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

Stale Godeps/_workspace/pkg breaking build #45

Closed
danp opened this Issue Dec 20, 2013 · 22 comments

Comments

Projects
None yet
8 participants
@danp

danp commented Dec 20, 2013

I've been tracking heroku/hk lately and have had trouble with my git pull && godep go build flow. Worked with @bgentry to track it down today.

The problem seems to be that objects in Godeps/_workspace/pkg are stale but not rebuilt. Nuking that directory then running godep go build works.

Haven't dug into why yet but wanted to get an issue opened.

@kr

This comment has been minimized.

Show comment
Hide comment
@kr

kr Dec 20, 2013

Member

Thanks. I don't know if I'll be able to fix this before
I go on vacation, but hopefully you can get by with
the workaround for a few weeks.

Member

kr commented Dec 20, 2013

Thanks. I don't know if I'll be able to fix this before
I go on vacation, but hopefully you can get by with
the workaround for a few weeks.

@danp

This comment has been minimized.

Show comment
Hide comment
@danp

danp Apr 26, 2014

Here is a repro for hk:

#!/bin/sh
set -ex
git checkout master
git branch -d old || true
git checkout -b old 208ceb0f3033800f0560913497490888b9c1cacf
rm -rf Godeps/_workspace/pkg
godep go install
git checkout master
godep go build -x

Initial observations:

  • things are only put in Godeps/_workspace/pkg when godep go install is used
  • if -a is added to godep go build it works but ends up doing a lot of extra recompiling of the runtime and stdlib

danp commented Apr 26, 2014

Here is a repro for hk:

#!/bin/sh
set -ex
git checkout master
git branch -d old || true
git checkout -b old 208ceb0f3033800f0560913497490888b9c1cacf
rm -rf Godeps/_workspace/pkg
godep go install
git checkout master
godep go build -x

Initial observations:

  • things are only put in Godeps/_workspace/pkg when godep go install is used
  • if -a is added to godep go build it works but ends up doing a lot of extra recompiling of the runtime and stdlib
@kr

This comment has been minimized.

Show comment
Hide comment
@kr

kr Apr 27, 2014

Member

Awesome. A reproducible test case makes this so much easier! Thanks.

Member

kr commented Apr 27, 2014

Awesome. A reproducible test case makes this so much easier! Thanks.

@artash

This comment has been minimized.

Show comment
Hide comment
@artash

artash Jul 17, 2014

Confirmed the issue is still there in current master c703322

artash commented Jul 17, 2014

Confirmed the issue is still there in current master c703322

@sqs

This comment has been minimized.

Show comment
Hide comment
@sqs

sqs Oct 31, 2014

I'm still seeing this and made a self-contained repro at https://github.com/sqs/godeps-stale-pkg-repro.

I don't think this is godep's fault, but my current guess is that something godep does is triggering it. Basically, dependency pkgs in Godeps/_workspace are shown as Stale with godep go list, are rebuilt if you run godep go install on them directly, but aren't rebuilt if you run godep go install on a pkg that imports them.

go help install says: "Install compiles and installs the packages named by the import paths, along with their dependencies." This seems to imply that the same operation should be performed on dependencies as on the named package arguments to go install.

My current guesses are either:

  1. I'm completely misunderstanding what go install should do.
  2. There's an special, undocumented way (or that I haven't seen the docs for) in which go install treats sub-GOPATHs or GOPATHs with underscore-leading names.
  3. There's a bug in how the go tool treats sub-GOPATHs or GOPATHs with underscore-leading names.

FWIW, go1.4beta1 has the same issue.

sqs commented Oct 31, 2014

I'm still seeing this and made a self-contained repro at https://github.com/sqs/godeps-stale-pkg-repro.

I don't think this is godep's fault, but my current guess is that something godep does is triggering it. Basically, dependency pkgs in Godeps/_workspace are shown as Stale with godep go list, are rebuilt if you run godep go install on them directly, but aren't rebuilt if you run godep go install on a pkg that imports them.

go help install says: "Install compiles and installs the packages named by the import paths, along with their dependencies." This seems to imply that the same operation should be performed on dependencies as on the named package arguments to go install.

