From b7dadf0f7bf8cc96aad84482159d0a63e5ef38fc Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 27 Apr 2020 12:20:20 -0700 Subject: [PATCH 1/8] Makefile: rm $(allpackages) This was added by commit 993cbf9db but since some time ago (go 1.13 for sure, but may be earlier) is no longer needed since all the tools are correctly skipping vendor subdir. Signed-off-by: Kir Kolyshkin --- Makefile | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/Makefile b/Makefile index 99d068c05..8b8ead9ad 100644 --- a/Makefile +++ b/Makefile @@ -59,8 +59,8 @@ dbuild: runcimage $(RUNC_IMAGE) make clean all lint: - $(GO) vet $(allpackages) - $(GO) fmt $(allpackages) + $(GO) vet ./... + $(GO) fmt ./... man: man/md2man-all.sh @@ -82,7 +82,7 @@ unittest: runcimage $(RUNC_IMAGE) make localunittest TESTFLAGS=$(TESTFLAGS) localunittest: all - $(GO) test -timeout 3m -tags "$(BUILDTAGS)" $(TESTFLAGS) -v $(allpackages) + $(GO) test -timeout 3m -tags "$(BUILDTAGS)" $(TESTFLAGS) -v ./... integration: runcimage $(CONTAINER_ENGINE) run $(CONTAINER_ENGINE_RUN_FLAGS) \ @@ -138,7 +138,7 @@ clean: validate: script/validate-gofmt script/validate-c - $(GO) vet $(allpackages) + $(GO) vet ./... ci: validate test release @@ -164,7 +164,3 @@ localcross: CGO_ENABLED=1 GOARCH=arm GOARM=7 CC=arm-linux-gnueabihf-gcc $(GO_BUILD) -o runc-armhf . CGO_ENABLED=1 GOARCH=arm64 CC=aarch64-linux-gnu-gcc $(GO_BUILD) -o runc-arm64 . CGO_ENABLED=1 GOARCH=ppc64le CC=powerpc64le-linux-gnu-gcc $(GO_BUILD) -o runc-ppc64le . - -# memoize allpackages, so that it's executed only once and only if used -_allpackages = $(shell $(GO) list ./... | grep -v vendor) -allpackages = $(if $(__allpackages),,$(eval __allpackages := $$(_allpackages)))$(__allpackages) From fc54f6d7db579fe05f878b291b0d1dc44539168d Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 27 Apr 2020 12:44:25 -0700 Subject: [PATCH 2/8] Makefile: rm $(SOURCES), mark targets as PHONY Since go has its own way to track dependencies and rebuild if needed, and it is efficient enough, let's drop using SOURCES variable, mark all targets as PHONY and let golang do its job. The primary motivation for this was concern about using find on every make invocation to build the list of all sources. Some unscientific performance analisys: Before: > $ time make > make: 'runc' is up to date. > > real 0m0.202s > user 0m0.177s > sys 0m0.031s After: > $ time make > go build -mod=vendor -buildmode=pie -tags "seccomp selinux apparmor" -ldflags "-X main.gitCommit="5a8210a58bd0f07cc987e6201b4174e5b93fa115" -X main.version=1.0.0-rc10+dev " -o runc . > > real 0m0.149s > user 0m0.315s > sys 0m0.106s So, it is slightly faster using the wall clock, uses more CPU, but we can be sure the binary is always up to date. This also fixes the Makefile to mark all targets as PHONY. The list was generated by `grep -E '^[a-z-]+:' Makefile | sed 's/:.*//'`. Signed-off-by: Kir Kolyshkin --- Makefile | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/Makefile b/Makefile index 8b8ead9ad..123dc6db6 100644 --- a/Makefile +++ b/Makefile @@ -1,12 +1,6 @@ -.PHONY: all shell dbuild man release \ - localtest localunittest localintegration \ - test unittest integration \ - cross localcross vendor verify-dependencies - CONTAINER_ENGINE := docker GO := go -SOURCES := $(shell find . 2>&1 | grep -E '.*\.(c|h|go)$$') PREFIX := $(DESTDIR)/usr/local BINDIR := $(PREFIX)/sbin GIT_BRANCH := $(shell git rev-parse --abbrev-ref HEAD 2>/dev/null) @@ -35,17 +29,15 @@ SHELL := $(shell command -v bash 2>/dev/null) .DEFAULT: runc -runc: $(SOURCES) +runc: $(GO_BUILD) -o runc . all: runc recvtty -recvtty: contrib/cmd/recvtty/recvtty - -contrib/cmd/recvtty/recvtty: $(SOURCES) +recvtty: $(GO_BUILD) -o contrib/cmd/recvtty/recvtty ./contrib/cmd/recvtty -static: $(SOURCES) +static: $(GO_BUILD_STATIC) -o runc . $(GO_BUILD_STATIC) -o contrib/cmd/recvtty/recvtty ./contrib/cmd/recvtty @@ -164,3 +156,9 @@ localcross: CGO_ENABLED=1 GOARCH=arm GOARM=7 CC=arm-linux-gnueabihf-gcc $(GO_BUILD) -o runc-armhf . CGO_ENABLED=1 GOARCH=arm64 CC=aarch64-linux-gnu-gcc $(GO_BUILD) -o runc-arm64 . CGO_ENABLED=1 GOARCH=ppc64le CC=powerpc64le-linux-gnu-gcc $(GO_BUILD) -o runc-ppc64le . + +.PHONY: runc all recvtty static release dbuild lint man runcimage \ + test localtest unittest localunittest integration localintegration \ + rootlessintegration localrootlessintegration shell install install-bash \ + install-man uninstall uninstall-bash uninstall-man clean validate ci \ + vendor verify-dependencies cross localcross From 19ba7688cb4e0922d53029e2f7c1f2af45d40938 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 27 Apr 2020 12:55:24 -0700 Subject: [PATCH 3/8] Makefile: test, localtest: no need to invoke make Signed-off-by: Kir Kolyshkin --- Makefile | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 123dc6db6..feb35faa3 100644 --- a/Makefile +++ b/Makefile @@ -60,11 +60,9 @@ man: runcimage: $(CONTAINER_ENGINE) build $(CONTAINER_ENGINE_BUILD_FLAGS) -t $(RUNC_IMAGE) . -test: - make unittest integration rootlessintegration +test: unittest integration rootlessintegration -localtest: - make localunittest localintegration localrootlessintegration +localtest: localunittest localintegration localrootlessintegration unittest: runcimage $(CONTAINER_ENGINE) run $(CONTAINER_ENGINE_RUN_FLAGS) \ From 2fe9e31aa9c5278cd48e702717a7f4e899bfb7fa Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 27 Apr 2020 13:15:44 -0700 Subject: [PATCH 4/8] Makefile: don't use -mod=vendor if GO111MODULE=off This fixes the following bug: > $ GO111MODULE=off make > go build "-mod=vendor" -buildmode=pie -tags "seccomp selinux apparmor" -ldflags "-X main.gitCommit="19ba7688cb4e0922d53029e2f7c1f2af45d40938-dirty" -X main.version=1.0.0-rc10+dev " -o runc . > build flag -mod=vendor only valid when using modules Signed-off-by: Kir Kolyshkin --- Makefile | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index feb35faa3..baa401896 100644 --- a/Makefile +++ b/Makefile @@ -12,10 +12,13 @@ COMMIT_NO := $(shell git rev-parse HEAD 2> /dev/null || true) COMMIT ?= $(if $(shell git status --porcelain --untracked-files=no),"$(COMMIT_NO)-dirty","$(COMMIT_NO)") VERSION := $(shell cat ./VERSION) -# TODO: rm -mod=vendor once go 1.13 will be unsupported -GO_BUILD := $(GO) build -mod=vendor -buildmode=pie $(EXTRA_FLAGS) -tags "$(BUILDTAGS)" \ +# TODO: rm -mod=vendor once go 1.13 is unsupported +ifneq ($(GO111MODULE),off) + MOD_VENDOR := "-mod=vendor" +endif +GO_BUILD := $(GO) build $(MOD_VENDOR) -buildmode=pie $(EXTRA_FLAGS) -tags "$(BUILDTAGS)" \ -ldflags "-X main.gitCommit=$(COMMIT) -X main.version=$(VERSION) $(EXTRA_LDFLAGS)" -GO_BUILD_STATIC := CGO_ENABLED=1 $(GO) build -mod=vendor $(EXTRA_FLAGS) -tags "$(BUILDTAGS) netgo osusergo" \ +GO_BUILD_STATIC := CGO_ENABLED=1 $(GO) build $(MOD_VENDOR) $(EXTRA_FLAGS) -tags "$(BUILDTAGS) netgo osusergo" \ -ldflags "-w -extldflags -static -X main.gitCommit=$(COMMIT) -X main.version=$(VERSION) $(EXTRA_LDFLAGS)" MAN_DIR := $(CURDIR)/man/man8 From a036e890b9ef394cd35850176f2bce0bde09e82f Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 27 Apr 2020 13:23:29 -0700 Subject: [PATCH 5/8] Makefile: add -mod=vendor to go test Otherwise, in case go < 1.14 is used, all the go deps are downloaded instead of using vendor subdir. Signed-off-by: Kir Kolyshkin --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index baa401896..a4510ecde 100644 --- a/Makefile +++ b/Makefile @@ -75,7 +75,7 @@ unittest: runcimage $(RUNC_IMAGE) make localunittest TESTFLAGS=$(TESTFLAGS) localunittest: all - $(GO) test -timeout 3m -tags "$(BUILDTAGS)" $(TESTFLAGS) -v ./... + $(GO) $(MOD_VENDOR) test -timeout 3m -tags "$(BUILDTAGS)" $(TESTFLAGS) -v ./... integration: runcimage $(CONTAINER_ENGINE) run $(CONTAINER_ENGINE_RUN_FLAGS) \ From df72e8989c97581029b0bb3bec6af539fc45c092 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 27 Apr 2020 13:37:36 -0700 Subject: [PATCH 6/8] Makefile: rm uninstall* targets These targets are not very reliable and, depending on environment variables, migth result in data loss. For example: make DESTDIR=`pwd`/tmp install ... make uninstall The first make command will install $CURDIR/tmp/usr/local/bin/runc, while the last command will remove /usr/local/bin/runc. One way to support uninstall would be to write a temp file during installation, which would contain the files we have installed. Signed-off-by: Kir Kolyshkin --- Makefile | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/Makefile b/Makefile index a4510ecde..41027c182 100644 --- a/Makefile +++ b/Makefile @@ -75,7 +75,7 @@ unittest: runcimage $(RUNC_IMAGE) make localunittest TESTFLAGS=$(TESTFLAGS) localunittest: all - $(GO) $(MOD_VENDOR) test -timeout 3m -tags "$(BUILDTAGS)" $(TESTFLAGS) -v ./... + $(GO) test $(MOD_VENDOR) -timeout 3m -tags "$(BUILDTAGS)" $(TESTFLAGS) -v ./... integration: runcimage $(CONTAINER_ENGINE) run $(CONTAINER_ENGINE_RUN_FLAGS) \ @@ -113,15 +113,6 @@ install-man: install -d -m 755 $(MAN_INSTALL_PATH) install -m 644 $(MAN_PAGES) $(MAN_INSTALL_PATH) -uninstall: - rm -f $(BINDIR)/runc - -uninstall-bash: - rm -f $(PREFIX)/share/bash-completion/completions/runc - -uninstall-man: - rm -f $(addprefix $(MAN_INSTALL_PATH),$(MAN_PAGES_BASE)) - clean: rm -f runc runc-* rm -f contrib/cmd/recvtty/recvtty @@ -161,5 +152,5 @@ localcross: .PHONY: runc all recvtty static release dbuild lint man runcimage \ test localtest unittest localunittest integration localintegration \ rootlessintegration localrootlessintegration shell install install-bash \ - install-man uninstall uninstall-bash uninstall-man clean validate ci \ + install-man clean validate ci \ vendor verify-dependencies cross localcross From 731947d5ecb8decd6b4f6a916640e7db29a41e2e Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 27 Apr 2020 13:49:39 -0700 Subject: [PATCH 7/8] Makefile: fix/clean install-man Target `install-man` was not dependent on `man`, meaning no man pages were installed unless one called `make man` beforehand. Fix this. Remove many man-related variables, only leaving MANDIR, which is an installation directory for man pages. Signed-off-by: Kir Kolyshkin --- Makefile | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/Makefile b/Makefile index 41027c182..a19cd7400 100644 --- a/Makefile +++ b/Makefile @@ -3,6 +3,8 @@ GO := go PREFIX := $(DESTDIR)/usr/local BINDIR := $(PREFIX)/sbin +MANDIR := $(PREFIX)/share/man + GIT_BRANCH := $(shell git rev-parse --abbrev-ref HEAD 2>/dev/null) GIT_BRANCH_CLEAN := $(shell echo $(GIT_BRANCH) | sed -e "s/[^[:alnum:]]/-/g") RUNC_IMAGE := runc_dev$(if $(GIT_BRANCH_CLEAN),:$(GIT_BRANCH_CLEAN)) @@ -21,11 +23,6 @@ GO_BUILD := $(GO) build $(MOD_VENDOR) -buildmode=pie $(EXTRA_FLAGS) -tags "$(BUI GO_BUILD_STATIC := CGO_ENABLED=1 $(GO) build $(MOD_VENDOR) $(EXTRA_FLAGS) -tags "$(BUILDTAGS) netgo osusergo" \ -ldflags "-w -extldflags -static -X main.gitCommit=$(COMMIT) -X main.version=$(VERSION) $(EXTRA_LDFLAGS)" -MAN_DIR := $(CURDIR)/man/man8 -MAN_PAGES = $(shell ls $(MAN_DIR)/*.8) -MAN_PAGES_BASE = $(notdir $(MAN_PAGES)) -MAN_INSTALL_PATH := $(PREFIX)/share/man/man8/ - RELEASE_DIR := $(CURDIR)/release SHELL := $(shell command -v bash 2>/dev/null) @@ -109,15 +106,15 @@ install: install-bash: install -D -m0644 contrib/completions/bash/runc $(PREFIX)/share/bash-completion/completions/runc -install-man: - install -d -m 755 $(MAN_INSTALL_PATH) - install -m 644 $(MAN_PAGES) $(MAN_INSTALL_PATH) +install-man: man + install -d -m 755 $(MANDIR)/man8 + install -D -m 644 man/man8/*.8 $(MANDIR)/man8 clean: rm -f runc runc-* rm -f contrib/cmd/recvtty/recvtty rm -rf $(RELEASE_DIR) - rm -rf $(MAN_DIR) + rm -rf man/man8 validate: script/validate-gofmt From 772d090930a8862b1ce0785019adf60776aa7b7f Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 27 Apr 2020 14:10:51 -0700 Subject: [PATCH 8/8] Makefile: rm RELEASE_DIR and SHELL RELEASE_DIR is only used once, so it doesn't make sense to have it. SHELL was introduced in commit 54390f89a76fef305c and was used implicitly (since Makefile contained some bash-specific code), but is no longer needed since commit ed68ee1e1054fee08d. Signed-off-by: Kir Kolyshkin --- Makefile | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/Makefile b/Makefile index a19cd7400..38683052c 100644 --- a/Makefile +++ b/Makefile @@ -23,10 +23,6 @@ GO_BUILD := $(GO) build $(MOD_VENDOR) -buildmode=pie $(EXTRA_FLAGS) -tags "$(BUI GO_BUILD_STATIC := CGO_ENABLED=1 $(GO) build $(MOD_VENDOR) $(EXTRA_FLAGS) -tags "$(BUILDTAGS) netgo osusergo" \ -ldflags "-w -extldflags -static -X main.gitCommit=$(COMMIT) -X main.version=$(VERSION) $(EXTRA_LDFLAGS)" -RELEASE_DIR := $(CURDIR)/release - -SHELL := $(shell command -v bash 2>/dev/null) - .DEFAULT: runc runc: @@ -113,7 +109,7 @@ install-man: man clean: rm -f runc runc-* rm -f contrib/cmd/recvtty/recvtty - rm -rf $(RELEASE_DIR) + rm -rf release rm -rf man/man8 validate: