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

Add global_cache_tracker stability tests. #13467

Merged
merged 6 commits into from
Feb 21, 2024

Conversation

ehuss
Copy link
Contributor

@ehuss ehuss commented Feb 20, 2024

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.

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.
@rustbot
Copy link
Collaborator

rustbot commented Feb 20, 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-infrastructure Area: infrastructure around the cargo repo, ci, releases, etc. A-testing-cargo-itself Area: cargo's tests S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 20, 2024
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.
Copy link
Member

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?

Copy link
Contributor Author

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.

Copy link
Member

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 :)

@weihanglo
Copy link
Member

Thanks for this!

@bors r+

@bors
Copy link
Contributor

bors commented Feb 21, 2024

📌 Commit a82794e has been approved by weihanglo

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 21, 2024
@bors
Copy link
Contributor

bors commented Feb 21, 2024

⌛ Testing commit a82794e with merge cafbc12...

@bors
Copy link
Contributor

bors commented Feb 21, 2024

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing cafbc12 to master...

@bors bors merged commit cafbc12 into rust-lang:master Feb 21, 2024
21 checks passed
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 28, 2024
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)
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 28, 2024
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)
@ehuss ehuss added this to the 1.78.0 milestone Mar 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-infrastructure Area: infrastructure around the cargo repo, ci, releases, etc. A-testing-cargo-itself Area: cargo's tests S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants