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

CARGO_PKG values should probably be in fingerprint, not metadata #6208

Closed
ehuss opened this issue Oct 23, 2018 · 3 comments · Fixed by #6785
Closed

CARGO_PKG values should probably be in fingerprint, not metadata #6208

ehuss opened this issue Oct 23, 2018 · 3 comments · Fixed by #6785

Comments

@ehuss
Copy link
Contributor

ehuss commented Oct 23, 2018

Currently, "authors", "description", and "homepage" are included in the metadata hash. This is done because the corresponding CARGO_PKG environment variables might be included in the crate. I think these values should be in the fingerprint instead. It is a minor distinction, but the consequence is that changing any of these values will cause a recompile, but leave the old crates in the target directory, forever to be unused until the next cargo clean. If they were in the fingerprint, the recompile would overwrite the old files.

Also, the recently added "repository" was not included in the hash, but it should have.

@alexcrichton
Copy link
Member

👍 sounds like a good idea to me!

@ehuss
Copy link
Contributor Author

ehuss commented Nov 6, 2018

@alexcrichton I've been thinking about this some more, and it seems like the fingerprint probably should contain all used environment variables. Do you think that would be a good idea? I noticed you mentioned something along these lines at rust-lang/rust#40364. I hacked something together that implements what you suggested (comments in the dep file). It wasn't hard, though it's a little messy passing the env names all the way through.

@alexcrichton
Copy link
Member

Yeah that seems fine by me, the only real problem hashing has ever caused is uncover situations where things were being rebuilt when they weren't expected to be, but often fixable via configuration changes.

bors added a commit that referenced this issue Mar 26, 2019
Some fingerprint cleanup.

Just a minor cleanup.

Move `CARGO_PKG_*` values from Metadata to Fingerprint (added in #3857).  Closes #6208.  This prevents stale artifacts from being left behind when these values change. Also tracks changes to the "repository" value (added in #6096).

Remove `edition` as a separate field. It is already tracked in `target`. This was required previously to #5816 which added per-target editions.

Also adds a helper to the testsuite to make globbing easier.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants