search

robfig / glock

MIT License
230 stars 27 forks source link

sync: `go get` everything at once #36

Closed tamird closed 8 years ago

tamird commented 8 years ago

Prevents a race condition on concurrent uses of go get.

@bdarnell @a-robinson @robfig

tamird commented 8 years ago

cc @raduberinde @clofresh @Viq111

a-robinson commented 8 years ago

You might want to leave a comment explaining why we're running go get on all packages in one command, but otherwise this looks reasonable and seems to fix the issue on my machine.

It's a shame that https://godoc.org/golang.org/x/tools/go/vcs didn't work for syncing to a revision.

RaduBerinde commented 8 years ago

+1 on the comment, reference the discussion in #34 LGTM

tamird commented 8 years ago

Added a comment.

RaduBerinde commented 8 years ago

LGTM

Viq111 commented 8 years ago

I think I have some perf regression for our glockfile (see PR #34).

For a fresh $GOPATH for each test:

ᐅ go version
go version go1.6.2 darwin/amd64
/go/src/github.com/tamird/glock (go-get-race✔) ᐅ go install
ᐅ time glock sync ...
44.47s user 19.22s system 73% cpu 1:27.11 total
/go/src/github.com/DD/glock (master✔) ᐅ go install
ᐅ time glock sync ...
53.35s user 23.63s system 227% cpu 33.855 total

(^ This glock is the master branch with a revert of PR #34)

tamird commented 8 years ago

That's not surprising since go get is probably not parallel. Better to be correct than fast.

robfig commented 8 years ago

Thank you!