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.

Sign up for GitHub

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

Jump to bottom

chore(ci): Ensure lockfile is respected during MSRV testing #13523

Merged
merged 1 commit into from
Mar 3, 2024
Merged

chore(ci): Ensure lockfile is respected during MSRV testing #13523

merged 1 commit into from
Mar 3, 2024

Conversation

epage
Copy link
Contributor

@epage epage commented Mar 3, 2024

As a hack in cargo-hack, it doesn't respect lockfiles when doing MSRV testing unless --locked is passed in.
This adds that so we make sure we don't run into problems with newer, MSRV-imcompatible dependencies come out that break our build.

See

@epage
chore(ci): Ensure lockfile is respected during MSRV testing
0564365 
As a hack in cargo-hack, it doesn't respect lockfiles when doing MSRV
testing unless `--locked` is passed in.
This adds that so we make sure we don't run into problems with newer,
MSRV-imcompatible dependencies come out that break our build.

See
- https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/gix-ref.20CI.20error/near/423319798
- taiki-e/cargo-hack#234
- taiki-e/cargo-hack#236
@rustbot
Copy link
Collaborator

rustbot commented Mar 3, 2024

r? @ehuss

rustbot has assigned @ehuss.
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. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 3, 2024
@epage epage mentioned this pull request Mar 3, 2024
Merged
@taiki-e
Copy link
Member

taiki-e commented Mar 3, 2024 •
edited
Loading

https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/gix-ref.20CI.20error/near/423319798

I didn't find out until recently that when doing MSRV testing, they blow away the lockfile because they assume you aren't committing your lockfile and they want to make sure they use a lockfile format that is compatible with your MSRV.

If this were the only reason, we could have changed that behavior without any problem, but unfortunately the fact that cargo has made incompatible lockfile changes in the past complicates problems.

Especially when running locally on a project with updated/uncommitted lockfiles, without this functionality, calling --rust-version/--version-range after a cargo update or a previous build by the latest cargo will cause the build to fail. Such a build fail is a very annoying behavior from a QoL perspective.

Ideally, I would like to determine on the cargo-hack side which older lockfile formats cause compatibility issues and call generate-lockfile only when necessary, but I'm not sure how much more complicated that would be.

@taiki-e taiki-e mentioned this pull request Mar 3, 2024
Open
@ehuss
Copy link
Contributor

ehuss commented Mar 3, 2024

Thanks!

@bors r

@bors
Copy link
Contributor

bors commented Mar 3, 2024

📌 Commit 0564365 has been approved by ehuss

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 Mar 3, 2024
@bors
Copy link
Contributor

bors commented Mar 3, 2024

⌛ Testing commit 0564365 with merge 352d2f3...

@bors
Copy link
Contributor

bors commented Mar 3, 2024

☀️ Test successful - checks-actions
Approved by: ehuss
Pushing 352d2f3 to master...

@bors bors merged commit 352d2f3 into rust-lang:master Mar 3, 2024
21 checks passed
@bors bors mentioned this pull request Mar 3, 2024
Closed
@epage epage deleted the locked branch March 4, 2024 15:27
@epage
Copy link
Contributor Author

epage commented Mar 4, 2024

https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/gix-ref.20CI.20error/near/423319798

I didn't find out until recently that when doing MSRV testing, they blow away the lockfile because they assume you aren't committing your lockfile and they want to make sure they use a lockfile format that is compatible with your MSRV.

If this were the only reason, we could have changed that behavior without any problem, but unfortunately the fact that cargo has made incompatible lockfile changes in the past complicates problems.

I don't see how what we said was different

Especially when running locally on a project with updated/uncommitted lockfiles, without this functionality, calling --rust-version/--version-range after a cargo update or a previous build by the latest cargo will cause the build to fail. Such a build fail is a very annoying behavior from a QoL perspective.

Ideally, I would like to determine on the cargo-hack side which older lockfile formats cause compatibility issues and call generate-lockfile only when necessary, but I'm not sure how much more complicated that would be.

FYI #12861 was merged which makes lockfile generation respect MSRV.

imo if working with a version older than that

  • If the lockfile exists. Only generate the lockfile if it isn't present
  • Maybe add a --generate-lockfile flag to opt-in to the current behavior

@weihanglo weihanglo mentioned this pull request Mar 6, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 6, 2024
@bors
Auto merge of rust-lang#122054 - weihanglo:update-cargo, r=weihanglo
07a39aa 
Update cargo

