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

Use shallow clones for submodules #89757

Merged
merged 1 commit into from
Oct 14, 2021
Merged

Use shallow clones for submodules #89757

merged 1 commit into from
Oct 14, 2021

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Oct 11, 2021

This reduces the amount of git history downloaded for submodules from ~67M to ~11M. For comparison, a shallow clone of rust-lang/rust is 103M and a deep clone is 740M, so this almost halves the amount of history necessary if you made a shallow clone to start, and it's a significant reduction even if not.

Closes #63978. r? @Mark-Simulacrum

This reduces the amount of git history downloaded from ~67M to ~11M.
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 11, 2021
@jyn514 jyn514 added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Oct 11, 2021
@Mark-Simulacrum
Copy link
Member

I've tried to tweak the lines in the above bootstrap.py script to limit the depth of the submodule update. Unfortunately, using --depth=1 doesn't work, because the submodule commit referenced here may not be the latest in the submodule repo, and apparently git cannot clone at a specific commit.

Is this fixed by something? IIRC, this was (one of) the reasons we did not do this historically. I also have some recollection that fetching new commits in a shallow repository is more work for GitHub and the client, but I could be wrong about that...

@Mark-Simulacrum Mark-Simulacrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 11, 2021
@jyn514
Copy link
Member Author

jyn514 commented Oct 13, 2021

using --depth=1 doesn't work, because the submodule commit referenced here may not be the latest in the submodule repo, and apparently git cannot clone at a specific commit.

Is this fixed by something?

I'm not sure what error they ran into originally, but I tested cloning rust-lang/rust from scratch and having x.py do a shallow clone for the first time, and it worked fine.

I also have some recollection that fetching new commits in a shallow repository is more work for GitHub and the client, but I could be wrong about that...

Hmm, so in CocoaPods/CocoaPods#4989 (comment) Github said shallow clones were more expensive over time if you don't subsequently use --depth=1. But we do use --depth=1 in further updates; e.g. here's a change from nightly to 1.55, which has to update most submodules:

$ git checkout 1.55
M	library/backtrace
M	library/stdarch
M	src/tools/cargo
M	src/tools/miri
M	src/tools/rls
$ x check
Updating only changed submodules
Updating submodule src/tools/cargo
Submodule path 'src/tools/cargo': checked out '32da73ab19417aa89686e1d85c1440b72fdf877d'
Updating submodule src/tools/rls
Submodule path 'src/tools/rls': checked out 'a82a0527057eab59c24a0452add35ccb39584e80'
Updating submodule src/tools/miri
Submodule path 'src/tools/miri': checked out 'e2872a3f2a26154b91a6a6085d56016509803c61'
Updating submodule library/backtrace
Submodule path 'library/backtrace': checked out '221483ebaf45df5c956adffee2d4024e7c3b96b8'
Submodule 'backtrace-sys/src/libbacktrace' (https://github.com/rust-lang/libbacktrace) registered for path 'library/backtrace/crates/backtrace-sys/src/libbacktrace'
Cloning into '/home/joshua/src/rust/rust-shallow/library/backtrace/crates/backtrace-sys/src/libbacktrace'...
remote: Enumerating objects: 363, done.        
remote: Total 363 (delta 0), reused 0 (delta 0), pack-reused 363        
Receiving objects: 100% (363/363), 846.61 KiB | 2.20 MiB/s, done.
Resolving deltas: 100% (262/262), done.
Submodule path 'library/backtrace/crates/backtrace-sys/src/libbacktrace': checked out '5c88e094a691bb803d4bba342403a10459abad9e'
Updating submodule library/stdarch
Submodule path 'library/stdarch': checked out '3001c75a1d2a81d2a76bef139c69387cb2ebb820'

So I'm not sure that problem is relevant here. (We also don't have many thousands of people running fetch every second, I would expect most contributors to only update submodules maybe once or twice a week depending how often they contribute.)

@jyn514 jyn514 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 13, 2021
@Mark-Simulacrum
Copy link
Member

I'm not sure what error they ran into originally, but I tested cloning rust-lang/rust from scratch and having x.py do a shallow clone for the first time, and it worked fine.

Great! This seems fine to try at least then, and we can always revert easily if folks run into problems.

It may also be that we can use this (presumably new) support to replace our current github archive fetching on CI (https://github.com/rust-lang/rust/blob/master/src/ci/init_repo.sh) which seems like a nice cleanup, but that doesn't need to happen in this PR.

@bors r+

@bors
Copy link
Contributor

bors commented Oct 13, 2021

📌 Commit 9177fa3 has been approved by Mark-Simulacrum

@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 Oct 13, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 13, 2021
…askrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#89347 (suggestion for typoed crate or module)
 - rust-lang#89670 (Improve `std::thread::available_parallelism` docs)
 - rust-lang#89757 (Use shallow clones for submodules)
 - rust-lang#89759 (Assemble the compiler when running `x.py build`)
 - rust-lang#89846 (Add `riscv32imc-esp-espidf` to 1.56 changelog)
 - rust-lang#89853 (Update the 1.56.0 release header for consistency)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 9f0ef18 into rust-lang:master Oct 14, 2021
@rustbot rustbot added this to the 1.57.0 milestone Oct 14, 2021
@jyn514 jyn514 deleted the submodule branch November 27, 2021 10:35
jyn514 added a commit to jyn514/rust that referenced this pull request Feb 7, 2022
…strap.py

I missed this in rust-lang#89757; it made
`x.py test src/bootstrap` very slow.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 7, 2022
…rk-Simulacrum

Use shallow clones for submodules managed by rustbuild, not just bootstrap.py

I missed this in rust-lang#89757; it made
`x.py test src/bootstrap` very slow.
m-ou-se added a commit to m-ou-se/rust that referenced this pull request Feb 7, 2022
…rk-Simulacrum

Use shallow clones for submodules managed by rustbuild, not just bootstrap.py

I missed this in rust-lang#89757; it made
`x.py test src/bootstrap` very slow.
m-ou-se added a commit to m-ou-se/rust that referenced this pull request Feb 7, 2022
…rk-Simulacrum

Use shallow clones for submodules managed by rustbuild, not just bootstrap.py

I missed this in rust-lang#89757; it made
`x.py test src/bootstrap` very slow.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reducing the memory/bandwidth footprint of submodules with ./x.py?
5 participants