bootstrap: RUSTFLAGS_*_BOOTSTRAP does not affect build scripts

[jonhoo]

It's unclear if this should be a bug report for Rust's bootstrap or not, but it feels like it's appropriate if only for tracking the problem for when others run into the same. I think closing this is also reasonable.

When RUSTFLAGS_BOOTSTRAP/RUSTFLAGS_NOT_BOOSTRAP is passed to to x.py (gets picked up here), those variables are forwarded as RUSTFLAGS here:

rust/src/bootstrap/builder.rs

Line 1856 in c5c610a

cargo.command.env("RUSTFLAGS", rustflags);

This works most of the time. However, there is a known bug in Cargo (rust-lang/cargo#5754 / rust-lang/cargo#4423) that makes it so that if --target is passed to a cargo invocation, even if the passed triple is the same as the host triple, RUSTFLAGS is not passed to build scripts. And since bootstrap always passes --target, RUSTFLAGS_*_BOOTSTRAP do not take effect for builds scripts:

rust/src/bootstrap/builder.rs

Line 1001 in c5c610a

cargo.arg("--target").arg(target.rustc_target_arg());

This is problematic when RUSTFLAGS includes arguments that are critical to building, most notably in the form of -Clink-args without which any attempt to link may fail.

There isn't an obvious fix to the problem beyond fixing the upstream problem(s) in Cargo. One possibility is to detect when target == host in the bootstrap logic and avoid passing --target to cargo in that instance. That way build scripts will respect RUSTFLAGS again, though it may still pose a problem for cross-compiling use-cases, I'm not sure?

Meta

rustc --version --verbose:

rustc 1.58.1 (db9d1b20b 2022-01-20)
binary: rustc
commit-hash: db9d1b20bba1968c1ec1fc49616d4742c1725b4b
commit-date: 2022-01-20
host: x86_64-unknown-linux-gnu
release: 1.58.1
LLVM version: 13.0.0

[jonhoo]

I just tried a quick-n-dirty build where I removed the line above that adds --target to every cargo invocation, and unfortunately then the build panics at

rust/src/bootstrap/compile.rs

Line 1325 in c5c610a

let contents = t!(target_deps_dir.read_dir())

with

thread 'main' panicked at 'target_deps_dir.read_dir() failed with No such file or directory (os error 2)', src/bootstrap/compile.rs:1307:20
stack backtrace:
   0: rust_begin_unwind
             at /rustc/db9d1b20bba1968c1ec1fc49616d4742c1725b4b/library/std/src/panicking.rs:498:5
   1: core::panicking::panic_fmt
             at /rustc/db9d1b20bba1968c1ec1fc49616d4742c1725b4b/library/core/src/panicking.rs:107:14
   2: bootstrap::compile::run_cargo
             at ./src/bootstrap/compile.rs:1307:20
   3: <bootstrap::compile::Std as bootstrap::builder::Step>::run
             at ./src/bootstrap/compile.rs:114:9
   4: bootstrap::builder::Builder::ensure
             at ./src/bootstrap/builder.rs:1585:23
   5: <bootstrap::compile::Rustc as bootstrap::builder::Step>::run
             at ./src/bootstrap/compile.rs:559:9
   6: bootstrap::builder::Builder::ensure
             at ./src/bootstrap/builder.rs:1585:23
   7: <bootstrap::compile::Assemble as bootstrap::builder::Step>::run
             at ./src/bootstrap/compile.rs:1081:9
   8: bootstrap::builder::Builder::ensure
             at ./src/bootstrap/builder.rs:1585:23
   9: bootstrap::builder::Builder::compiler
             at ./src/bootstrap/builder.rs:624:9
  10: <bootstrap::compile::Assemble as bootstrap::builder::Step>::run
             at ./src/bootstrap/compile.rs:1068:30
  11: bootstrap::builder::Builder::ensure
             at ./src/bootstrap/builder.rs:1585:23
  12: bootstrap::builder::Builder::compiler
             at ./src/bootstrap/builder.rs:624:9
  13: <bootstrap::dist::Rustc as bootstrap::builder::Step>::make_run
             at ./src/bootstrap/dist.rs:324:39
  14: bootstrap::builder::StepDescription::maybe_run
             at ./src/bootstrap/builder.rs:175:13
  15: bootstrap::builder::StepDescription::run
             at ./src/bootstrap/builder.rs:211:25
  16: bootstrap::builder::Builder::run_step_descriptions
             at ./src/bootstrap/builder.rs:616:9
  17: bootstrap::builder::Builder::execute_cli
             at ./src/bootstrap/builder.rs:596:9
  18: bootstrap::Build::build
             at ./src/bootstrap/lib.rs:623:13
  19: bootstrap::main
             at ./src/bootstrap/bin/main.rs:33:5
  20: core::ops::function::FnOnce::call_once
             at /rustc/db9d1b20bba1968c1ec1fc49616d4742c1725b4b/library/core/src/ops/function.rs:227:5 

I believe that happens due to the hard-coding of target in the build output directory path here:

rust/src/bootstrap/lib.rs

Line 727 in c5c610a

self.stage_out(compiler, mode).join(&*target.triple).join(self.cargo_dir())

This assumption is also present here:

rust/src/bootstrap/compile.rs

Lines 1241 to 1251 in c5c610a

	// `target_root_dir` looks like $dir/$target/release
	let target_root_dir = stamp.parent().unwrap();
	// `target_deps_dir` looks like $dir/$target/release/deps
	let target_deps_dir = target_root_dir.join("deps");
	// `host_root_dir` looks like $dir/release
	let host_root_dir = target_root_dir
	.parent()
	.unwrap() // chop off `release`
	.parent()
	.unwrap() // chop off `$target`
	.join(target_root_dir.file_name().unwrap());

[jonhoo]

For others who run into this, this patch worked for me against Rust 1.58.1, though should almost certainly not be upstreamed in its current form:

diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs
index 6ba1b1b6036..d389209150f 100644
--- a/src/bootstrap/builder.rs
+++ b/src/bootstrap/builder.rs
@@ -898,11 +898,7 @@ pub fn cargo(
             Color::Auto => {} // nothing to do
         }

-        if cmd != "install" {
-            cargo.arg("--target").arg(target.rustc_target_arg());
-        } else {
-            assert_eq!(target, compiler.host);
-        }
+        assert_eq!(target, compiler.host);

         // Set a flag for `check`/`clippy`/`fix`, so that certain build
         // scripts can do less work (i.e. not building/requiring LLVM).
diff --git a/src/bootstrap/compile.rs b/src/bootstrap/compile.rs
index 007ca9f7f5a..a6859b297aa 100644
--- a/src/bootstrap/compile.rs
+++ b/src/bootstrap/compile.rs
@@ -1225,12 +1225,7 @@ pub fn run_cargo(
     // `target_deps_dir` looks like $dir/$target/release/deps
     let target_deps_dir = target_root_dir.join("deps");
     // `host_root_dir` looks like $dir/release
-    let host_root_dir = target_root_dir
-        .parent()
-        .unwrap() // chop off `release`
-        .parent()
-        .unwrap() // chop off `$target`
-        .join(target_root_dir.file_name().unwrap());
+    let host_root_dir = target_root_dir.clone();

     // Spawn Cargo slurping up its JSON output. We'll start building up the
     // `deps` array of all files it generated along with a `toplevel` array of
diff --git a/src/bootstrap/lib.rs b/src/bootstrap/lib.rs
index 1667dfc3f85..04a7036cf4b 100644
--- a/src/bootstrap/lib.rs
+++ b/src/bootstrap/lib.rs
@@ -729,8 +733,8 @@ fn stage_out(&self, compiler: Compiler, mode: Mode) -> PathBuf {
     /// Returns the root output directory for all Cargo output in a given stage,
     /// running a particular compiler, whether or not we're building the
     /// standard library, and targeting the specified architecture.
-    fn cargo_out(&self, compiler: Compiler, mode: Mode, target: TargetSelection) -> PathBuf {
-        self.stage_out(compiler, mode).join(&*target.triple).join(self.cargo_dir())
+    fn cargo_out(&self, compiler: Compiler, mode: Mode, _target: TargetSelection) -> PathBuf {
+        self.stage_out(compiler, mode).join(self.cargo_dir())
     }

     /// Root output directory for LLVM compiled for `target`

[Mark-Simulacrum]

Yeah, I'm pretty sure this needs upstream support in Cargo if we want a nice workaround (somehow).

Locally to rustbuild, we could manually pass through opts in our wrapper shim in theory, but I would prefer not to try to fix this in rustbuild rather than upstream, but if there's a -Z flag or similar that could get passed that would likely be fine.

[jonhoo]

Unfortunately I don't think there's a -Z flag to make "the right thing" happen here. @alexcrichton sorry to loop you in, but I think your familiarity with the intricacies may be necessary to point us towards the best workaround here. Which, to be fair, could be "if this hits you, apply the above patch before building" in the short-to-medium-term.

[Mark-Simulacrum]

Yes, I meant that given the backwards compat concern with just changing RUSTFLAGS behavior in Cargo, a -Z flag to opt-in may be one path forward.

[tmandry]

Fuchsia also just ran into this in #94003 🤔

[jyn514]

The -Z flag is target-applies-to-host, but it doesn't work.

[alexcrichton]

This is what -Zhost-config is for but testing it locally I don't think it's ready for this use case yet, I think it may be buggy internally or not have a finished implementation.

[jonhoo]

Interestingly, the patch I posted above (and probably more generally when the bootstrap rustflags are applied to build scripts) somehow trips up indexmap's build.rs std detection when building stage0 compiler artifacts and makes it think it doesn't have std available when it does. This in turn causes it to not provide a default type for the its build hasher generic argument (S), leading to errors like:

error[E0107]: this struct takes 3 generic arguments but 2 generic arguments were supplied
   --> /rustc/1.58.1/vendor/petgraph/src/graphmap.rs:580:16
    |
580 |     edges: &'a IndexMap<(N, N), E>,
    |                ^^^^^^^^ ------  - supplied 2 generic arguments
    |                |
    |                expected 3 generic arguments
    |
help: add missing generic argument
    |
580 |     edges: &'a IndexMap<(N, N), E, S>,
    |                                  +++ 

Still digging into why that is. The RUSTFLAGS that gets passed down to the build script (that weren't previously) are:

-Zsymbol-mangling-version=v0
-Zmacro-backtrace
-Clink-args=-Wl,-rpath,$ORIGIN/../lib
-Ztls-model=initial-exec
-Zunstable-options
-Wrustc::internal
-Cprefer-dynamic

@cuviper given your knowledge of autocfg and indexmap, any idea why the above rustflags may cause indexmap's build.rs to conclude that std isn't available? Interestingly enough, the STDERR of the build script holds

error[E0463]: can't find crate for `std`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0463`.
error[E0463]: can't find crate for `core`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0463`.
warning: autocfg could not probe for `std`
error[E0463]: can't find crate for `std`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0463`.

Which suggests that the problem may be somewhere higher up that std isn't made available in the first place for some reason just because we don't build it with --target 🤔

[cuviper]

I only added CARGO_ENCODED_RUSTFLAGS recently in autocfg 1.1.0, so the vendor in Rust 1.58.1 is probably not using the right flags at all, since cargo has been un-setting RUSTFLAGS entirely after the encoded version landed.

[jonhoo]

Ah, right, so these flags would be applied to the compilation of indexmap's build.rs, but would not actually be passed to build.rs. That makes it sound even more as though what's at fault here is that there's some logic I missed that tries to copy stage0's std under the assumption that --target being passed to cargo. The digging continues!

You make a good point though that I'll probably also need to update the vendored autocfg so that its compile probe also gets the right rustflags. What a rabbit hole 😅

[jonhoo]

I missed the early continue for proc-macros when the host prefix is matched (which is now identical to the target prefix) here:

rust/src/bootstrap/compile.rs

Lines 1281 to 1289 in 6bf3008

	// If this was an output file in the "host dir" we don't actually
	// worry about it, it's not relevant for us
	if filename.starts_with(&host_root_dir) {
	// Unless it's a proc macro used in the compiler
	if crate_types.iter().any(|t| t == "proc-macro") {
	deps.push((filename.to_path_buf(), DependencyType::Host));
	}
	continue;
	}

Still figuring out how to properly replace that check. My earlier patch seems to fail to find the stage0 libstd, and instead links the libstd from the bootstrapping compiler, which is no good. Will post an updated patch once I have one.

[jonhoo]

I wonder if I'm running into the fact that with --target more artifacts are supposedly generated (rust-lang/cargo#5730 (comment)). It may be that I instead need to resort to setting CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_RUSTFLAGS, modifying .cargo/config, or something along those lines. Or fix -Ztarget-is-host 😅

EDIT: Looking at the root cause for rust-lang/cargo#10206, I don't think any workaround will work here — either --target must be left off, or -Ztarget-is-host must be fixed. Fun times. The former is turning out to be quite painful to fix, and I suspect it may not be doable due to just how ingrained the assumption about --target being passed is to the bootstrapping process, so I guess I'll get cracking on the latter.

[jonhoo]

Fix for the Cargo issue is in rust-lang/cargo#10395.

[jonhoo]

When rust-lang/cargo#10395 lands, I think there's still an open question about what Rust's bootstrap logic should do. Should it generate a [host] section in .cargo/config to ensure its rustflags also apply to the build scripts? Or should it not, and make callers generate said .cargo/config if they want rustflags to apply to build scripts (in which case the bootstrap-generated rustflags will not apply to build scripts)? Does it make sense for all the bootstrap-set RUSTFLAGS to apply to both the host and target platform, or should some of them only apply to one or the other?

[chbaker0]

I think we're seeing this in Chromium, when trying to bootstrap our Rust toolchain: https://issues.chromium.org/issues/40280340

[onur-ozkan]

In #123593 (comment), I attempted to solve the issue with the host RUSTFLAGS but it turned out to be problematic (causing conflicts with design of the bootstrap stages). I'm wondering why cargo behaves this way regarding the --target flag and whether it assumes that when --target is used, RUSTFLAGS shouldn't be passed. Does it make this assumption because it considers the target to be a cross-target? Or is there another reason behind it? If it's because it assumes it's a cross-target then it should verify if it actually is (this should be quite easy check I think?), since --target could refer to the host triple.

This isn't a bug, so.. @rustbot label -C-bug

[onur-ozkan]

When rust-lang/cargo#10395 lands, I think there's still an open question about what Rust's bootstrap logic should do. Should it generate a [host] section in .cargo/config to ensure its rustflags also apply to the build scripts? Or should it not, and make callers generate said .cargo/config if they want rustflags to apply to build scripts (in which case the bootstrap-generated rustflags will not apply to build scripts)? Does it make sense for all the bootstrap-set RUSTFLAGS to apply to both the host and target platform, or should some of them only apply to one or the other?

Given that bootstrapping is inherently somewhat a hacky flow, I would prefer leaving such solutions (e.g., generating a [host] section in .cargo/config) to be handled externally, as long as it's feasible and not significantly more complex than handling them in the bootstrap.

[chbaker0]

I would like to use a [host] section in a .cargo/config.toml for my project's workflow, but it seems bootstrap auto-deletes .cargo/config.toml if vendoring is not enabled.

[bjorn3]

The CARGO_HOST_* env vars should work.

[chbaker0]

That'll work without -Zhost-config?

[bjorn3]

Based on how other configs can be set using env vars, I expect CARGO_UNSTABLE_HOST_CONFIG=true would work in the place of -Zhost-config.