Turning cargo into a git subtree in rust-lang/rust

[weihanglo]

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 to let _ 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

It generally doesn't seem like its being too 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 close rust-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
  • test: relax panic output assertion #14989
  • 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
• 1.84.0
  • add unstable -Zroot-dir flag to configure the path from which rustc should be invoked #14752
  • test: adjust cargo_test_env to unblock rust submodule update #14803
• 1.83.0
  • chore: revert change to Cargo.lock in f25806c #14547
  • test: relax panic output assertion #14602
  • initial version of checksum based freshness #14137
• 1.82.0
  • test: Fix some test based on rustc version #14282
  • Change tests to support rustc wording changes #14341
  • fix(build-std): remove hack on creating virtual std workspace  #14358
  • fix: std Cargo.lock moved to library dir #14370
• 1.81.0
  • Change tests to support rustc wording changes #14135
  • test: fix several assertions #14167
  • test: relax redactions for rust-lang/rust #14203
  • Fix compatible_with_older_cargo test. #14212
• 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 to let _ due to nightly rustc change #13964
• 1.79.0
  • Make sure to also wrap the initial -vV invocation #13659
• 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
  • chore: tracing be compat with rustc_log #13239
  • fix(json-msg): use pkgid spec in in JSON messages #13311
• 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's omit-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

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

[weihanglo]

@rustbot note "Case: target pointer width changed to int in target-spec schema"

• test: avoid hardcoded target spec json #15880
• Make target pointer width in target json an integer rust#144443
• #t-compiler > Is it possible to add a commit to a submodule? @ 💬

[weihanglo]

@rustbot note "Case: rustc lint message changed (with group info)"

• update tests to match lint message changes from rust-lang/rust#140794 #15849
• mention lint group in default level lint note rust#140794

[weihanglo]

@rustbot note "Case: remove unnecessary c-int-width in target spec json schema"

• Remove unnecessary target-c-int-width from target specs #15759
• Use serde for target spec json deserialize rust#144218

[weihanglo]

@rustbot note "Case: bump cc-rs for all tools in rust-lang/rust including cargo"

• Bump cc to 1.2.16 to fix x86 windows jobs in rust-lang/rust CI #15245
• Tracking issue for i686 msvc CI issues with latest Windows 2022 runner image 20250224.5.0+ rust#137733

[weihanglo]

@rustbot note "Case: In linker verbose output Cargo test suite emitted unnecessary warnings"

• Remove unused -C link-arg=-fuse-ld=lld #15097
• show linker output even if the linker succeeds rust#119286

[weihanglo]

@rustbot note "Case: missing_abi lint"

• Make "C" explicit in extern "C". #15034
• Make missing_abi lint warn-by-default. rust#132397

[Kobzol]

One additional motivation: experimenting with nightly rustc features used by Cargo. If I change e.g. the output of --json=timings, I need to get the corresponding change landed in Cargo and sync it back to r-l/r in the same day the Rust change landed (which is quite unlikely), or it will break nightly users of that feature for a few days.

Also perf. experiments would be much easier, currently if I want to test some perf. change in Cargo (that interacts with rustc), I need to do the change in my cargo fork, update the submodule URL in r-l/r and then do a perf. run. Not many people do this today, but if it was a subtree, these changes would be much simpler to be experimented upon.

Reverting to a submodule is trivial. Going then back from a submodule to a subtree again should also be possible.

The general policy for PRs changing only the subtree is to send them directly to the subtree repo. We have triagebot automation that warns people about this.

[epage]

As an anecdote, working on cargo script frontmatter, I had started to contribute to src/tools/rustfmt which is a subtree. I'm familiar with the Project as a whole and I've even interacted with the upstream rust-lang/rustfmt before and I didn't notice my mistake. That PR was abandoned but it wasn't until months later when I took up a subset of it again that I realized my mistake. As an aside, in the end I still ended up making the contribution to rust-lang/rust because the rust-lang/rustfmt repo has gone dark.

[epage]

@weihanglo could we link out to or describe the differences in test coverage? I'm concerned people will make changes to rust-lang/rust's cargo which fail when synced to cargo and the burden that puts on the person babysitting the sync process particularly when they may be unfamiliar with the problem. I see a couple cases where this may come up

• Making a change to cargo and expecting the test suite to be sufficient
• Making a general change and assuming the test suite will catch everything that needs updating

[epage]

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.

For me, a minimum requirement should be

• beta branch and release should be blocked on all rust-lang/rust changes being pushed to rust-lang/cargo.
• release should be blocked on all rust-lang/cargo changes being pushed to rust-lang/rust

[ehuss]

@weihanglo, I think it would be helpful to collect the issues and concerns about making the transition, along with potential solutions to address them. I think we've all known that there are many benefits, but there were various technical, procedural, and policy issues that have held it back.

Roughly from what I remember being discussed in the past:

• Who is going to do all the work to actually integrate everything and do the migration?
  • Setting up the tooling.
  • Setting up the automated syncing.
  • Documenting the processes.
  • Making changes to bootstrap.
  • Making changes to the CI scripts which check out submodules and such.
  • Fixing things that might break (like tidy).
  • Updating triagebot config.
  • Setting up rustfmt checks.
• Who is going to monitor changes for cargo on rust-lang/rust?
• Who is going to do the syncs, and approve/merge them?
• There are concerns about people having uncertainty about which repo to make changes, and when. What will be the exact policy? Will a triagebot comment be sufficient to inform people?
• Who is going to approve changes that need to happen on rust-lang/rust?
• When a change is merged in rust-lang/rust that breaks cargo's CI, who will be responsible for quickly getting that fixed?
• What test changes will happen in rust-lang/rust? Should we disable the "no nightly tests" option to ensure changes in rust-lang/rust are correct?
  • What about the rest of cargo's coverage, such as targets that aren't tested on rust-lang/rust, cross-compilation, or all the other things that we test that aren't tested there?
• What about the issue tracker problem? Will triagebot's issue-links warning be sufficient? What about the roughly 16,000 issues that could potentially be impacted?
• How will we ensure that the beta and stable branches stay in sync with cargo's repo? It is crucial that rust-lang/rust doesn't have commits that aren't synced back (but also crucial that we are able to make changes in rust-lang/cargo that aren't pushed to rust-lang/rust).

We've had many discussions about this in the past, but I can't find the summaries, so I'm probably missing some concerns we had raised. (Some of them are no longer relevant since switching to an isolated workspace.) Hopefully this is food for thinking of any other things that come to mind.

[Kobzol]

Who is going to do all the work to actually integrate everything and do the migration?

Happy to work on all that, I did similar work for a bunch of subtrees lately.

What about the issue tracker problem? Will triagebot's issue-links warning be sufficient? What about the roughly 16,000 issues that could potentially be impacted?

Triagebot is able to rewrite PR descriptions to make issue links absolute, we already use that in several subtrees. So as long as commit messages don't contain "Fixes: XYZ", it should be fine.