Rustflags in metadata #6503
Rustflags in metadata #6503
Conversation
|
Nice doc addition too! The test seems overly complex for what it's testing (does it need dependencies? two of them?). |
|
The doc improvements are just a rewrite of how @ehuss explained it to me. I guess it is an example of "the best time to wright documentation is when you are new." I know what I did not understand and where I had looked, he new the anceer. Good call on the test! (I was under the impression that this would only cache dependencies, hence adding them, I was wrong.) |
|
@ehuss dose that look better? |
|
Cursory comment: As I understand, there are two bits of information which track "dependencies": the hash of the file and the However, I believe we've broken that "automatic gc" logic: if I today build a crate, and then add So perhaps we can do something more radical here and just define the file hash to be exactly the fingerprint? That should simplify internal representation. |
|
I think there are still some important things that are not in |
|
I agree that we're not quite ready yet to merge these two hashes, the one for fingerprint calculation wasn't ever designed to be in the filename so it'd at least need a review to make sure it's suitable to always throw all of it in the filename. Incrementally adding more to the filename hash though I think is fine (like this PR) |
|
So what is this waiting on? |
|
@bors: r=ehuss |
|
📌 Commit 6d24d57 has been approved by |
Rustflags in metadata This is a small change that adds Rustflags to the `Metadata`. This means, for example, that if you do a plane build then build with "-C target-cpu=native", you will get 2 copies of the dependencies. Con, more space if you are changing Rustflags. Pro, not rebuilding all the dependencies when you switch back. Suggested by <https://internals.rust-lang.org/t/idea-cargo-global-binary-cache/9002/31>
|
☀️ Test successful - status-appveyor, status-travis |
Update cargo 13 commits in 34320d212dca8cd27d06ce93c16c6151f46fcf2e..2b4a5f1f0bb6e13759e88ea9512527b0beba154f 2019-01-03 19:12:38 +0000 to 2019-01-12 04:13:12 +0000 - Add test for publish with [patch] + cleanup. (rust-lang/cargo#6544) - Fix clippy warning (rust-lang/cargo#6546) - Revert "Workaround by using yesterday's nightly" (rust-lang/cargo#6540) - Adding feature-flags to `cargo publish` and `cargo package` (rust-lang/cargo#6453) - Fix the Travis CI badge (rust-lang/cargo#6530) - Add helpful text for Windows exceptions like Unix (rust-lang/cargo#6532) - Report fix bugs to Rust instead of Cargo (rust-lang/cargo#6531) - --{example,bin,bench,test} with no argument now lists all available targets (rust-lang/cargo#6505) - Rebuild on mid build file modification (rust-lang/cargo#6484) - Derive Clone for TomlDependency (rust-lang/cargo#6527) - publish: rework the crates.io detection logic. (rust-lang/cargo#6525) - avoid duplicates in ignore files (rust-lang/cargo#6521) - Rustflags in metadata (rust-lang/cargo#6503) r? @alexcrichton
|
I find this change rather problematic. It keeps breaking things (path remapping, PGO, reproducible builds) because it goes against what rustc's If there is some kind of caching of compiled artifacts, then that layer should take care of parsing |
|
I am sorry this is making problems! The only problem where the report has come to my attention is the
There definitely is, cargo has never rebuilt from scratch at every invocation. |
I just opened this issue: #7416
I know. I think that improving caching via different metadata strings has problems though:
I'm sorry for complaining about your feature, @Eh2406. You wrote such a nice doc string for it |
|
This PR was intended to help unlock work on the "zombie artifacts" problem, unfortunately that work has not yet progressed far enough to clean up the mess. I do not object to a rewrite or removal if this is making more problems than it is worth. |
This is a moral revert of rust-lang#6503 but not a literal code revert. This switches Cargo's behavior to avoid hashing compiler flags into `-Cmetadata` since we've now had multiple requests of excluding flags from the `-Cmetadata` hash: usage of `--remap-path-prefix` and PGO options. These options should only affect how the compiler is invoked/compiled and not radical changes such as symbol names, but symbol names are changed based on `-Cmetadata`. Instead Cargo will still track these flags internally, but only for reinvoking rustc, and not for caching separately based on rustc flags. Closes rust-lang#7416
Go back to not hashing `RUSTFLAGS` in `-Cmetadata` This is a moral revert of #6503 but not a literal code revert. This switches Cargo's behavior to avoid hashing compiler flags into `-Cmetadata` since we've now had multiple requests of excluding flags from the `-Cmetadata` hash: usage of `--remap-path-prefix` and PGO options. These options should only affect how the compiler is invoked/compiled and not radical changes such as symbol names, but symbol names are changed based on `-Cmetadata`. Instead Cargo will still track these flags internally, but only for reinvoking rustc, and not for caching separately based on rustc flags. Closes #7416
This is a moral revert of rust-lang#6503 but not a literal code revert. This switches Cargo's behavior to avoid hashing compiler flags into `-Cmetadata` since we've now had multiple requests of excluding flags from the `-Cmetadata` hash: usage of `--remap-path-prefix` and PGO options. These options should only affect how the compiler is invoked/compiled and not radical changes such as symbol names, but symbol names are changed based on `-Cmetadata`. Instead Cargo will still track these flags internally, but only for reinvoking rustc, and not for caching separately based on rustc flags. Closes rust-lang#7416
This is a small change that adds Rustflags to the
Metadata. This means, for example, that if you do a plane build then build with "-C target-cpu=native", you will get 2 copies of the dependencies. Con, more space if you are changing Rustflags. Pro, not rebuilding all the dependencies when you switch back.Suggested by https://internals.rust-lang.org/t/idea-cargo-global-binary-cache/9002/31