Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved build.sh taking as a base the work of margueritev. #337

Merged
merged 8 commits into from
Jan 28, 2017

Conversation

jgmdev
Copy link
Contributor

@jgmdev jgmdev commented Jan 25, 2017

Improved build.sh taking as a base the work of margueritev on the opensuse package build.
LZMA will always static link even if the linux distro doesn't ships a static build.
OPENSSL will always static link even if the linux distro doesn't ships a static build.
Root priviliges are not needeed anymore to build.
Changed build-appdirs.sh to not depend on a specific libarchive version.
Redirect errors of find command in build-appdirs.sh to stop script execution due to permission denied messages.

…nsuse package build.

LZMA will always static link even if the linux distro doesn't ships a static build.
OPENSSL will always static link even if the linux distro doesn't ships a static build.
Root priviliges are not needeed anymore to build.
Changed build-appdirs.sh to not depend on a specific libarchive version.
Redirect errors of find command in build-appdirs.sh to stop script execution due to permission denied messages.
@probonopd
Copy link
Member

probonopd commented Jan 25, 2017

Thanks @jgmdev looks very promising indeed, great work of @marguerite and you.

The Travis CI build fails with mksquashfs: error while loading shared libraries: liblzma.so.5: cannot open shared object file: No such file or directory - can you fix the Travis CI build?

@jgmdev
Copy link
Contributor Author

jgmdev commented Jan 26, 2017

Strange, I tested the changes on last commit to see if mksquashfs was now properly static linking to lzma and got:

ldd build/mksquashfs 
	linux-vdso.so.1 (0x00007ffd275fe000)
	libpthread.so.0 => /usr/lib/libpthread.so.0 (0x00007f0b0c864000)
	libm.so.6 => /usr/lib/libm.so.6 (0x00007f0b0c55c000)
	libz.so.1 => /usr/lib/libz.so.1 (0x00007f0b0c344000)
	libc.so.6 => /usr/lib/libc.so.6 (0x00007f0b0bfa4000)
	/lib64/ld-linux-x86-64.so.2 (0x00007f0b0ca84000)

So I don't quite understand why it is still insisting on travis to link against the dynamic version of lzma, maybe the compiler version works differently...

@probonopd
Copy link
Member

probonopd commented Jan 26, 2017

So I don't quite understand why it is still insisting on travis to link against the dynamic version of lzma, maybe the compiler version works differently...

Same here, this is why I took quite drastic measures (e.g., deleting dynamic versions) to enforce it...

Will be happy to merge this kind of work if we can get it to work properly both on Travis CI and locally, but unfortunately I am not an expert at all in build systems/linkers, so I have not much to offer in terms of advice here.

Fixed removal of .so files called before exiting child directory.
@jgmdev
Copy link
Contributor Author

jgmdev commented Jan 26, 2017

Well, finally is working :), I guess it can be merged now unless some cleanup is needed for install-build-deps.sh

@probonopd
Copy link
Member

Is it still possible to use xz as one of the compression options when built on CentOS? That is a feature that I would like to keep:
https://github.com/probonopd/AppImageKit/blob/appimagetool/master/appimagetool.c#L122-L129

@jgmdev
Copy link
Contributor Author

jgmdev commented Jan 27, 2017

mksquashfs is still linking to the static version of lzma, so the functionality should stay intact. I just disabled the old method of building the xz utilities on the dependencies install script with the new method found on build.sh which does not requires root priviliges. Also I moved the build of mksquashfs from build-appdir script to build.sh. The idea is to always build static versions of the most important libraries so we don't have to depend on a specific linux distro. Maybe we can do the same with fuse, glib, etc...

@probonopd
Copy link
Member

probonopd commented Jan 28, 2017

Build succeeds also locally on Ubuntu 16.04. 👍

Question, why should we deal with OpenSSL at all and not rely on whatever the distribution provides?

  • When appimagetool is built for packaging it as an AppImage, we could statically link OpenSSL at build time so that the resulting AppImage has no runtime dependency on ever-changing OpenSSL (more ideally even, we would find a way to be able to use whatever OpenSSL is provided by the distribution at runtime)
  • When appimagetool is built for packaging it as a native distribution package (e.g., the @marguerite build), we could link OpenSSL dynamically and declare a dependency on the distribution package of OpenSSL

I would really like to avoid having to deal with OpenSSL as part of the AppImageKit build.

Copy link
Member

@probonopd probonopd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is there a need to build OpenSSL?

@jgmdev
Copy link
Contributor Author

jgmdev commented Jan 28, 2017

Why is there a need to build OpenSSL?

Because not all linux distributions provide static versions of libssl/libcrypto. That seems to be the reason why @marguerite also added a custom build of openssl to the appimage opensuse package. Also on archlinux there is no static version of the openssl libraries.

Also this way the life of distro packagers is easier because modifying the build.sh becomes minimal. What would be needed is a way to make install-build-deps.sh an optional call, something like ./build.sh --no-deps, this way I would say distro packagers don't have to patch build.sh and with openssl been built statically by default the appimagetool would run on most other distro's.

