-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Description
This is an issue tracking how many issue we encountered because Cargo is not a Git subtree in rust-lang/rust repository. See also the discussion in #t-cargo > Turning cargo into a subtree.
Below is a summary from t-cargo originally posted in #t-cargo > Turning cargo into a subtree @ 💬.
The Cargo team has discussed subtree migration last week. We've got a couple of questions and would like to understand before proceeding:
- Motiviation of turning into a subtree.
- Burden of resolving conflicts
- Issue / PR managements
- Possibility to revert to submodule
Motivation of turning into a subtree
It seems that it was brought up again because there were some recent pull requests blocked by Cargo's overly-fitted test assertions. Apart from that, are there any other reasons or benefits people wanting Cargo to be a subtree? How painful is it that people need it to happen soon otherwise it toils the development?
From 1.75 (2023-12-28) to 1.83 (as of 2025-01-07), there are around 11 pull requests opened in rust-lang/cargo because test failures in Cargo's test suite really blocked their pull request in rust-lang/rust.
See a list of pull requests
There are other cross-repo efforts like check-cfg and checksum-freshness. However I don't think it is better do it in subtree, as those changes are way larger and may require scrutiny from the Cargo team. See the full list of PRs in the end of this post
- test: relax
bad_crate_type
to only match error message prefix #14990 - Add the
test
cfg as a well known cfg before of compiler change #14963 - test: relax panic output assertion #14602
- Change tests to support
rustc
wording changes #14341 - Change tests to support
rustc
wording changes #14135 - test: switch from
drop
tolet _
due to nightly rustc change #13964 - Relax a test to permit warnings to be emitted, too. #13415
- Apply
-Zpanic-abort-tests
to doctests too #13388 - Change tests to support changes to suggestion #13382
- remove jobserver env var in some tests #13072
- fix: preserve jobserver file descriptors on rustc invocation in
fix_exec_rustc
#12951 - rustdoc: remove the word "Version" from test cases #12800
We generally doesn't seem it being to much a burden for the current submodule process. To myself I have been actively helping people to unblock Cargo and never feel stressful about it. We would like to understand how other teams' feelings. Maybe it is really frustrating to other? Maybe they just didn't know who to ask for helps?
Burden of resolving conflicts
According to feedback from miri and other repos, conflicts seem rare and easy to deal with. The current Cargo maintainer doing sync (me) is also willing to take resposibility to do subtree maintenance as well. However, team still has concerns of the burden.
I've collected data from other subtree projects of how often they synced back to the original repositories:
- miri
- From 2024-01-08 to 2025-01-08
- 160 total bot PR (daily cronjob)
- 39 human (RalfJung) intervention
git log --format='%h %an: %s' --since 2024-01-07 -- rust-version
- https://github.com/rust-lang/miri/commits/dda834f266cc78c049fc0b19fa7b884217368c0a/rust-version
- rust-analyzer
- From 2024-05-19 to 2025-01-08
- 18 manual syncs
git log --format='%h %an: %s' --since 2024-01-07 -- rust-version
- https://github.com/rust-lang/rust-analyzer/commits/f8fce005d7501f3f3db86e2ed4cce2e3f1f5b50a/rust-version
- rust-clippy
- From 2024-01-25 to 2025-01-08
- 20 manual syncs
git log --format='%h %an: %s' --since 2024-01-07 -- rust-toolchain | rg clippy-subtree-update
- rustfmt
- From 2024-06-12 to 2025-01-08: 2 manual syncs
git log --format='%h %an: %s' --since 2024-01-07 -- rust-toolchain
The number of subtree-to-repo syncs are not really as many as I expect. Apparently Miri outnumbered others, though those PRs are mostly one-click-mergeable.
However, rust-lang/cargo has weird CI — it needs to pass all stable, beta, and nightly channel. A change in subtreee may accidentally fail on stable or beta. What if we discovered it only after beta cutoff, should we backport the fix of conflits?
Also, if a subtree-to-repo sync happens after beta cutoff, that sync needs to merge into beta branch (rust-1.y.z
branch ) in rust-lang/cargo. That means cargo might not be able to reuse scripts from existing subtree, or need more tweaks. The CI infra migration possibly requires more works than the recent rustc-dev-guide one.
Issue / PR managements
These are not really big problems but a compilation of small questions about the project management.
- Will GitHub magic keywords close issues with the same number for both repos? For example, in cargo a commit message says
closes #12345
. Will it also closerust-lang/rust#12345
when syncing to subtree? - Similarly, the commit hash changes between repos for the same commit. Which one commit should be the de facto one when discussing/linking/sharing? The question can be seen as "which one is the real source." or "don't duplicate source code" which @Josh Triplett has called out one year ago.
- If a pull request was opened in rust-lang/rust but has nothing to do with compiler, should we merge it then, or request a reopen in rust-lang/cargo? What is the general rule of the current subtrees?
- Should PR to cargo subtree require approvals from the Cargo team? That requires a more granular bors permission though, and we've seen some objections to that because people may want to get a faster merge for small changes without waiting for specific teams/people.
Possibility to revert to submodule
If it turns out not suitable for Cargo's workflow, is it possible to revert back to Git submodule, and how would it be?
If reverting is easier, it is more likely we could test out and see if it is worth it.
Anyhow, the cargo team is more on weighing the benefit and tradeoff. Dont take these questions as objection :)
See the full list of cross-repo PR in rust-lang/cargo
Note: not all of them really need to happen in subtree.
- 1.85.0
- 1.84.0
- 1.83.0
- 1.82.0
- 1.81.0
- 1.80.0
- Populate git information when building Cargo from Rust's source tarball #13832
- Stabilize
-Zcheck-cfg
as always enabled #13571 - test(rustfix): run some tests only on nightly #13890
- Update benchmark formatting for new nightly #13901
- Temporarily fix standard_lib tests on linux. #13931
- test: switch from
drop
tolet _
due to nightly rustc change #13964
- 1.79.0
- 1.78.0
- fix(cli): Skip tracing-chrome for platforms without 64bit atomics #13551
- chore: downgrade to openssl v1.1.1 (again) #13550
- test: relax help text assertion #13488
- Update tests for changes in latest nightly #13444
- Relax a test to permit warnings to be emitted, too. #13415
- Apply
-Zpanic-abort-tests
to doctests too #13388 - test: data layout fix for
x86_64-unknown-none-gnu
#13362 - Change tests to support changes to suggestion #13382
- 1.77.0
- 1.76.0
- make some debug assertion failures more informative #12963
- fix: do not panic when failed to parse rustc commit-hash #12965
- Ignore changing_spec_relearns_crate_types on windows-gnu #12972
- Switch from AtomicU64 to Mutex. #12981
- Respect
rust-lang/rust
'somit-git-hash
#12968 - Handle $message_type in JSON diagnostics #13016
- remove jobserver env var in some tests #13072
- chore: downgrade to openssl v1.1.1 #13144
- 1.75.0
- Adjust
-Zcheck-cfg
for new rustc syntax and behavior #12845 - fix: preserve jobserver file descriptors on rustc invocation in
fix_exec_rustc
#12951 - test(build): generalize test assertion for non-rustup env #12804
- rustdoc: remove the word "Version" from test cases #12800
- Disable custom_target::custom_bin_target on windows-gnu #12763
- Adjust
Summary Notes
- Case: target pointer width changed to int in target-spec schema by weihanglo
- Case: rustc lint message changed (with group info) by weihanglo
- Case: remove unnecessary c-int-width in target spec json schema by weihanglo
- Case: bump cc-rs for all tools in rust-lang/rust including cargo by weihanglo
- Case: In linker verbose output Cargo test suite emitted unnecessary warnings by weihanglo
- Case: missing_abi lint by weihanglo
Managed by @rustbot
—see help for details