From b20fd9d10e4a7b673d59eeb9611a4eb5b00d9ba4 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Tue, 16 Apr 2019 16:00:22 +0100 Subject: [PATCH 1/6] build: remove duplicated COLLECT_SCRIPT from clean GENERATED_FILES already includes COLLECT_SCRIPT, so there's no need to specify it again: GENERATED_FILES += $(COLLECT_SCRIPT) ... clean: $(QUIET_CLEAN)rm -f ... $(GENERATED_FILES) $(COLLECT_SCRIPT) Signed-off-by: Stefan Hajnoczi --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 072aca548..628e6fe6f 100644 --- a/Makefile +++ b/Makefile @@ -564,7 +564,7 @@ install-completions: $(QUIET_INST)install --mode 0644 -D $(BASH_COMPLETIONS) $(DESTDIR)/$(BASH_COMPLETIONSDIR)/$(notdir $(BASH_COMPLETIONS)); clean: - $(QUIET_CLEAN)rm -f $(TARGET) $(SHIMV2) $(NETMON_TARGET) $(CONFIGS) $(GENERATED_GO_FILES) $(GENERATED_FILES) $(COLLECT_SCRIPT) + $(QUIET_CLEAN)rm -f $(TARGET) $(SHIMV2) $(NETMON_TARGET) $(CONFIGS) $(GENERATED_GO_FILES) $(GENERATED_FILES) show-usage: show-header @printf "• Overview:\n" From bbf92533f4bbc35ae7e0bdd5c0bf9c4c5c25e88e Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Wed, 17 Apr 2019 10:04:30 +0100 Subject: [PATCH 2/6] build: add VERSION dependency to netmon target The netmon target must be rebuilt when the VERSION file changes since it uses the value of VERSION on the build command-line. Signed-off-by: Stefan Hajnoczi --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 628e6fe6f..b27e580af 100644 --- a/Makefile +++ b/Makefile @@ -361,7 +361,7 @@ containerd-shim-v2: $(SHIMV2_OUTPUT) netmon: $(NETMON_TARGET_OUTPUT) -$(NETMON_TARGET_OUTPUT): $(SOURCES) +$(NETMON_TARGET_OUTPUT): $(SOURCES) VERSION $(QUIET_BUILD)(cd $(NETMON_DIR) && go build $(BUILDFLAGS) -o $@ -ldflags "-X main.version=$(VERSION)") runtime: $(TARGET_OUTPUT) $(CONFIGS) From 1eb5d6c900886eedd9117a1777cc39c02bdd0c78 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Wed, 17 Apr 2019 10:05:44 +0100 Subject: [PATCH 3/6] build: use MAKEFILE_LIST for a more complete Makefile dependency Depending on Makefile is not enough to detect all changes. We must rebuild when included makefiles change, too. The MAKEFILE_LIST special variable contains the filenames of all included makefiles and Makefile itself. Signed-off-by: Stefan Hajnoczi --- Makefile | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index b27e580af..ab54c581d 100644 --- a/Makefile +++ b/Makefile @@ -444,10 +444,10 @@ GENERATED_CONFIG = $(CLI_DIR)/config-generated.go GENERATED_GO_FILES += $(GENERATED_CONFIG) -$(GENERATED_CONFIG): Makefile VERSION +$(GENERATED_CONFIG): $(MAKEFILE_LIST) VERSION $(QUIET_GENERATE)echo "$$GENERATED_CODE" >$@ -$(TARGET_OUTPUT): $(EXTRA_DEPS) $(SOURCES) $(GENERATED_GO_FILES) $(GENERATED_FILES) Makefile | show-summary +$(TARGET_OUTPUT): $(EXTRA_DEPS) $(SOURCES) $(GENERATED_GO_FILES) $(GENERATED_FILES) $(MAKEFILE_LIST) | show-summary $(QUIET_BUILD)(cd $(CLI_DIR) && go build $(BUILDFLAGS) -o $@ .) $(SHIMV2_OUTPUT): @@ -464,12 +464,12 @@ $(SHIMV2_OUTPUT): show-summary \ show-variables -$(TARGET).coverage: $(SOURCES) $(GENERATED_FILES) Makefile +$(TARGET).coverage: $(SOURCES) $(GENERATED_FILES) $(MAKEFILE_LIST) $(QUIET_TEST)go test -o $@ -covermode count GENERATED_FILES += $(CONFIGS) -$(GENERATED_FILES): %: %.in Makefile VERSION +$(GENERATED_FILES): %: %.in $(MAKEFILE_LIST) VERSION $(QUIET_CONFIG)$(SED) \ -e "s|@COMMIT@|$(COMMIT)|g" \ -e "s|@VERSION@|$(VERSION)|g" \ From 0f7bb25cf7da481c92c2ee8f8467b5a296deeffb Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Wed, 17 Apr 2019 11:17:47 +0100 Subject: [PATCH 4/6] build: extract config-generated.go.in from Makefile Makefile had a template for cli/config-generated.go embedded inside it. There is already a templating mechanism for .in files. Using a .in file is cleaner since it avoids make's awkward interaction with shell escaping and line splitting. This patch moves the template into cli/config-generated.go.in and reuses the existing .in file templating mechanism. Only the PKGRUNDIR variable needs to be added. Signed-off-by: Stefan Hajnoczi --- Makefile | 72 +++++--------------------------------- cli/config-generated.go.in | 49 ++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 63 deletions(-) create mode 100644 cli/config-generated.go.in diff --git a/Makefile b/Makefile index ab54c581d..2137bb978 100644 --- a/Makefile +++ b/Makefile @@ -334,13 +334,12 @@ USER_VARS += BUILDFLAGS V = @ Q = $(V:1=) -QUIET_BUILD = $(Q:@=@echo ' BUILD '$@;) -QUIET_CHECK = $(Q:@=@echo ' CHECK '$@;) -QUIET_CLEAN = $(Q:@=@echo ' CLEAN '$@;) -QUIET_CONFIG = $(Q:@=@echo ' CONFIG '$@;) +QUIET_BUILD = $(Q:@=@echo ' BUILD '$@;) +QUIET_CHECK = $(Q:@=@echo ' CHECK '$@;) +QUIET_CLEAN = $(Q:@=@echo ' CLEAN '$@;) QUIET_GENERATE = $(Q:@=@echo ' GENERATE '$@;) -QUIET_INST = $(Q:@=@echo ' INSTALL '$@;) -QUIET_TEST = $(Q:@=@echo ' TEST '$@;) +QUIET_INST = $(Q:@=@echo ' INSTALL '$@;) +QUIET_TEST = $(Q:@=@echo ' TEST '$@;) # go build common flags BUILDFLAGS := -buildmode=pie @@ -369,55 +368,6 @@ runtime: $(TARGET_OUTPUT) $(CONFIGS) build: default -define GENERATED_CODE -// WARNING: This file is auto-generated - DO NOT EDIT! -// -// Note that some variables are "var" to allow them to be modified -// by the tests. -package main - -import ( - "fmt" -) - -// name is the name of the runtime -const name = "$(TARGET)" - -// name of the project -const project = "$(PROJECT_NAME)" - -// prefix used to denote non-standard CLI commands and options. -const projectPrefix = "$(PROJECT_TYPE)" - -// original URL for this project -const projectURL = "$(PROJECT_URL)" - -const defaultRootDirectory = "$(PKGRUNDIR)" - -// commit is the git commit the runtime is compiled from. -var commit = "$(COMMIT)" - -// version is the runtime version. -var version = "$(VERSION)" - -// project-specific command names -var envCmd = fmt.Sprintf("%s-env", projectPrefix) -var checkCmd = fmt.Sprintf("%s-check", projectPrefix) - -// project-specific option names -var configFilePathOption = fmt.Sprintf("%s-config", projectPrefix) -var showConfigPathsOption = fmt.Sprintf("%s-show-default-config-paths", projectPrefix) - -// Default config file used by stateless systems. -var defaultRuntimeConfiguration = "$(CONFIG_PATH)" - -// Alternate config file that takes precedence over -// defaultRuntimeConfiguration. -var defaultSysConfRuntimeConfiguration = "$(SYSCONFIG)" -endef - -export GENERATED_CODE - #Install an executable file # params: # $1 : file to install @@ -440,14 +390,9 @@ define MAKE_KERNEL_NAME $(if $(findstring uncompressed,$1),vmlinux.container,vmlinuz.container) endef -GENERATED_CONFIG = $(CLI_DIR)/config-generated.go +GENERATED_FILES += $(CLI_DIR)/config-generated.go -GENERATED_GO_FILES += $(GENERATED_CONFIG) - -$(GENERATED_CONFIG): $(MAKEFILE_LIST) VERSION - $(QUIET_GENERATE)echo "$$GENERATED_CODE" >$@ - -$(TARGET_OUTPUT): $(EXTRA_DEPS) $(SOURCES) $(GENERATED_GO_FILES) $(GENERATED_FILES) $(MAKEFILE_LIST) | show-summary +$(TARGET_OUTPUT): $(EXTRA_DEPS) $(SOURCES) $(GENERATED_FILES) $(MAKEFILE_LIST) | show-summary $(QUIET_BUILD)(cd $(CLI_DIR) && go build $(BUILDFLAGS) -o $@ .) $(SHIMV2_OUTPUT): @@ -470,7 +415,7 @@ $(TARGET).coverage: $(SOURCES) $(GENERATED_FILES) $(MAKEFILE_LIST) GENERATED_FILES += $(CONFIGS) $(GENERATED_FILES): %: %.in $(MAKEFILE_LIST) VERSION - $(QUIET_CONFIG)$(SED) \ + $(QUIET_GENERATE)$(SED) \ -e "s|@COMMIT@|$(COMMIT)|g" \ -e "s|@VERSION@|$(VERSION)|g" \ -e "s|@CONFIG_QEMU_IN@|$(CONFIG_QEMU_IN)|g" \ @@ -487,6 +432,7 @@ $(GENERATED_FILES): %: %.in $(MAKEFILE_LIST) VERSION -e "s|@KERNELPARAMS@|$(KERNELPARAMS)|g" \ -e "s|@LOCALSTATEDIR@|$(LOCALSTATEDIR)|g" \ -e "s|@PKGLIBEXECDIR@|$(PKGLIBEXECDIR)|g" \ + -e "s|@PKGRUNDIR@|$(PKGRUNDIR)|g" \ -e "s|@PROXYPATH@|$(PROXYPATH)|g" \ -e "s|@NETMONPATH@|$(NETMONPATH)|g" \ -e "s|@PROJECT_BUG_URL@|$(PROJECT_BUG_URL)|g" \ diff --git a/cli/config-generated.go.in b/cli/config-generated.go.in new file mode 100644 index 000000000..f0a9571b8 --- /dev/null +++ b/cli/config-generated.go.in @@ -0,0 +1,49 @@ +// +// Copyright (c) 2018-2019 Intel Corporation +// +// SPDX-License-Identifier: Apache-2.0 +// +// WARNING: This file is auto-generated - DO NOT EDIT! +// +// Note that some variables are "var" to allow them to be modified +// by the tests. +package main + +import ( + "fmt" +) + +// name is the name of the runtime +const name = "@RUNTIME_NAME@" + +// name of the project +const project = "@PROJECT_NAME@" + +// prefix used to denote non-standard CLI commands and options. +const projectPrefix = "@PROJECT_TYPE@" + +// original URL for this project +const projectURL = "@PROJECT_URL@" + +const defaultRootDirectory = "@PKGRUNDIR@" + +// commit is the git commit the runtime is compiled from. +var commit = "@COMMIT@" + +// version is the runtime version. +var version = "@VERSION@" + +// project-specific command names +var envCmd = fmt.Sprintf("%s-env", projectPrefix) +var checkCmd = fmt.Sprintf("%s-check", projectPrefix) + +// project-specific option names +var configFilePathOption = fmt.Sprintf("%s-config", projectPrefix) +var showConfigPathsOption = fmt.Sprintf("%s-show-default-config-paths", projectPrefix) + +// Default config file used by stateless systems. +var defaultRuntimeConfiguration = "@CONFIG_PATH@" + +// Alternate config file that takes precedence over +// defaultRuntimeConfiguration. +var defaultSysConfRuntimeConfiguration = "@SYSCONFIG@" From 7949cd6ebce3cf985e283cff28db56b4eebac5c8 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Wed, 17 Apr 2019 11:21:29 +0100 Subject: [PATCH 5/6] build: turn COMMIT into a file dependency Makefile uses $(shell) to build a git commit string. Unfortunately this means make targets cannot be rebuilt when COMMIT changes value. We need to reflect this string value into files on which make can process dependencies. I stole a solution from QEMU's Makefile: 1. Print the string into .git-commit.tmp 2. If .git-commit.tmp differs from .git-commit, copy it to .git-commit 3. Depend on .git-commit from all targets that need $COMMIT This way targets are only rebuilt if the commit string value actually changes. Signed-off-by: Stefan Hajnoczi --- .gitignore | 2 ++ Makefile | 22 +++++++++++++++------- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/.gitignore b/.gitignore index 7b6eee072..79ceb2387 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,8 @@ *.o *.patch *.swp +.git-commit +.git-commit.tmp /cli/config/configuration-fc.toml /cli/config/configuration-qemu.toml /cli/config-generated.go diff --git a/Makefile b/Makefile index 2137bb978..e68912f6f 100644 --- a/Makefile +++ b/Makefile @@ -182,8 +182,16 @@ SHIMV2_DIR = $(CLI_DIR)/$(SHIMV2) SOURCES := $(shell find . 2>&1 | grep -E '.*\.(c|h|go)$$') VERSION := ${shell cat ./VERSION} -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}) + +# Targets that depend on .git-commit can use $(shell cat .git-commit) to get a +# git revision string. They will only be rebuilt if the revision string +# actually changes. +.PHONY: .git-commit.tmp +.git-commit: .git-commit.tmp + @cmp $< $@ >/dev/null 2>&1 || cp $< $@ +.git-commit.tmp: + @echo -n "$$(git rev-parse HEAD 2>/dev/null)" >$@ + @test -n "$$(git status --porcelain --untracked-files=no)" && echo -n "-dirty" >>$@ || true # List of configuration files to build and install CONFIGS = @@ -414,9 +422,9 @@ $(TARGET).coverage: $(SOURCES) $(GENERATED_FILES) $(MAKEFILE_LIST) GENERATED_FILES += $(CONFIGS) -$(GENERATED_FILES): %: %.in $(MAKEFILE_LIST) VERSION +$(GENERATED_FILES): %: %.in $(MAKEFILE_LIST) VERSION .git-commit $(QUIET_GENERATE)$(SED) \ - -e "s|@COMMIT@|$(COMMIT)|g" \ + -e "s|@COMMIT@|$(shell cat .git-commit)|g" \ -e "s|@VERSION@|$(VERSION)|g" \ -e "s|@CONFIG_QEMU_IN@|$(CONFIG_QEMU_IN)|g" \ -e "s|@CONFIG_FC_IN@|$(CONFIG_FC_IN)|g" \ @@ -510,7 +518,7 @@ install-completions: $(QUIET_INST)install --mode 0644 -D $(BASH_COMPLETIONS) $(DESTDIR)/$(BASH_COMPLETIONSDIR)/$(notdir $(BASH_COMPLETIONS)); clean: - $(QUIET_CLEAN)rm -f $(TARGET) $(SHIMV2) $(NETMON_TARGET) $(CONFIGS) $(GENERATED_GO_FILES) $(GENERATED_FILES) + $(QUIET_CLEAN)rm -f $(TARGET) $(SHIMV2) $(NETMON_TARGET) $(CONFIGS) $(GENERATED_FILES) .git-commit .git-commit.tmp show-usage: show-header @printf "• Overview:\n" @@ -549,8 +557,8 @@ show-variables: "$(foreach v,$(sort $(USER_VARS)),$(shell printf "\\t$(v)='$($(v))'\\\n"))" @printf "\n" -show-header: - @printf "%s - version %s (commit %s)\n\n" $(TARGET) $(VERSION) $(COMMIT) +show-header: .git-commit + @printf "%s - version %s (commit %s)\n\n" $(TARGET) $(VERSION) $(shell cat .git-commit) show-arches: show-header @printf "Supported architectures (possible values for ARCH variable):\n\n" From 53ebe51f1c8f346869894e227a8f182511b51fb0 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Tue, 16 Apr 2019 15:44:45 +0100 Subject: [PATCH 6/6] build: fix race between 'clean' and generated files When a parallel build is invoked using "make -j4" there is a race between EXTRA_DEPS ('clean') and generating files: CPU1 CPU2 ---- ---- create cli/generated-config.go rm cli/generated-config.go go build -> error: generated-config.go doesn't exist! Previous commits ensured that targets relying on version information like VERSION and COMMIT declare appropriate dependencies. Therefore make is now able to detect changes and rebuild targets as needed. It is no longer necessary to abuse the clean target to force a rebuild. Fixes: #1540 Signed-off-by: Stefan Hajnoczi --- Makefile | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/Makefile b/Makefile index e68912f6f..32b4f1d3c 100644 --- a/Makefile +++ b/Makefile @@ -69,8 +69,6 @@ BINLIBEXECLIST += $(NETMON_TARGET) DESTDIR := / -installing = $(findstring install,$(MAKECMDGOALS)) - ifeq ($(PREFIX),) PREFIX := /usr EXEC_PREFIX := $(PREFIX)/local @@ -85,12 +83,6 @@ FCBINDIR := $(PREFIXDEPS)/bin SYSCONFDIR := /etc LOCALSTATEDIR := /var -ifeq (,$(installing)) - # Force a rebuild to ensure version details are correct - # (but only for a non-install build phase). - EXTRA_DEPS = clean -endif - LIBEXECDIR := $(PREFIXDEPS)/libexec SHAREDIR := $(PREFIX)/share DEFAULTSDIR := $(SHAREDIR)/defaults @@ -400,7 +392,7 @@ endef GENERATED_FILES += $(CLI_DIR)/config-generated.go -$(TARGET_OUTPUT): $(EXTRA_DEPS) $(SOURCES) $(GENERATED_FILES) $(MAKEFILE_LIST) | show-summary +$(TARGET_OUTPUT): $(SOURCES) $(GENERATED_FILES) $(MAKEFILE_LIST) | show-summary $(QUIET_BUILD)(cd $(CLI_DIR) && go build $(BUILDFLAGS) -o $@ .) $(SHIMV2_OUTPUT):