I wonder now if static linking fuse and glib would make the resulting appimagetool binary more universal. The other dependencies like zlib, pthread seem to not change abi compatibility, but im not an expert on this matter, maybe glib and fuse abi also doesn't changes but im not sure.

@probonopd
Copy link
Member

Also this way the life of distro packagers is easier because modifying the build.sh becomes minimal

It is this i am not sure about. When appimagetool is packaged as a native distribution package, why shouldn't it dynamically link against the distro-provided libssl/libcrypto?

@jgmdev
Copy link
Contributor Author

jgmdev commented Jan 28, 2017

Mmm, maybe another flag can be added to choose from static or dynamic. This way the travis can always build with static libraries and distro packagers can choose to turn off the static option and use the dynamic ones. right now appimagetool with current build.sh changes only has the following dynamic library dependencies:

libfuse.so.2
libpthread.so.0
libglib-2.0.so.0
libz.so.1
libc.so.6
libdl.so.2
/lib64/ld-linux-x86-64.so.2
libpcre.so.1

From the above I guess that libc, libdl and ld-linux-x86-64.so.2 don't matter, but what if we could get appimagetool to link against static builds of the remaining libraries, would using a old distro like centos 6 still matter? Does libc, libdl have broken abi compatibility over the years? The point is that if some one chooses to distribute a build of appimagetool done on a recent linux distro it would still work on some older distributions if most libraries are statically linked.

@probonopd
Copy link
Member

probonopd commented Jan 28, 2017

Agree a flag would be a good idea.

So far we are assuming that the libraries you mentioned will be provided by the target systems. Then there are libraries like libssl/libcrypto which apparently we cannot take for granted in the same way, but which I think should come from the distributions at least in the case of native distribution packages.

@jgmdev
Copy link
Contributor Author

jgmdev commented Jan 28, 2017

Ok, im adding a --no-dependencies (to prevent calling the install-build-deps.sh) and --use-shared-libs to disable compiling of openssl as static. So, when appimagetool generates the .AppImage, only the runtime file is injected to it? The only dependency that should be compiled as static is lzma right?

@probonopd
Copy link
Member

Yes, only runtime and AppRun end up in the generated AppImage. These should be the same regardless whether appimagetool from an AppImage or from a distribution package was used.

appimagetool, when packaged as a distribution package, could link to lzma dynamically, shouldn't it? (I am not talking about runtime.c but about appimagetool itself)

@jgmdev
Copy link
Contributor Author

jgmdev commented Jan 28, 2017

I forgot to pull before pushing :( but it seems that everything is working properly :). I tested the 2 build modes (static, shared) locally. Basically now a distro packager can just do ./build.sh -n -s and the binaries will link against the shared libraries provided by the distro except for runtime which still builds with static lzma.

Maybe the -n flag (--no-dependencies) should check if the system meets all required dependencies and display errors when not, so that way is easier for packagers to know which dependencies are needed.

@probonopd
Copy link
Member

probonopd commented Jan 28, 2017

Most likely unrelated, but can you find out why appimaged has a runtime dependency on libselinux.so.1? (compiled on ubuntu-16.04-desktop-amd64.iso)

@jgmdev
Copy link
Contributor Author

jgmdev commented Jan 28, 2017

it gotta be because ubuntu is enabling it for gio or cairo, even libarchive, not sure which, maybe you can corroborate that with ldd gio. ldd cairo or ldd archive.

@marguerite
Copy link

I have also been advised by openSUSE people not to have openssl builtin. If it has to be static, it is better to modify the system package to have static build results, which will gather all security stuff to one place. So I think a flag of dynamic linking is perfect, saving us a lot of works/communications and having great backward compatibility because modifying a already released distro is not possible.

Secondly I think we should put mksquashfs to libexec instead of /usr/bin. our mksquashfs is patched and static linked to xz (dynamic linking is good here), so it can't be replaced by the one from the system squashfs package. But they do conflict because they both needs the namespace /usr/bin/mksquashfs

@marguerite
Copy link

and mksquashfs should be found by /usr/bin/appimagetool directly, instead of exposing "libexecdir/appimagetool" to the system path because some of the distros also put AppRun in the same directory

@probonopd
Copy link
Member

probonopd commented Jan 28, 2017

The need for a custom mksquashfs is only until plougher/squashfs-tools#13 is merged. However it looks like the squashfs-tools project may be stalled and not accepting patches. If you think it would be better, we could also rename our patched version of it. libexec doesn't exist on all distributions, I think debian/Ubuntu are not using it. So we could put it somewhere in /usr/lib/appimagekit/libexec/mksquashfs (or something like that) which is not on the $PATH, and modify appimagetool to pick it up from there.

@probonopd
Copy link
Member

@marguerite

I have also been advised by openSUSE people not to have openssl builtin. If it has to be static

I don't think it has to. It should be static only when we distribute appimaged inside an AppImage. For native distribution packages, it should be dynamic.

@probonopd probonopd merged commit 1dc201b into AppImage:appimagetool/master Jan 28, 2017
@probonopd
Copy link
Member

plougher/squashfs-tools#13 (comment) was merged, so we can use stock mksquashfs in the future (once the change arrives in the distros).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants