Error sometimes with gpm install on existing projects

[technosophos]

I just noticed this a few times today. It looks like sometimes a race condition happens:

>> Setting github.com/lann/builder to version
# cd /Users/mbutcher/Code/Go/src/bitbucket.org/mobiplug/sugarmill/.godeps/src/github.com/lann/builder; git checkout master
fatal: Unable to create '/Users/mbutcher/Code/Go/src/bitbucket.org/mobiplug/sugarmill/.godeps/src/github.com/lann/builder/.git/index.lock': File exists.

If no other git process is currently running, this probably means a
git process crashed in this repository earlier. Make sure no other git
process is running and remove the file manually to continue.
package github.com/lann/squirrel
    imports github.com/lann/builder: exit status 128

I am having trouble reproducing this, but every once in a while I see the same error (on different projects each time).

[elcuervo]

@technosophos Every of the projects that fail are in the Godeps file? Can you provide a failed Godeps so I can check?

[pote]

Mmm, these race conditions very possibly can't be avoided from the gpm side as we're running multiple go get calls in parallel, even if we can (and do) make sure that our own git checkouts avoid this conditions two go get calls could be trying to install or update the same dependency at the same time.

This is a shame, I very much like package installation happening in parallel, maybe it's something we should scrap? I don't see a way around it without evaluating the whole dependency tree - and that's completely outside of the scope of gpm. Performance will be worse, but at least we will ensure that we have no race conditions.

What do you guys think?

[technosophos]

I pulled the parallelization out of my gpm-git plugin to test it out, and the result is (predictably) slower but safer.

My feeling is that safer is better in this case. I would prefer removing the part that backgrounds the go get even at the cost of performance.

[elcuervo]

An known failing example could help us see if there's a way to prevent it
or removing parallel is the way to go.
Can you help us with that?

2014-06-04 18:16 GMT-03:00 Matt Butcher [email protected]:

I pulled the parallelization out of my gpm-git plugin to test it out, and
the result is (predictably) slower but safer.

My feeling is that safer is better in this case. I would prefer removing
the part that backgrounds the go get even at the cost of performance.

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

[pote]

I think removing parallel installation will be the best option but want to chase this down a bit to see if we can find a way around it that doesn't hinder performance. As @elcuervo says: If you have a sample Godeps file you wouldn't mind sharing that'd be fantastic!

[elcuervo]

Btw, an easy way to try the race condition is to have the same pkg multiple times so the parallel execution screws something. #36 retries go get until the lock is over.

[pote]

Ok, I just merged in #36 by @elcuervo , this should solve this particular race condition or fail gracefully when errors happen.

I'll keep this issue open though, as we want to make sure this use case is solved. @technosophos please get back to us in a week or two so we know how to proceed, in case we still get errors we'll be forced to remove parallel package installation for good.

Thanks everyone!

[elcuervo]

[technosophos]

Okay. I will test.

Here is a sequence of packages that seemed to trigger the issue for me:

github.com/lann/builder
github.com/lann/squirrel
github.com/technosophos/structable 1.0.0

squirrel imports builder and structable imports squirrel, so it is possible for a parallelized build to break here.

[technosophos]

I've tested several runs of gpm install (including destroying and re-creating a major project), and it all seems to work fine. Yay!

[elcuervo]

[elcuervo]

Ready for prime time?

[pote]

I think so, let's close this and make a new release. :)

[elcuervo]

[technosophos]

Having done a few dozen releases since the last patch, I'm willing to say this one is fixed.