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

feat(git): blobless git #11401

Merged
merged 12 commits into from Sep 9, 2021
Merged

feat(git): blobless git #11401

merged 12 commits into from Sep 9, 2021

Conversation

rarkins
Copy link
Collaborator

@rarkins rarkins commented Aug 24, 2021

Changes:

Uses blobless git cloning instead of shallow cloning.

Context:

Closes #11398

Documentation (please check one with an [x])

  • I have updated the documentation, or
  • No documentation update is required

How I've tested my work (please tick one)

I have verified these changes via:

  • Code inspection only, or
  • Newly added/modified unit tests, or
  • No unit tests but ran on a real repository, or
  • Both unit tests + ran on a real repository

Uses blobless git cloning instead of shallow cloning.

Closes #11398
@rarkins rarkins requested a review from viceice August 24, 2021 07:54
@rarkins
Copy link
Collaborator Author

rarkins commented Aug 24, 2021

Using this repo as an example, comparing shallow to blobless:

❯ time git clone https://github.com/renovatebot/renovate --depth=2
Cloning into 'renovate'...
remote: Enumerating objects: 2302, done.
remote: Counting objects: 100% (2302/2302), done.
remote: Compressing objects: 100% (1964/1964), done.
remote: Total 2302 (delta 281), reused 1007 (delta 158), pack-reused 0
Receiving objects: 100% (2302/2302), 5.32 MiB | 20.32 MiB/s, done.
Resolving deltas: 100% (281/281), done.
git clone https://github.com/renovatebot/renovate --depth=2  0.32s user 0.41s system 39% cpu 1.854 total

/tmp/git
❯ du -sh renovate
 42M    renovate
❯ time git clone https://github.com/renovatebot/renovate --filter=blob:none
Cloning into 'renovate'...
remote: Enumerating objects: 65735, done.
remote: Counting objects: 100% (315/315), done.
remote: Compressing objects: 100% (228/228), done.
remote: Total 65735 (delta 108), reused 171 (delta 77), pack-reused 65420
Receiving objects: 100% (65735/65735), 8.47 MiB | 17.38 MiB/s, done.
Resolving deltas: 100% (41332/41332), done.
remote: Enumerating objects: 1781, done.
remote: Counting objects: 100% (1225/1225), done.
remote: Compressing objects: 100% (1144/1144), done.
remote: Total 1781 (delta 194), reused 119 (delta 81), pack-reused 556
Receiving objects: 100% (1781/1781), 5.24 MiB | 5.42 MiB/s, done.
Resolving deltas: 100% (266/266), done.
Updating files: 100% (1811/1811), done.
git clone https://github.com/renovatebot/renovate --filter=blob:none  1.23s user 1.05s system 46% cpu 4.929 total

/tmp/git
❯ du -sh renovate                                                          
 53M    renovate

When I re-ran this multiple times I had shallow clone vary between 1.9 and 4.5s too, but I'll go with 1.9 for now (best case).

Time taken increased from 1.9s to 4.9s and disk increased from 42MB to 53MB.

On the scale of things, a 3s increase is not important for such a large repo, and nor is ~10MB. Also need to consider that the shallow approach requires ref syncing/fetching later which adds to the time.

Copy link
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should do some real runs to verify everything works as expected. otherwise LGTM

viceice
viceice previously approved these changes Aug 25, 2021
@rarkins
Copy link
Collaborator Author

rarkins commented Aug 25, 2021

@viceice I think I'll wait until I update the app to v26 before merging this. Just one less thing..

@rarkins rarkins marked this pull request as draft August 25, 2021 07:46
@rarkins rarkins marked this pull request as ready for review September 1, 2021 18:01
# Conflicts:
#	lib/util/git/index.spec.ts
#	lib/util/git/index.ts
@rarkins rarkins requested a review from viceice September 8, 2021 11:44
Copy link
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you update the devel docs, as this requires git v2.26 ? Not sure aabout the version. Otherwise LGTM

@rarkins
Copy link
Collaborator Author

rarkins commented Sep 8, 2021

Maybe I'll include this in the major release and say git 2.33.0 is required? It's hard to tell which "improvements" to e.g. sparse checkout will be necessary or not but I'd like to add it soon so asking for 2.32.0 or 2.33.0 seems like a good idea. https://github.blog/2021-08-16-highlights-from-git-2-33/

@rarkins rarkins marked this pull request as draft September 8, 2021 13:04
@viceice viceice added the breaking Breaking change, requires major version bump label Sep 8, 2021
@viceice
Copy link
Member

viceice commented Sep 8, 2021

Yes, lets do it in major bump, as it's potentially breaking some self-hosted users.

@rarkins rarkins marked this pull request as ready for review September 8, 2021 14:14
Copy link
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we bump node js to v14.17? We should also consider removing node v12 from engines?

@HonkingGoose
Copy link
Collaborator

Should we bump node js to v14.17? We should also consider removing node v12 from engines?

If you want to do this it should go in a new PR so that it's captured in a changelog from the commit. 😉

@rarkins
Copy link
Collaborator Author

rarkins commented Sep 9, 2021

Let's leave the engines-next setting for now

@rarkins rarkins merged commit d6d4124 into main Sep 9, 2021
@rarkins rarkins deleted the feat/11398-blobless-git branch September 9, 2021 07:25
@renovate-release
Copy link
Collaborator

🎉 This PR is included in version 27.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
breaking Breaking change, requires major version bump
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace shallow clones with blobless
4 participants