Skip to content
This repository has been archived by the owner on Nov 25, 2024. It is now read-only.

Go: miscellaneous cleanup #196

Merged
merged 6 commits into from
Jul 28, 2022
Merged

Go: miscellaneous cleanup #196

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
175726e
Go: cleanup
epk Jul 27, 2022
e0e4050
Update gomplate to 3.11.1
epk Jul 27, 2022
8acbfc4
Add new go.modules targets
epk Jul 27, 2022
b276a37
Ping down versions
epk Jul 27, 2022
700a1cf
rm go.distclean
epk Jul 27, 2022
e040ab1
Keep GOFMT_VERSION same as GO_VERSION
epk Jul 27, 2022
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
90 changes: 9 additions & 81 deletions makelib/golang.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,6 @@ GO_SUBDIRS ?= cmd pkg
# Optional. Additional subdirs used for integration or e2e testings
GO_INTEGRATION_TESTS_SUBDIRS ?=

# Optional directories (relative to CURDIR)
GO_VENDOR_DIR ?= vendor
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't vendor if GO111MODULE is on. GO111MODULE is set to on automatically if there is a go.mod file since Go 1.16: https://go.dev/blog/go116-module-changes

GO_PKG_DIR ?= $(WORK_DIR)/pkg
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use the default/system global $GOPATH/pkg directory, this helps go mod cache ($GOPATH/pkg/mod) reuse between different repos.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not to mention having $(WORK_DIR)/pkg messes up with vscode + gopls when it sees several thousand files.

Copy link
Member

Choose a reason for hiding this comment

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

I think I'm mostly concerned about this part of the changes. I agree with the cache reuse aspect; however I want to dig into why the mod cache was placed in the directories to get that context.

Copy link
Member

Choose a reason for hiding this comment

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

however I want to dig into why the mod cache was placed in the directories to get that context

To tie a bow on this, it looks like this behavior was introduced during the introduction to using Go modules here #74. Most likely this was done for two reasons:

  1. we were still supporting dep at that time
  2. it wasn't entirely clear how to work with go modules yet

With that context, I'm comfortable moving forward with this changeset.


# Optional build flags passed to go tools
GO_BUILDFLAGS ?=
GO_LDFLAGS ?=
Expand Down Expand Up @@ -81,8 +77,6 @@ endif
GOPATH := $(shell $(GO) env GOPATH)

# setup tools used during the build
DEP_VERSION=v0.5.1
DEP := $(TOOLS_HOST_DIR)/dep-$(DEP_VERSION)
Comment on lines -84 to -85
Copy link
Contributor Author

Choose a reason for hiding this comment

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

DEP was deprecated long ago (Go 1.13 iirc)

GOJUNIT := $(TOOLS_HOST_DIR)/go-junit-report
GOCOVER_COBERTURA := $(TOOLS_HOST_DIR)/gocover-cobertura
GOIMPORTS := $(TOOLS_HOST_DIR)/goimports
Expand Down Expand Up @@ -124,28 +118,10 @@ ifeq ($(RUNNING_IN_CI),true)
GO_LINT_ARGS += --timeout 10m0s --out-format=checkstyle > $(GO_LINT_OUTPUT)/checkstyle.xml
endif

# NOTE: the install suffixes are matched with the build container to speed up the
# the build. Please keep them in sync.

ifneq ($(GO_PKG_DIR),)
GO_PKG_BASE_DIR := $(abspath $(GO_PKG_DIR)/$(PLATFORM))
GO_PKG_STATIC_FLAGS := -pkgdir $(GO_PKG_BASE_DIR)_static
endif

GO_COMMON_FLAGS = $(GO_BUILDFLAGS) -tags '$(GO_TAGS)'
GO_STATIC_FLAGS = $(GO_COMMON_FLAGS) $(GO_PKG_STATIC_FLAGS) -installsuffix static -ldflags '$(GO_LDFLAGS)'
Comment on lines -127 to -136
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use default/system global $GOPATH/pkg directory, helps with reuse across multiple repos

GO_COMMON_FLAGS = $(GO_BUILDFLAGS) -tags '$(GO_TAGS)' -trimpath
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add -trimpath to Go invocations:

	-trimpath
		remove all file system paths from the resulting executable.
		Instead of absolute file system paths, the recorded file names
		will begin either a module path@version (when using modules),
		or a plain import path (when using the standard library, or GOPATH).

GO_STATIC_FLAGS = $(GO_COMMON_FLAGS) -installsuffix static -ldflags '$(GO_LDFLAGS)'
GO_GENERATE_FLAGS = $(GO_BUILDFLAGS) -tags 'generate $(GO_TAGS)'

export GO111MODULE

# switch for go modules
ifeq ($(GO111MODULE),on)

# set GOPATH to $(GO_PKG_DIR), so that the go modules are installed there, instead of the default $HOME/go/pkg/mod
export GOPATH=$(abspath $(GO_PKG_DIR))

endif

# ====================================================================================
# Go Targets

Expand Down Expand Up @@ -217,8 +193,6 @@ go.fmt.simplify: $(GOFMT)

go.validate: go.vet go.fmt

ifeq ($(GO111MODULE),on)

go.vendor.lite go.vendor.check:
@$(INFO) verify dependencies have expected content
@$(GOHOST) mod verify || $(FAIL)
Expand All @@ -230,47 +204,15 @@ go.vendor.update:
@$(OK) update go modules

go.vendor:
@$(INFO) go mod vendor
@$(GOHOST) mod vendor || $(FAIL)
@$(OK) go mod vendor

else

go.vendor.lite: $(DEP)
# dep ensure blindly updates the whole vendor tree causing everything to be rebuilt. This workaround
# will only call dep ensure if the .lock file changes or if the vendor dir is non-existent.
@if [ ! -d $(GO_VENDOR_DIR) ]; then \
$(MAKE) vendor; \
elif ! $(DEP) ensure -no-vendor -dry-run &> /dev/null; then \
$(MAKE) vendor; \
fi

go.vendor.check: $(DEP)
@$(INFO) checking if vendor deps changed
@$(DEP) check -skip-vendor || $(FAIL)
@$(OK) vendor deps have not changed

go.vendor.update: $(DEP)
@$(INFO) updating vendor deps
@$(DEP) ensure -update -v || $(FAIL)
@$(OK) updating vendor deps

go.vendor: $(DEP)
@$(INFO) dep ensure
@$(DEP) ensure || $(FAIL)
@$(OK) dep ensure

endif
@$(INFO) mod download
@$(GOHOST) mod download || $(FAIL)
@$(OK) mod download

go.clean:
@# `go modules` creates read-only folders under WORK_DIR
@# make all folders within WORK_DIR writable, so they can be deleted
@if [ -d $(WORK_DIR) ]; then chmod -R +w $(WORK_DIR); fi

@rm -fr $(GO_BIN_DIR) $(GO_TEST_DIR)

go.distclean:
@rm -rf $(GO_VENDOR_DIR) $(GO_PKG_DIR)
epk marked this conversation as resolved.
Show resolved Hide resolved
@$(OK) distclean

go.generate:
@$(INFO) go generate $(PLATFORM)
Expand Down Expand Up @@ -334,14 +276,6 @@ help-special: go.help
# ====================================================================================
# Tools install targets

$(DEP):
@$(INFO) installing dep-$(DEP_VERSION) $(SAFEHOSTPLATFORM)
@mkdir -p $(TOOLS_HOST_DIR)/tmp-dep || $(FAIL)
@curl -fsSL -o $(DEP) https://github.com/golang/dep/releases/download/$(DEP_VERSION)/dep-$(SAFEHOSTPLATFORM) || $(FAIL)
@chmod +x $(DEP) || $(FAIL)
@rm -fr $(TOOLS_HOST_DIR)/tmp-dep
@$(OK) installing dep-$(DEP_VERSION) $(SAFEHOSTPLATFORM)

$(GOLANGCILINT):
@$(INFO) installing golangci-lint-v$(GOLANGCILINT_VERSION) $(SAFEHOSTPLATFORM)
@mkdir -p $(TOOLS_HOST_DIR)/tmp-golangci-lint || $(FAIL)
Expand All @@ -360,21 +294,15 @@ $(GOFMT):

$(GOIMPORTS):
@$(INFO) installing goimports
@mkdir -p $(TOOLS_HOST_DIR)/tmp-goimports || $(FAIL)
@GO111MODULE=off GOPATH=$(TOOLS_HOST_DIR)/tmp-goimports GOBIN=$(TOOLS_HOST_DIR) $(GOHOST) get golang.org/x/tools/cmd/goimports || rm -fr $(TOOLS_HOST_DIR)/tmp-goimports || $(FAIL)
@rm -fr $(TOOLS_HOST_DIR)/tmp-goimports
@GOBIN=$(TOOLS_HOST_DIR) $(GOHOST) install golang.org/x/tools/cmd/goimports@latest || $(FAIL)
epk marked this conversation as resolved.
Show resolved Hide resolved
@$(OK) installing goimports

$(GOJUNIT):
@$(INFO) installing go-junit-report
@mkdir -p $(TOOLS_HOST_DIR)/tmp-junit || $(FAIL)
@GO111MODULE=off GOPATH=$(TOOLS_HOST_DIR)/tmp-junit GOBIN=$(TOOLS_HOST_DIR) $(GOHOST) get github.com/jstemmer/go-junit-report || rm -fr $(TOOLS_HOST_DIR)/tmp-junit || $(FAIL)
@rm -fr $(TOOLS_HOST_DIR)/tmp-junit
@GOBIN=$(TOOLS_HOST_DIR) $(GOHOST) install github.com/jstemmer/go-junit-report/v2@latest || $(FAIL)
@$(OK) installing go-junit-report

$(GOCOVER_COBERTURA):
@$(INFO) installing gocover-cobertura
@mkdir -p $(TOOLS_HOST_DIR)/tmp-gocover-cobertura || $(FAIL)
@GO111MODULE=off GOPATH=$(TOOLS_HOST_DIR)/tmp-gocover-cobertura GOBIN=$(TOOLS_HOST_DIR) $(GOHOST) get github.com/t-yuki/gocover-cobertura || rm -fr $(TOOLS_HOST_DIR)/tmp-covcover-cobertura || $(FAIL)
@rm -fr $(TOOLS_HOST_DIR)/tmp-gocover-cobertura
@GOBIN=$(TOOLS_HOST_DIR) $(GOHOST) install github.com/t-yuki/gocover-cobertura@latest || $(FAIL)
epk marked this conversation as resolved.
Show resolved Hide resolved
@$(OK) installing gocover-cobertura
1 change: 0 additions & 1 deletion run
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,6 @@ docker run \
-h ${BUILD_HOST} \
-e BUILD_REGISTRY=${BUILD_REGISTRY} \
-e GOPATH="${BUILDER_HOME}/go" \
-e GO_PKG_DIR="" \
-e GITHUB_TOKEN \
-e VERSION \
-e CHANNEL \
Expand Down