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

refactor(fingerprint): Track the intent for each use of UnitHash #14826

Merged
merged 3 commits into from
Nov 16, 2024

Conversation

epage
Copy link
Contributor

@epage epage commented Nov 15, 2024

What does this PR try to resolve?

We have

  • -C metadata
  • -C extra-filename
  • Uniquifying build scripts
  • Uniquifying scraped documentation

Currently, these are all the same value. For #8716, we might want to have -C metadata and -C extra-filename hash different parts of the Unit. I figured that this change helps to clarify intent so that even if we don't change the definitions, this could still be worth it.

The last two I'm tracking as a unit_id. As we evolve the first two
hashes, we can decide which would be a better fit for backing the unit_id.
For scraping, I could have treated this as
-C extra-filename but then we'd have a lot of .expect()s.

I moved the accessors from CompilationFiles to MetaInfo so we could closely associate the documentation, API, and implementation. Making CompilationFiles::c_metadata the main entry point that gets documented would be weird because the hashing is associated with MetaInfo (umbrella type, was private, now Metadata) or Metadata (agnostic of each use, now UnitHash)

How should we test and review this PR?

Additional information

Now called `UnitHash` and `Metadata`.

The goal is to provide role-specific `UnitHash` accessors on the new `Metadata`.
I chose this method, instead of changing the accessors on
`CompilationFiles`, to have the least impact on the documentation as
what is called `Metadata` can remain the focus of the documentation,
rather than having it apply to different `CompilationFiles` accessors
which don't house the actual logic.
We have
- `-C metadata`
- `-C extra-filename`
- Uniquifying build scripts
- Uniquifying scraped documentation

The last two I'm tracking as a `unit_id`.  As we evolve the first two
hashes, we can decide which would be a better fit for backing this.
For scraping,  we could treat this as
`-C extra-filename` but then we'd have a lot of `.expect()`s.
@rustbot
Copy link
Collaborator

rustbot commented Nov 15, 2024

r? @weihanglo

rustbot has assigned @weihanglo.
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 A-build-execution Area: anything dealing with executing the compiler A-build-scripts Area: build.rs scripts A-layout Area: target output directory layout, naming, and organization Command-test S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 15, 2024
@epage epage force-pushed the rustflags branch 3 times, most recently from 6bacd81 to 1f4eeb4 Compare November 15, 2024 21:52
Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

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

Thanks!

@weihanglo weihanglo added this pull request to the merge queue Nov 16, 2024
Merged via the queue into rust-lang:master with commit 69e5959 Nov 16, 2024
20 checks passed
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 16, 2024
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)
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 16, 2024
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)
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 17, 2024
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)
@epage epage deleted the rustflags branch November 21, 2024 23:02
@weihanglo weihanglo added this to the 1.84.0 milestone Nov 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-build-execution Area: anything dealing with executing the compiler A-build-scripts Area: build.rs scripts A-layout Area: target output directory layout, naming, and organization Command-test 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