Skip to content

bootstrap: support shell argfiles in compiler shims#159083

Open
kn1g78 wants to merge 1 commit into
rust-lang:mainfrom
kn1g78:fix-bootstrap-shell-argfiles
Open

bootstrap: support shell argfiles in compiler shims#159083
kn1g78 wants to merge 1 commit into
rust-lang:mainfrom
kn1g78:fix-bootstrap-shell-argfiles

Conversation

@kn1g78

@kn1g78 kn1g78 commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #159036.

The bootstrap rustc and rustdoc shims expand argument files so that they can
inspect arguments such as --target before invoking the actual compiler.

However, the current expansion treats every @... argument as a regular
line-based argument file. When rustc's -Zshell-argfiles option is enabled,
an argument such as @shell:<path> is therefore interpreted as a file named
shell:<path>. The shim fails while opening that path before the real compiler
can process it.

This change makes the shim's argument expansion follow rustc's behavior:

  • Track both -Zshell-argfiles and -Z shell-argfiles.
  • Allow the option to be enabled from a regular argument file.
  • Parse @shell:<path> using shlex.
  • Preserve the existing behavior for regular argument files.

Tests cover both supported -Z forms, including enabling shell argument files
from another argument file.

Tested with:

python x.py test bootstrap --set build.submodules=false \
--test-args utils::tests::shared_helpers_tests

python x.py check bootstrap --set build.submodules=false

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Jul 10, 2026
@kn1g78 kn1g78 marked this pull request as ready for review July 10, 2026 14:59
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 10, 2026
@rustbot rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jul 10, 2026
@rustbot

rustbot commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: bootstrap
  • bootstrap expanded to 6 candidates
  • Random selection from Mark-Simulacrum, clubby789, jieyouxu

@Mark-Simulacrum

Copy link
Copy Markdown
Member

Hm, I guess Miri will also need this support (I assume it doesn't have it) if we continue to perpetuate shell files containing the arguments most wrappers look for (e.g., --target, --sysroot probably). cc @RalfJung

I'm wondering if we should instead be teaching Cargo to not put those inside the argfile rather than copying the wrapper code to undo the argfiles into a bunch of places. I guess the undoing of argfiles is more general but it also seems fairly hard to get right (based on needing several PRs) -- maybe the right answer is that we need to have a crates.io crate that implements that, perhaps published from Cargo repo -- so that we can at least reuse a single implementation?

Skimming the implementation here it looks OK though.

Is there a tracking issue for shell-argfiles? I'm not seeing one right now. The original MCP (rust-lang/compiler-team#684) seems to suggest it was needed for Ninja compatibility, but it's not clear to me if that actually materialized / we are compatible.

@RalfJung

Copy link
Copy Markdown
Member

Miri has no support for argfiles indeed. When this came up with rustdoc, we were able to keep the relevant arguments out of the argfile and only stuff the long list of linker flags there, which Miri does not care about. Would something like that also be possible here?

@kn1g78

kn1g78 commented Jul 12, 2026

Copy link
Copy Markdown
Contributor Author

Hm, I guess Miri will also need this support (I assume it doesn't have it) if we continue to perpetuate shell files containing the arguments most wrappers look for (e.g., --target, --sysroot probably). cc @RalfJung

I'm wondering if we should instead be teaching Cargo to not put those inside the argfile rather than copying the wrapper code to undo the argfiles into a bunch of places. I guess the undoing of argfiles is more general but it also seems fairly hard to get right (based on needing several PRs) -- maybe the right answer is that we need to have a crates.io crate that implements that, perhaps published from Cargo repo -- so that we can at least reuse a single implementation?

Skimming the implementation here it looks OK though.

Is there a tracking issue for shell-argfiles? I'm not seeing one right now. The original MCP (rust-lang/compiler-team#684) seems to suggest it was needed for Ninja compatibility, but it's not clear to me if that actually materialized / we are compatible.

Thanks, that makes sense. The current change mirrors rustc's expansion so the bootstrap shims see the same arguments,
but that duplicates the parser and does not help Miri.

Cargo currently writes all rustc/rustdoc arguments to its response file. I’ll investigate keeping wrapper-visible
options such as --target, --sysroot, --crate-name, and --crate-type on the command line, while putting the remaining
arguments in the argfile. That should let the bootstrap shims and Miri inspect what they need without each wrapper
learning shell argfiles.

I have not found a dedicated tracking issue for -Zshell-argfiles; rust-lang/compiler-team#684 appears to be the
original MCP. Would you prefer this PR to stay as a narrow compatibility fix while that work is pursued separately, or
should I close/supersede it in favor of the Cargo-side solution?

@RalfJung

Copy link
Copy Markdown
Member

Would you prefer this PR to stay as a narrow compatibility fix while that work is pursued separately, or
should I close/supersede it in favor of the Cargo-side solution?

That's up to the bootstrap maintainers. This PR doesn't make things any worse for Miri, but it may become unnecessary if a cargo-side solution gets implemented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants