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.

Sign up for GitHub

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

Jump to bottom

Fetch only master of index, and in shallow mode #403

Open
corneliusweig opened this issue Nov 24, 2019 · 5 comments · May be fixed by #836
Open

Fetch only master of index, and in shallow mode #403

corneliusweig opened this issue Nov 24, 2019 · 5 comments · May be fixed by #836
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. priority/backlog Higher priority than priority/awaiting-more-evidence.

Comments

@corneliusweig
Copy link
Contributor

Currently, we do a plain git fetch:

krew/pkg/gitutil/git.go

Lines 52 to 54 in b303e3a

if err := exec(destinationPath, "fetch", "-v"); err != nil {
return errors.Wrapf(err, "fetch index at %q failed", destinationPath)
}

This has some drawbacks, because it

  • fetches all remote branches and tags
  • .. and it fetches every commit in each branch.

It would be better, if instead we could

  • just fetch the relevant branch (master)
  • .. and only fetch the tip commit of that branch.

That way, we could keep the index folder as lean as possible.

This should do the trick:

if err := git(destinationPath, "fetch", "origin", "master", "--verbose", "--depth=1"); err != nil {
		return errors.Wrapf(err, "fetch index at %q failed", destinationPath)
}
func EnsureCloned(uri, destinationPath string) error {
	if ok, err := IsGitCloned(destinationPath); err != nil {
		return err
	} else if !ok {
		return exec("", "clone", "--branch=master", "--depth=1", "-v", uri, destinationPath)
	}
	return nil
}
@corneliusweig corneliusweig changed the title Maybe fetch krew-index in shallow mode? Maybe fetch index in shallow mode? Nov 24, 2019
@ahmetb
Copy link
Member

ahmetb commented Nov 24, 2019

For now, I think this is not a serious issue.

(1) We are still at a very small number of git objects to make any perf issue noticeable. (2) We don't really do branches that would harm clients, and as you said we can later on migrate to a new model without any pain.

@corneliusweig corneliusweig added the priority/backlog Higher priority than priority/awaiting-more-evidence. label Nov 25, 2019
@corneliusweig
Copy link
Contributor Author

Yes, the --depth is an optimization and not important any time soon.

But only fetching master would already be useful. That way we avoid that people pull useless references from origin. For example, there is also a v0.2.1 branch in my index dir.

@ahmetb
Copy link
Member

ahmetb commented Nov 25, 2019

Yeah, that's not big of a deal but if you want to fix it, I think we can add fetch origin master

@corneliusweig corneliusweig added the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Nov 25, 2019
@ahmetb
Copy link
Member

ahmetb commented Nov 25, 2019

/retitle Fetch only master of index, and in shallow mode

Something else we'd need to consider is going to be private Git repos.

For the centralized index we can hardcode master as the branch name to fetch. However, other index repos (even on GitHub) may choose a different main branch name. So this won’t work for them.

I say let's keep things as is, we can refactor over time if needed. I'm not too worried about extra branches getting fetched atm.
/lifecycle frozen

@k8s-ci-robot k8s-ci-robot added the lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. label Nov 25, 2019
@k8s-ci-robot k8s-ci-robot changed the title Maybe fetch index in shallow mode? Fetch only master of index, and in shallow mode Nov 25, 2019
@slimm609 slimm609 linked a pull request Aug 18, 2023 that will close this issue
Open
@slimm609
Copy link

Put up a PR for this issue. --single-branch with out the --branch=master only clones the default branch so this supports private repos where the branch is different than master. Tested with master in one repo and main in another repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. priority/backlog Higher priority than priority/awaiting-more-evidence.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

clone single branch with depth of 1 slimm609/krew
4 participants
@ahmetb @slimm609 @k8s-ci-robot @corneliusweig