Use shallow cloning to add pack registries #444
Conversation
Signed-off-by: Bram Vogelaar <[email protected]>
chore: add changelog item Signed-off-by: Bram Vogelaar <[email protected]>
attachmentgenie
force-pushed
the
shallow_clone_registries
branch
from
e2d6d78 to
80cb4ac
Compare
Signed-off-by: Bram Vogelaar <[email protected]>
angrycub
added
stage/accepted
type/enhancement
theme/registry/cache
There was a problem hiding this comment.
Thanks for the PR @attachmentgenie! This is good thinking, in many scenarios (big repo, using pack in CI) a shallow clone makes much more sense than a full clone. However, shallow clones affect what we can do with the repository, and that's why I'd prefer shallow-cloning to be a flag.
For example, if we were to support updating registries, we'd have to introduce special treatment for shallow-cloned repositories, and their updates would take much longer. Secondly, users might want to switch branches of their registries (granted, a niche use-case, but possible with current implementation and defaulting to shallow-clones would break it).
I don't have a strong opinion on whether this should be an opt-in or an opt-out flag, perhaps @angrycub has an opinion? But I think it should be a flag nonetheless.
| @@ -154,6 +154,9 @@ func (c *Cache) cloneRemoteGitRegistry(opts *AddOpts) (string, error) { | |||
| // If ref is set, add query string variable | |||
| if !opts.IsLatest() { | |||
| url = fmt.Sprintf("%s?ref=%s", url, opts.Ref) | |||
| } else { | |||
| // Attempt to shallow clone the constructed url | |||
| url = fmt.Sprintf("%s?depth=1", url) | |||
There was a problem hiding this comment.
I'd rather have the shallow clone be a a flag to the add command than handling it this way.
|
I debated with myself about feature flagging it but ultimately (lazily) decided not to do it because as long as , this bit of code is in place, there are IMHO no benefits to keeping any history more that the commit your are looking for since nomad-pack cleans that up for you automatically after adding the registry But i am more than happy to add the feature flag code if you are considering removing that code in the (near) future |
Right! The team has been thinking about adding an update feature though, and I just thought fetching would be a natural way to do this. The more I think about shallow clones, the more I think this could just be an opt-out feature, as in: we'd default to a shallow clone. |
alright, will update to do just that |
There was a problem hiding this comment.
I've reconsidered, you're right. Shallow-clones are the way to go. Vide #447 (review)
We use a mono repo to build out all of our platforms, as a down side of this our git history and .git directory are huge (think 10x the actual code size)which causes git clones to be quite slow.
This PR switches the git clone behaviour of
nomad-pack registry addto use shallow clone instead. In our particular case it drops the length of the process from ~9s to 0.5s.Reminders
CHANGELOG.mdentry