Skip to content
This repository has been archived by the owner on Feb 26, 2019. It is now read-only.

godep restore makes impossible to update project with go get #453

Open
Raffo opened this issue Apr 7, 2016 · 26 comments
Open

godep restore makes impossible to update project with go get #453

Raffo opened this issue Apr 7, 2016 · 26 comments

Comments

@Raffo
Copy link

Raffo commented Apr 7, 2016

Currently godep restore puts the repositories in detached HEAD and this makes impossible to execute a go get -u github.com/foo/bar. If a high number of projects using godeps are used and if godep restore is used frequently, this leads to a very high number of projects in detached state which makes very hard to update them.
Are we able to avoid this somehow?

@freeformz
Copy link

Longer term, yes, by removing the need to godep restore. A go tooling change recently (go1.6) made this a problem AFAIK.

@freeformz
Copy link

PS: godep restore is doing what it's always done, restore the recorded versions to the $GOPATH. it can only do this by checking out the various shas (AFAIK) it's recorded.

As a temp work around I could implement a godep unrestore, which would check out master again. But I'm hesitant to add another command that should be removed in the future and I'm not sure it's super helpful in the workflow ... ie. you have to remember to use it.

Thoughts?

@Raffo
Copy link
Author

Raffo commented Apr 7, 2016

I was about to implement a godep unrestore myself, I think it would be useful (I can work on it and submit a PR, but if you want to work on it, as you whish). More important, though is to document in the README what happens now if a user executes godep restore.

Also, can you give me the detail of this change in 1.6 you are mentioning above?

@freeformz
Copy link

PRs welcome. I don't have an exact commit to golang where this broke. Let me see if I can dig it up though.

@freeformz
Copy link

It was likely this one: golang/go@4220659

@freeformz
Copy link

@Raffo What would you suggest be added to the README? It states what godep restore already does in a non VCS specific manner, since godep handles multiple types of VCS.

@Raffo
Copy link
Author

Raffo commented Apr 7, 2016

Thanks for the link to the commit.

Do you think that a warning like "godep restore could leave some of the dependencies in a detached state. This could lead to problems to update your dependencies" would be too much? Maybe we could add some text to specify that the users should avoid using godep restore if not strictly necessary.

@freeformz
Copy link

How about

NOTE: godep restore leaves git repositories in a detached state. go1.6+ no longer checks out the master branch when doing a go get, see here.

Then if you land an unrestore PR we can adjust that to recommend the usage of unrestore afterwards?

@Raffo
Copy link
Author

Raffo commented Apr 7, 2016

Sounds good to me.

@freeformz
Copy link

@Raffo README / restore help text updated in 584a3f5

@Raffo
Copy link
Author

Raffo commented Apr 8, 2016

👍

@Raffo
Copy link
Author

Raffo commented Apr 8, 2016

As I imagined, it's very easy to hack an "unrestore" command to checkout the master, but it's not so easy to make the unrestore command so general that can handle cases in which the default remote branch is different from the master. I would have to add this information to Godeps.json (the remote HEAD), but I'm not sure we want that.
If we agree that we will implement that, I wouldn't know any valid command that will give me the current remote HEAD. The only valid info I know how to get is in git remote show origin, but I don't really want to parse the output. Any suggestion/help?

@Raffo
Copy link
Author

Raffo commented Apr 11, 2016

@freeformz Any input on what I wrote above?

@freeformz
Copy link

Why does the remote branch matter? I don't think we want to pull new code with unrestore, just check out the local master.

@Raffo
Copy link
Author

Raffo commented Apr 13, 2016

@freeformz because it's not obvious that the main branch is the master. If you clone a repository which remote HEAD is not the master you will end up with something different than a master branch. In such a case, if godep unrestore tries to restore it to the master branch this could lead to wrong results.

@freeformz
Copy link

It only matters what it's merged to locally AFAIK, so git config --local branch.master.merge should tell us what to checkout.

I could totally be wrong here, I don't think I've ever worked with remotes who HEAD was not master.

@Raffo
Copy link
Author

Raffo commented Apr 18, 2016

Take the gin repository: the master branch exist, but the remote HEAD is develop. I think that in this case it will not work.

@freeformz
Copy link

freeformz commented Apr 18, 2016

I see. If there is only one branch returned from git config --local --get-regexp 'branch.*' we could check that out and otherwise punt with instructions to the user to start?

@Raffo
Copy link
Author

Raffo commented Apr 18, 2016

Or we just modify Godep.json to be something like:

    {
            "ImportPath": "github.com/spf13/pflag",
            "Rev": "8f6a28b0916586e7f22fe931ae2fcfc380b1c0e6",
            "RemoteHEAD": "master"
        },

And use the RemoteHEAD when executing unrestore. This will break only in case the remote head of the repo changes from the last time the Godep.json is updated, which is not perfect, but acceptable (IMO).

@freeformz
Copy link

I'm not sure I really want to require people to populate that field manually. Also AFAICT RemoteHEAD is detectable through either:

$ git remote show origin
...
HEAD branch: develop
...

or

$ git branch -r | grep "origin/HEAD ->"
  origin/HEAD -> origin/develop

possibly some other means as well.

@Raffo
Copy link
Author

Raffo commented Apr 19, 2016

This is exactly what I meant, RemoteHEAD would be populated when godep save is executed. When doing godep unrestore we could also add a --force option (I'm not sure if this flag is already used in another way in other godep commands) to remove the dependency and re-executing a go get from scratch in case of failures. Obviously after a warning and a confirmation from the user.

@freeformz
Copy link

I'm not sure we should save RemoteHEAD though and just honor the remote head from git info for what's already there.

@Raffo
Copy link
Author

Raffo commented Apr 29, 2016

So, what you suggest is to just use the info from git remote show origin , i.e:

* remote origin
  Fetch URL: https://github.com/kubernetes/kubernetes.git
  Push  URL: https://github.com/kubernetes/kubernetes.git
  HEAD branch: master
  Remote branches:

Is this right?

@eduncan911
Copy link

eduncan911 commented May 11, 2016

FWIW, today I got fed up with this and just _banned_ godep restore from my entire system. I did this with a script:

https://github.com/eduncan911/dotfiles/blob/master/bin/godep

If you want it, first make sure your personal ~/bin comes before your $GOPATH/bin:

$ echo $PATH
...:/Users/eric/bin:/Users/eric/go/bin:...

If so, you are good to go. If not, you may want to tweak your $PATH to use your personal ~/bin before your GOPATH/bin.

Get the file locally:

$ curl -L https://raw.githubusercontent.com/eduncan911/dotfiles/master/bin/godep --create-dirs -o ~/bin/godep
$ chmod 755 ~/bin/godep

Use it normally:

$ godep go run main.go
$ godep get foo/bar

...etc. And when you try to do godep restore:

$ godep restore
WARNING: godep restore <- this will frack with your $GOPATH !!
WARNING: to override, use your installed version of godep like this:
WARNING:      $GOPATH/bin/godep restore
WARNING: Aborting ...

hehe. 👍

I'm in the habit of using the godep prefix for all my go needs anyways:

$ godep go test ./...
$ godep go run main.go
$ godep get foo/bar

...etc. But I've accidently used godep restore a few times, like today when I ran a Makefile for a new repo I was working on. Argh! Spent over an hour restoring all of my $GOPATH repos back to master.

No more!

@tsuna
Copy link
Contributor

tsuna commented May 11, 2016

No need to spend an hour restoring them all, just do:

find $GOPATH/src -type d -name .git -execdir git --git-dir={} checkout master \;

@eduncan911
Copy link

i have to exclude the dozen or so that I am working on... that do have pending commits, on different branches, etc.

chungers pushed a commit to docker-archive/deploykit that referenced this issue Jul 15, 2016
Note: due to issue 453 [1], there's currently a bit of legwork involved
in updating dependencies.  I used the approach of first switching all
repos in `$GOPATH` to master as a workaround.

[1] tools/godep#453
[2] tools/godep#453 (comment)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants