-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
[infra] Update base-builder image to support go-fuzz (#2714). #2735
Conversation
Travis is unhappy because this updates the base-builder image. |
|
||
# $SHELL is needed for the installer. | ||
ENV SHELL "bash" | ||
RUN chmod +x $SRC/installer_linux && \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: why not add SHELL="bash" when calling the command itself, this removes lines 34 and 38.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, didn't know it was possible this way. Thanks!
ENV SHELL "" | ||
|
||
# Set up Golang environment variables (copied from /root/.bash_profile). | ||
ENV PATH $PATH:/root/.go/bin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: this block can be shortened to
ENV GOPATH /root/go
ENV PATH $PATH:/root/.go/bin:$GOPATH/bin
Also, why do we have /root/go and /root/.go ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Also added a comment to clarify:
# /root/.go/bin is the standard Go binaries (i.e. go, gofmt, etc).
# $GOPATH/bin is the binaries from the dependencies installed via "go get".
ENV GOPATH /root/go | ||
ENV PATH $PATH:$GOPATH/bin | ||
|
||
# Dependency of go-fuzz |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: end with period.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Confirmed locally that this doesn't break golang project integration. |
If we agree on this, then we can update golang scripts and also document this.
In the meantime, I'll check locally that this does not break golang project. If it does, I'll just update its scripts in this PR as well.