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

cmd/go: use shallow clones for new git checkouts #13078

Open
mengzhuo opened this Issue Oct 28, 2015 · 32 comments

Comments

Projects
None yet
@mengzhuo
Contributor

mengzhuo commented Oct 28, 2015

Currently go get * will download all changes data of repositories.
We can improve the download waiting time/size by passing depth argument.

I have already submit a code-review for this at
https://go-review.googlesource.com/#/c/16360/
as @bradfitz asked.

@bradfitz bradfitz changed the title from cmd: improve git pull download size and speed by passing --depth arguments to cmd/go: improve git pull download size and speed by passing --depth arguments Oct 28, 2015

@rakyll

This comment has been minimized.

Show comment
Hide comment
@rakyll

rakyll Oct 28, 2015

Member

Some users depend on go get to clone the repos. How is this going to affect them?

Member

rakyll commented Oct 28, 2015

Some users depend on go get to clone the repos. How is this going to affect them?

@rakyll rakyll added this to the Unplanned milestone Oct 28, 2015

@garukun

This comment has been minimized.

Show comment
Hide comment
@garukun

garukun Oct 28, 2015

+1 for the CL. Maybe add go clone to support @rakyll's case?

garukun commented Oct 28, 2015

+1 for the CL. Maybe add go clone to support @rakyll's case?

@ALTree

This comment has been minimized.

Show comment
Hide comment
@ALTree

ALTree Oct 28, 2015

Member

A suggestion: some data about the magnitude of the improvement in the size of the downloaded repository could probably help the reviewers.

Member

ALTree commented Oct 28, 2015

A suggestion: some data about the magnitude of the improvement in the size of the downloaded repository could probably help the reviewers.

@bradfitz

This comment has been minimized.

Show comment
Hide comment
@bradfitz

bradfitz Oct 28, 2015

Member

It's already the case that "go get" won't always fetch the full history in some cases (subversion), if that precedent matters.

I'm also in favor or switching to shallow clones. Personal motivation: Camlistore's Git repo is huge, but mostly because we used to have some huge assets and other libraries in it, since gone. But that means for a user just wanting a few files, they have to download megabytes.

People who want history can go into the git directories and run the git commands.

Or we can add a new -full or -deep flag to go get.

Member

bradfitz commented Oct 28, 2015

It's already the case that "go get" won't always fetch the full history in some cases (subversion), if that precedent matters.

I'm also in favor or switching to shallow clones. Personal motivation: Camlistore's Git repo is huge, but mostly because we used to have some huge assets and other libraries in it, since gone. But that means for a user just wanting a few files, they have to download megabytes.

People who want history can go into the git directories and run the git commands.

Or we can add a new -full or -deep flag to go get.

@bradfitz

This comment has been minimized.

Show comment
Hide comment
@bradfitz
Member

bradfitz commented Oct 28, 2015

/cc @adg @rsc

@fsouza

This comment has been minimized.

Show comment
Hide comment
@fsouza

fsouza Oct 28, 2015

Contributor

+1 for the feature and for @bradfitz's -full flag, although I think users can always unshallow their repositories manually running git fetch --unshallow.

Contributor

fsouza commented Oct 28, 2015

+1 for the feature and for @bradfitz's -full flag, although I think users can always unshallow their repositories manually running git fetch --unshallow.

@adg

This comment has been minimized.

Show comment
Hide comment
@adg

adg Oct 29, 2015

Contributor

I do think it would be worth switching to shallow clones.

I don't see much value in adding a flag to the go tool.

Contributor

adg commented Oct 29, 2015

I do think it would be worth switching to shallow clones.

I don't see much value in adding a flag to the go tool.

@fsouza

This comment has been minimized.

Show comment
Hide comment
@fsouza

fsouza Oct 29, 2015

Contributor

Maybe having an environment variable for the git command (as well as other vcs commands) would do the trick and make it more flexible, if that's the case. Just like git allows customizing the ssh command with the GIT_SSH_COMMAND variable. Probably off-topic, though.

Contributor

fsouza commented Oct 29, 2015

Maybe having an environment variable for the git command (as well as other vcs commands) would do the trick and make it more flexible, if that's the case. Just like git allows customizing the ssh command with the GIT_SSH_COMMAND variable. Probably off-topic, though.

@davecheney

This comment has been minimized.

Show comment
Hide comment
@davecheney

davecheney Oct 29, 2015

Contributor

I think that these requests are outside the remit of go get.

We all know go get is just a wrapper around git clone. As time goes on, a few little tweaks have been added to the clone model, and the result is more requests for more tweaks, more flags, etc.

This is natural and expected, go get is a simplified wrapper around a more complicated command so it is a logical conclusion that go get itself would have to become complex in turn.

Rather than this outcome, I suggest that we document the limitations of go get and recommend to people who need more that they simply skip go get, and use git clone; giving them the full power of the tool for their specific scenario without having to further complicate the simple model that go get provides.

Contributor

davecheney commented Oct 29, 2015

I think that these requests are outside the remit of go get.

We all know go get is just a wrapper around git clone. As time goes on, a few little tweaks have been added to the clone model, and the result is more requests for more tweaks, more flags, etc.

This is natural and expected, go get is a simplified wrapper around a more complicated command so it is a logical conclusion that go get itself would have to become complex in turn.

Rather than this outcome, I suggest that we document the limitations of go get and recommend to people who need more that they simply skip go get, and use git clone; giving them the full power of the tool for their specific scenario without having to further complicate the simple model that go get provides.

@mengzhuo

This comment has been minimized.

Show comment
Hide comment
@mengzhuo

mengzhuo Oct 30, 2015

Contributor

@davecheney
I agree that we should think about what "go get" for at the beginning.
IMHO "get" means "get the right source code" for build/compile instead of "get the whole repository for investigation/study" and git --depth arguments making "go get" do the right thing.

Contributor

mengzhuo commented Oct 30, 2015

@davecheney
I agree that we should think about what "go get" for at the beginning.
IMHO "get" means "get the right source code" for build/compile instead of "get the whole repository for investigation/study" and git --depth arguments making "go get" do the right thing.

@rakyll

This comment has been minimized.

Show comment
Hide comment
@rakyll

rakyll Oct 30, 2015

Member

For the sake of clarity...

Even though, it is not the core job of the go tool, I raised to issue of being able to deep copy because of the canonical URLs.

I simply want to be able to run

go get -d golang.org/x/tools/...

rather than figuring out what exact repo it is pointing to. But I simply can run git fetch --unshallow once it is cloned shallowly.

Member

rakyll commented Oct 30, 2015

For the sake of clarity...

Even though, it is not the core job of the go tool, I raised to issue of being able to deep copy because of the canonical URLs.

I simply want to be able to run

go get -d golang.org/x/tools/...

rather than figuring out what exact repo it is pointing to. But I simply can run git fetch --unshallow once it is cloned shallowly.

@omeid

This comment has been minimized.

Show comment
Hide comment
@omeid

omeid Nov 3, 2015

+1 shallow clone.

I just tried to do go get github.com/openshift/origin/cmd/oc and after a very long wait I ended up selectively shallow cloning docker/docker, kubernetes/kubernetes, openshift/origin and couple of other dependencies.

omeid commented Nov 3, 2015

+1 shallow clone.

I just tried to do go get github.com/openshift/origin/cmd/oc and after a very long wait I ended up selectively shallow cloning docker/docker, kubernetes/kubernetes, openshift/origin and couple of other dependencies.

@mengzhuo

This comment has been minimized.

Show comment
Hide comment
@mengzhuo

mengzhuo Nov 3, 2015

Contributor

@davecheney
@adg

The title is miss leading.
It should be:
cmd/go: improve git pull download size and speed (by adding --depth arguments into git VCS command)
It doesn't add any params into "go get" cmd
Sorry for my poor English (sigh)

Contributor

mengzhuo commented Nov 3, 2015

@davecheney
@adg

The title is miss leading.
It should be:
cmd/go: improve git pull download size and speed (by adding --depth arguments into git VCS command)
It doesn't add any params into "go get" cmd
Sorry for my poor English (sigh)

@mengzhuo mengzhuo changed the title from cmd/go: improve git pull download size and speed by passing --depth arguments to cmd/go: improve git pull download size and speed (by adding --depth arguments into git VCS command) Nov 3, 2015

