Open jonhoo opened 2 years ago
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
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:
This assumption is also present here:
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`
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.
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.
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.
Fuchsia also just ran into this in #94003 🤔
The -Z flag is target-applies-to-host
, but it doesn't work.
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.
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
🤔
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.
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 😅
I missed the early continue
for proc-macros when the host prefix is matched (which is now identical to the target prefix) here:
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.
I wonder if I'm running into the fact that with --target
more artifacts are supposedly generated (https://github.com/rust-lang/cargo/issues/5730#issuecomment-405629218). 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 https://github.com/rust-lang/cargo/issues/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.
Fix for the Cargo issue is in https://github.com/rust-lang/cargo/pull/10395.
When https://github.com/rust-lang/cargo/pull/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?
I think we're seeing this in Chromium, when trying to bootstrap our Rust toolchain: https://issues.chromium.org/issues/40280340
In https://github.com/rust-lang/rust/pull/123593#issuecomment-2041550659, 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
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-setRUSTFLAGS
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.
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.
The CARGO_HOST_*
env vars should work.
That'll work without -Zhost-config
?
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
.
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 tox.py
(gets picked up here), those variables are forwarded asRUSTFLAGS
here:https://github.com/rust-lang/rust/blob/c5c610aad0a012a9228ecb83cc19e77111a52140/src/bootstrap/builder.rs#L1856
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 acargo
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:https://github.com/rust-lang/rust/blob/c5c610aad0a012a9228ecb83cc19e77111a52140/src/bootstrap/builder.rs#L1001
This is problematic when
RUSTFLAGS
includes arguments that are critical to building, most notably in the form of-Clink-arg
s 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
tocargo
in that instance. That way build scripts will respectRUSTFLAGS
again, though it may still pose a problem for cross-compiling use-cases, I'm not sure?Meta
rustc --version --verbose
: