bootstrap: fix panic when repo path contains spaces by switching to CARGO_ENCODED_RUSTFLAGS#158073
Conversation
|
Thanks for the pull request, and welcome! The Rust Project is excited to review your changes, and you should hear from @Mark-Simulacrum (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
Why was this reviewer chosen?The reviewer was selected based on:
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
cc @rust-lang/miri |
|
Can you compare with a previous PR in this area (#156096) and evaluate whether there's anything from there that should also be done here? I'm also curious if we can get some kind of testing in place for this. Maybe one of the CI runners can adjust its build directory to have a space in it? |
Sure i will look into it ! |
|
Updated to use String with \x1f as the internal delimiter (matching #156096's approach) and added the \x1f assertion in arg(). For the CI test, I can add --set build.build-dir with a space in the path to one existing Linux job in jobs.yml would that be the right approach? cc : @Mark-Simulacrum |
|
This looks reasonable to me; at this point it is essentially the same as #156096. I don't think that we have to test this in CI, it will just create another testing hack in a bash script that we can't run locally through bootstrap. We don't test much more important things in bootstrap on our CI.. 😆 |
Thanks! Happy to skip the CI test then. Let me know if there's anything else needed before this can be merged. |
| let rustdocflags = &cargo.rustdocflags.0; | ||
| if !rustdocflags.is_empty() { | ||
| cargo.command.env("RUSTDOCFLAGS", rustdocflags); | ||
| cargo.command.env("CARGO_ENCODED_RUSTDOCFLAGS", rustdocflags); |
There was a problem hiding this comment.
Could we explicitly unset RUSTFLAGS/RUSTDOCFLAGS as well, just to avoid any confusion over those having different values? I assume Cargo will prioritize the encoded form but not sure if downstream things (e.g., build.rs scripts) will do so.
There was a problem hiding this comment.
Done, I now explicitly unset RUSTFLAGS and RUSTDOCFLAGS before setting the encoded forms, so cargo and any build.rs scripts only see CARGO_ENCODED_RUSTFLAGS/CARGO_ENCODED_RUSTDOCFLAGS.
Any flags from the caller's environment have already been folded into the Rustflags struct via propagate_cargo_env, so nothing is lost.
|
r? Kobzol |
|
|
|
cc : @Kobzol |
|
Thanks, let's try. @bors r=kobzol,mark-simulacrum,bjorn3 |
… r=kobzol,mark-simulacrum,bjorn3 bootstrap: fix panic when repo path contains spaces by switching to CARGO_ENCODED_RUSTFLAGS Fixes rust-lang#158052 Closes: rust-lang#156096 ## Problem `./x build` panics with a cryptic assertion error when the repository is checked out under a directory path containing spaces (e.g. `/Users/foo/Open Source/rust`): thread 'main' panicked at src/bootstrap/src/core/builder/cargo.rs:54:9: assertion left == right failed left: 2 right: 1 The root cause: when building tools in `ToolRustcPrivate` or `Codegen` mode, bootstrap calls `llvm-config --libdir` and passes the result as a `-Clink-arg=-L<path>` rustflag. The `Rustflags::arg()` method asserted that arguments contain no spaces, but if the repo path has a space the libdir path inherits it and the assertion fires. The error gives no hint that the path is the problem. A secondary bug: `llvm-config --libdir` output has a trailing newline that was previously stripped accidentally by `RUSTFLAGS` whitespace splitting. Nothing was trimming it explicitly. ## Fix Two changes in `src/bootstrap/src/core/builder/cargo.rs`: 1. **Switch `RUSTFLAGS` → `CARGO_ENCODED_RUSTFLAGS`**: Change `Rustflags` to store args as `Vec<String>` and join with `\x1f` (ASCII unit separator) when setting the env var. Cargo's `CARGO_ENCODED_RUSTFLAGS` (stable since Cargo 1.55) uses `\x1f` as delimiter, which never appears in filesystem paths, so paths with spaces are handled correctly. The space-based assertion in `arg()` is removed. 2. **Trim `llvm-config --libdir` output**: Explicitly `.trim()` the captured stdout so the trailing newline is not included in the linker search path. `RUSTDOCFLAGS` is left as-is (space-joined) since no llvm paths are added to rustdocflags. cc: @Kobzol
… r=kobzol,mark-simulacrum,bjorn3 bootstrap: fix panic when repo path contains spaces by switching to CARGO_ENCODED_RUSTFLAGS Fixes rust-lang#158052 Closes: rust-lang#156096 ## Problem `./x build` panics with a cryptic assertion error when the repository is checked out under a directory path containing spaces (e.g. `/Users/foo/Open Source/rust`): thread 'main' panicked at src/bootstrap/src/core/builder/cargo.rs:54:9: assertion left == right failed left: 2 right: 1 The root cause: when building tools in `ToolRustcPrivate` or `Codegen` mode, bootstrap calls `llvm-config --libdir` and passes the result as a `-Clink-arg=-L<path>` rustflag. The `Rustflags::arg()` method asserted that arguments contain no spaces, but if the repo path has a space the libdir path inherits it and the assertion fires. The error gives no hint that the path is the problem. A secondary bug: `llvm-config --libdir` output has a trailing newline that was previously stripped accidentally by `RUSTFLAGS` whitespace splitting. Nothing was trimming it explicitly. ## Fix Two changes in `src/bootstrap/src/core/builder/cargo.rs`: 1. **Switch `RUSTFLAGS` → `CARGO_ENCODED_RUSTFLAGS`**: Change `Rustflags` to store args as `Vec<String>` and join with `\x1f` (ASCII unit separator) when setting the env var. Cargo's `CARGO_ENCODED_RUSTFLAGS` (stable since Cargo 1.55) uses `\x1f` as delimiter, which never appears in filesystem paths, so paths with spaces are handled correctly. The space-based assertion in `arg()` is removed. 2. **Trim `llvm-config --libdir` output**: Explicitly `.trim()` the captured stdout so the trailing newline is not included in the linker search path. `RUSTDOCFLAGS` is left as-is (space-joined) since no llvm paths are added to rustdocflags. cc: @Kobzol
Rollup of 6 pull requests Successful merges: - #158073 (bootstrap: fix panic when repo path contains spaces by switching to CARGO_ENCODED_RUSTFLAGS) - #158256 (Avoid parser panics bubbling out to proc macros) - #158081 (trait-system: Recover deferred closure calls after errors) - #158323 (rustc: improve diagnostics for file-open failures) - #158327 (Move attribute and keyword docs from `std` to `core`) - #158468 (Include default-stability info in rustdoc JSON.)
… r=kobzol,mark-simulacrum,bjorn3 bootstrap: fix panic when repo path contains spaces by switching to CARGO_ENCODED_RUSTFLAGS Fixes rust-lang#158052 Closes: rust-lang#156096 ## Problem `./x build` panics with a cryptic assertion error when the repository is checked out under a directory path containing spaces (e.g. `/Users/foo/Open Source/rust`): thread 'main' panicked at src/bootstrap/src/core/builder/cargo.rs:54:9: assertion left == right failed left: 2 right: 1 The root cause: when building tools in `ToolRustcPrivate` or `Codegen` mode, bootstrap calls `llvm-config --libdir` and passes the result as a `-Clink-arg=-L<path>` rustflag. The `Rustflags::arg()` method asserted that arguments contain no spaces, but if the repo path has a space the libdir path inherits it and the assertion fires. The error gives no hint that the path is the problem. A secondary bug: `llvm-config --libdir` output has a trailing newline that was previously stripped accidentally by `RUSTFLAGS` whitespace splitting. Nothing was trimming it explicitly. ## Fix Two changes in `src/bootstrap/src/core/builder/cargo.rs`: 1. **Switch `RUSTFLAGS` → `CARGO_ENCODED_RUSTFLAGS`**: Change `Rustflags` to store args as `Vec<String>` and join with `\x1f` (ASCII unit separator) when setting the env var. Cargo's `CARGO_ENCODED_RUSTFLAGS` (stable since Cargo 1.55) uses `\x1f` as delimiter, which never appears in filesystem paths, so paths with spaces are handled correctly. The space-based assertion in `arg()` is removed. 2. **Trim `llvm-config --libdir` output**: Explicitly `.trim()` the captured stdout so the trailing newline is not included in the linker search path. `RUSTDOCFLAGS` is left as-is (space-joined) since no llvm paths are added to rustdocflags. cc: @Kobzol
Rollup of 12 pull requests Successful merges: - #158073 (bootstrap: fix panic when repo path contains spaces by switching to CARGO_ENCODED_RUSTFLAGS) - #158169 (Fix debuginfo compression in bootstrap) - #158256 (Avoid parser panics bubbling out to proc macros) - #158375 (Support `DefKind::InlineConst` in `ConstKind::Unevaluated`) - #158417 (Avoid ICE when cfg_eval recovers no item from derive input) - #158556 (delegation: store child segment flag in `PathSegment`) - #158561 (Avoid building rustdoc for tests without doctests) - #158562 (Improve tracing of steps in bootstrap) - #157445 (Allow section override when using patchable-function-entries) - #158081 (trait-system: Recover deferred closure calls after errors) - #158327 (Move attribute and keyword docs from `std` to `core`) - #158468 (Include default-stability info in rustdoc JSON.)
… r=kobzol,mark-simulacrum,bjorn3 bootstrap: fix panic when repo path contains spaces by switching to CARGO_ENCODED_RUSTFLAGS Fixes rust-lang#158052 Closes: rust-lang#156096 ## Problem `./x build` panics with a cryptic assertion error when the repository is checked out under a directory path containing spaces (e.g. `/Users/foo/Open Source/rust`): thread 'main' panicked at src/bootstrap/src/core/builder/cargo.rs:54:9: assertion left == right failed left: 2 right: 1 The root cause: when building tools in `ToolRustcPrivate` or `Codegen` mode, bootstrap calls `llvm-config --libdir` and passes the result as a `-Clink-arg=-L<path>` rustflag. The `Rustflags::arg()` method asserted that arguments contain no spaces, but if the repo path has a space the libdir path inherits it and the assertion fires. The error gives no hint that the path is the problem. A secondary bug: `llvm-config --libdir` output has a trailing newline that was previously stripped accidentally by `RUSTFLAGS` whitespace splitting. Nothing was trimming it explicitly. ## Fix Two changes in `src/bootstrap/src/core/builder/cargo.rs`: 1. **Switch `RUSTFLAGS` → `CARGO_ENCODED_RUSTFLAGS`**: Change `Rustflags` to store args as `Vec<String>` and join with `\x1f` (ASCII unit separator) when setting the env var. Cargo's `CARGO_ENCODED_RUSTFLAGS` (stable since Cargo 1.55) uses `\x1f` as delimiter, which never appears in filesystem paths, so paths with spaces are handled correctly. The space-based assertion in `arg()` is removed. 2. **Trim `llvm-config --libdir` output**: Explicitly `.trim()` the captured stdout so the trailing newline is not included in the linker search path. `RUSTDOCFLAGS` is left as-is (space-joined) since no llvm paths are added to rustdocflags. cc: @Kobzol
… r=kobzol,mark-simulacrum,bjorn3 bootstrap: fix panic when repo path contains spaces by switching to CARGO_ENCODED_RUSTFLAGS Fixes rust-lang#158052 Closes: rust-lang#156096 ## Problem `./x build` panics with a cryptic assertion error when the repository is checked out under a directory path containing spaces (e.g. `/Users/foo/Open Source/rust`): thread 'main' panicked at src/bootstrap/src/core/builder/cargo.rs:54:9: assertion left == right failed left: 2 right: 1 The root cause: when building tools in `ToolRustcPrivate` or `Codegen` mode, bootstrap calls `llvm-config --libdir` and passes the result as a `-Clink-arg=-L<path>` rustflag. The `Rustflags::arg()` method asserted that arguments contain no spaces, but if the repo path has a space the libdir path inherits it and the assertion fires. The error gives no hint that the path is the problem. A secondary bug: `llvm-config --libdir` output has a trailing newline that was previously stripped accidentally by `RUSTFLAGS` whitespace splitting. Nothing was trimming it explicitly. ## Fix Two changes in `src/bootstrap/src/core/builder/cargo.rs`: 1. **Switch `RUSTFLAGS` → `CARGO_ENCODED_RUSTFLAGS`**: Change `Rustflags` to store args as `Vec<String>` and join with `\x1f` (ASCII unit separator) when setting the env var. Cargo's `CARGO_ENCODED_RUSTFLAGS` (stable since Cargo 1.55) uses `\x1f` as delimiter, which never appears in filesystem paths, so paths with spaces are handled correctly. The space-based assertion in `arg()` is removed. 2. **Trim `llvm-config --libdir` output**: Explicitly `.trim()` the captured stdout so the trailing newline is not included in the linker search path. `RUSTDOCFLAGS` is left as-is (space-joined) since no llvm paths are added to rustdocflags. cc: @Kobzol
… r=kobzol,mark-simulacrum,bjorn3 bootstrap: fix panic when repo path contains spaces by switching to CARGO_ENCODED_RUSTFLAGS Fixes rust-lang#158052 Closes: rust-lang#156096 ## Problem `./x build` panics with a cryptic assertion error when the repository is checked out under a directory path containing spaces (e.g. `/Users/foo/Open Source/rust`): thread 'main' panicked at src/bootstrap/src/core/builder/cargo.rs:54:9: assertion left == right failed left: 2 right: 1 The root cause: when building tools in `ToolRustcPrivate` or `Codegen` mode, bootstrap calls `llvm-config --libdir` and passes the result as a `-Clink-arg=-L<path>` rustflag. The `Rustflags::arg()` method asserted that arguments contain no spaces, but if the repo path has a space the libdir path inherits it and the assertion fires. The error gives no hint that the path is the problem. A secondary bug: `llvm-config --libdir` output has a trailing newline that was previously stripped accidentally by `RUSTFLAGS` whitespace splitting. Nothing was trimming it explicitly. ## Fix Two changes in `src/bootstrap/src/core/builder/cargo.rs`: 1. **Switch `RUSTFLAGS` → `CARGO_ENCODED_RUSTFLAGS`**: Change `Rustflags` to store args as `Vec<String>` and join with `\x1f` (ASCII unit separator) when setting the env var. Cargo's `CARGO_ENCODED_RUSTFLAGS` (stable since Cargo 1.55) uses `\x1f` as delimiter, which never appears in filesystem paths, so paths with spaces are handled correctly. The space-based assertion in `arg()` is removed. 2. **Trim `llvm-config --libdir` output**: Explicitly `.trim()` the captured stdout so the trailing newline is not included in the linker search path. `RUSTDOCFLAGS` is left as-is (space-joined) since no llvm paths are added to rustdocflags. cc: @Kobzol
…uwer Rollup of 16 pull requests Successful merges: - #155722 (Introduce aarch64-unknown-linux-pauthtest target) - #156230 (tests: check wasm compiler_builtins object architecture) - #158073 (bootstrap: fix panic when repo path contains spaces by switching to CARGO_ENCODED_RUSTFLAGS) - #158169 (Fix debuginfo compression in bootstrap) - #158256 (Avoid parser panics bubbling out to proc macros) - #158375 (Support `DefKind::InlineConst` in `ConstKind::Unevaluated`) - #158556 (delegation: store child segment flag in `PathSegment`) - #158561 (Avoid building rustdoc for tests without doctests) - #158562 (Improve tracing of steps in bootstrap) - #157445 (Allow section override when using patchable-function-entries) - #158081 (trait-system: Recover deferred closure calls after errors) - #158327 (Move attribute and keyword docs from `std` to `core`) - #158468 (Include default-stability info in rustdoc JSON.) - #158564 (fix `-Z min-recursion-limit` unstable chapter name) - #158568 (llvm-wrapper: use accessors for private fields in LLVM 23+) - #158582 (Comment on needed RAM in huge-stacks.rs)
…uwer Rollup of 16 pull requests Successful merges: - #155722 (Introduce aarch64-unknown-linux-pauthtest target) - #156230 (tests: check wasm compiler_builtins object architecture) - #158073 (bootstrap: fix panic when repo path contains spaces by switching to CARGO_ENCODED_RUSTFLAGS) - #158169 (Fix debuginfo compression in bootstrap) - #158256 (Avoid parser panics bubbling out to proc macros) - #158375 (Support `DefKind::InlineConst` in `ConstKind::Unevaluated`) - #158556 (delegation: store child segment flag in `PathSegment`) - #158561 (Avoid building rustdoc for tests without doctests) - #158562 (Improve tracing of steps in bootstrap) - #157445 (Allow section override when using patchable-function-entries) - #158081 (trait-system: Recover deferred closure calls after errors) - #158327 (Move attribute and keyword docs from `std` to `core`) - #158468 (Include default-stability info in rustdoc JSON.) - #158564 (fix `-Z min-recursion-limit` unstable chapter name) - #158568 (llvm-wrapper: use accessors for private fields in LLVM 23+) - #158582 (Comment on needed RAM in huge-stacks.rs)
|
@bors try jobs=dist-x86_64-msvc |
This comment has been minimized.
This comment has been minimized.
bootstrap: fix panic when repo path contains spaces by switching to CARGO_ENCODED_RUSTFLAGS try-job: dist-x86_64-msvc
…uwer Rollup of 7 pull requests Successful merges: - #158073 (bootstrap: fix panic when repo path contains spaces by switching to CARGO_ENCODED_RUSTFLAGS) - #158256 (Avoid parser panics bubbling out to proc macros) - #158561 (Avoid building rustdoc for tests without doctests) - #158562 (Improve tracing of steps in bootstrap) - #157445 (Allow section override when using patchable-function-entries) - #158327 (Move attribute and keyword docs from `std` to `core`) - #158591 (Fix spacing issue for unused parentheses lint)
…uwer Rollup of 7 pull requests Successful merges: - #158073 (bootstrap: fix panic when repo path contains spaces by switching to CARGO_ENCODED_RUSTFLAGS) - #158256 (Avoid parser panics bubbling out to proc macros) - #158561 (Avoid building rustdoc for tests without doctests) - #158562 (Improve tracing of steps in bootstrap) - #157445 (Allow section override when using patchable-function-entries) - #158327 (Move attribute and keyword docs from `std` to `core`) - #158591 (Fix spacing issue for unused parentheses lint)
…uwer Rollup of 7 pull requests Successful merges: - #158073 (bootstrap: fix panic when repo path contains spaces by switching to CARGO_ENCODED_RUSTFLAGS) - #158256 (Avoid parser panics bubbling out to proc macros) - #158561 (Avoid building rustdoc for tests without doctests) - #158562 (Improve tracing of steps in bootstrap) - #157445 (Allow section override when using patchable-function-entries) - #158327 (Move attribute and keyword docs from `std` to `core`) - #158591 (Fix spacing issue for unused parentheses lint)
|
Not sure what the first issue was supposed to be, maybe a wrong link? But yeah, this will close #156096, as it arrived at essentially the same implementation. |
…uwer Rollup of 7 pull requests Successful merges: - #158073 (bootstrap: fix panic when repo path contains spaces by switching to CARGO_ENCODED_RUSTFLAGS) - #158256 (Avoid parser panics bubbling out to proc macros) - #158561 (Avoid building rustdoc for tests without doctests) - #158562 (Improve tracing of steps in bootstrap) - #157445 (Allow section override when using patchable-function-entries) - #158327 (Move attribute and keyword docs from `std` to `core`) - #158591 (Fix spacing issue for unused parentheses lint)
Rollup merge of #158073 - Rohan-Singla:fix/#158052, r=kobzol,mark-simulacrum,bjorn3 bootstrap: fix panic when repo path contains spaces by switching to CARGO_ENCODED_RUSTFLAGS Fixes #158052 Closes: #156096 ## Problem `./x build` panics with a cryptic assertion error when the repository is checked out under a directory path containing spaces (e.g. `/Users/foo/Open Source/rust`): thread 'main' panicked at src/bootstrap/src/core/builder/cargo.rs:54:9: assertion left == right failed left: 2 right: 1 The root cause: when building tools in `ToolRustcPrivate` or `Codegen` mode, bootstrap calls `llvm-config --libdir` and passes the result as a `-Clink-arg=-L<path>` rustflag. The `Rustflags::arg()` method asserted that arguments contain no spaces, but if the repo path has a space the libdir path inherits it and the assertion fires. The error gives no hint that the path is the problem. A secondary bug: `llvm-config --libdir` output has a trailing newline that was previously stripped accidentally by `RUSTFLAGS` whitespace splitting. Nothing was trimming it explicitly. ## Fix Two changes in `src/bootstrap/src/core/builder/cargo.rs`: 1. **Switch `RUSTFLAGS` → `CARGO_ENCODED_RUSTFLAGS`**: Change `Rustflags` to store args as `Vec<String>` and join with `\x1f` (ASCII unit separator) when setting the env var. Cargo's `CARGO_ENCODED_RUSTFLAGS` (stable since Cargo 1.55) uses `\x1f` as delimiter, which never appears in filesystem paths, so paths with spaces are handled correctly. The space-based assertion in `arg()` is removed. 2. **Trim `llvm-config --libdir` output**: Explicitly `.trim()` the captured stdout so the trailing newline is not included in the linker search path. `RUSTDOCFLAGS` is left as-is (space-joined) since no llvm paths are added to rustdocflags. cc: @Kobzol
View all comments
Fixes #158052
Closes: #156096
Problem
./x buildpanics with a cryptic assertion error when the repository ischecked out under a directory path containing spaces (e.g.
/Users/foo/Open Source/rust):thread 'main' panicked at src/bootstrap/src/core/builder/cargo.rs:54:9:
assertion left == right failed
left: 2
right: 1
The root cause: when building tools in
ToolRustcPrivateorCodegenmode, bootstrap calls
llvm-config --libdirand passes the result as a-Clink-arg=-L<path>rustflag. TheRustflags::arg()method assertedthat arguments contain no spaces, but if the repo path has a space the
libdir path inherits it and the assertion fires. The error gives no hint
that the path is the problem.
A secondary bug:
llvm-config --libdiroutput has a trailing newlinethat was previously stripped accidentally by
RUSTFLAGSwhitespacesplitting. Nothing was trimming it explicitly.
Fix
Two changes in
src/bootstrap/src/core/builder/cargo.rs:Switch
RUSTFLAGS→CARGO_ENCODED_RUSTFLAGS: ChangeRustflagsto store args asVec<String>and join with\x1f(ASCII unit separator) when setting the env var. Cargo's
CARGO_ENCODED_RUSTFLAGS(stable since Cargo 1.55) uses\x1fasdelimiter, which never appears in filesystem paths, so paths with
spaces are handled correctly. The space-based assertion in
arg()isremoved.
Trim
llvm-config --libdiroutput: Explicitly.trim()thecaptured stdout so the trailing newline is not included in the linker
search path.
RUSTDOCFLAGSis left as-is (space-joined) since no llvm paths areadded to rustdocflags.
cc: @Kobzol