rust-lang / cargo

The Rust package manager
https://doc.rust-lang.org/cargo
Apache License 2.0
12.61k stars 2.4k forks source link

There is no way to set RUSTFLAGS for build scripts when cross-compiling #4423

Open RalfJung opened 7 years ago

RalfJung commented 7 years ago

The cargo documentation environment-variables.md says

RUSTFLAGS - A space-separated list of custom flags to pass to all compiler invocations that Cargo performs. In contrast with cargo rustc, this is useful for passing a flag to all compiler instances.

This is wrong, unfortunately. The flag is not passed to all compiler instances: When cross-compiling (i.e., when --target is set), the flag is not passed to build scripts. It seems there is currently no way to pass anything to build scripts when --target is set, which clearly is a feature gap. Cargo should provide a way to set flags for build scripts even when cross-compiling.

alexcrichton commented 7 years ago

This is currently intentional as it is the conservative choice of how to interpret RUSTFLAGS, but it's always possible to expand it!

RalfJung commented 7 years ago

This is currently intentional as it is the conservative choice of how to interpret RUSTFLAGS, but it's always possible to expand it!

So the fact that whether RUSTFLAGS applies to build scripts or not depends on whether --target was set or not is how things ought to be? That's a really surprising side-effect of setting a target! The option should be called --target-and-ignore-rustflags-for-buildscripts, then... ;) To make matters worse, this behavior is actually stable and hence can probably never be changed :(

Actually the code says this is just a hack:

    // We *want* to apply RUSTFLAGS only to builds for the
    // requested target architecture, and not to things like build
    // scripts and plugins, which may be for an entirely different
    // architecture. Cargo's present architecture makes it quite
    // hard to only apply flags to things that are not build
    // scripts and plugins though, so we do something more hacky
    // instead to avoid applying the same RUSTFLAGS to multiple targets
    // arches:

It seems that what we really want is two environment variables; one applied to plugins/build scripts and one applied to the "actual" code. (And maybe a third applied to both.) However, that's currently not easy to do because the function setting the flags has no idea whether this is for the host or target. Correct? That's what this comment sounds like.

Judging from trouble that xargo has wrt. plugins, I assume the problem is that when I just run cargo build, the kind is actually Kind::Host for everything? IOW, cargo build and cargo build --target $host_arch are very different. Not sure why this is, sounds like a bug to me.

One "easy" fix for this problem would be to double-down on the existing hack, and change env_args such that rather than not passing any arguments when it detects a build script in a cross-build, it just goes looking for a different environment variable, like RUSTFLAGS_HOST or so. That would be enough to solve my problem, but I'm not sure if you'd be willing to accept such a patch -- and anyway, it'd all still be a hack. (My problem is described in https://github.com/japaric/xargo/issues/162: I want a way to run cargo build --target <...> such that all compiler invocations have --cap-lint=allow set.)

The alternative I guess is to fix cargo such that kind actually reliably says whether this is a build script or part of the "real" build. Judging from the fact that nobody did this so far, I expect that to be non-trivial, and there may also be good reasons for why things are the way they are right now.

RalfJung commented 7 years ago

Browsing through the code, I have found other code using Target::for_host to distinguish builds for plugins from builds for the target. What is the reason that env_args relies on a different signal?

alexcrichton commented 7 years ago

er just to be clear, none of this behavior is a bug today, it's all intentional. RUSTFLAGS is intended to only apply to the "final artifact" which when cross compiling doesn't account for build scripts and such. We left ourselves room to grow other environment variables, though, so we can certainly add some more!

RalfJung commented 7 years ago

RUSTFLAGS is intended to only apply to the "final artifact" which when cross compiling doesn't account for build scripts and such

But that's not what happens. If I run just cargo build, RUSTFLAGS is applied to build scripts and the artifact alike. The entire behavior in cross-compilation vs. "native" compilation mode is pretty inconsistent. Also see the xargo README which observes that

When using compiler plugins (e.g. serde_derive) the target triple must be provided even when compiling for the host platform due to the way cargo handles compiler plugins. I.e., use xargo build --target x86_64-unknown-linux-gnu instead of just xargo build. (Surprisingly, these commands are NOT the same to Cargo.)

RalfJung commented 7 years ago

Well anyway I submitted https://github.com/rust-lang/cargo/pull/4428 which tries to not touch this existing logic, but is just enough to fix my problem. Some other day I'll try to distill my trouble with this logic into a separate bugreport. ;)

gnzlbg commented 6 years ago

@alexcrichton

RUSTFLAGS is intended to only apply to the "final artifact" which when cross compiling doesn't account for build scripts and such.

Why does it account for build scripts and plugins when not cross compiling?

Cross-compiling to the host and compiling to the host are the same thing, therefore I think that cargo build and cargo build --target=$host_arch should be equivalent.

I find the current behavior of RUSTFLAGS when --target is used to be sane, so I wouldn't mind if that behavior would apply to cargo build as well.


@RalfJung

It seems that what we really want is two environment variables; one applied to plugins/build scripts and one applied to the "actual" code.

We can keep RUSTFLAGS to apply to the code of the "target", and add RUSTFLAGS_BUILD_DEPENDENCIES to apply to all build dependencies like crates, plugins, build.rs, ...

EDIT: an alternative is to allow finer grained rustflags RUSTFLAGS_BACKTRACE_V_1_0_1_BUILD_RS to apply only to the build.rs of the dependency backtrace version 1.0.1 ...

alexcrichton commented 6 years ago

@gnzlbg for historical reasons cargo build is subtly different from cargo build --target $host, but it does indeed cause a lot of confusion and was likely a bad decision

gnzlbg commented 6 years ago

Could something be done about it in the 2018 edition? Probably would need an RFC, but that’s only worth writing if this can be technically changed without “too much hassle”.

alexcrichton commented 6 years ago

Perhaps! No matter what though it needs to be a smooth breaking change, if any.

rbalicki2 commented 6 years ago

Could someone clarify if the following is impossible given the above decision?

I have a dependency which relies on proc_macro2 being compiled with RUSTFLAGS='--cfg procmacro2_semver_exempt', as it uses span().start() and span().end(). I want to use this procedural macro in another library which targets wasm32-unknown-unknown.

I'd like to run RUSTFLAGS='--cfg procmacro2_semver_exempt --target wasm32-unknown-unknown' cargo +nightly build --verbose, but of course that doesn't work as the RUSTFLAGS is dropped.

I am open to doing things like: forking proc_macro2 (seems easiest), or manually listing out all of the rustc commands I need to execute (this seems possible, but I tried to execute all of the rustc commands printed when I ran cargo build --verbose, but it did not work. So I am presumably missing something.)

Thank you for your help in advance!

alexcrichton commented 6 years ago

@rbalicki2 hm I think that may be accidental poor planning on our part! Currently if you're cross compiling you'll want to configure CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_RUSTFLAGS=... as an environment variable or configure the [target.x86_64-unknown-linux-gnu.rustflags] key in .cargo/config.

rbalicki2 commented 6 years ago

Thanks! I will try that.

mbrubeck commented 6 years ago

@Boscop noticed that build.rustflags being passed to build scripts when --target omitted, but not when it is passed, causes extra rebuilds when building for multiple targets.

For example, if you have build.rustflags set in .cargo/config, then running cargo build; cargo build --target $OTHER; cargo build will cause all build scripts (and anything that depends on them) to be rebuilt every time. In practice this can mean that the third build command rebuilds nearly the entire dependency tree, even though everything should be cached from the first build.

A workaround is to run cargo build --target $host when not cross-compiling.

Boscop commented 6 years ago

[...] will cause all build scripts (and anything that depends on them) to be rebuilt.

(proc-macros and their deps, too.)

A workaround is to run cargo build --target $host when not cross-compiling.

That avoids the rebuild issue but it seems the rustflags aren't getting applied, so it's not really a working workaround. The resulting exe still links to the crt dynamically when I have this .cargo/config (so I guess the target-cpu setting is also ignored):

[build] 
target = "x86_64-pc-windows-msvc" 

[target.'cfg(all(target_arch = "x86_64", not(debug_assertions)))'] 
rustflags = ["-Ctarget-feature=+crt-static", "-Ctarget-cpu=haswell"]

So it has the same effect as no rustflags setting, that's why it "fixes" the rebuild issue.

Due to this (https://github.com/rust-lang/cargo/issues/5777) issue it doesn't seem to be possible right now to set different rustflags in release mode, so I switched to setting them in my justfile as env var in my release build recipe for now..

mbrubeck commented 6 years ago

The workaround also works for cases where rustflags are applied, e.g.

[build] 
target = "x86_64-pc-windows-msvc" 

[target.'cfg(target_arch = "x86_64")']
rustflags = ["-Ctarget-feature=+crt-static", "-Ctarget-cpu=haswell"]

The fact that there's no way to set rustflags for release builds only is a separate issue, as you note, but the workaround prevents rebuilds whether you hit that bug or not.

Boscop commented 6 years ago

Ah yes, originally I wanted to use those rust flags only for release, but I guess it's ok if they are also used for debug, but do they make the build take longer?

But for this use case I'd prefer to build my release build into target/release instead of a different folder and I had a justfile for post build packaging anyway so I'm keeping the env var solution for now. (But I'm keeping the workaround in mind for the future.. )

I hope it will soon be possible to use different rustflags for different profiles..

nox commented 5 years ago

This is hitting us hard in Servo. https://github.com/servo/servo/issues/20380#issuecomment-442039196

elichai commented 5 years ago

@alexcrichton I need to set the -C link-args=-zstack-size=X flag, but I want to choose the X at compile time. Is this not possible in build.rs right now? Do this mean I have to use a Makefile?

RalfJung commented 5 years ago

@elichai does this question have anything to do with the issue we are discussing here? From what I can tell, it does not. In that case, please open a new issue -- or, probably better for this, ask on Discord.

gnzlbg commented 5 years ago

Is this not possible in build.rs right now? Do this mean I have to use a Makefile?

@elichai this is possible in build.rs (see this, ping me on Discord or Zulip if you need help).

retep998 commented 5 years ago

@gnzlbg It's not possible to set arbitrary linker arguments in build.rs. Currently build scripts are limited to specifying libraries to link to and directories for the linker to search in.

gnzlbg commented 5 years ago

Indeed, I misremembered @elichai, buils.rs cannot do that. You can use a json target spec or a .cargo/config to specify these at compile-time, but then they are "static" - if you want to ship this somewhere, and have it do something else, then that's not a good solution. The targets specification of rustc_target are written in Rust and they can do these things programmatically.

elichai commented 5 years ago

I guess i'll need to use a Makefile for that. @retep998 Is this a wanted feature? If so I can work on a PR to cargo

nox commented 5 years ago

@gnzlbg for historical reasons cargo build is subtly different from cargo build --target $host, but it does indeed cause a lot of confusion and was likely a bad decision

Is there any documentation about those historical reasons somewhere? What would be the proper way to change that? Would a crater run make us confident that aligning cargo build with cargo build --target $host is ok?

retep998 commented 5 years ago

@elichai The problem is that while it is a wanted feature, the big blocker is not implementing it but rather convincing the appropriate teams that it should be exposed. There is currently opposition to this feature on the basis of it trying Rust to a particular linker implementation (which would make it harder to switch to rustc using LLD as a library for example), or hampering the user's ability to compile your code for other platforms.

wkennington commented 5 years ago

This makes it much harder for us to build rustc when our rust-std installation is separate from rustc. Normally, we pass RUSTFLAGS to cargo to target the correct rust-std location. For building the initial libtest, we need the build.rs to link against the old rust-std libs but the libtest to link against the new libstd. We currently have no way to override the flags for just the build process. We could make a wrapper around rustc to always append our flags, but this breaks compiling against the new libstd since we get duplicate crates in the search path (this is definitely undesirable). It's not trivially possible to tell whether or not we are building the target crate or build script artifacts.

If I could just pass RUSTFLAGS_(BUILD|HOST) it would make this easy.

QuietMisdreavus commented 4 years ago

This also happens with cargo doc, proc-macros, and RUSTDOCFLAGS; see https://github.com/rust-lang/rust/issues/66796. It's causing issues with docs.rs collecting docs for proc-macro crates.

michaelforney commented 4 years ago

I'm running into this issue when trying to build firefox for x86_64-unknown-linux-musl. Since the default for this target is crt-static, I have set target.x86_64-unknown-linux-musl.rustflags to ["-C", "target-feature=-crt-static"] in my ~/.cargo/config.

However, the firefox build system always passes --target=... to cargo, which is preventing the flags from getting applied, producing the error:

error: cannot produce proc-macro for `cssparser-macros v0.3.6` as the target `x86_64-unknown-linux-musl` does not support these crate types

The only workarounds I've found for this are

Is there any workaround that doesn't involve any patching and works with the musl rust toolchains obtained through rustup?

saurik commented 3 years ago

FWIW, the reason why #9089 ended up mentioning this issue is because these build scripts are being built using CARGO_TARGET_..._LINKER, and yet ignore CARGO_TARGET_..._RUSTFLAGS, which might include special/magic target-specific -C link-arg= arguments to make the special/magic target-specific linker actually link; and so without the ability to have the build scripts consistently use my rustflags for my linker or, at least, just not try to use my linker (I would be totally happy with this: if one wants to configure the linker for build scripts, maybe a CARGO_BUILD_LINKER is needed... though, that begs the question as to why CARGO_BUILD_RUSTFLAGS doesn't work for build scripts, as I'm not sure what it actually means otherwise? ;P), I can't get builds to work at all with some compiler stacks :(.

jameshilliard commented 3 years ago

There are some fixes that might help here in #9322.

jonhoo commented 2 years ago

I think this is probably a better place to mention it, so I'll carry over my comment from https://github.com/rust-lang/cargo/pull/10395#issuecomment-1055691480 here:

I wonder if the way to go here is to take a lesson from https://github.com/alexcrichton/cc-rs/pull/9 and add support for RUSTFLAGS_$TARGET and the like. That way we can say that RUSTFLAGS applies to all targets (including the host), TARGET_RUSTFLAGS applies only to the target(s), HOST_RUSTFLAGS applies only to the host, RUSTFLAGS_$TARGET applies only to $TARGET, etc. What do you think?

messense commented 2 years ago

@jonhoo I really want this and I want TARGET_RUSTFLAGS take precedence over the existing RUSTFLAGS. Cargo already supports CARGO_TARGET_<triple>_RUSTFLAGS but unfortunately it has lower priority over RUSTFLAGS which could lead to breakage when user specified RUSTFLAGS.

  1. https://github.com/BLAKE3-team/BLAKE3/pull/101#issuecomment-1057777114
  2. The fix: https://github.com/messense/cargo-xwinbuild/commit/49a15a06b5b2563bfc989ff508793cf95ae8571c