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

Remove gox in favor of go build. #16353

Merged
merged 3 commits into from
Jul 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ EXTENDED_TEST_TIMEOUT=60m
INTEG_TEST_TIMEOUT=120m
VETARGS?=-asmdecl -atomic -bool -buildtags -copylocks -methods -nilfunc -printf -rangeloops -shift -structtags -unsafeptr
EXTERNAL_TOOLS_CI=\
github.com/mitchellh/gox \
golang.org/x/tools/cmd/goimports
EXTERNAL_TOOLS=\
github.com/client9/misspell/cmd/misspell
Expand Down Expand Up @@ -202,10 +201,10 @@ fmtcheck:
fmt:
find . -name '*.go' | grep -v pb.go | grep -v vendor | xargs gofumpt -w

semgrep:
semgrep:
semgrep --include '*.go' --exclude 'vendor' -a -f tools/semgrep .

semgrep-ci:
semgrep-ci:
semgrep --error --include '*.go' --exclude 'vendor' -f tools/semgrep/ci .

assetcheck:
Expand Down
3 changes: 3 additions & 0 deletions changelog/16353.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
core: remove gox
```
2 changes: 0 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,6 @@ require (
github.com/mitchellh/go-homedir v1.1.0
github.com/mitchellh/go-testing-interface v1.14.1
github.com/mitchellh/go-wordwrap v1.0.0
github.com/mitchellh/gox v1.0.1
github.com/mitchellh/mapstructure v1.5.0
github.com/mitchellh/reflectwalk v1.0.2
github.com/natefinch/atomic v0.0.0-20150920032501-a62ce929ffcc
Expand Down Expand Up @@ -329,7 +328,6 @@ require (
github.com/matttproud/golang_protobuf_extensions v1.0.2-0.20181231171920-c182affec369 // indirect
github.com/miekg/dns v1.1.41 // indirect
github.com/mitchellh/hashstructure v1.0.0 // indirect
github.com/mitchellh/iochan v1.0.0 // indirect
github.com/mitchellh/pointerstructure v1.2.0 // indirect
github.com/moby/sys/mount v0.2.0 // indirect
github.com/moby/sys/mountinfo v0.4.1 // indirect
Expand Down
4 changes: 0 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -978,7 +978,6 @@ github.com/hashicorp/go-uuid v1.0.1/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/b
github.com/hashicorp/go-uuid v1.0.2/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro=
github.com/hashicorp/go-uuid v1.0.3 h1:2gKiV6YVmrJ1i2CKKa9obLvRieoRGviZFL26PcT/Co8=
github.com/hashicorp/go-uuid v1.0.3/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro=
github.com/hashicorp/go-version v1.0.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA=
github.com/hashicorp/go-version v1.2.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA=
github.com/hashicorp/go-version v1.3.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA=
github.com/hashicorp/go-version v1.4.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA=
Expand Down Expand Up @@ -1324,11 +1323,8 @@ github.com/mitchellh/go-testing-interface v1.14.1/go.mod h1:gfgS7OtZj6MA4U1UrDRp
github.com/mitchellh/go-wordwrap v1.0.0 h1:6GlHJ/LTGMrIJbwgdqdl2eEH8o+Exx/0m8ir9Gns0u4=
github.com/mitchellh/go-wordwrap v1.0.0/go.mod h1:ZXFpozHsX6DPmq2I0TCekCxypsnAUbP2oI0UX1GXzOo=
github.com/mitchellh/gox v0.4.0/go.mod h1:Sd9lOJ0+aimLBi73mGofS1ycjY8lL3uZM3JPS42BGNg=
github.com/mitchellh/gox v1.0.1 h1:x0jD3dcHk9a9xPSDN6YEL4xL6Qz0dvNYm8yZqui5chI=
github.com/mitchellh/gox v1.0.1/go.mod h1:ED6BioOGXMswlXa2zxfh/xdd5QhwYliBFn9V18Ap4z4=
github.com/mitchellh/hashstructure v1.0.0 h1:ZkRJX1CyOoTkar7p/mLS5TZU4nJ1Rn/F8u9dGS02Q3Y=
github.com/mitchellh/hashstructure v1.0.0/go.mod h1:QjSHrPWS+BGUVBYkbTZWEnOh3G1DutKwClXU/ABz6AQ=
github.com/mitchellh/iochan v1.0.0 h1:C+X3KsSTLFVBr/tK1eYN/vs4rJcvsiLU338UhYPJWeY=
github.com/mitchellh/iochan v1.0.0/go.mod h1:JwYml1nuB7xOzsp52dPpHFffvOCDupsG0QubkSMEySY=
github.com/mitchellh/mapstructure v0.0.0-20160808181253-ca63d7c062ee/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y=
github.com/mitchellh/mapstructure v1.1.2/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y=
Expand Down
4 changes: 2 additions & 2 deletions make.bat
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ REM If no target is provided, default to test.
if [%1]==[] goto test

set _TARGETS=bin,bootstrap,dev,generate,test,testacc,testrace,vet
set _EXTERNAL_TOOLS=github.com/mitchellh/gox,github.com/kardianos/govendor
set _EXTERNAL_TOOLS=github.com/kardianos/govendor

REM Run target.
for %%a in (%_TARGETS%) do (if x%1==x%%a goto %%a)
Expand Down Expand Up @@ -82,7 +82,7 @@ REM any common errors.

go tool vet 2>nul
if %ERRORLEVEL% equ 3 go get golang.org/x/tools/cmd/vet

set _vetExitCode=0
set _VAULT_PKG_DIRS=%TEMP%\vault-pkg-dirs.txt

Expand Down
49 changes: 6 additions & 43 deletions scripts/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -23,21 +23,6 @@ GIT_DIRTY="$(test -n "`git status --porcelain`" && echo "+CHANGES" || true)"

BUILD_DATE=$("$SOURCE_DIR"/build_date.sh)

# If its dev mode, only build for ourself
if [ "${VAULT_DEV_BUILD}x" != "x" ] && [ "${XC_OSARCH}x" == "x" ]; then
XC_OS=$(${GO_CMD} env GOOS)
XC_ARCH=$(${GO_CMD} env GOARCH)
XC_OSARCH=$(${GO_CMD} env GOOS)/$(${GO_CMD} env GOARCH)
elif [ "${XC_OSARCH}x" != "x" ]; then
IFS='/' read -ra SPLITXC <<< "${XC_OSARCH}"
DEV_PLATFORM="./pkg/${SPLITXC[0]}_${SPLITXC[1]}"
fi

# Determine the arch/os combos we're building for
XC_ARCH=${XC_ARCH:-"386 amd64"}
XC_OS=${XC_OS:-linux darwin windows freebsd openbsd netbsd solaris}
XC_OSARCH=${XC_OSARCH:-"linux/386 linux/amd64 linux/arm linux/arm64 darwin/386 darwin/amd64 darwin/arm64 windows/386 windows/amd64 freebsd/386 freebsd/amd64 freebsd/arm openbsd/386 openbsd/amd64 openbsd/arm netbsd/386 netbsd/amd64 solaris/amd64"}

GOPATH=${GOPATH:-$(${GO_CMD} env GOPATH)}
case $(uname) in
CYGWIN*)
Expand All @@ -52,43 +37,21 @@ rm -rf pkg/*
mkdir -p bin/

# Build!
# If GOX_PARALLEL_BUILDS is set, it will be used to add a "-parallel=${GOX_PARALLEL_BUILDS}" gox parameter
echo "==> Building..."
gox \
-osarch="${XC_OSARCH}" \
${GO_CMD} build \
-gcflags "${GCFLAGS}" \
-ldflags "${LD_FLAGS}-X github.com/hashicorp/vault/sdk/version.GitCommit='${GIT_COMMIT}${GIT_DIRTY}' -X github.com/hashicorp/vault/sdk/version.BuildDate=${BUILD_DATE}" \
-output "pkg/{{.OS}}_{{.Arch}}/vault" \
${GOX_PARALLEL_BUILDS+-parallel="${GOX_PARALLEL_BUILDS}"} \
-tags="${BUILD_TAGS}" \
-gocmd="${GO_CMD}" \
-ldflags "${LD_FLAGS} -X github.com/hashicorp/vault/sdk/version.GitCommit='${GIT_COMMIT}${GIT_DIRTY}' -X github.com/hashicorp/vault/sdk/version.BuildDate=${BUILD_DATE}" \
-o "bin/vault" \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This bit will probably be missed:

    rm -f ${MAIN_GOPATH}/bin/vault
    cp ${F} ${MAIN_GOPATH}/bin/

Can you do something equivalent? People are used to the newly built being in their path.
Also the rm is important on macos, something to do with their binary scanning/validation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

go build does this under the hood already: golang/go@cf7aa58 (thanks to mitchellh for the link).

I'll re-add the bit to copy into the GOPATH.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does that for the build target, but if we're copying into GOPATH, that binary will still be vulnerable to the same problem, I think. Happy to test it once you've made the change if you like.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kept the rm for the copy into GOPATH. It should be committed now if you would like to test.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I just tested with a manual cp, and it gets sigkilled.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested your branch, works!

-tags "${BUILD_TAGS}" \
.

# Move all the compiled things to the $GOPATH/bin
OLDIFS=$IFS
IFS=: MAIN_GOPATH=($GOPATH)
IFS=$OLDIFS

# Copy our OS/Arch to the bin/ directory
DEV_PLATFORM=${DEV_PLATFORM:-"./pkg/$(${GO_CMD} env GOOS)_$(${GO_CMD} env GOARCH)"}
for F in $(find ${DEV_PLATFORM} -mindepth 1 -maxdepth 1 -type f); do
cp ${F} bin/
rm -f ${MAIN_GOPATH}/bin/vault
cp ${F} ${MAIN_GOPATH}/bin/
done

if [ "${VAULT_DEV_BUILD}x" = "x" ]; then
# Zip and copy to the dist dir
echo "==> Packaging..."
for PLATFORM in $(find ./pkg -mindepth 1 -maxdepth 1 -type d); do
OSARCH=$(basename ${PLATFORM})
echo "--> ${OSARCH}"

pushd $PLATFORM >/dev/null 2>&1
zip ../${OSARCH}.zip ./*
popd >/dev/null 2>&1
done
fi
rm -f ${MAIN_GOPATH}/bin/vault
cp bin/vault ${MAIN_GOPATH}/bin/

# Done!
echo
Expand Down
1 change: 0 additions & 1 deletion scripts/cross/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ ENV GOROOT /goroot
ENV PATH $GOROOT/bin:$GOPATH/bin:$PATH

RUN go get golang.org/x/tools/cmd/goimports
RUN go get github.com/mitchellh/gox

RUN mkdir -p /gopath/src/github.com/hashicorp/vault
WORKDIR /gopath/src/github.com/hashicorp/vault
Expand Down
7 changes: 3 additions & 4 deletions scripts/docker/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
# Multi-stage builder to avoid polluting users environment with wrong
# architecture binaries. Since this binary is used in an alpine container,
# we're explicitly compiling for 'linux/amd64'
# architecture binaries.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed this comment about linux/amd64 as I was able to successfully get this to work on linux/arm64 on my Mac as well (but not Dockerfile.ui due to problems with the debian distro).

ARG VERSION=1.17.12

FROM golang:${VERSION} AS builder
Expand All @@ -12,7 +11,7 @@ WORKDIR /go/src/github.com/hashicorp/vault
COPY . .

RUN make bootstrap \
&& CGO_ENABLED=$CGO_ENABLED BUILD_TAGS="${BUILD_TAGS}" VAULT_DEV_BUILD=1 XC_OSARCH='linux/amd64' sh -c "'./scripts/build.sh'"
&& CGO_ENABLED=$CGO_ENABLED BUILD_TAGS="${BUILD_TAGS}" VAULT_DEV_BUILD=1 sh -c "'./scripts/build.sh'"

# Docker Image

Expand All @@ -27,7 +26,7 @@ RUN addgroup vault && \
RUN set -eux; \
apk add --no-cache ca-certificates libcap su-exec dumb-init tzdata

COPY --from=builder /go/bin/vault /bin/vault
COPY --from=builder /go/src/github.com/hashicorp/vault/bin/vault /bin/vault

# /vault/logs is made available to use as a location to store audit logs, if
# desired; /vault/file is made available to use as a location with the file
Expand Down
7 changes: 3 additions & 4 deletions scripts/docker/Dockerfile.ui
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
# Multi-stage builder to avoid polluting users environment with wrong
# architecture binaries. Since this binary is used in an alpine container,
# we're explicitly compiling for 'linux/amd64'
# architecture binaries. This file only currently works for linux/amd64.
FROM debian:buster AS builder

ARG VERSION=1.17.12
Expand Down Expand Up @@ -38,7 +37,7 @@ ENV PATH $GOROOT/bin:$GOPATH/bin:$PATH
WORKDIR /go/src/github.com/hashicorp/vault
COPY . .
RUN make bootstrap static-dist \
&& CGO_ENABLED=$CGO_ENABLED BUILD_TAGS="${BUILD_TAGS} ui" VAULT_DEV_BUILD=1 XC_OSARCH='linux/amd64' sh -c "'./scripts/build.sh'"
&& CGO_ENABLED=$CGO_ENABLED BUILD_TAGS="${BUILD_TAGS} ui" VAULT_DEV_BUILD=1 GOOS=linux GOARCH=amd64 sh -c "'./scripts/build.sh'"

# Docker Image

Expand All @@ -53,7 +52,7 @@ RUN addgroup vault && \
RUN set -eux; \
apk add --no-cache ca-certificates libcap su-exec dumb-init tzdata

COPY --from=builder /go/bin/vault /bin/vault
COPY --from=builder /go/src/github.com/hashicorp/vault/bin/vault /bin/vault

# /vault/logs is made available to use as a location to store audit logs, if
# desired; /vault/file is made available to use as a location with the file
Expand Down
14 changes: 2 additions & 12 deletions scripts/windows/build.bat
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,9 @@ del /f "%_GO_ENV_TMP_FILE%" 2>nul
:build
REM Build!
echo ==^> Building...
gox^
-os="%_XC_OS%"^
-arch="%_XC_ARCH%"^
go build^
-ldflags "-X github.com/hashicorp/vault/sdk/version.GitCommit=%_GIT_COMMIT%%_GIT_DIRTY% -X github.com/hashicorp/vault/sdk/version.BuildDate=%_BUILD_DATE%"^
-output "pkg/{{.OS}}_{{.Arch}}/vault"^
-o "bin/vault.exe"^
.

if %ERRORLEVEL% equ 1 set %_EXITCODE%=1
Expand All @@ -87,14 +85,6 @@ go env GOOS >"%_GO_ENV_TMP_FILE%"
set /p _GOOS=<"%_GO_ENV_TMP_FILE%"
del /f "%_GO_ENV_TMP_FILE%" 2>nul

REM Copy our OS/Arch to the bin/ directory
set _DEV_PLATFORM=pkg\%_GOOS%_%_GOARCH%

for /r %%f in (%_DEV_PLATFORM%) do (
copy /b /y %%f bin\ >nul
copy /b /y %%f %_GOPATH%\bin\ >nul
)

REM TODO(ceh): package dist

REM Done!
Expand Down
3 changes: 0 additions & 3 deletions tools/tools.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
package tools

//go:generate go install golang.org/x/tools/cmd/goimports
//go:generate go install github.com/mitchellh/gox
//go:generate go install github.com/client9/misspell/cmd/misspell
//go:generate go install mvdan.cc/gofumpt
//go:generate go install google.golang.org/protobuf/cmd/protoc-gen-go
Expand All @@ -20,8 +19,6 @@ package tools
import (
_ "golang.org/x/tools/cmd/goimports"

_ "github.com/mitchellh/gox"

_ "github.com/client9/misspell/cmd/misspell"

_ "mvdan.cc/gofumpt"
Expand Down