-
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
Add global_cache_tracker stability tests. #13467
Conversation
This fixes a bug in the `GlobalCacheTracker::git_checkout_all` function which was using the wrong SQL column name. This function belongs to the set of of functions only used for testing, and this particular function was not yet used.
This adds the `Execs::args` method which forwards to `ProcessBuilder::args` to specify multiple command arguments at once.
This adds the `requires_rustup_stable` option to the cargo_test macro to require `rustup` to exist, and for the `stable` toolchain to be installed. It is required that stable be installed in Cargo's CI. On a developer machine, the tests will be ignored if rustup is not installed. It will also be ignored on rust-lang/rust's CI since it does not have rustup.
This adds some tests to ensure that the database used in the global cache tracker stays compatible across versions. These tests work by using rustup to run both the current cargo and the stable cargo, and verifying that when switching versions, everything works as expected.
r? @weihanglo rustbot has assigned @weihanglo. Use r? to explicitly pick a reviewer |
On Windows, rustup has an issue with recursive cargo invocations. This commit can be removed once rust-lang/rustup#3036 is resolved.
Cargo likes to modify PATH, which circumvents the ability to choose the correct "cargo" executable to run on Windows (because Windows uses PATH for both binary and shared library searching).
check_command("cargo", &["+stable", "--version"]) | ||
// Cargo mucks with PATH on Windows, adding sysroot host libdir, which is | ||
// "bin", which circumvents the rustup wrapper. Use the path directly from | ||
// CARGO_HOME. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would #13468 fix this issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe! It will need to wait until that hits stable, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, yeah. We need it in stable :)
Thanks for this! @bors r+ |
☀️ Test successful - checks-actions |
Update cargo 16 commits in 194a60b2952bd5d12ba15dd2577a97eed7d3c587..8964c8ccff6e420e2a38b8696d178d69fab84d9d 2024-02-21 01:53:45 +0000 to 2024-02-27 19:22:46 +0000 - feat: Add "-Zpublic-dependency" for public-dependency feature. (rust-lang/cargo#13340) - Stabilize global cache data tracking. (rust-lang/cargo#13492) - chore: bump baseline version requirement of sub crates (rust-lang/cargo#13494) - fix(doctest): search native libs in build script outputs (rust-lang/cargo#13490) - chore: fixed a typo(two->too) (rust-lang/cargo#13489) - test: relax help text assertion (rust-lang/cargo#13488) - refactor: clean up for `GlobalContext` rename (rust-lang/cargo#13486) - test(cli): Verify terminal styling (rust-lang/cargo#13461) - fix(cli): Respect CARGO_TERM_COLOR in '--list' and '-Zhelp' (rust-lang/cargo#13479) - Error messages when collecting workspace members now mention the workspace root location (rust-lang/cargo#13480) - fix(add): Improve error when adding registry packages while vendored (rust-lang/cargo#13281) - [docs]:Add missing jump links (rust-lang/cargo#13478) - Add global_cache_tracker stability tests. (rust-lang/cargo#13467) - fix(cli): Control clap colors through config (rust-lang/cargo#13463) - chore: remove the unused function (rust-lang/cargo#13472) - Fix missing brackets (rust-lang/cargo#13470)
Update cargo 16 commits in 194a60b2952bd5d12ba15dd2577a97eed7d3c587..8964c8ccff6e420e2a38b8696d178d69fab84d9d 2024-02-21 01:53:45 +0000 to 2024-02-27 19:22:46 +0000 - feat: Add "-Zpublic-dependency" for public-dependency feature. (rust-lang/cargo#13340) - Stabilize global cache data tracking. (rust-lang/cargo#13492) - chore: bump baseline version requirement of sub crates (rust-lang/cargo#13494) - fix(doctest): search native libs in build script outputs (rust-lang/cargo#13490) - chore: fixed a typo(two->too) (rust-lang/cargo#13489) - test: relax help text assertion (rust-lang/cargo#13488) - refactor: clean up for `GlobalContext` rename (rust-lang/cargo#13486) - test(cli): Verify terminal styling (rust-lang/cargo#13461) - fix(cli): Respect CARGO_TERM_COLOR in '--list' and '-Zhelp' (rust-lang/cargo#13479) - Error messages when collecting workspace members now mention the workspace root location (rust-lang/cargo#13480) - fix(add): Improve error when adding registry packages while vendored (rust-lang/cargo#13281) - [docs]:Add missing jump links (rust-lang/cargo#13478) - Add global_cache_tracker stability tests. (rust-lang/cargo#13467) - fix(cli): Control clap colors through config (rust-lang/cargo#13463) - chore: remove the unused function (rust-lang/cargo#13472) - Fix missing brackets (rust-lang/cargo#13470)
This adds some tests to ensure that the database used in the global cache tracker stays compatible across versions. These tests work by using rustup to run both the current cargo and the stable cargo, and verifying that when switching versions, everything works as expected.
These tests will be ignored on developer environments if they don't have rustup, or don't have the stable toolchain installed. It does assume that "stable" is at least 1.76. It is required for the tests to run in CI, but will be disabled in rust-lang/rust since it does not have rustup.
I'm not expecting too much trouble with these tests, but if they become too fiddly or broken, they can always be changed or removed.
The support code for running "cargo +stable" is very basic right now. If we expand to add similar tests in the future, then I think we could consider adding support functions (such as
tc_process
) to make it easier or more reliable.