Skip to content

Conversation

Urgau
Copy link
Member

@Urgau Urgau commented Sep 4, 2025

This PR is similar to #146130, where we disallowed frontmatter in --cfg and --check-cfg arguments. While fixing the other one we also discovered that shebang #!/usr/bin/shebang are currently also allowed in --cfg and --check-cfg arguments.

Allowing shebang in them (which are just ignored) was never intended, this PR fixes that by not stripping shebang for --cfg and --check-cfg arguments.

This is technically a breaking-change, although I don't expect anyone to actually rely on this unintended behavior.

Fixes #146130 (comment)
r? fmease

@rustbot
Copy link
Collaborator

rustbot commented Sep 4, 2025

fmease is not on the review rotation at the moment.
They may take a while to respond.

@rustbot rustbot added A-run-make Area: port run-make Makefiles to rmake.rs S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 4, 2025
@fmease fmease added the relnotes Marks issues that should be documented in the release notes of the next release. label Sep 5, 2025
@fmease
Copy link
Member

fmease commented Sep 5, 2025

Okay, so process-wise I'm not quite sure how to proceed. An MCP is overkill and a T-compiler FCP is also slightly overkill. I did add relnotes for formality. I'm inclined to just approve this unilaterally which I've done a few times for minor T-lang breaking changes over the years (notably only ones which were obviously not affecting real users and which were quite clearly bugs) (most recently in RUST-145604).

Re. crater, it's not worth testing (esp. because of the high infra costs) and the beta crater runs would be sufficient in covering that.

@fmease
Copy link
Member

fmease commented Sep 5, 2025

r=me with nitpicks addressed
@bors rollup

@fmease fmease added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 5, 2025
@Urgau Urgau force-pushed the cfg-disallow-shebang branch from 2983825 to d224d3a Compare September 5, 2025 22:22
@rustbot
Copy link
Collaborator

rustbot commented Sep 5, 2025

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@fmease
Copy link
Member

fmease commented Sep 6, 2025

@bors r+

@bors
Copy link
Collaborator

bors commented Sep 6, 2025

📌 Commit d224d3a has been approved by fmease

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 6, 2025
@jieyouxu
Copy link
Member

jieyouxu commented Sep 6, 2025

Okay, so process-wise I'm not quite sure how to proceed. An MCP is overkill and a T-compiler FCP is also slightly overkill.

Honestly I'd say just do this, shebang was never intended to be supported in --cfg/check-cfg, and if someone was somehow relying on this, I rather break them ASAP than let this become more relied upon (intentionally or unintentionally).

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 6, 2025
Disallow shebang in `--cfg` and `--check-cfg` arguments

This PR is similar to rust-lang#146130, where we disallowed frontmatter in `--cfg` and `--check-cfg` arguments. While fixing the other one we also discovered that shebang `#!/usr/bin/shebang` are currently also allowed in `--cfg` and `--check-cfg` arguments.

Allowing shebang in them (which are just ignored) was never intended, this PR fixes that by not stripping shebang for `--cfg` and `--check-cfg` arguments.

This is technically a breaking-change, although I don't expect anyone to actually rely on this unintended behavior.

Fixes rust-lang#146130 (comment)
r? fmease
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 6, 2025
Disallow shebang in `--cfg` and `--check-cfg` arguments

This PR is similar to rust-lang#146130, where we disallowed frontmatter in `--cfg` and `--check-cfg` arguments. While fixing the other one we also discovered that shebang `#!/usr/bin/shebang` are currently also allowed in `--cfg` and `--check-cfg` arguments.

Allowing shebang in them (which are just ignored) was never intended, this PR fixes that by not stripping shebang for `--cfg` and `--check-cfg` arguments.

This is technically a breaking-change, although I don't expect anyone to actually rely on this unintended behavior.

Fixes rust-lang#146130 (comment)
r? fmease
bors added a commit that referenced this pull request Sep 6, 2025
Rollup of 3 pull requests

Successful merges:

 - #127316 (move pinned version from tracing_core to tracing)
 - #144801 (Suggest bounds in more cases, accounting for type parameters referenced in predicate)
 - #146211 (Disallow shebang in `--cfg` and `--check-cfg` arguments)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit that referenced this pull request Sep 6, 2025
Rollup of 5 pull requests

Successful merges:

 - #127316 (move pinned version from tracing_core to tracing)
 - #144801 (Suggest bounds in more cases, accounting for type parameters referenced in predicate)
 - #146211 (Disallow shebang in `--cfg` and `--check-cfg` arguments)
 - #146263 (Fix `bump-stage0` build failure, and check-build `bump-stage0` in CI)
 - #146266 (miri std tests: skip all of sys::)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit a0c5e6b into rust-lang:master Sep 6, 2025
10 checks passed
@rustbot rustbot added this to the 1.91.0 milestone Sep 6, 2025
rust-timer added a commit that referenced this pull request Sep 6, 2025
Rollup merge of #146211 - Urgau:cfg-disallow-shebang, r=fmease

Disallow shebang in `--cfg` and `--check-cfg` arguments

This PR is similar to #146130, where we disallowed frontmatter in `--cfg` and `--check-cfg` arguments. While fixing the other one we also discovered that shebang `#!/usr/bin/shebang` are currently also allowed in `--cfg` and `--check-cfg` arguments.

Allowing shebang in them (which are just ignored) was never intended, this PR fixes that by not stripping shebang for `--cfg` and `--check-cfg` arguments.

This is technically a breaking-change, although I don't expect anyone to actually rely on this unintended behavior.

Fixes #146130 (comment)
r? fmease
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-run-make Area: port run-make Makefiles to rmake.rs relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Frontmatter inside --cfg and --check-cfg is not feature-gated
5 participants