rustc-link-arg does not propagate transitively

[tronical]

Problem
With #9523 the following use-case does not compile anymore with nightly. It works with current stable.

1. A -sys crate links against an external library using cargo:rustc-link-lib/cargo:rustc-link-lib-framework in its build.rs. The crate is to be consumed by a cdylib crate. To make consumption easy, build.rs of the -sys crate also prints out cargo:rustc-cdylib-link-arg=-Wl,-rpath,/some/path.
2. The cdylib crate depends on the -sys crate and the cdylib is linked in a way that the external library becomes a dependency for the dynamic linker and the rpath is set due to the propagation of rustc-cdylib-link-arg from the -sys crate to the final cdylib crate.

With stable rust this works as described, with nightly this now produces a build error when compiling the -sys crate due to the use of rustc-cdylib-link-arg in the non-cdylib -sys crate.

Concrete example:

Building the qttypes crate from https://github.com/woboq/qmetaobject-rs/tree/master/qttypes produces the error that #9523 introduced:

$ cargo +nightly build
   Compiling qttypes v0.2.1 (/Users/simon/src/qmetaobject-rs/qttypes)
error: invalid instruction `cargo:rustc-cdylib-link-arg` from build script of `qttypes v0.2.1 (/Users/simon/src/qmetaobject-rs/qttypes)`
The package qttypes v0.2.1 (/Users/simon/src/qmetaobject-rs/qttypes) does not have a cdylib target.

Possible Solution(s)

Allow rustc-cdylib-link-arg again in non-cdylib crates to avoid breaking their build with the next stable release.

Notes

Output of cargo version:

cargo 1.54.0-nightly (0cecbd6 2021-06-01)
cargo 1.52.0 (6976741 2021-04-21)

[joshtriplett]

@tronical We discussed this in the @rust-lang/cargo meeting today.

The behavior on stable wasn't intended to pass the flags to any package other than the current one. Up till 1.50, it was silently ignored. From 1.50 to 1.52, it was incorrectly propagating to the final linking package. On nightly, an error was added to indicate that the package doesn't have a cdylib. The behavior on stable wasn't known at the time.

A crate shouldn't be able to silently affect the link arguments of the crate depending on it; the crate depending on it should have to opt into that by calling a dependency-provided function from its own build script or similar. (Such a function could reference metadata provided by the dependency, just as it could currently reference such metadata to find things like C include paths.)

In general, rpath is something that a dependency can't safely set for a crate that depends on it; that might work for a specific cooperating set of crates, but not for an arbitrary dependency. Different crates may want to set rpath in different ways. (And, for that matter, different builders/distributors of executables may want to set it differently; for instance, Linux distributions typically do not want rpath set.)

So, rustc-cdylib-link-arg shouldn't affect the linker invocations of depending crates. To avoid breaking builds, we will make this a warning rather than an error for now, though we may make it an error in the future. We suggest altering your build scripts to only pass rustc-cdylib-link-arg within the package that contains the cdylib.

[ogoffart]

A crate shouldn't be able to silently affect the link arguments of the crate depending on it;

Why not?
For example, in C++ with cmake, this is quite typical to do target_link_options with INTERFACE and the link flags are propagated to all the dependees.
I would say that this is a valid use case to add some flags from the -sys crate, so that everything is setup for any crate using it.
I wouldn't call this behavior "silent".

the crate depending on it should have to opt into that by calling a dependency-provided function from its own build script or similar.

That's quite an overkill workaround. I've seen crates doing that (e.g, neon) but i guess it's only because it did not work with previous versions.
Asking every crate at the bottom of the tree to use a build script and depending on a build crate because one crate deep in the hierarchy might need a specific flag is quite not convenient when one could just tell cargo to do the right thing with something like rustc-cdylib-link-arg

In general, rpath is something that a dependency can't safely set for a crate that depends on it [...]. Different crates may want to set rpath in different ways.

This is off topic, but i was under the impression rpath are additive, so that different crates just append all the rpath they need.

(And, for that matter, different builders/distributors of executables may want to set it differently; for instance, Linux distributions typically do not want rpath set.)

That's true, but on the other hand, we added the rpath on our project because people filling github issues about our crate "not working" as it couldn't load shared libraries. We thought rpath was a good way to fix this problem to get a better out-of-the-box experience. Even of packagers, rpath doesn't hurt, does it?

[joshtriplett]

I would say that this is a valid use case to add some flags from the -sys crate, so that everything is setup for any crate using it.

Linker options can conflict or create various issues, and may break if the depending crate is doing something non-trivial with linking, or if they have different needs than one of their dependencies might expect.

And there's no way for the depending crate to control or filter out those options, if they're propagated from a dependency. In many cases, the depending crate may not even see those options; nothing will even display them unless you verbosely display your link line or the output of dependency build scripts.

