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

Design of system packages #65

Open
springmeyer opened this issue Feb 3, 2015 · 10 comments
Open

Design of system packages #65

springmeyer opened this issue Feb 3, 2015 · 10 comments

Comments

@springmeyer
Copy link
Contributor

A ticket to discuss how to design "system" packages for mason, which are a very unique kind of package. This is a place for @kkaefer and @springmeyer to sync up on goals and others to read to get some background on why system packages are tricky.

My understanding is that a system package:

  • makes it possible to declare an explicit dependency on something like zlib without installing a custom version. (**This can be very important because something like zlib is often a dependency of so many apps that mixing a custom version of zlib would be a path to ABI conflicts and much pain).
  • still allows mason to report cflags and ldflags for where the system's zlib is located such that a build system can depend on mason to supply platform-specific flags.
  • does not get published since there is nothing binary to publish. Therefore the script.sh does all the work dynamically on each system.

But one challenge remains: is it safe to put the -L/path and -I/path of the system package on the compile and link paths? This can result in conflicts with other system libraries that you might be intending to statically link.

I ran into a situation like this with libpq (postgres client library) on OS X, which is the motivation for trying to have mason cflags and mason ldflags return symlinks to the location of a system package rather than the real location (4e14abb). However, due to the nature of the postgres build system I ultimately found I could not avoid -L/usr/lib ending up on the link flags, even if mason did not put it there. So my changes were not enough. And in addition they were not correct for every platform and therefore caused other unintended breakages downstream in mbgl which I'm sorry about.

@kkaefer has made some progress further cleaning up the zlib-system package (https://github.com/mapbox/mason/commits/zlib-system). Hopefully we'll be in good shape now and we can keep the symlinking addition. Or we can remove it. Let's discuss.

@springmeyer
Copy link
Contributor Author

As some background for anyone who has not dealt with conflicting system libraries and custom libraries before I'll add a bit more background...

Say that your app depends on zlib-system and libfoo-1.0.0, both packaged with mason. You want to link statically against /.mason_packages/.link/lib/libfoo.a but dynamically against libz.so. But let's pretend that libz.so exists at /usr/lib/libz.so and so does a system version of libfoo.so.In this case you'll need to be sure that the mason packaged /mason_packages/.link/lib/libfoo.a is linked instead of /usr/lib/libfoo.so .

This is impossible to ensure if your app uses -lfoo to link libfoo unless /usr/lib is not found in the linker flags. So, one potential solution is to not have mason ldflags zlib system return -L/usr/lib but rather return a symlink that points to a mason-local directory that only contains a symlink like ./mason_packages/{platform}/zlib-system/lib/libz.so -> /usr/lib/libz.so. This would allow zlib to be linked dynamically to /usr/lib/libz.so without needing to have -L/usr/lib on the linking paths.

The other option is to have your app link to libfoo.a using /full/path/to/libfoo.a instead of -lfoo. Then you can be sure that only your custom libfoo.a will be linked and not /usr/lib/libfoo.so even if -L/usr/lib end up on the linker paths.

But the problem of conflicting headers still exists. To point a compiler at headers you have only the -I/path/to/header option. Passing the absolute path to a header is not realistic. So, in this case - as far as I know - if you need to avoid a conflict with both /usr/include/libfoo.h and ./mason_packages/{platform}/libfoo/include/libfoo.h the only way to do this is to ensure that -I/usr/include is not in the compile paths. Note: there is also the -sysroot and the CPLUS_INCLUDE_PATH and C_INCLUDE_PATH options to help tell the compiler what are the system headers it should prioritize using (and LIBRARY_PATH for linking). But I've not found these a predictable substitute and so my gut says it is better to avoid -L/usr/lib and -I/usr/include on the link/compile paths than rely on anything else.

@springmeyer
Copy link
Contributor Author

Working on trying to understand the failure at mapbox/mapbox-gl-native#802 by starting to add tests to zlib-system package: ecdd669

@ljbade
Copy link
Contributor

ljbade commented Feb 3, 2015

What's wrong with building and statically linking to our own zlib? Then we don't need to worry about system conflicts.

@springmeyer
Copy link
Contributor Author

Statically linking zlib is fine if you can be sure that all code statically links the same zlib (allowing the linker to discard duplicate symbols). Situations where this would not work and would be asking for trouble:

  • On iOS you want to use a Apple provided Framework that dynamically links zlib (it is bound to be a different version and will clash).
  • On OS X or Linux you want your binary to be able to be linked into node.js and to use the node.js binaries provided by upstream (rather than source compiling your own node.js). In this case node statically links zlib and trouble will ensure if your statically linked zlib is not exactly the same version.

@ljbade
Copy link
Contributor

ljbade commented Feb 3, 2015

Hmm, you cant get the linker to resolve the zlib symbols in our .a/.o files with or zlib, and let other dynamically loaded libs resolve their own zlib?

@springmeyer
Copy link
Contributor Author

In adding tests for zlib-system (#66) I've noticed that MASON_SDK_PATH gets clobbered for ios such that the SDK for the iPhoneSimulator gets used instead of the iPhone: https://github.com/mapbox/mason/blob/master/mason.sh#L58-L67. This makes me worried about the impact of https://github.com/mapbox/mason/blob/zlib-system/script.sh#L14-L15. That code works for android because the android make-standalone-toolchain.sh generates the right toolchain and places it at MASON_SDK_PATH. It also works for MacOSX because there is only one toolchain for MacOSX. But for IOS it will effectively be saying that the library for both the iPhone and iPhoneSimulator is the one inside the simulator toolchain, which is wrong. This would be avoidable if we broke out iOS into new platforms like MASON_PLATFORM=iPhone and MASON_PLATFORM=iPhoneSimulator, but I'm not sure what other issues that would create.

Hmm, you cant get the linker to resolve the zlib symbols in our .a/.o files with or zlib, and let other dynamically loaded libs resolve their own zlib?

@ljbade not that I'm aware. But I've never been keen to try such a thing enough to learn the intricacies of how you might go about ensuring it works 100% safely. What I see when apps truly need to vendor a library that is also a system/shared library they will rename all the symbols so you can call mylib_inflate instead of just inflate and be sure you're getting the right ABI. This is not something we want to resort to.

@kkaefer
Copy link
Contributor

kkaefer commented Feb 5, 2015

But for IOS it will effectively be saying that the library for both the iPhone and iPhoneSimulator is the one inside the simulator toolchain, which is wrong.

I'm running into this problem. The root cause of this is that MASON_SDK_PATH shouldn't actually be available to the script file, but it is, because the mason.sh file is sourced.

Android uses separate package dirs per toolchain, but we don't want that for iOS since it means we'd have to build separate projects for Simulator and actual device. All of our static archives have five architectures (armv7, armv7a, arm64, x86 and x86_64, the latter two are for the simulator), so we're clear here. I think the quickest fix for now is to not output include and lib paths for PLATFORM == 'ios'.

kkaefer added a commit that referenced this issue Feb 5, 2015
@springmeyer
Copy link
Contributor Author

sounds good @kkaefer

@danpat
Copy link
Contributor

danpat commented May 15, 2015

Under GNU ld, it should be possible to be explict about what to link statically and what to link dynamically by passing -Bstatic and -Bdynamic to ld, like so:

gcc whatever.c -L/usr/lib -Lmason_packages/.link/lib -Wl,-Bstatic -lfoo -Wl,-Bdynamic -lzlib -o whatever

Apple's ld unfortunately doesn't support -Bstatic/dynamic, but there is a possible solution outlined in QA1393: https://developer.apple.com/library/mac/qa/qa1393/_index.html

Normally, the OS/X linker searches all paths for dynamic libraries, then all paths for static libraries. This can be changed to "search each path for dynamic, then static, then continue to the next path" by supplying -Wl,-search_paths_first. Under this strategy, putting mason_packages/.link/lib at the beginning of the ld search path should result in static libraries being linked in preference to dynamic libraries elsewhere on the system.

@danpat
Copy link
Contributor

danpat commented May 15, 2015

For reference, there are some other (ugly) techniques for handling symbol conflicts discussed here:

http://stackoverflow.com/questions/3232822/linking-with-multiple-versions-of-a-library

Summary:

  • use dlopen() with RTLD_GLOBAL|RTLD_LAZY to dynamically load specific shared libraries and get references to specific symbols using dlsym()
  • use objcopy with --localize-symbols to rename symbols after compilation (Linux only)
  • use objconv to do the same on OS X (http://www.agner.org/optimize/#objconv)

I wouldn't want to go down any of these paths unless absolutely necessary.

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

No branches or pull requests

4 participants