11 commits in f772ec0224d3755ce52ac5128a80319fb2eb45d0..c3c417b85e01a1de1633317fa55e4f1a31e346d4
2024-03-01 22:57:35  0000 to 2024-03-06 01:16:08  0000
- fix(rustdoc-map): dedup `--extern-html-too-url` for same unit (rust-lang/cargo#13544)
- test: Add test for packaging a public dependency (rust-lang/cargo#13536)
- doc: Edits for git/path dependency sections (rust-lang/cargo#13341)
- feat(cli): Allow logging to chrome traces (rust-lang/cargo#13399)
- fix(log): Trace parameters to align with profile (rust-lang/cargo#13538)
- fix(toml): Don't warn on unset Edition if only 2015 is compatible (rust-lang/cargo#13533)
- fix(cli): Trace core cargo operations (rust-lang/cargo#13532)
- chore: update pulldown-cmark to 0.10.0 (rust-lang/cargo#13517)
- feat(add): Fallback to `rustc -v` when no MSRV is set (rust-lang/cargo#13516)
- chore(ci): Ensure lockfile is respected during MSRV testing (rust-lang/cargo#13523)
- feat: Use consistent colors when testing (rust-lang/cargo#13520)

r? ghost
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 7, 2024
@bors
Auto merge of rust-lang#122054 - weihanglo:update-cargo, r=weihanglo
1508a03 
Update cargo

14 commits in f772ec0224d3755ce52ac5128a80319fb2eb45d0..a4c63fe5388beaa09e5f91196c86addab0a03580
2024-03-01 22:57:35  0000 to 2024-03-06 22:15:17  0000
- fix(cli): Skip tracing-chrome for platforms without 64bit atomics (rust-lang/cargo#13551)
- chore: downgrade to openssl v1.1.1 (again) (rust-lang/cargo#13550)
- fix(cli): Add traces to clarify where time is going (rust-lang/cargo#13545)
- fix(rustdoc-map): dedup `--extern-html-too-url` for same unit (rust-lang/cargo#13544)
- test: Add test for packaging a public dependency (rust-lang/cargo#13536)
- doc: Edits for git/path dependency sections (rust-lang/cargo#13341)
- feat(cli): Allow logging to chrome traces (rust-lang/cargo#13399)
- fix(log): Trace parameters to align with profile (rust-lang/cargo#13538)
- fix(toml): Don't warn on unset Edition if only 2015 is compatible (rust-lang/cargo#13533)
- fix(cli): Trace core cargo operations (rust-lang/cargo#13532)
- chore: update pulldown-cmark to 0.10.0 (rust-lang/cargo#13517)
- feat(add): Fallback to `rustc -v` when no MSRV is set (rust-lang/cargo#13516)
- chore(ci): Ensure lockfile is respected during MSRV testing (rust-lang/cargo#13523)
- feat: Use consistent colors when testing (rust-lang/cargo#13520)
@ehuss ehuss added this to the 1.78.0 milestone Mar 10, 2024
@taiki-e taiki-e mentioned this pull request Mar 31, 2024
Merged
lnicola pushed a commit to lnicola/rust-analyzer that referenced this pull request Apr 7, 2024
@bors
Auto merge of #122054 - weihanglo:update-cargo, r=weihanglo
0a760e2 
Update cargo

14 commits in f772ec0224d3755ce52ac5128a80319fb2eb45d0..a4c63fe5388beaa09e5f91196c86addab0a03580
2024-03-01 22:57:35  0000 to 2024-03-06 22:15:17  0000
- fix(cli): Skip tracing-chrome for platforms without 64bit atomics (rust-lang/cargo#13551)
- chore: downgrade to openssl v1.1.1 (again) (rust-lang/cargo#13550)
- fix(cli): Add traces to clarify where time is going (rust-lang/cargo#13545)
- fix(rustdoc-map): dedup `--extern-html-too-url` for same unit (rust-lang/cargo#13544)
- test: Add test for packaging a public dependency (rust-lang/cargo#13536)
- doc: Edits for git/path dependency sections (rust-lang/cargo#13341)
- feat(cli): Allow logging to chrome traces (rust-lang/cargo#13399)
- fix(log): Trace parameters to align with profile (rust-lang/cargo#13538)
- fix(toml): Don't warn on unset Edition if only 2015 is compatible (rust-lang/cargo#13533)
- fix(cli): Trace core cargo operations (rust-lang/cargo#13532)
- chore: update pulldown-cmark to 0.10.0 (rust-lang/cargo#13517)
- feat(add): Fallback to `rustc -v` when no MSRV is set (rust-lang/cargo#13516)
- chore(ci): Ensure lockfile is respected during MSRV testing (rust-lang/cargo#13523)
- feat: Use consistent colors when testing (rust-lang/cargo#13520)
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request Apr 27, 2024
@bors
Auto merge of #122054 - weihanglo:update-cargo, r=weihanglo
a6f9708 
Update cargo

14 commits in f772ec0224d3755ce52ac5128a80319fb2eb45d0..a4c63fe5388beaa09e5f91196c86addab0a03580
2024-03-01 22:57:35  0000 to 2024-03-06 22:15:17  0000
- fix(cli): Skip tracing-chrome for platforms without 64bit atomics (rust-lang/cargo#13551)
- chore: downgrade to openssl v1.1.1 (again) (rust-lang/cargo#13550)
- fix(cli): Add traces to clarify where time is going (rust-lang/cargo#13545)
- fix(rustdoc-map): dedup `--extern-html-too-url` for same unit (rust-lang/cargo#13544)
- test: Add test for packaging a public dependency (rust-lang/cargo#13536)
- doc: Edits for git/path dependency sections (rust-lang/cargo#13341)
- feat(cli): Allow logging to chrome traces (rust-lang/cargo#13399)
- fix(log): Trace parameters to align with profile (rust-lang/cargo#13538)
- fix(toml): Don't warn on unset Edition if only 2015 is compatible (rust-lang/cargo#13533)
- fix(cli): Trace core cargo operations (rust-lang/cargo#13532)
- chore: update pulldown-cmark to 0.10.0 (rust-lang/cargo#13517)
- feat(add): Fallback to `rustc -v` when no MSRV is set (rust-lang/cargo#13516)
- chore(ci): Ensure lockfile is respected during MSRV testing (rust-lang/cargo#13523)
- feat: Use consistent colors when testing (rust-lang/cargo#13520)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@ehuss ehuss

Labels
A-infrastructure Area: infrastructure around the cargo repo, ci, releases, etc. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.78.0
Development

Successfully merging this pull request may close these issues.
5 participants
@epage @rustbot @taiki-e @ehuss @bors