search

carvel-dev / vendir

Easy way to vendor portions of git repos, github releases, helm charts, docker image contents, etc. declaratively
https://carvel.dev/vendir
Apache License 2.0
269 stars 47 forks source link

Avoid fetching unnecessary git tags and branches #251

Closed scothis closed 1 year ago

scothis commented 1 year ago

When a git ref is specified, it's not necessary to pull refs for every tag and branch in the repo. For large repos with lots of branch and tags this change can result in a multiple orders of magnitude speed up.

scothis commented 1 year ago

I believe this to be an optimization that is compatible with existing usage patterns, but I'm not an expert in all the ways git repos are consumed with vendir.

Using vendir 0.33.1 fetching a branch took 93 seconds. With this change, syncing the same content took 17 seconds.

neil-hickey commented 1 year ago

LGTM! Makes a lot of sense. I can't see a reason this would be a breaking change.... By specifying a branch / tag, you are effectively only ever going to checkout that code anyways. I guess it's possible someone is relying on the fact we are pulling down all the branches locally. But it's not really the intended outcome from specifying a tag. At least I as a user wouldn't expect to rely on this behaviour...

scothis commented 1 year ago

@neil-hickey what are the next steps?

kumaritanushree commented 1 year ago

@scothis did you tested your changes with ref: . In case of tag it is still fetching all branches and tags. Is it expected? I think you would want to avoid fetching all tags and branch in case of tag as well.

scothis commented 1 year ago

@kumaritanushree ~good catch, it works with branches but not tags. I'll keep digging to find a way the makes both happy.~

edit: I had misconfigured my vendir.yml for a tag, I tried again and the same vendir.yml works with git branches, tags, and commits with both vendir 0.32.2 and this PR. All of the tags and branches are still fetched for tags and commits. It doesn't take advantage of the optimized flow that exists for branches, but it still works.

kumaritanushree commented 1 year ago

@scothis I verified your changes as well and my doubt was do we want same functionality for tag and commit or not?

scothis commented 1 year ago

@kumaritanushree sorry for the confusion. It's possible to efficiently clone tags and branches using git clone -b <branchname | tagname> --depth 1 <repository>, but this will fail when the ref is a commit digest. We could try to sniff a commit based on the length of the ref, but it won't be perfect.

etirta commented 1 year ago

Just want to say I really want this changes so bad. Thx @scothis for improving this. When can this be merged? Cheers.