@davecheney davecheney changed the title from cmd/go: improve git pull download size and speed (by adding --depth arguments into git VCS command) to cmd/go: improve go get download size and speed (by adding --depth arguments into git VCS command) Nov 3, 2015

@hyper0x

This comment has been minimized.

Show comment
Hide comment
@hyper0x

hyper0x Nov 10, 2015

+1 I agree with what @bradfitz said.

hyper0x commented Nov 10, 2015

+1 I agree with what @bradfitz said.

@mattn

This comment has been minimized.

Show comment
Hide comment
@mattn

mattn Nov 10, 2015

Member

+1 to -deep

Member

mattn commented Nov 10, 2015

+1 to -deep

@fsouza

This comment has been minimized.

Show comment
Hide comment
@fsouza

fsouza Nov 10, 2015

Contributor

Just to make it clear: I'm -1 to add a new flag to go get, I think it should always do shallow clones, and if the user wants the full repository, he/she can cd into the repository directory and unshallow it.

Contributor

fsouza commented Nov 10, 2015

Just to make it clear: I'm -1 to add a new flag to go get, I think it should always do shallow clones, and if the user wants the full repository, he/she can cd into the repository directory and unshallow it.

@bradfitz

This comment has been minimized.

Show comment
Hide comment
@bradfitz

bradfitz Nov 10, 2015

Member

I'm also -1 on a new flag. I probably shouldn't have even mentioned it.

I'll just make my https://github.com/bradfitz/gitutil/tree/master/git-allgoupdate code unshallow everything it finds in GOPATH instead.

I think we're all in agreement that we should go with shallow clones. The difference in size between shallow and deep for some of the repos is quite large.

Member

bradfitz commented Nov 10, 2015

I'm also -1 on a new flag. I probably shouldn't have even mentioned it.

I'll just make my https://github.com/bradfitz/gitutil/tree/master/git-allgoupdate code unshallow everything it finds in GOPATH instead.

I think we're all in agreement that we should go with shallow clones. The difference in size between shallow and deep for some of the repos is quite large.

@mattn

This comment has been minimized.

Show comment
Hide comment
@mattn

mattn Nov 10, 2015

Member

I'm thinking that is good to provide flag or environment variable to enable or disable full/shallow because CI server hope to get shallow.

Member

mattn commented Nov 10, 2015

I'm thinking that is good to provide flag or environment variable to enable or disable full/shallow because CI server hope to get shallow.

@rsc

This comment has been minimized.

Show comment
Hide comment
@rsc

rsc Nov 10, 2015

Contributor

No flags, no environment variables, none of that complexity or configuration. The go command should do the right thing. People who want something else can type git commands. It sounds like there is general agreement that the right thing is shallow clones.

Contributor

rsc commented Nov 10, 2015

No flags, no environment variables, none of that complexity or configuration. The go command should do the right thing. People who want something else can type git commands. It sounds like there is general agreement that the right thing is shallow clones.

@rsc rsc changed the title from cmd/go: improve go get download size and speed (by adding --depth arguments into git VCS command) to cmd/go: use shallow clones for new git checkouts Nov 10, 2015

@rsc rsc modified the milestones: Go1.6, Unplanned Nov 10, 2015

@mengzhuo

This comment has been minimized.

Show comment
Hide comment
@mengzhuo

mengzhuo Nov 11, 2015

Contributor

@rsc @bradfitz I had fixed patch to depth=1

Contributor

mengzhuo commented Nov 11, 2015

@rsc @bradfitz I had fixed patch to depth=1

@bradfitz

This comment has been minimized.

Show comment
Hide comment
@bradfitz

bradfitz Nov 11, 2015

Member

@mengzhuo, did you forget to upload it? I don't see it at https://go-review.googlesource.com/#/c/16360/

Member

bradfitz commented Nov 11, 2015

@mengzhuo, did you forget to upload it? I don't see it at https://go-review.googlesource.com/#/c/16360/

@mengzhuo

This comment has been minimized.

Show comment
Hide comment
@mengzhuo

mengzhuo Nov 11, 2015

Contributor

@bradfitz Sorry, I'm new to Gerrit. I had clicked "fix" on Russ's comment and changed into 1.

Contributor

mengzhuo commented Nov 11, 2015