Also, where to load shared libraries from can be a very project-specific or distribution-specific problem of release engineering. Binaries may be built on one system, packaged up, and deployed to other systems, with shared libraries in a different place than they were on the build system. In general, the -sys crate can't know the right solution for many use cases.

If we were going to support this, I could imagine doing so by allowing crates to declaratively (without using a build.rs script) state that they want to use link flags from a specific dependency's metadata (DEP_THEDEPNAME_LINK_FLAGS, for instance).

But beyond that, rpath is one of many things that we should really have a standard solution to. People should be able to make a single top-level decision like "I want rpath set in this common way" or "I want rpath set in this uncommon way" or "I don't want rpath set at all", and then crates supplying shared libraries should be able to give cargo enough information that it could set that consistently with the user's preferences and those of the top-level crate.

This is off topic, but i was under the impression rpath are additive, so that different crates just append all the rpath they need.
Even of packagers, rpath doesn't hurt, does it?

It can hurt, for various reasons. https://wiki.debian.org/RpathIssue gives one explanation. An rpath may also cause libraries to be loaded from an unexpected path; that may have security implications, or may break if there are other libraries in that path. It's important for people building crates to be able to control where they load libraries from.

[tronical]

Perhaps the -sys crate can't know the right solution for many use-cases, but I believe that it should be in a position to make some decisions and it can take "user input" into account. If the consumers of the crate have to express that they want to use link flags from a dependency's meta-data and it also needs to be specified by the consumers of the consuming crate, then it becomes hard to use. We want the crate that we are developing to be easy to use. It's okay if the build fails, but it's bad if everything compiles but the program can't be started because the dynamic linker can't find a library that the regular linker clearly found at build time.

The feature that build systems like CMake offer is the encapsulation, that a CMake target can be used without having to know what other things it depends on. If I create a library with CMake and say that it needs this particular linker flag, then I can be sure that in a dynamic build the library will be linked with the flag and in a static build that flag will be forwarded until a shared library (or regular binary) is created. The only equivalent that I can think of from before that were lib tool archives.

I agree that built-in support for rpath in Cargo would be awesome, combined with a single top-level decision as you pointed out. It would be great to have a solution in the meantime, which is what rustc-cdylib-link-arg seems to offer - at the risk of being imperfect.

In any case, thanks for considering this issue and our input :-). It's much appreciated that you plan on making this a warning again, instead of an error.

[ivmarkov]

Given that this issue was recently closed as a duplicate of this one, let me state that the scope of the problem is not only about passing an rpath from a -SYS crate down to a 'cdylib' crate.

We would like to pass - transitively of course - ALL sorts of linker args from a -SYS crate all the way down to a binary (as opposed to cdylib) crate.

The above is very, very useful in the embedded world when the SYS crate is modelled to represent the unsafe bindings to the Vendor-provided SDK for the MCU, as well as the vendor-specific linker args (including custom linker scripts and whatnot) that have to be passed through to the binary crate.

[kaspar030]

Just to chime in: This also broke our build, in embedded context. In RIOT-rs, one crate compiles and links a static library (from C code). that crate also needs to link in newlib (-lc_nano). Previously, with "-Zextra-link-args", that worked fine. Not anymore.

[joshtriplett]

Opening a poll to check consensus among the rest of the team.

Should we confirm that the current behavior is correct, and dependencies shouldn't be able to affect the linker command-line of the depending crate without some kind of explicit opt-in from the depending crate?

@rfcbot close

[rfcbot]

Team member @joshtriplett has proposed to close this. The next step is review by the rest of the tagged team members:

• @Eh2406
• @alexcrichton
• @ehuss
• @joshtriplett

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[Lokathor]

My final comment is that this is poor for the embedded space where the API crate for a particular device, which includes the necessary linker script for said device, can't send the linker script along to binaries built for the device that depend on the API crate.

[ivmarkov]

I do agree with @Lokathor that not having any form of transitive arguments passing from the API/SYS crate to the binary one is very suboptimal. At the same time, I also agree that such a scheme would ideally require an explicit opt-in from the binary crate.

My only concern is that if the current restricted scheme is stabilized in its current form, future transitive + explicit opt-in extensions might take a lot of time to stabilize or not receive enough attention for that to happen at all in the foreseeable future.

[joshtriplett]

To be clear, I'm quite amenable to proposals for opt-in linker arguments; I just wouldn't want them to happen implicitly without the explicit opt-in from the top-level crate.

We also already have the DEP_ mechanism, which is enough to support manually propagating these arguments via build.rs. Any further mechanism would be to make that more automatic and declarative without requiring a build.rs in the top-level crate.

[rfcbot]

The final comment period, with a disposition to close, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.