Skip to content

Cargo should accumulate rustflags from all sources, rather than from only one source #6338

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

Closed
infinity0 opened this issue Nov 20, 2018 · 9 comments
Labels
A-configuration Area: cargo config files and env vars A-rustflags Area: rustflags C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted`

Comments

@infinity0
Copy link
Contributor

In Debian Rust package builds we would like to apply e.g. -C linker and --remap-path-prefix in all situations. We currently do this by setting CARGO_HOME to point to a directory containing a config file with contents similar to the following:

[source.crates-io]
replace-with = "dh-cargo-registry"

[source.dh-cargo-registry]
directory = "$PWD/vendor" # or /usr/share/cargo/registry

[build]
rustflags = [... "-C", "linker=arm-linux-gnueabi-gcc", "-C", "link-arg=-Wl,-z,relro", "--remap-path-prefix", "$PWD=/usr/share/cargo/registry/cargo-0.31.0"]

However if RUSTFLAGS is set (e.g. to empty) in the environment, the above build.rustflags will be totally ignored.

The only method we have currently, to absolutely ensure that certain flags are set, is to append to RUSTFLAGS itself, but this is ugly because it does not support escaping of spaces.

The proposal here is to change env_args to instead use all the sources listed there, in order of priority - that is, set the effective rust flags to be config/build.rustflags + Cargo.toml/target.rustflags + RUSTFLAGS, with later flags taking precedence.

@dwijnand
Copy link
Member

While I'm not sure of how to solve the issue, the setup of having environment variables trump configuration files is common to the point I think it's the expected behaviour, making accumulating break many people's (IMHO) expectations.

@infinity0
Copy link
Contributor Author

infinity0 commented Nov 21, 2018

the setup of having environment variables trump configuration files is common to the point

[citation needed]

I haven't seen many build tools read both an envvar and a user configuration file for the same setting. Usually a high-level tool controls the envvar and that is passed to the low-level tool as a command-line arg, there is a clear separation of concerns.

A common pattern is to offer multiple envvars and use all of them, with the intention / naming reflecting who is supposed to set them (system-wide vs per-package vs user default vs user temporary override) and to have later ones take precedence and be able to override earlier ones. For example, see autoconf. This sort of behaviour is more predictable as people don't really expect the presence/absence of an envvar to completely change something else, but only to tweak it. To disable a config file completely, it is customary to explicitly require a command-line flag like "--no-rc" or something.

The main thing I am concerned about here is that if a crate author sets target.rustflags then I cannot add to this, I (as the system-wide rust packager for Debian) can only completely override it with RUSTFLAGS. But what I really want to do is set some system-wide defaults and then have target.rustflags appended onto it, not to have a false dichotomy between the different sources of flags.

@dwijnand
Copy link
Member

This sort of behaviour is more predictable as people don't really expect the presence/absence of an envvar to completely change something else, but only to tweak it. To disable a config file completely, it is customary to explicitly require a command-line flag like "--no-rc" or something.

Apologies, I meant an environment variable overrides a specific setting in a configuration file. I have no citation, but my intuition is: command line option > environment flag > configuration file.

However, when the setting is not for a single value but for a list of values I don't think there's an established practice, especially one that differentiates between overriding, prepending and appending, which is exactly the case you're facing.

If you need to prepend to target.rustflags maybe we should introduce RUSTFLAGS_PREPEND? And in terms of RUSTFLAGS I think that should continue to always win, ignoring target.rustflags and RUSTFLAGS_PREPEND.

WDYT?

@infinity0
Copy link
Contributor Author

infinity0 commented Nov 22, 2018

when the setting is not for a single value but for a list of values I don't think there's an established practice

Look at the autoconf practise that goes back several decades, it (to repeat myself):

offers multiple envvars and use all of them, with the intention / naming reflecting who is supposed to set them (system-wide vs per-package vs user default vs user temporary override) and to have later ones take precedence and be able to override earlier ones.

This relies on the fact that gcc and other compilers (e.g. ghc) all generally allow later CLI flags to override earlier ones, e.g. there is a --no-X flag for each --X flag, or the flag is of the form --X=Y where a later flag --X=default can override the earlier =Y.

If you need to prepend to target.rustflags maybe we should introduce RUSTFLAGS_PREPEND

It would be better to do this in $CARGO_HOME/config in build.rustflags_prepend so arguments can contain spaces. That is one problem with the older *FLAGS convention. I would be happy with this solution yes.

However I still consider this suboptimal to cargo just simply reading from all sources and composing everything together. I'd say this is most intuitive and predictable when looking at large-scale behaviour of how you want complex buildsystems to interact. For example your "intuition":

but my intuition is: command line option > environment flag > configuration file.

doesn't even match the current situation, it is not "command line option > environment flag", what happens is that cargo reads "environment flag > user config > crate config" and then passes one of the previous as the command-line flags to rustc.

Anyway what is actually the issue here is not the mechanism (cli vs envvar vs config) but reading flags from different conceptual sources i.e.

  1. local user
  2. crate developer
  3. system/distribution

Here I agree it is "obvious" the precedence should go 1 > 2 > 3, however the behaviour shouldn't be to override lower-precedence ones, it should be to append to the lower-precedence ones. Then rustc will treat later (appended) flags as higher-precedence if they "conflict" with an earlier (prepended) flag - and it already does this, this is conventional behaviour for pretty much all compilers and other programs.

@dwijnand
Copy link
Member

Ah, I understand what you're saying. Makes sense to me now (sorry).

@alexcrichton
Copy link
Member

In the wild I've seen both use cases for overriding and for appending, so we may not want to use RUSTFLAGS literally but it seems reasonable to have some configuration for pulling in custom flags from multiple sources.

@alexcrichton alexcrichton added the C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` label Nov 28, 2018
@ehuss ehuss added the A-configuration Area: cargo config files and env vars label Feb 21, 2019
@Abdillah
Copy link

For the case of prepending and overwriting, those might be useful when cross compiling.

In my case though, cross-compile led me to "file not recognized: file format not recognized" error. For example,

$ PKG_CONFIG_ALLOW_CROSS=1 cargo rustc --target arm-unknown-linux-gnueabihf --bin server -- -C linker=arm-linux-gnueabihf-gcc -C link-arg="-L/usr/arm-linux-gnueabihf/lib/"
/usr/lib/libdl.so: file not recognized: file format not recognized
collect2: error: ld returned 1 exit status

Maybe I haven't found a better way, but from my perspective as user, prepending the flag will solve my common cross-compiling problem here. Which I think due to the precedence of "-L" "/usr/lib" over the -L/path/to/arm/lib.

@richardwhiuk
Copy link

There's a similar issue here:

build.rustflags in .cargo/config are overriden by target specific rust flags in target.x86_64-unknown-linux-gnu

This is hazardous because we use the second workaround suggested in rust-lang/rust#47551 (comment) to avoid a musl backtrace issue.

That means that crates which define custom .cargo/config have it ignored on musl only targets.

@ehuss ehuss added the A-rustflags Area: rustflags label Apr 6, 2020
@ehuss
Copy link
Contributor

ehuss commented Apr 13, 2021

This issue is almost identical to #5376, so I am going to close in favor of that so that this is tracked/discussed in one place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-configuration Area: cargo config files and env vars A-rustflags Area: rustflags C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted`
Projects
None yet
Development

No branches or pull requests

6 participants