fix(build-rs)!: Updates from an audit #14817
Conversation
At this point, it is assumed to always be up-to-date
rustbot
added
the
S-waiting-on-review
| var_os(key).is_some() | ||
| } | ||
|
|
||
| #[track_caller] |
There was a problem hiding this comment.
Note to reviewers: buildscripts are compiled without debug info by default, so backtraces aren't helpful. That's why #[track_caller] is applied so liberally here, so the caller actually knows where the bad call was made.
There was a problem hiding this comment.
I wonder where we should record design principles like that. For example, I had considered moving to env! but realized that re-running is cheaper than recompilation
There was a problem hiding this comment.
The default answer would be a DESIGN.md alongside README.md. That seems as reasonable a choice as any other1, although any such document always is difficult to organize and keep up to date.
As for env! — it's lots of "fun" to keep track of whether we're querying the environment for building build-rs (set at our compile time) or for the client buildscript (only set at our run time).
Footnotes
-
The main alternative being a wiki page, which is even less likely to be updated or even looked at ime since it isn't colocated. ↩
There was a problem hiding this comment.
For build-rs I think we should in general query only runtime envs, though I don't fully get the context of using env! for this function.
There was a problem hiding this comment.
The only time we use env! is to get build-rss MSRV. If we were to do things at compile time, we'd have to turn all of these functions into macros so that env! runs in the callers context.
| match value.parse() { | ||
| Ok(s) => s, | ||
| Err(err) => { | ||
| panic!("{err}") | ||
| } | ||
| } |
There was a problem hiding this comment.
nit: any reason we can't do this?
| match value.parse() { | |
| Ok(s) => s, | |
| Err(err) => { | |
| panic!("{err}") | |
| } | |
| } | |
| value.parse().unwrap() |
There was a problem hiding this comment.
The question is whether to report Debug or Display
|
Anyway, the only nit is just a nit. Going to merge this now. |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Update cargo 15 commits in 4a2d8dc636445b276288543882e076f254b3ae95..69e595908e2c420e7f0d1be34e6c5b984c8cfb84 2024-11-09 19:10:33 +0000 to 2024-11-16 01:26:11 +0000 - refactor(fingerprint): Track the intent for each use of `UnitHash` (rust-lang/cargo#14826) - fix(toml): Update frontmatter parser for RFC 3503 (rust-lang/cargo#14792) - docs(unstable): Move -Zwarnings from stable to unstable section (rust-lang/cargo#14827) - Simplify English used in guide (rust-lang/cargo#14825) - feat(resolver): Stabilize resolver v3 (rust-lang/cargo#14754) - docs: Clean up doc comments (rust-lang/cargo#14823) - fix(remove): On error, suggest other dependencies (rust-lang/cargo#14818) - Always include Cargo.lock in published crates (rust-lang/cargo#14815) - fix(build-rs)!: Updates from an audit (rust-lang/cargo#14817) - feat(rustdoc): diplay env vars in extra verbose mode (rust-lang/cargo#14812) - Migrate build-rs to the Cargo repo (rust-lang/cargo#14786) - chore(ci): Check for clippy `correctness` (rust-lang/cargo#14796) - git: do not validate submodules of fresh checkouts (rust-lang/cargo#14605) - refactor: clone-on-write when needed for InternedString (rust-lang/cargo#14808) - fix(docs): typo in cargo-fmt.md (rust-lang/cargo#14805)
Update cargo 15 commits in 4a2d8dc636445b276288543882e076f254b3ae95..69e595908e2c420e7f0d1be34e6c5b984c8cfb84 2024-11-09 19:10:33 +0000 to 2024-11-16 01:26:11 +0000 - refactor(fingerprint): Track the intent for each use of `UnitHash` (rust-lang/cargo#14826) - fix(toml): Update frontmatter parser for RFC 3503 (rust-lang/cargo#14792) - docs(unstable): Move -Zwarnings from stable to unstable section (rust-lang/cargo#14827) - Simplify English used in guide (rust-lang/cargo#14825) - feat(resolver): Stabilize resolver v3 (rust-lang/cargo#14754) - docs: Clean up doc comments (rust-lang/cargo#14823) - fix(remove): On error, suggest other dependencies (rust-lang/cargo#14818) - Always include Cargo.lock in published crates (rust-lang/cargo#14815) - fix(build-rs)!: Updates from an audit (rust-lang/cargo#14817) - feat(rustdoc): diplay env vars in extra verbose mode (rust-lang/cargo#14812) - Migrate build-rs to the Cargo repo (rust-lang/cargo#14786) - chore(ci): Check for clippy `correctness` (rust-lang/cargo#14796) - git: do not validate submodules of fresh checkouts (rust-lang/cargo#14605) - refactor: clone-on-write when needed for InternedString (rust-lang/cargo#14808) - fix(docs): typo in cargo-fmt.md (rust-lang/cargo#14805)
Update cargo 15 commits in 4a2d8dc636445b276288543882e076f254b3ae95..69e595908e2c420e7f0d1be34e6c5b984c8cfb84 2024-11-09 19:10:33 +0000 to 2024-11-16 01:26:11 +0000 - refactor(fingerprint): Track the intent for each use of `UnitHash` (rust-lang/cargo#14826) - fix(toml): Update frontmatter parser for RFC 3503 (rust-lang/cargo#14792) - docs(unstable): Move -Zwarnings from stable to unstable section (rust-lang/cargo#14827) - Simplify English used in guide (rust-lang/cargo#14825) - feat(resolver): Stabilize resolver v3 (rust-lang/cargo#14754) - docs: Clean up doc comments (rust-lang/cargo#14823) - fix(remove): On error, suggest other dependencies (rust-lang/cargo#14818) - Always include Cargo.lock in published crates (rust-lang/cargo#14815) - fix(build-rs)!: Updates from an audit (rust-lang/cargo#14817) - feat(rustdoc): diplay env vars in extra verbose mode (rust-lang/cargo#14812) - Migrate build-rs to the Cargo repo (rust-lang/cargo#14786) - chore(ci): Check for clippy `correctness` (rust-lang/cargo#14796) - git: do not validate submodules of fresh checkouts (rust-lang/cargo#14605) - refactor: clone-on-write when needed for InternedString (rust-lang/cargo#14808) - fix(docs): typo in cargo-fmt.md (rust-lang/cargo#14805)
What does this PR try to resolve?
Did a quick audit for parts of the API and implementation that stood out to me. So far, I've mostly focused on "input".
How should we test and review this PR?
Additional information
CC @CAD97