My current guesses are either:

  1. I'm completely misunderstanding what go install should do.
  2. There's an special, undocumented way (or that I haven't seen the docs for) in which go install treats sub-GOPATHs or GOPATHs with underscore-leading names.
  3. There's a bug in how the go tool treats sub-GOPATHs or GOPATHs with underscore-leading names.

FWIW, go1.4beta1 has the same issue.

@jkodumal

This comment has been minimized.

Show comment
Hide comment
@jkodumal

jkodumal Nov 24, 2014

I've been running into this quite a bit with a few recent projects. I've just added a step that cleans the pkg directory as part of my build, but it'd be good to see this fixed.

jkodumal commented Nov 24, 2014

I've been running into this quite a bit with a few recent projects. I've just added a step that cleans the pkg directory as part of my build, but it'd be good to see this fixed.

@danp

This comment has been minimized.

Show comment
Hide comment
@danp

danp Jan 16, 2015

Not sure if it's related to this or not but yesterday I had to nuke $GOPATH/pkg to get a godep go install to work.

danp commented Jan 16, 2015

Not sure if it's related to this or not but yesterday I had to nuke $GOPATH/pkg to get a godep go install to work.

@sqs

This comment has been minimized.

Show comment
Hide comment
@sqs

sqs Feb 2, 2015

Following up on this, we experience this 100% of the time when checking out git branches with different files in Godeps/_workspace/src (because the old pkg files are there), git pulling commits that change files in Godeps/_workspace/src, editing .go files in Godeps/_workspace/src, etc. It's also something that confuses people we're onboarding. The workaround is to nuke the Godeps/_workspace/pkg dir, of course.

The thing that's kept me from digging into this more (beyond the repro at https://github.com/sqs/godeps-stale-pkg-repro) is that nobody else seems to experience this issue with such regularity. And that makes me suspect that something is different about our setup. For the other folks who have reported this, does this issue occur all of the time, or just occasionally?

sqs commented Feb 2, 2015

Following up on this, we experience this 100% of the time when checking out git branches with different files in Godeps/_workspace/src (because the old pkg files are there), git pulling commits that change files in Godeps/_workspace/src, editing .go files in Godeps/_workspace/src, etc. It's also something that confuses people we're onboarding. The workaround is to nuke the Godeps/_workspace/pkg dir, of course.

The thing that's kept me from digging into this more (beyond the repro at https://github.com/sqs/godeps-stale-pkg-repro) is that nobody else seems to experience this issue with such regularity. And that makes me suspect that something is different about our setup. For the other folks who have reported this, does this issue occur all of the time, or just occasionally?

@collinvandyck

This comment has been minimized.

Show comment
Hide comment
@collinvandyck

collinvandyck Feb 6, 2015

@sqs hi I actually just ran into this today. I changed a dependency and ran a godep go update [dep]. The JSON file seemed to be correctly updated, but the .a files in pkg were stale from a previous build. Nuking the pkg directory fixed it.

This is the first time I've run into this that I have noticed. I suspect/fear that it has been happening all along, periodically, but I just haven't noticed it for whatever reason.

collinvandyck commented Feb 6, 2015

@sqs hi I actually just ran into this today. I changed a dependency and ran a godep go update [dep]. The JSON file seemed to be correctly updated, but the .a files in pkg were stale from a previous build. Nuking the pkg directory fixed it.

This is the first time I've run into this that I have noticed. I suspect/fear that it has been happening all along, periodically, but I just haven't noticed it for whatever reason.

@sqs

This comment has been minimized.

Show comment
Hide comment
@sqs

sqs Feb 8, 2015

I think I found the root cause:

https://github.com/golang/go/blob/906aefb038b6baddc38f165bdfb6ecf624db398d/src/cmd/go/pkg.go#L784-L796

    // Have installed copy, probably built using current compilers,
    // and built after its imported packages.  The only reason now
    // that we'd have to rebuild it is if the sources were newer than
    // the package.   If a package p is not in the same tree as any
    // package named on the command-line, assume it is up-to-date
    // no matter what the modification times on the source files indicate.
    // This avoids rebuilding $GOROOT packages when people are
    // working outside the Go root, and it effectively makes each tree
    // listed in $GOPATH a separate compilation world.
    // See issue 3149.
    if p.Root != "" && !topRoot[p.Root] {
        return false
    }

It refers to golang/go#3149.

I can see why you wouldn't want to recompile $GOROOT packages, but I wonder if the side effect of "mak[ing] each tree listed in $GOPATH a separate compilation world" is desirable. In this case, it is not.

sqs commented Feb 8, 2015

I think I found the root cause:

https://github.com/golang/go/blob/906aefb038b6baddc38f165bdfb6ecf624db398d/src/cmd/go/pkg.go#L784-L796

    // Have installed copy, probably built using current compilers,
    // and built after its imported packages.  The only reason now
    // that we'd have to rebuild it is if the sources were newer than
    // the package.   If a package p is not in the same tree as any
    // package named on the command-line, assume it is up-to-date
    // no matter what the modification times on the source files indicate.
    // This avoids rebuilding $GOROOT packages when people are
    // working outside the Go root, and it effectively makes each tree
    // listed in $GOPATH a separate compilation world.
    // See issue 3149.
    if p.Root != "" && !topRoot[p.Root] {
        return false
    }

It refers to golang/go#3149.

I can see why you wouldn't want to recompile $GOROOT packages, but I wonder if the side effect of "mak[ing] each tree listed in $GOPATH a separate compilation world" is desirable. In this case, it is not.

@danp

This comment has been minimized.

Show comment
Hide comment
@danp

danp Mar 3, 2015

On twitter @kr suggested godep save -r as a workaround. I have not been able to test yet.

danp commented Mar 3, 2015

On twitter @kr suggested godep save -r as a workaround. I have not been able to test yet.

@fkautz

This comment has been minimized.

Show comment
Hide comment
@fkautz

fkautz Apr 2, 2015

As a workaround, we should be able to use:

godep go build -a

-a tells go build to ignore mtime and assume everything is stale.

fkautz commented Apr 2, 2015

As a workaround, we should be able to use:

godep go build -a

-a tells go build to ignore mtime and assume everything is stale.

@fkautz

This comment has been minimized.

Show comment
Hide comment
@fkautz

fkautz Apr 2, 2015

Can anyone see any reason why godep should not append -a on every build and install automatically?

fkautz commented Apr 2, 2015

Can anyone see any reason why godep should not append -a on every build and install automatically?

@sqs

This comment has been minimized.

Show comment
Hide comment
@sqs

sqs Apr 2, 2015

Always appending -a would slow down partial rebuilds, which would be undesirable for us (from a dev speed and iteration POV).

sqs commented Apr 2, 2015

Always appending -a would slow down partial rebuilds, which would be undesirable for us (from a dev speed and iteration POV).

@fkautz

This comment has been minimized.

Show comment
Hide comment
@fkautz

fkautz Apr 2, 2015

It would slow down builds, but by how much? On one of my projects, I saw build times increase by around 200ms. The increase in build times is likely acceptable for the added benefit of having correct builds. :)

fkautz commented Apr 2, 2015

It would slow down builds, but by how much? On one of my projects, I saw build times increase by around 200ms. The increase in build times is likely acceptable for the added benefit of having correct builds. :)

@fkautz

This comment has been minimized.

Show comment
Hide comment
@fkautz

fkautz Apr 3, 2015

Had a talk with @sqs offline, main problem is incremental builds, which typically run in 500ms vs full builds which run in around 7s.

We should probably push this upstream to the go team and get them to fix go build instead.

fkautz commented Apr 3, 2015

Had a talk with @sqs offline, main problem is incremental builds, which typically run in 500ms vs full builds which run in around 7s.

We should probably push this upstream to the go team and get them to fix go build instead.

@dmitshur

This comment has been minimized.

Show comment
Hide comment
@dmitshur

dmitshur Apr 14, 2015

Contributor

There's a related issue (from Jan 2014) with discussion here:

jimmyfrasche/goutil#2

We made a post on golang-nuts to try to motivate improving the situation upstream in Go, but it did not catch much attention:

https://groups.google.com/d/msg/golang-nuts/Z5uJf6mEF6E/K7yb6Xc5zQUJ

It was suggested to me by minux to make the thread in golang-dev (since Russ was away at the time), which I did, but it didn't get any traction either:

https://groups.google.com/forum/#!topic/golang-dev/svxuGkGJTD8

(Just mentioning relevant links here. I am extremely familiar with this issue and would really like to see it fixed, because it forces me to avoid installing my packages and thus lose benefits of faster compilation, auto completion, etc.)

Contributor

dmitshur commented Apr 14, 2015

There's a related issue (from Jan 2014) with discussion here:

jimmyfrasche/goutil#2

We made a post on golang-nuts to try to motivate improving the situation upstream in Go, but it did not catch much attention:

https://groups.google.com/d/msg/golang-nuts/Z5uJf6mEF6E/K7yb6Xc5zQUJ

It was suggested to me by minux to make the thread in golang-dev (since Russ was away at the time), which I did, but it didn't get any traction either:

https://groups.google.com/forum/#!topic/golang-dev/svxuGkGJTD8

(Just mentioning relevant links here. I am extremely familiar with this issue and would really like to see it fixed, because it forces me to avoid installing my packages and thus lose benefits of faster compilation, auto completion, etc.)

@fkautz

This comment has been minimized.

Show comment
Hide comment
@fkautz

fkautz Apr 14, 2015

@shurcooL How difficult would it be to submit a patch? One option would be to send them one and see if they accept it.

fkautz commented Apr 14, 2015

@shurcooL How difficult would it be to submit a patch? One option would be to send them one and see if they accept it.

@dmitshur

This comment has been minimized.

Show comment
Hide comment
@dmitshur

dmitshur Apr 14, 2015

Contributor

Summary

@fkautz, it may be quite difficult to come up with a patch that addresses the problem but doesn't create additional problems in edge cases.

Details

The difficulty comes from having to consider all edge cases - the go command has to deal with a lot. Most of those edge cases are less common, but still valid conditions where the go command is expected to behave correctly. An example would be building Go from source. In some cases, the go command needs to write to GOROOT workspace, in most other cases it should not.

It would require very careful consideration of all edge cases and planning, and seeing if it's possible to change the behavior such that this problem is resolved without compromising anything. If that can be done, I would imagine they would accept such a patch (after very thorough review).

At some point I had plans to try to do that, by making a fork of the go tool, making the changes I would want, and seeing if everything works out. I haven't gotten around to doing that (and I don't have short term plans to do it atm).

In the end, it may end up being trivial - or impossible. You can only find out after starting the work. It's quite hard to predict.

Contributor

dmitshur commented Apr 14, 2015

Summary

@fkautz, it may be quite difficult to come up with a patch that addresses the problem but doesn't create additional problems in edge cases.

Details

The difficulty comes from having to consider all edge cases - the go command has to deal with a lot. Most of those edge cases are less common, but still valid conditions where the go command is expected to behave correctly. An example would be building Go from source. In some cases, the go command needs to write to GOROOT workspace, in most other cases it should not.

It would require very careful consideration of all edge cases and planning, and seeing if it's possible to change the behavior such that this problem is resolved without compromising anything. If that can be done, I would imagine they would accept such a patch (after very thorough review).

At some point I had plans to try to do that, by making a fork of the go tool, making the changes I would want, and seeing if everything works out. I haven't gotten around to doing that (and I don't have short term plans to do it atm).

In the end, it may end up being trivial - or impossible. You can only find out after starting the work. It's quite hard to predict.

@dmitshur

This comment has been minimized.

Show comment
Hide comment
@dmitshur

dmitshur Jun 19, 2015

Contributor

This issue has been resolved in Go 1.5. See golang/go#10509.

Contributor

dmitshur commented Jun 19, 2015

This issue has been resolved in Go 1.5. See golang/go#10509.

@kr

This comment has been minimized.

Show comment
Hide comment
@kr

kr Jun 19, 2015

Member

That is a relief. I'll go ahead and close this issue now. If folks come asking about it in the future we can direct them to update to Go 1.5.

@freeformz

Member

kr commented Jun 19, 2015

That is a relief. I'll go ahead and close this issue now. If folks come asking about it in the future we can direct them to update to Go 1.5.

@freeformz

@kr kr closed this Jun 19, 2015

@fkautz

This comment has been minimized.

Show comment
Hide comment
@fkautz

fkautz Jun 19, 2015

@shurcooL @kr Fantastic news, thanks for following up on this.

fkautz commented Jun 19, 2015

@shurcooL @kr Fantastic news, thanks for following up on this.

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