@bradfitz Sorry, I'm new to Gerrit. I had clicked "fix" on Russ's comment and changed into 1.

@bradfitz bradfitz closed this in bc1f9d2 Nov 11, 2015

@mengzhuo

This comment has been minimized.

Show comment
Hide comment
@mengzhuo

mengzhuo Nov 12, 2015

Contributor

@bradfitz First of all, I'm sorry for the trouble.
After Russ reviewed, I found that none of us ( including testcases) missed "go get -u" which will raise an git error "missing option --ff-only".

It appears to be git's behavior that "--ff-only" trigger merge however "--depth" trigger fetch.
To fix this issue, we can delete --depth option in downloadCmd
https://go-review.googlesource.com/#/c/16360/5/src/cmd/go/vcs.go

My git version: 1.9

Contributor

mengzhuo commented Nov 12, 2015

@bradfitz First of all, I'm sorry for the trouble.
After Russ reviewed, I found that none of us ( including testcases) missed "go get -u" which will raise an git error "missing option --ff-only".

It appears to be git's behavior that "--ff-only" trigger merge however "--depth" trigger fetch.
To fix this issue, we can delete --depth option in downloadCmd
https://go-review.googlesource.com/#/c/16360/5/src/cmd/go/vcs.go

My git version: 1.9

@mengzhuo

This comment has been minimized.

Show comment
Hide comment
@mengzhuo
Contributor

mengzhuo commented Nov 12, 2015

@bradfitz

This comment has been minimized.

Show comment
Hide comment
@bradfitz

bradfitz Nov 12, 2015

Member

This is being reverted in https://go-review.googlesource.com/#/c/16832/ so re-opening this bug.

This will need better tests to re-introduce.

Member

bradfitz commented Nov 12, 2015

This is being reverted in https://go-review.googlesource.com/#/c/16832/ so re-opening this bug.

This will need better tests to re-introduce.

@bradfitz bradfitz reopened this Nov 12, 2015

@mengzhuo

This comment has been minimized.

Show comment
Hide comment
@mengzhuo

mengzhuo Nov 12, 2015

Contributor

Yes, This is my fault.
As Russ's comment, we should ONLY use it in clone.


Light up the darkness

2015-11-12 16:35 GMT+08:00 Brad Fitzpatrick notifications@github.com:

This is being reverted in https://go-review.googlesource.com/#/c/16832/
so re-opening this bug.

This will need better tests to re-introduce.


Reply to this email directly or view it on GitHub
#13078 (comment).

Contributor

mengzhuo commented Nov 12, 2015

Yes, This is my fault.
As Russ's comment, we should ONLY use it in clone.


Light up the darkness

2015-11-12 16:35 GMT+08:00 Brad Fitzpatrick notifications@github.com:

This is being reverted in https://go-review.googlesource.com/#/c/16832/
so re-opening this bug.

This will need better tests to re-introduce.


Reply to this email directly or view it on GitHub
#13078 (comment).

@rsc

This comment has been minimized.

Show comment
Hide comment
@rsc

rsc Nov 12, 2015

Contributor

Complication: some servers do not support --depth=1.
Possible complication: do any servers appear to support it but not support it correctly? I've seen notes to that effect on StackOverflow but they were in fairly old comment threads.

Contributor

rsc commented Nov 12, 2015

Complication: some servers do not support --depth=1.
Possible complication: do any servers appear to support it but not support it correctly? I've seen notes to that effect on StackOverflow but they were in fairly old comment threads.

@rsc

This comment has been minimized.

Show comment
Hide comment
@rsc

rsc Nov 23, 2015

Contributor

It's now too late to do this in Go 1.6. It would need to be reintroduced in a non-freeze time, such as early in Go 1.7. That said, please consider whether doing a shallow clone fetches all the necessary tags for the go command to do its tag-based checkout selection. If a shallow clone does not fetch all the necessary tags, then a separate tag fetch would be needed, and if that's not possible then we can't do shallow clones at all.

Contributor

rsc commented Nov 23, 2015

It's now too late to do this in Go 1.6. It would need to be reintroduced in a non-freeze time, such as early in Go 1.7. That said, please consider whether doing a shallow clone fetches all the necessary tags for the go command to do its tag-based checkout selection. If a shallow clone does not fetch all the necessary tags, then a separate tag fetch would be needed, and if that's not possible then we can't do shallow clones at all.

@rsc rsc modified the milestones: Unplanned, Go1.6 Nov 23, 2015

@kokizzu

This comment has been minimized.

Show comment
Hide comment
@kokizzu

kokizzu Feb 22, 2016

+1 for -depth 1, sometimes go getting some repository takes too long for country with slow internet (like mine: Indonesia)

kokizzu commented Feb 22, 2016

+1 for -depth 1, sometimes go getting some repository takes too long for country with slow internet (like mine: Indonesia)

@bradfitz

This comment has been minimized.

Show comment
Hide comment
@bradfitz

bradfitz Feb 22, 2016

Member

@kokizzu, we don't vote on issues. We all want this. That's not the issue. The issue is how to implement it correctly.

Member

bradfitz commented Feb 22, 2016

@kokizzu, we don't vote on issues. We all want this. That's not the issue. The issue is how to implement it correctly.

fsouza added a commit to NYTimes/video-transcoding-api that referenced this issue Feb 24, 2016

bin/build: speed-up local build time
In case we have GOPATH defined, we probably have the dependencies, than
we don't need to download them again. It still works as expected in a
box with no GOPATH defined.

It's way faster to avoid downloading the dependencies. This is because
`go get` is dumb when cloning Git repositories. This should probably
change when golang/go#13078 is fixed (Go 1.7, hopefully).

fsouza added a commit to fsouza/go-dockerclient that referenced this issue May 1, 2016

all: stop vendoring external dependencies
It's not good practice for a library, but we vendored our dependencies
in the past because go get github.com/fsouza/go-dockerclient was too
slow, but now vendoring is well advised in Go 1.5 and standard in Go
1.6. I'm also dropping Go 1.4 as some standard tools developed by the Go
core team already did that.

This will make tests on Travis run even faster, but I can hope that
golang/go#13078 will eventually get fixed.

rheinwein added a commit to codeship/go-dockerclient that referenced this issue May 3, 2016

all: stop vendoring external dependencies
It's not good practice for a library, but we vendored our dependencies
in the past because go get github.com/fsouza/go-dockerclient was too
slow, but now vendoring is well advised in Go 1.5 and standard in Go
1.6. I'm also dropping Go 1.4 as some standard tools developed by the Go
core team already did that.

This will make tests on Travis run even faster, but I can hope that
golang/go#13078 will eventually get fixed.

rheinwein added a commit to codeship/go-dockerclient that referenced this issue May 4, 2016

all: stop vendoring external dependencies
It's not good practice for a library, but we vendored our dependencies
in the past because go get github.com/fsouza/go-dockerclient was too
slow, but now vendoring is well advised in Go 1.5 and standard in Go
1.6. I'm also dropping Go 1.4 as some standard tools developed by the Go
core team already did that.

This will make tests on Travis run even faster, but I can hope that
golang/go#13078 will eventually get fixed.

rheinwein added a commit to codeship/go-dockerclient that referenced this issue Aug 18, 2016

all: stop vendoring external dependencies
It's not good practice for a library, but we vendored our dependencies
in the past because go get github.com/fsouza/go-dockerclient was too
slow, but now vendoring is well advised in Go 1.5 and standard in Go
1.6. I'm also dropping Go 1.4 as some standard tools developed by the Go
core team already did that.

This will make tests on Travis run even faster, but I can hope that
golang/go#13078 will eventually get fixed.

@eparis eparis referenced this issue Jan 9, 2017

Merged

Remove dns #39448

@Tim15

This comment has been minimized.

Show comment
Hide comment
@Tim15

Tim15 Aug 28, 2017

Any new developments on this? (no pun intended)

Tim15 commented Aug 28, 2017

Any new developments on this? (no pun intended)

@rsc

This comment has been minimized.

Show comment
Hide comment
@rsc

rsc Aug 29, 2017

Contributor

No new developments. In the long term I think the answer will be that the go command will move to downloading snapshots instead of git repos, and then everything will be "shallow".

Contributor

rsc commented Aug 29, 2017

No new developments. In the long term I think the answer will be that the go command will move to downloading snapshots instead of git repos, and then everything will be "shallow".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment