Skip to content
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

build: really use different go pkg directories for docker builds #7703

Merged

Conversation

petermattis
Copy link
Collaborator

@petermattis petermattis commented Jul 8, 2016

Fixes #7699.


This change is Reviewable

@petermattis
Copy link
Collaborator Author

@glycerine I'd appreciate it if you could test this.

@RaduBerinde
Copy link
Member

Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.


build/builder.sh, line 84 [r1] (raw file):

  --volume="${gopath0}/pkg/docker_amd64_race:/go/pkg/linux_amd64_race" \
  --volume="${gopath0}/pkg/docker_amd64:/usr/local/go/pkg/linux_amd64" \
  --volume="${gopath0}/pkg/docker_amd64_race:/usr/local/go/pkg/linux_amd64_race" \

So if a file is writtein into /go/pkg/linux_amd64, it will automatically show up into /usr/local/go/pkg/linux_amd64 right? I hope different files with the same relative paths are never written..

More generally - it is a little strange that we are effectively dumping our build artifacts directly into $GOPATH/pkg. Would it make more sense to put them in the cockroach dir (e.g. cockroach/build/docker/)? We could have a more fine-grained tree in there, like goroot/pkg, goroot/pkg-race, gopath/pkg, gopath/pkg-race, gopath/bin


Comments from Reviewable

@RaduBerinde
Copy link
Member

Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.


build/builder.sh, line 84 [r1] (raw file):

Previously, RaduBerinde wrote…

So if a file is writtein into /go/pkg/linux_amd64, it will automatically show up into /usr/local/go/pkg/linux_amd64 right? I hope different files with the same relative paths are never written..

More generally - it is a little strange that we are effectively dumping our build artifacts directly into $GOPATH/pkg. Would it make more sense to put them in the cockroach dir (e.g. cockroach/build/docker/)? We could have a more fine-grained tree in there, like goroot/pkg, goroot/pkg-race, gopath/pkg, gopath/pkg-race, gopath/bin

To clarify - when I say "dumping our build artifacts directly into `$GOPATH/pkg`" I mean on the host, not inside the container.

Comments from Reviewable

@petermattis
Copy link
Collaborator Author

Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.


build/builder.sh, line 84 [r1] (raw file):

Previously, RaduBerinde wrote…

To clarify - when I say "dumping our build artifacts directly into $GOPATH/pkg" I mean on the host, not inside the container.

Yes, if a file is written inside the container to `/go/pkg/linux_amd64` it will automatically show up in `/usr/local/go/pkg/linux_amd64`. This was the setup when this was originally written and somehow diverged over time.

Yeah, I could see putting the acceptance build artifacts in cockroach/build/docker, but I'm not seeing a strong reason to change it at this point. Using the docker_amd64 prefix ensures that we're not going to be clashing with host files (as we previously were).


Comments from Reviewable

@RaduBerinde
Copy link
Member

:lgtm:


Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.


build/builder.sh, line 84 [r1] (raw file):

Previously, petermattis (Peter Mattis) wrote…

Yes, if a file is written inside the container to /go/pkg/linux_amd64 it will automatically show up in /usr/local/go/pkg/linux_amd64. This was the setup when this was originally written and somehow diverged over time.

Yeah, I could see putting the acceptance build artifacts in cockroach/build/docker, but I'm not seeing a strong reason to change it at this point. Using the docker_amd64 prefix ensures that we're not going to be clashing with host files (as we previously were).

I guess everything under `gopath/pkg` starts with a hostname (`golang.org`, etc) and everything in `goroot/pkg` does not so there shouldn't be any collisions.

Comments from Reviewable

@petermattis
Copy link
Collaborator Author

Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.


build/builder.sh, line 84 [r1] (raw file):

Previously, RaduBerinde wrote…

I guess everything under gopath/pkg starts with a hostname (golang.org, etc) and everything in goroot/pkg does not so there shouldn't be any collisions.

Correct.

Comments from Reviewable

@petermattis petermattis merged commit 97f4679 into cockroachdb:master Jul 8, 2016
@petermattis petermattis deleted the pmattis/really-fix-build-builder branch July 8, 2016 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants