Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(build-rs)!: Remove meaningless 'cargo_cfg_debug_assertions' #14901

Merged
merged 2 commits into from
Dec 6, 2024

Conversation

epage
Copy link
Contributor

@epage epage commented Dec 6, 2024

What does this PR try to resolve?

The documentation that was added was pulled straight from a comment in custom_build.rs:

This cfg is always true and misleading, so avoid setting it.
That is because Cargo queries rustc without any profile settings.

It is therefore misleading to expose this in the API

How should we test and review this PR?

Additional information

@rustbot
Copy link
Collaborator

rustbot commented Dec 6, 2024

r? @ehuss

rustbot has assigned @ehuss.
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

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 6, 2024
epage added 2 commits December 6, 2024 09:33
The documentation that was added was pulled straight from a comment in
`custom_build.rs`.
Comment on lines 113 to 115
/// This cfg is always true and misleading, so avoid setting it.
/// That is because Cargo queries rustc without any profile settings.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// This cfg is always true and misleading, so avoid setting it.
/// That is because Cargo queries rustc without any profile settings.
/// This cfg is always false and misleading, so avoid setting it.
/// That is because Cargo explicitly filters out this setting since it queries rustc without any profile settings, and historically led to confusion.

Correct? The cfg is filtered so it is always unset.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, copying it directly can be misleading. I'm wanting to keep the comment in line with where its filtered, so I tweaked it in a different way.

Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see. I guess there are two ways to look at this, from the angle of the build script (the env is never set and thus false) and the angle from cargo (rustc always tells us it is always true).

Thanks!

@ehuss ehuss added this pull request to the merge queue Dec 6, 2024
Merged via the queue into rust-lang:master with commit 2560340 Dec 6, 2024
22 checks passed
@epage epage deleted the build-rs-debug branch December 6, 2024 19:18
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 7, 2024
Update cargo

6 commits in 05f54fdc34310f458033af8a63ce1d699fae8bf6..20a443231846b81c7b909691ec3f15eb173f2b18
2024-12-03 03:14:12 +0000 to 2024-12-06 21:56:56 +0000
- fix(fingerprint): Don't throwaway the cache on RUSTFLAGS changes  (rust-lang/cargo#14830)
- fix(build-rs)!: Remove meaningless 'cargo_cfg_debug_assertions' (rust-lang/cargo#14901)
- docs(fingerprint): cargo-rustc extra flags do not affect the metadata  (rust-lang/cargo#14898)
- fix(add): Don't select yanked versions when normalizing names (rust-lang/cargo#14895)
- fix(fix): Migrate workspace dependencies (rust-lang/cargo#14890)
- test(build-std): make mock-std closer to real world (rust-lang/cargo#14896)
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 8, 2024
Update cargo

6 commits in 05f54fdc34310f458033af8a63ce1d699fae8bf6..20a443231846b81c7b909691ec3f15eb173f2b18
2024-12-03 03:14:12 +0000 to 2024-12-06 21:56:56 +0000
- fix(fingerprint): Don't throwaway the cache on RUSTFLAGS changes  (rust-lang/cargo#14830)
- fix(build-rs)!: Remove meaningless 'cargo_cfg_debug_assertions' (rust-lang/cargo#14901)
- docs(fingerprint): cargo-rustc extra flags do not affect the metadata  (rust-lang/cargo#14898)
- fix(add): Don't select yanked versions when normalizing names (rust-lang/cargo#14895)
- fix(fix): Migrate workspace dependencies (rust-lang/cargo#14890)
- test(build-std): make mock-std closer to real world (rust-lang/cargo#14896)
@rustbot rustbot added this to the 1.85.0 milestone Dec 8, 2024
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Dec 8, 2024
Update cargo

6 commits in 05f54fdc34310f458033af8a63ce1d699fae8bf6..20a443231846b81c7b909691ec3f15eb173f2b18
2024-12-03 03:14:12 +0000 to 2024-12-06 21:56:56 +0000
- fix(fingerprint): Don't throwaway the cache on RUSTFLAGS changes  (rust-lang/cargo#14830)
- fix(build-rs)!: Remove meaningless 'cargo_cfg_debug_assertions' (rust-lang/cargo#14901)
- docs(fingerprint): cargo-rustc extra flags do not affect the metadata  (rust-lang/cargo#14898)
- fix(add): Don't select yanked versions when normalizing names (rust-lang/cargo#14895)
- fix(fix): Migrate workspace dependencies (rust-lang/cargo#14890)
- test(build-std): make mock-std closer to real world (rust-lang/cargo#14896)
lnicola pushed a commit to lnicola/rust-analyzer that referenced this pull request Dec 11, 2024
Update cargo

6 commits in 05f54fdc34310f458033af8a63ce1d699fae8bf6..20a443231846b81c7b909691ec3f15eb173f2b18
2024-12-03 03:14:12 +0000 to 2024-12-06 21:56:56 +0000
- fix(fingerprint): Don't throwaway the cache on RUSTFLAGS changes  (rust-lang/cargo#14830)
- fix(build-rs)!: Remove meaningless 'cargo_cfg_debug_assertions' (rust-lang/cargo#14901)
- docs(fingerprint): cargo-rustc extra flags do not affect the metadata  (rust-lang/cargo#14898)
- fix(add): Don't select yanked versions when normalizing names (rust-lang/cargo#14895)
- fix(fix): Migrate workspace dependencies (rust-lang/cargo#14890)
- test(build-std): make mock-std closer to real world (rust-lang/cargo#14896)
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants