-
-
Notifications
You must be signed in to change notification settings - Fork 10.1k
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
Support git partial clones with sparse checkouts #13232
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good so far, nice work!
I'd be interested to see a PR to a homebrew-* repository which makes use of this new feature so we can see what the DSL looks like from that side. |
Thanks for the review @MikeMcQuaid 🙌 Here's a draft PR showing how the new DSL could be used: Homebrew/homebrew-cask-fonts#5758 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great to me. I have only one question about naming but I'm happy with the implementation here.
Will merge a few other maintainers chime in on their preferred DSL naming here. Great work @hmarr!
Thanks @MikeMcQuaid! Before merging, we should probably also decide on a way of handling upgrades from sparse to full checkouts (which would happen in the rare case someone removes
|
This seems fine 👍🏻 |
There is a small change that should be made related to the recently merged in PR #13358 (more information can be found there). Essentially, Before def checksumable?
url.using.blank? || url.using == :post
end After def checksumable?
(url.using.blank? && url.only_paths.blank?) || url.using == :post
end Let me know if that makes sense. |
Co-authored-by: Mike McQuaid <[email protected]>
920bc9b
to
a8c9d38
Compare
@apainintheneck thanks for the heads up, I made that change in ccdd45d @MikeMcQuaid I made the change to the cache path for sparse checkouts in a8c9d38. That was the last thing I had on my list, so I we're ready to go now. Once this is merged, we can start switching the font casks over to git from svn. |
@hmarr Just before this gets merged: interested if you have any thoughts on the comments in https://github.com/Homebrew/homebrew-cask-fonts/pull/5758/files. I ask because I want to ensure that we're creating the optimal API here before we merge and are (somewhat) stuck with it. Thanks! |
The url documentation over at the Cask Cookbook should also probably be updated at some point too. |
@MikeMcQuaid good point! I just replied to your comments there (been away for a couple of weeks, sorry for being slow to get back to you 🐢) |
@hmarr Thanks! Replied over there. |
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
brew style
with your changes locally?brew typecheck
with your changes locally?brew tests
with your changes locally?This adds a new option to enable downloads that use git partial clones with sparse checkouts, which can dramatically speed up clones of large repositories as well as reducing the checkout size on disk. The background and motivation is described in more detail in #13188.
Example (in a relatively small repo so there's not a huge difference)
Without partial clone and sparse checkout: 7.6s.
With partial clone and sparse checkout: 4.8s.
How it works
It relies on two git features working in tandem:
blob:none
and the--no-checkout
option to avoid fetching any blobs during the initial clone.git checkout
is run, only the blobs required for the sparse checkout are fetched (thanks to the partial clone), then only those specified paths appear in the repository working tree.The sparse checkout is created by enabling the
core.sparseCheckout
option, and writing the paths to.git/info/sparse-checkout
. There is agit-sparse-checkout
command that makes this easier, but I've avoided using it as it's relatively new and marked as experimental, and doesn't exist in older versions of git.Git version compatibility
It's a little hard to find builds for old git versions, but I built sevearl load of versions git from source to see what worked.
Sparse checkouts work with git 2.19, but although there's some support for partial clones, they don't work well. It seems to fetch more blobs than are necessary, and it fetches them one at a time, which is very slow.
From git 2.20, both sparse checkouts and partial clones seem to work well. This is the version of git that comes with macOS 10.15, which is the minimum recommended macOS verison Homebrew supports right now. For earlier versions of git, the option is ignored, meaning the full repository is checked out. This feel in line with the "best effort" support guarantees for older versions of macOS that are promised.
Handling repository updates
I did some testing to make sure that things don't to awry after the initial clone.
only_paths
option for the first time, the repository should be configured correctly to use partial clones and sparse checkoutsonly_paths
. It's not totally trivial as sparse checkouts set the skip-worktree bit, which needs to be removed to transition from a sparse checkout to a full checkout. I can see three options:only_paths
isnil
, check to see if thecore.sparseCheckout
option is enabled. If it is, set the paths in.git/info/sparse-checkout
to*
. This technically leaves the repository as a sparse checkout but includes all paths.core.sparseCheckout
option and deleting the.git/info/sparse-checkout
file aftergit checkout
has been run.-sparse
to thecache_tag
ifonly_paths
isn'tnil
, meaning we'd get a different cached repository if the option disabled.Open questions / things I'd love feedback on
only_paths
) is an array of strings, where each string is a path that will be checked out. I suspect that most of the time people will only provide one path, so we could go with a string rather than an array.only_paths
clear enough? Other options I considered weresparse_checkout_paths
,only_checkout_paths
,paths_to_fetch
.Thanks for all your hard work keeping homebrew awesome! ❤️