-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
fingerprint error causing rebuild on feature flag changes #3923
Comments
Sounds bad! Could you try enabling |
Here's the full log:
|
So much for being more illuminating, alas! |
Oh wait sorry I think I misunderstood the issue, I believe I see the problem now. So cargo recently started caching crates based on features as well as debug/release, meaning that So for one compilation graph here no features are enabled. Next a feature in Rocket and transitively, ring, a feature is enabled. Now cookie depends on ring so when it's recompiled it means cookie is recompiled, and later Rocket is recompiled. This basically means that the Rocket with no features uses a different version of the cookie crate than the version of Rocket with features. Both versions of the cookie crate have no features enabled, but the dependencies are different. I believe the solution here, and one we forgot to do earlier, is to simply hash dependencies when hashing a crate. I believe the relevant function already has all the information necessary to do this, we basically just need to call |
Gecko would like to turn on the stylo layout tests (tests/unit/stylo) in Gecko CI. The plan for doing this is to add the tests as a dev-dependency of Gecko's main Rust library, from which `cargo test` can be run in the usual fashion. Doing this creates problems for normal development, because the stylo tests need the `selectors` crate to be compiled with `gecko_like_types`, whereas the `geckolib` crate does not. So if we compile `geckolib` in a non-test build configuration, the `selectors` crate is compiled without `gecko_like_types`...but then if we compile `geckolib` in a test build configuration, cargo will evict the previous rlib for the `selectors` crate and replace it with a `selectors` compiled with gecko_like_types. And then compiling `geckolib` in a non-test configuration repeats the process, and so forth. Needless to say, this is highly annoying behavior. It is due to a bug in cargo: rust-lang/cargo#3923 but it's not known when that bug will get fixed. In the meantime, we can just make sure that geckolib's `selectors` is compiled with the same features as the `selectors` crate in the stylo tests.
Gecko would like to turn on the stylo layout tests (tests/unit/stylo) in Gecko CI. The plan for doing this is to add the tests as a dev-dependency of Gecko's main Rust library, from which `cargo test` can be run in the usual fashion. Doing this creates problems for normal development, because the stylo tests need the `selectors` crate to be compiled with `gecko_like_types`, whereas the `geckolib` crate does not. So if we compile `geckolib` in a non-test build configuration, the `selectors` crate is compiled without `gecko_like_types`...but then if we compile `geckolib` in a test build configuration, cargo will evict the previous rlib for the `selectors` crate and replace it with a `selectors` compiled with gecko_like_types. And then compiling `geckolib` in a non-test configuration repeats the process, and so forth. Needless to say, this is highly annoying behavior. It is due to a bug in cargo: rust-lang/cargo#3923 but it's not known when that bug will get fixed. In the meantime, we can just make sure that geckolib's `selectors` is compiled with the same features as the `selectors` crate in the stylo tests.
align selectors's features in geckolib and stylo_tests Gecko would like to turn on the stylo layout tests (tests/unit/stylo) in Gecko CI. The plan for doing this is to add the tests as a dev-dependency of Gecko's main Rust library, from which `cargo test` can be run in the usual fashion. Doing this creates problems for normal development, because the stylo tests need the `selectors` crate to be compiled with `gecko_like_types`, whereas the `geckolib` crate does not. So if we compile `geckolib` in a non-test build configuration, the `selectors` crate is compiled without `gecko_like_types`...but then if we compile `geckolib` in a test build configuration, cargo will evict the previous rlib for the `selectors` crate and replace it with a `selectors` compiled with gecko_like_types. And then compiling `geckolib` in a non-test configuration repeats the process, and so forth. Needless to say, this is highly annoying behavior. It is due to a bug in cargo: rust-lang/cargo#3923 but it's not known when that bug will get fixed. In the meantime, we can just make sure that geckolib's `selectors` is compiled with the same features as the `selectors` crate in the stylo tests. - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18489) <!-- Reviewable:end -->
…o_tests (from froydnj:geckolib-feature-alignment); r=froydnj Gecko would like to turn on the stylo layout tests (tests/unit/stylo) in Gecko CI. The plan for doing this is to add the tests as a dev-dependency of Gecko's main Rust library, from which `cargo test` can be run in the usual fashion. Doing this creates problems for normal development, because the stylo tests need the `selectors` crate to be compiled with `gecko_like_types`, whereas the `geckolib` crate does not. So if we compile `geckolib` in a non-test build configuration, the `selectors` crate is compiled without `gecko_like_types`...but then if we compile `geckolib` in a test build configuration, cargo will evict the previous rlib for the `selectors` crate and replace it with a `selectors` compiled with gecko_like_types. And then compiling `geckolib` in a non-test configuration repeats the process, and so forth. Needless to say, this is highly annoying behavior. It is due to a bug in cargo: rust-lang/cargo#3923 but it's not known when that bug will get fixed. In the meantime, we can just make sure that geckolib's `selectors` is compiled with the same features as the `selectors` crate in the stylo tests. - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors Source-Repo: https://github.com/servo/servo Source-Revision: 1aa8be392b0ab8e7a8426f525361b40b69d70b4f --HG-- extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear extra : subtree_revision : f496fa2a771c7765dae4da1edbf160e298a426f3
…o_tests (from froydnj:geckolib-feature-alignment); r=froydnj Gecko would like to turn on the stylo layout tests (tests/unit/stylo) in Gecko CI. The plan for doing this is to add the tests as a dev-dependency of Gecko's main Rust library, from which `cargo test` can be run in the usual fashion. Doing this creates problems for normal development, because the stylo tests need the `selectors` crate to be compiled with `gecko_like_types`, whereas the `geckolib` crate does not. So if we compile `geckolib` in a non-test build configuration, the `selectors` crate is compiled without `gecko_like_types`...but then if we compile `geckolib` in a test build configuration, cargo will evict the previous rlib for the `selectors` crate and replace it with a `selectors` compiled with gecko_like_types. And then compiling `geckolib` in a non-test configuration repeats the process, and so forth. Needless to say, this is highly annoying behavior. It is due to a bug in cargo: rust-lang/cargo#3923 but it's not known when that bug will get fixed. In the meantime, we can just make sure that geckolib's `selectors` is compiled with the same features as the `selectors` crate in the stylo tests. - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors Source-Repo: https://github.com/servo/servo Source-Revision: 1aa8be392b0ab8e7a8426f525361b40b69d70b4f
…o_tests (from froydnj:geckolib-feature-alignment); r=froydnj Gecko would like to turn on the stylo layout tests (tests/unit/stylo) in Gecko CI. The plan for doing this is to add the tests as a dev-dependency of Gecko's main Rust library, from which `cargo test` can be run in the usual fashion. Doing this creates problems for normal development, because the stylo tests need the `selectors` crate to be compiled with `gecko_like_types`, whereas the `geckolib` crate does not. So if we compile `geckolib` in a non-test build configuration, the `selectors` crate is compiled without `gecko_like_types`...but then if we compile `geckolib` in a test build configuration, cargo will evict the previous rlib for the `selectors` crate and replace it with a `selectors` compiled with gecko_like_types. And then compiling `geckolib` in a non-test configuration repeats the process, and so forth. Needless to say, this is highly annoying behavior. It is due to a bug in cargo: rust-lang/cargo#3923 but it's not known when that bug will get fixed. In the meantime, we can just make sure that geckolib's `selectors` is compiled with the same features as the `selectors` crate in the stylo tests. - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors Source-Repo: https://github.com/servo/servo Source-Revision: 1aa8be392b0ab8e7a8426f525361b40b69d70b4f
I've been troubleshooting this in nikomatsakis/lalrpop#266, and I've put together a small project to easily reproduce it. I can reproduce this on stable cargo ( |
Ah yes, this was indeed fixed on master! |
The easy part of this patch is the addition of the RustTest itself. The more difficult to understand part of the patch is the changes to all of our Rust build configuration. We do this due to a bug in cargo: rust-lang/cargo#3923 where features on dependent crates are not correctly taken into account when determining whether cached artifacts on disk are valid and whether they should be evicted from the disk cache. The practical upshot of this behavior is that, say, running gtests during normal development when files in libxul are modified will: * rebuild some Rust dependencies for libxul; * link libxul; * rebuild those same Rust dependencies *again* for libxul-gtest, since we have different features active and therefore the old artifacts look to be out of date; * link libxul-gtest. Needless to say, this is highly annoying and counterproductive behavior. The "fix" is to ensure that the gkrust-shared crate explicitly depends on crates and assigns features to them such that the feature sets do not change between normal builds and testing builds. This is admittedly fragile, but it is not the first time this has come up, and is probably not the last.
The easy part of this patch is the addition of the RustTest itself. The more difficult to understand part of the patch is the changes to all of our Rust build configuration. We do this due to a bug in cargo: rust-lang/cargo#3923 where features on dependent crates are not correctly taken into account when determining whether cached artifacts on disk are valid and whether they should be evicted from the disk cache. The practical upshot of this behavior is that, say, running gtests during normal development when files in libxul are modified will: * rebuild some Rust dependencies for libxul; * link libxul; * rebuild those same Rust dependencies *again* for libxul-gtest, since we have different features active and therefore the old artifacts look to be out of date; * link libxul-gtest. Needless to say, this is highly annoying and counterproductive behavior. The "fix" is to ensure that the gkrust-shared crate explicitly depends on crates and assigns features to them such that the feature sets do not change between normal builds and testing builds. This is admittedly fragile, but it is not the first time this has come up, and is probably not the last.
The easy part of this patch is the addition of the RustTest itself. The more difficult to understand part of the patch is the changes to all of our Rust build configuration. We do this due to a bug in cargo: rust-lang/cargo#3923 where features on dependent crates are not correctly taken into account when determining whether cached artifacts on disk are valid and whether they should be evicted from the disk cache. The practical upshot of this behavior is that, say, running gtests during normal development when files in libxul are modified will: * rebuild some Rust dependencies for libxul; * link libxul; * rebuild those same Rust dependencies *again* for libxul-gtest, since we have different features active and therefore the old artifacts look to be out of date; * link libxul-gtest. Needless to say, this is highly annoying and counterproductive behavior. The "fix" is to ensure that the gkrust-shared crate explicitly depends on crates and assigns features to them such that the feature sets do not change between normal builds and testing builds. This is admittedly fragile, but it is not the first time this has come up, and is probably not the last.
Remove the syn dependency from gkrust-shared, which was added in bug 1373878 as a workaround for this Cargo bug: rust-lang/cargo#3923 MozReview-Commit-ID: L34J0davEYd --HG-- extra : rebase_source : 79e5684cba143f6215e7dcf9367f82227bca48c5
…o_tests (from froydnj:geckolib-feature-alignment); r=froydnj Gecko would like to turn on the stylo layout tests (tests/unit/stylo) in Gecko CI. The plan for doing this is to add the tests as a dev-dependency of Gecko's main Rust library, from which `cargo test` can be run in the usual fashion. Doing this creates problems for normal development, because the stylo tests need the `selectors` crate to be compiled with `gecko_like_types`, whereas the `geckolib` crate does not. So if we compile `geckolib` in a non-test build configuration, the `selectors` crate is compiled without `gecko_like_types`...but then if we compile `geckolib` in a test build configuration, cargo will evict the previous rlib for the `selectors` crate and replace it with a `selectors` compiled with gecko_like_types. And then compiling `geckolib` in a non-test configuration repeats the process, and so forth. Needless to say, this is highly annoying behavior. It is due to a bug in cargo: rust-lang/cargo#3923 but it's not known when that bug will get fixed. In the meantime, we can just make sure that geckolib's `selectors` is compiled with the same features as the `selectors` crate in the stylo tests. - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors Source-Repo: https://github.com/servo/servo Source-Revision: 1aa8be392b0ab8e7a8426f525361b40b69d70b4f UltraBlame original commit: 962216b7844402a956a9b410fed0567506f96261
…o_tests (from froydnj:geckolib-feature-alignment); r=froydnj Gecko would like to turn on the stylo layout tests (tests/unit/stylo) in Gecko CI. The plan for doing this is to add the tests as a dev-dependency of Gecko's main Rust library, from which `cargo test` can be run in the usual fashion. Doing this creates problems for normal development, because the stylo tests need the `selectors` crate to be compiled with `gecko_like_types`, whereas the `geckolib` crate does not. So if we compile `geckolib` in a non-test build configuration, the `selectors` crate is compiled without `gecko_like_types`...but then if we compile `geckolib` in a test build configuration, cargo will evict the previous rlib for the `selectors` crate and replace it with a `selectors` compiled with gecko_like_types. And then compiling `geckolib` in a non-test configuration repeats the process, and so forth. Needless to say, this is highly annoying behavior. It is due to a bug in cargo: rust-lang/cargo#3923 but it's not known when that bug will get fixed. In the meantime, we can just make sure that geckolib's `selectors` is compiled with the same features as the `selectors` crate in the stylo tests. - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors Source-Repo: https://github.com/servo/servo Source-Revision: 1aa8be392b0ab8e7a8426f525361b40b69d70b4f UltraBlame original commit: 962216b7844402a956a9b410fed0567506f96261
…o_tests (from froydnj:geckolib-feature-alignment); r=froydnj Gecko would like to turn on the stylo layout tests (tests/unit/stylo) in Gecko CI. The plan for doing this is to add the tests as a dev-dependency of Gecko's main Rust library, from which `cargo test` can be run in the usual fashion. Doing this creates problems for normal development, because the stylo tests need the `selectors` crate to be compiled with `gecko_like_types`, whereas the `geckolib` crate does not. So if we compile `geckolib` in a non-test build configuration, the `selectors` crate is compiled without `gecko_like_types`...but then if we compile `geckolib` in a test build configuration, cargo will evict the previous rlib for the `selectors` crate and replace it with a `selectors` compiled with gecko_like_types. And then compiling `geckolib` in a non-test configuration repeats the process, and so forth. Needless to say, this is highly annoying behavior. It is due to a bug in cargo: rust-lang/cargo#3923 but it's not known when that bug will get fixed. In the meantime, we can just make sure that geckolib's `selectors` is compiled with the same features as the `selectors` crate in the stylo tests. - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors Source-Repo: https://github.com/servo/servo Source-Revision: 1aa8be392b0ab8e7a8426f525361b40b69d70b4f UltraBlame original commit: 962216b7844402a956a9b410fed0567506f96261
The easy part of this patch is the addition of the RustTest itself. The more difficult to understand part of the patch is the changes to all of our Rust build configuration. We do this due to a bug in cargo: rust-lang/cargo#3923 where features on dependent crates are not correctly taken into account when determining whether cached artifacts on disk are valid and whether they should be evicted from the disk cache. The practical upshot of this behavior is that, say, running gtests during normal development when files in libxul are modified will: * rebuild some Rust dependencies for libxul; * link libxul; * rebuild those same Rust dependencies *again* for libxul-gtest, since we have different features active and therefore the old artifacts look to be out of date; * link libxul-gtest. Needless to say, this is highly annoying and counterproductive behavior. The "fix" is to ensure that the gkrust-shared crate explicitly depends on crates and assigns features to them such that the feature sets do not change between normal builds and testing builds. This is admittedly fragile, but it is not the first time this has come up, and is probably not the last. UltraBlame original commit: 7d0cf5897a623819500a45b042cce3732c8ad86d
The easy part of this patch is the addition of the RustTest itself. The more difficult to understand part of the patch is the changes to all of our Rust build configuration. We do this due to a bug in cargo: rust-lang/cargo#3923 where features on dependent crates are not correctly taken into account when determining whether cached artifacts on disk are valid and whether they should be evicted from the disk cache. The practical upshot of this behavior is that, say, running gtests during normal development when files in libxul are modified will: * rebuild some Rust dependencies for libxul; * link libxul; * rebuild those same Rust dependencies *again* for libxul-gtest, since we have different features active and therefore the old artifacts look to be out of date; * link libxul-gtest. Needless to say, this is highly annoying and counterproductive behavior. The "fix" is to ensure that the gkrust-shared crate explicitly depends on crates and assigns features to them such that the feature sets do not change between normal builds and testing builds. This is admittedly fragile, but it is not the first time this has come up, and is probably not the last. UltraBlame original commit: 7d0cf5897a623819500a45b042cce3732c8ad86d
The easy part of this patch is the addition of the RustTest itself. The more difficult to understand part of the patch is the changes to all of our Rust build configuration. We do this due to a bug in cargo: rust-lang/cargo#3923 where features on dependent crates are not correctly taken into account when determining whether cached artifacts on disk are valid and whether they should be evicted from the disk cache. The practical upshot of this behavior is that, say, running gtests during normal development when files in libxul are modified will: * rebuild some Rust dependencies for libxul; * link libxul; * rebuild those same Rust dependencies *again* for libxul-gtest, since we have different features active and therefore the old artifacts look to be out of date; * link libxul-gtest. Needless to say, this is highly annoying and counterproductive behavior. The "fix" is to ensure that the gkrust-shared crate explicitly depends on crates and assigns features to them such that the feature sets do not change between normal builds and testing builds. This is admittedly fragile, but it is not the first time this has come up, and is probably not the last. UltraBlame original commit: 7d0cf5897a623819500a45b042cce3732c8ad86d
Remove the syn dependency from gkrust-shared, which was added in bug 1373878 as a workaround for this Cargo bug: rust-lang/cargo#3923 MozReview-Commit-ID: L34J0davEYd UltraBlame original commit: 580a27dbc127ef6ea45de4122395361bc22190d9
Remove the syn dependency from gkrust-shared, which was added in bug 1373878 as a workaround for this Cargo bug: rust-lang/cargo#3923 MozReview-Commit-ID: L34J0davEYd UltraBlame original commit: 580a27dbc127ef6ea45de4122395361bc22190d9
Remove the syn dependency from gkrust-shared, which was added in bug 1373878 as a workaround for this Cargo bug: rust-lang/cargo#3923 MozReview-Commit-ID: L34J0davEYd UltraBlame original commit: 580a27dbc127ef6ea45de4122395361bc22190d9
Switching between building rocket with and without feature flags causes the
cookie
dependency to be rebuilt each item. The following logs show this:Running with
RUST_LOG=cargo::ops::cargo_rustc::fingerprint=info
shows that this is caused by a fingerprint error due tocookie
in turn due toring
.cc @briansmith
The text was updated successfully, but these errors were encountered: