Prevent silently falling back to dynamic linking when static linking is explicitly requested

[golddranks]

There were two problems with the current code:

1. if lib flags are parsed before directory flags, the full search paths are not available yet, and a library may be mistakenly deemed as a system library. Resolution: process the lib flags only after the loop, when search paths are fully known.

2. The variable statik is true when static linking has been explicitly requested. However, the is_system check can prevent the static branch from being selected, and dynamic link flags are passed. Because static build has been explicitly requested, it would be better just to panic with an error message instead of doing unexpected behaviour.

[alexcrichton]

Hm is this really the behavior we want? This means that if you set PKG_CONFIG_ALL_STATIC you're basically guaranteed to get a build failure in most situations.

I could imagine though that if you say config.statik(true) we should generate an error though?

[golddranks]

Hm, I thought that that flag is precisely for signalling that you want to have everything linked statically? I couldn't imagine setting that flag unless I'm deliberately trying to link everything statically. But that might very well be because of my lack of imagination :D

[kornelski]

Is it possible to check whether the static library exists in the filesystem?

On macOS some critical libraries are dynamic only, e.g. /usr/lib/libSystem.B.dylib, /usr/lib/libobjc.A.dylib, /System/Library/Frameworks/*.

OTOH I need everything else to be really static (this might require cooperation from the gcc crate too). For example, on macOS gcc does not produce redistributable executables unless you compile with -static-libgcc (otherwise it dynamically links to /usr/local/lib/gcc/6/libgcc_s.1.dylib which is only installed with the compiler, and no normal user has this).

Similarly /usr/local/opt/gcc/lib/gcc/6/libgomp.1.dylib is very compiler-specific and nobody has it, so it must be linked statically for the executable to work.

[alexcrichton]

@golddranks my intention with this was to just pass --static to pkg-config itself, not do much more than that. I'd inteprete .statik(true) as a request that only the library in question is desired to be static, but not necessarily saying anything about other libraries.

@pornel yeah makes sense to me that we'd use the filesystem to drive decisions about linkage

[golddranks]

Ah, I see. In that case, (a part of) the current behaviour makes totally sense.

I still think that resolving whether a lib is a system lib or not before knowing all the search paths may be a bug; what do you think?

For example: pkg-config --libs --cflags --static openssl → -I/musl/include -lssl -lcrypto -L/musl/lib -ldl. The -L flag is in the middle, and that's why the search path isn't yet known when checking whether ssl and crypto are system libs. (Btw. -ldl seemed an odd library to link when linking statically, but in a discussion I found from the internet it was said that it shouldn't matter if it's a musl version of it that is meant to be linked statically.) So at least if it checks the libs, it should do that only after all the search paths are known, right? Or then there is some significance to the order of -l and -L flags that's escaping me.

[alexcrichton]

I mean in general linkage is just impossible to get right. Everyone wants something different so no matter what heuristic you have someone will complain that it's not correct for them. If you have all the heuristics then others will complain that it's too complicated and they can't just do what they know is right.

I don't mind bug fixes of course, but there's got to be a line somewhere at some point... Where do lookup paths come into play here? I don't think this crate currently probes the filesystem yet?

[golddranks]

The lookup paths come into play because the is_system check currently performed by this crate uses them. It actually probes with fs::metadata() whether the library exists:

        !d.starts_with(root) && fs::metadata(&d.join(&libname)).is_ok()

[golddranks]

This check was introduced in this commit, if it helps to refresh your memory of its purpose: 9a57960

[alexcrichton]

Could this change to just check self.statik instead of statik? That'd be fine by me.