From 6052dd9e173318dc5b98cd3589648e98ffd4b372 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Thu, 20 Feb 2020 09:34:41 +0100 Subject: [PATCH 1/7] =?UTF-8?q?Don=E2=80=99t=20use=20simply=20expanded=20v?= =?UTF-8?q?ariables=20for=20constants?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: David Gageot --- Makefile | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/Makefile b/Makefile index e3833630e76..75956277ced 100644 --- a/Makefile +++ b/Makefile @@ -14,8 +14,8 @@ GOOS ?= $(shell go env GOOS) GOARCH = amd64 BUILD_DIR ?= ./out -ORG := github.com/GoogleContainerTools -PROJECT := skaffold +ORG = github.com/GoogleContainerTools +PROJECT = skaffold REPOPATH ?= $(ORG)/$(PROJECT) RELEASE_BUCKET ?= $(PROJECT) GSC_BUILD_PATH ?= gs://$(RELEASE_BUCKET)/builds/$(COMMIT) @@ -28,10 +28,12 @@ GCP_PROJECT ?= k8s-skaffold GKE_CLUSTER_NAME ?= integration-tests GKE_ZONE ?= us-central1-a -SUPPORTED_PLATFORMS := linux-$(GOARCH) darwin-$(GOARCH) windows-$(GOARCH).exe +SUPPORTED_PLATFORMS = linux-$(GOARCH) darwin-$(GOARCH) windows-$(GOARCH).exe BUILD_PACKAGE = $(REPOPATH)/cmd/skaffold SKAFFOLD_TEST_PACKAGES := $(shell go list ./... | grep -v diag) +GO_FILES := $(shell find . -type f -name '*.go' -not -path "./vendor/*" -not -path "./pkg/diag/*") +DEPS_DIGEST := $(shell ./hack/skaffold-deps-sha1.sh) VERSION_PACKAGE = $(REPOPATH)/pkg/skaffold/version COMMIT = $(shell git rev-parse HEAD) @@ -45,16 +47,16 @@ endif export GO111MODULE = on export GOFLAGS = -mod=vendor -GO_GCFLAGS := "all=-trimpath=${PWD}" -GO_ASMFLAGS := "all=-trimpath=${PWD}" +GO_GCFLAGS = "all=-trimpath=${PWD}" +GO_ASMFLAGS = "all=-trimpath=${PWD}" LDFLAGS_linux = -static LDFLAGS_darwin = LDFLAGS_windows = -GO_BUILD_TAGS_linux := "osusergo netgo static_build release" -GO_BUILD_TAGS_darwin := "release" -GO_BUILD_TAGS_windows := "release" +GO_BUILD_TAGS_linux = "osusergo netgo static_build release" +GO_BUILD_TAGS_darwin = "release" +GO_BUILD_TAGS_windows = "release" GO_LDFLAGS = -X $(VERSION_PACKAGE).version=$(VERSION) GO_LDFLAGS += -X $(VERSION_PACKAGE).buildDate=$(shell date +'%Y-%m-%dT%H:%M:%SZ') @@ -66,9 +68,6 @@ GO_LDFLAGS_windows =" $(GO_LDFLAGS) -extldflags \"$(LDFLAGS_windows)\"" GO_LDFLAGS_darwin =" $(GO_LDFLAGS) -extldflags \"$(LDFLAGS_darwin)\"" GO_LDFLAGS_linux =" $(GO_LDFLAGS) -extldflags \"$(LDFLAGS_linux)\"" -GO_FILES := $(shell find . -type f -name '*.go' -not -path "./vendor/*" -not -path "./pkg/diag/*") -DEPS_DIGEST := $(shell ./hack/skaffold-deps-sha1.sh) - $(BUILD_DIR)/$(PROJECT): $(BUILD_DIR)/$(PROJECT)-$(GOOS)-$(GOARCH) cp $(BUILD_DIR)/$(PROJECT)-$(GOOS)-$(GOARCH) $@ From 9c42a4a4c3faaafe20c1fc3abeaa2e16ed708bed Mon Sep 17 00:00:00 2001 From: David Gageot Date: Thu, 20 Feb 2020 09:36:41 +0100 Subject: [PATCH 2/7] Eval only when needed This makes the Makefile faster for other targets. Signed-off-by: David Gageot --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 75956277ced..fc0d1bd1d75 100644 --- a/Makefile +++ b/Makefile @@ -33,7 +33,6 @@ BUILD_PACKAGE = $(REPOPATH)/cmd/skaffold SKAFFOLD_TEST_PACKAGES := $(shell go list ./... | grep -v diag) GO_FILES := $(shell find . -type f -name '*.go' -not -path "./vendor/*" -not -path "./pkg/diag/*") -DEPS_DIGEST := $(shell ./hack/skaffold-deps-sha1.sh) VERSION_PACKAGE = $(REPOPATH)/pkg/skaffold/version COMMIT = $(shell git rev-parse HEAD) @@ -165,6 +164,7 @@ clean: .PHONY: build_deps build_deps: + $(eval DEPS_DIGEST := $(shell ./hack/skaffold-deps-sha1.sh)) docker build \ -f deploy/skaffold/Dockerfile.deps \ -t gcr.io/$(GCP_PROJECT)/build_deps:$(DEPS_DIGEST) \ From d2cdb3dda0f618f943f4ef4e872ed85d660f4e98 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Thu, 20 Feb 2020 09:41:29 +0100 Subject: [PATCH 3/7] Use built-in variable for current path Signed-off-by: David Gageot --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index fc0d1bd1d75..d30aebef82f 100644 --- a/Makefile +++ b/Makefile @@ -46,8 +46,8 @@ endif export GO111MODULE = on export GOFLAGS = -mod=vendor -GO_GCFLAGS = "all=-trimpath=${PWD}" -GO_ASMFLAGS = "all=-trimpath=${PWD}" +GO_GCFLAGS = "all=-trimpath=$(CURDIR)" +GO_ASMFLAGS = "all=-trimpath=$(CURDIR)" LDFLAGS_linux = -static LDFLAGS_darwin = From a5d174ea9406988ae435a39626ed242314129c26 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Thu, 20 Feb 2020 09:49:27 +0100 Subject: [PATCH 4/7] Simplify `make` and `make install` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We don’t need a target to go build for every GOOS since this done by `make cross`. Signed-off-by: David Gageot --- Makefile | 25 ++++++++++++------------- deploy/skaffold/Dockerfile | 2 +- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/Makefile b/Makefile index d30aebef82f..b357d493478 100644 --- a/Makefile +++ b/Makefile @@ -67,12 +67,20 @@ GO_LDFLAGS_windows =" $(GO_LDFLAGS) -extldflags \"$(LDFLAGS_windows)\"" GO_LDFLAGS_darwin =" $(GO_LDFLAGS) -extldflags \"$(LDFLAGS_darwin)\"" GO_LDFLAGS_linux =" $(GO_LDFLAGS) -extldflags \"$(LDFLAGS_linux)\"" -$(BUILD_DIR)/$(PROJECT): $(BUILD_DIR)/$(PROJECT)-$(GOOS)-$(GOARCH) - cp $(BUILD_DIR)/$(PROJECT)-$(GOOS)-$(GOARCH) $@ - -$(BUILD_DIR)/$(PROJECT)-$(GOOS)-$(GOARCH): generate-statik $(GO_FILES) $(BUILD_DIR) +# Build for local development. +$(BUILD_DIR)/$(PROJECT): generate-statik $(GO_FILES) $(BUILD_DIR) GOOS=$(GOOS) GOARCH=$(GOARCH) CGO_ENABLED=1 go build -tags $(GO_BUILD_TAGS_$(GOOS)) -ldflags $(GO_LDFLAGS_$(GOOS)) -gcflags $(GO_GCFLAGS) -asmflags $(GO_ASMFLAGS) -o $@ $(BUILD_PACKAGE) +.PHONY: install +install: $(BUILD_DIR)/$(PROJECT) + cp $(BUILD_DIR)/$(PROJECT) $(GOPATH)/bin/$(PROJECT) + +# Build for a release. +.PRECIOUS: $(foreach platform, $(SUPPORTED_PLATFORMS), $(BUILD_DIR)/$(PROJECT)-$(platform)) + +.PHONY: cross +cross: $(foreach platform, $(SUPPORTED_PLATFORMS), $(BUILD_DIR)/$(PROJECT)-$(platform).sha256) + $(BUILD_DIR)/$(PROJECT)-%-$(GOARCH): generate-statik $(GO_FILES) $(BUILD_DIR) docker build \ --build-arg PROJECT=$(REPOPATH) \ @@ -97,11 +105,6 @@ $(BUILD_DIR)/VERSION: $(BUILD_DIR) $(BUILD_DIR): mkdir -p $(BUILD_DIR) -.PRECIOUS: $(foreach platform, $(SUPPORTED_PLATFORMS), $(BUILD_DIR)/$(PROJECT)-$(platform)) - -.PHONY: cross -cross: $(foreach platform, $(SUPPORTED_PLATFORMS), $(BUILD_DIR)/$(PROJECT)-$(platform).sha256) - .PHONY: test test: $(BUILD_DIR) @ ./hack/gotest.sh -count=1 -race -short -timeout=90s $(SKAFFOLD_TEST_PACKAGES) @@ -120,10 +123,6 @@ checks: $(BUILD_DIR) quicktest: @ ./hack/gotest.sh -short -timeout=60s $(SKAFFOLD_TEST_PACKAGES) -.PHONY: install -install: $(GO_FILES) $(BUILD_DIR) - GOOS=$(GOOS) GOARCH=$(GOARCH) CGO_ENABLED=1 go install -tags $(GO_BUILD_TAGS_$(GOOS)) -ldflags $(GO_LDFLAGS_$(GOOS)) -gcflags $(GO_GCFLAGS) -asmflags $(GO_ASMFLAGS) $(BUILD_PACKAGE) - .PHONY: integration integration: generate-statik install ifeq ($(GCP_ONLY),true) diff --git a/deploy/skaffold/Dockerfile b/deploy/skaffold/Dockerfile index 8906341c250..5d7965be857 100644 --- a/deploy/skaffold/Dockerfile +++ b/deploy/skaffold/Dockerfile @@ -19,5 +19,5 @@ COPY . . FROM builder as release ARG VERSION -RUN make clean && make out/skaffold-linux-amd64 VERSION=$VERSION && mv out/skaffold-linux-amd64 /usr/bin/skaffold +RUN make clean out/skaffold VERSION=$VERSION && mv out/skaffold /usr/bin/skaffold RUN skaffold credits -d /THIRD_PARTY_NOTICES From ddc7c9b10c9eef144ae51a381c3d4e0e33785e82 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Thu, 20 Feb 2020 09:54:58 +0100 Subject: [PATCH 5/7] Not useful for local dev and not used at release time. At release time, we build in a Docker container so the path is predictable. Signed-off-by: David Gageot --- Makefile | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Makefile b/Makefile index b357d493478..4ad94e09936 100644 --- a/Makefile +++ b/Makefile @@ -46,9 +46,6 @@ endif export GO111MODULE = on export GOFLAGS = -mod=vendor -GO_GCFLAGS = "all=-trimpath=$(CURDIR)" -GO_ASMFLAGS = "all=-trimpath=$(CURDIR)" - LDFLAGS_linux = -static LDFLAGS_darwin = LDFLAGS_windows = @@ -69,7 +66,7 @@ GO_LDFLAGS_linux =" $(GO_LDFLAGS) -extldflags \"$(LDFLAGS_linux)\"" # Build for local development. $(BUILD_DIR)/$(PROJECT): generate-statik $(GO_FILES) $(BUILD_DIR) - GOOS=$(GOOS) GOARCH=$(GOARCH) CGO_ENABLED=1 go build -tags $(GO_BUILD_TAGS_$(GOOS)) -ldflags $(GO_LDFLAGS_$(GOOS)) -gcflags $(GO_GCFLAGS) -asmflags $(GO_ASMFLAGS) -o $@ $(BUILD_PACKAGE) + GOOS=$(GOOS) GOARCH=$(GOARCH) CGO_ENABLED=1 go build -tags $(GO_BUILD_TAGS_$(GOOS)) -ldflags $(GO_LDFLAGS_$(GOOS)) -o $@ $(BUILD_PACKAGE) .PHONY: install install: $(BUILD_DIR)/$(PROJECT) From 18f775eda6fd02243fdceacb0b2c5f6dadd429cf Mon Sep 17 00:00:00 2001 From: David Gageot Date: Thu, 20 Feb 2020 10:00:27 +0100 Subject: [PATCH 6/7] Generate statik files only when needed Signed-off-by: David Gageot --- Makefile | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index 4ad94e09936..492f6f39c36 100644 --- a/Makefile +++ b/Makefile @@ -64,8 +64,10 @@ GO_LDFLAGS_windows =" $(GO_LDFLAGS) -extldflags \"$(LDFLAGS_windows)\"" GO_LDFLAGS_darwin =" $(GO_LDFLAGS) -extldflags \"$(LDFLAGS_darwin)\"" GO_LDFLAGS_linux =" $(GO_LDFLAGS) -extldflags \"$(LDFLAGS_linux)\"" +STATIK_FILES = cmd/skaffold/app/cmd/statik/statik.go + # Build for local development. -$(BUILD_DIR)/$(PROJECT): generate-statik $(GO_FILES) $(BUILD_DIR) +$(BUILD_DIR)/$(PROJECT): $(STATIK_FILES) $(GO_FILES) $(BUILD_DIR) GOOS=$(GOOS) GOARCH=$(GOARCH) CGO_ENABLED=1 go build -tags $(GO_BUILD_TAGS_$(GOOS)) -ldflags $(GO_LDFLAGS_$(GOOS)) -o $@ $(BUILD_PACKAGE) .PHONY: install @@ -78,7 +80,7 @@ install: $(BUILD_DIR)/$(PROJECT) .PHONY: cross cross: $(foreach platform, $(SUPPORTED_PLATFORMS), $(BUILD_DIR)/$(PROJECT)-$(platform).sha256) -$(BUILD_DIR)/$(PROJECT)-%-$(GOARCH): generate-statik $(GO_FILES) $(BUILD_DIR) +$(BUILD_DIR)/$(PROJECT)-%-$(GOARCH): $(STATIK_FILES) $(GO_FILES) $(BUILD_DIR) docker build \ --build-arg PROJECT=$(REPOPATH) \ --build-arg TARGETS=$*/$(GOARCH) \ @@ -121,7 +123,7 @@ quicktest: @ ./hack/gotest.sh -short -timeout=60s $(SKAFFOLD_TEST_PACKAGES) .PHONY: integration -integration: generate-statik install +integration: install ifeq ($(GCP_ONLY),true) gcloud container clusters get-credentials \ $(GKE_CLUSTER_NAME) \ @@ -238,6 +240,7 @@ build-docs-preview: generate-schemas: go run hack/schemas/main.go -.PHONY: generate-statik -generate-statik: +# static files + +$(STATIK_FILES): go.mod docs/content/en/schemas/* hack/generate-statik.sh From 15c6699381e1dca6e5f1faa939df350576912668 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Thu, 20 Feb 2020 10:09:10 +0100 Subject: [PATCH 7/7] These variables can be deferred Signed-off-by: David Gageot --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 492f6f39c36..4e7fe6fc8c4 100644 --- a/Makefile +++ b/Makefile @@ -31,8 +31,8 @@ GKE_ZONE ?= us-central1-a SUPPORTED_PLATFORMS = linux-$(GOARCH) darwin-$(GOARCH) windows-$(GOARCH).exe BUILD_PACKAGE = $(REPOPATH)/cmd/skaffold -SKAFFOLD_TEST_PACKAGES := $(shell go list ./... | grep -v diag) -GO_FILES := $(shell find . -type f -name '*.go' -not -path "./vendor/*" -not -path "./pkg/diag/*") +SKAFFOLD_TEST_PACKAGES = $(shell go list ./... | grep -v diag) +GO_FILES = $(shell find . -type f -name '*.go' -not -path "./vendor/*" -not -path "./pkg/diag/*") VERSION_PACKAGE = $(REPOPATH)/pkg/skaffold/version COMMIT = $(shell git rev-parse HEAD)