From ab703e0406303d488543b9e32d15494defd5d095 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Tue, 19 Jul 2016 16:42:28 -0700 Subject: [PATCH 1/5] Make 'make clean all' work In this case, the 'clean' step would nuke the metadata files, but they have already been read, so in-memory state is fine. This triggered a couple of pathological conditions that would not normally be hit. This commit fills in those nodes in the DAG, even though they are not directly needed in most builds. Also fix some whitespace for readability. --- Makefile.generated_files | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/Makefile.generated_files b/Makefile.generated_files index 1e4a83c9137..c1c6a4472cb 100644 --- a/Makefile.generated_files +++ b/Makefile.generated_files @@ -139,12 +139,18 @@ $(foreach dir, $(ALL_GO_DIRS), $(eval \ # We regenerate the output file in order to satisfy make's "newer than" rules, # but we only need to rebuild targets if the contents actually changed. That # is what the .stamp file represents. -$(foreach dir, $(ALL_GO_DIRS), $(META_DIR)/$(dir)/$(GOFILES_META)): +$(foreach dir, $(ALL_GO_DIRS), \ + $(META_DIR)/$(dir)/$(GOFILES_META)): FILES=$$(ls $$@.tmp; \ cmp -s $@.tmp $@ || touch $@.stamp; \ mv $@.tmp $@ +# This is required to fill in the DAG, since some cases (e.g. 'make clean all') +# will reference the .stamp file when it doesn't exist. We don't need to +# rebuild it in that case, just keep make happy. +$(foreach dir, $(ALL_GO_DIRS), \ + $(META_DIR)/$(dir)/$(GOFILES_META).stamp): # Include any deps files as additional Makefile rules. This triggers make to # consider the deps files for rebuild, which makes the whole @@ -219,7 +225,7 @@ gen_deepcopy: $(DEEPCOPY_FILES) # has changed. This allows us to detect deleted input files. $(foreach dir, $(DEEPCOPY_DIRS), $(eval \ $(dir)/$(DEEPCOPY_FILENAME): $(META_DIR)/$(dir)/$(GOFILES_META).stamp \ - $(gofiles__$(dir)) \ + $(gofiles__$(dir)) \ )) # Unilaterally remove any leftovers from previous runs. @@ -228,6 +234,7 @@ $(shell rm -f $(META_DIR)/$(DEEPCOPY_GEN)*.todo) # How to regenerate deep-copy code. This is a little slow to run, so we batch # it up and trigger the batch from the 'generated_files' target. $(DEEPCOPY_FILES): $(DEEPCOPY_GEN) + mkdir -p $$(dirname $(META_DIR)/$(DEEPCOPY_GEN)) echo $(PRJ_SRC_PATH)/$(@D) >> $(META_DIR)/$(DEEPCOPY_GEN).todo # This calculates the dependencies for the generator tool, so we only rebuild @@ -341,8 +348,9 @@ $(foreach dir, $(CONVERSION_DIRS), $(eval \ # We regenerate the output file in order to satisfy make's "newer than" rules, # but we only need to rebuild targets if the contents actually changed. That # is what the .stamp file represents. -$(foreach dir, $(CONVERSION_DIRS), $(META_DIR)/$(dir)/$(CONVERSIONS_META)): - TAGS=$$(grep --color=never -h '^// *+k8s:conversion-gen=' $> $(META_DIR)/$(CONVERSION_GEN).todo # This calculates the dependencies for the generator tool, so we only rebuild From 8fb8f3d41b6a0c97f8b76bb11b062a9a4fde2e52 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Tue, 19 Jul 2016 17:24:51 -0700 Subject: [PATCH 2/5] Cleanup unneeded deps on generated_files --- Makefile | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 9711c5d54bc..76c38d459df 100644 --- a/Makefile +++ b/Makefile @@ -90,7 +90,7 @@ ginkgo: # Runs all the presubmission verifications. # # Args: -# BRANCH: Branch to be passed to hack/verify-godeps.sh script. +# BRANCH: Branch to be passed to verify-godeps.sh script. # # Example: # make verify @@ -174,7 +174,7 @@ test-e2e-node: ginkgo generated_files # # Example: # make test-cmd -test-cmd: +test-cmd: generated_files @hack/make-rules/test-cmd.sh .PHONY: test-cmd @@ -217,7 +217,7 @@ vet: # Example: # make release .PHONY: release -release: generated_files +release: build/release.sh # Build a release, but skip tests @@ -225,7 +225,7 @@ release: generated_files # Example: # make release-skip-tests .PHONY: release-skip-tests quick-release -release-skip-tests quick-release: generated_files +release-skip-tests quick-release: KUBE_RELEASE_RUN_TESTS=n KUBE_FASTBUILD=true build/release.sh # Cross-compile for all platforms From 5930c57378d9eb0d3f80552250b986a3aa4544d3 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Tue, 19 Jul 2016 20:13:01 -0700 Subject: [PATCH 3/5] Remove Makefile undefined variables logic It didn't do what I thought it did and was just noise. Specifically, it only kicks in for recursive `make` invocations. --- Makefile | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/Makefile b/Makefile index 76c38d459df..d9c9aa75e07 100644 --- a/Makefile +++ b/Makefile @@ -36,9 +36,6 @@ SHELL := /bin/bash MAKEFLAGS += --no-builtin-rules .SUFFIXES: -# We want make to yell at us if we use undefined variables. -MAKEFLAGS += --warn-undefined-variables - # Constants used throughout. OUT_DIR ?= _output BIN_DIR := $(OUT_DIR)/bin @@ -47,20 +44,9 @@ PRJ_SRC_PATH := k8s.io/kubernetes # Metadata for driving the build lives here. META_DIR := .make -# -# Define variables that we use as inputs so we can warn about undefined variables. -# +export KUBE_GOFLAGS := $(GOFLAGS) -WHAT ?= -TESTS ?= - -GOFLAGS ?= -KUBE_GOFLAGS = $(GOFLAGS) -export KUBE_GOFLAGS GOFLAGS - -GOLDFLAGS ?= -KUBE_GOLDFLAGS = $(GOLDFLAGS) -export KUBE_GOLDFLAGS GOLDFLAGS +export KUBE_GOLDFLAGS := $(GOLDFLAGS) # Build code. # From 5315fd6786c32d39378961e1fcebd15eeb451304 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Tue, 19 Jul 2016 20:47:36 -0700 Subject: [PATCH 4/5] Make 'make clean' not depend on genfiles metadata Previously even 'make clean' would first produce dependency information (slow) and sinclude all the metadata, possibly restarting the make, before removing the things it just did. Now dependencies are only processed when something explicitly depends on them, by calling 'make' on the rules around generated files, rather than sincluding them. Net result: make clean is faster and safer. Additionally, bash tab-completion for 'make' does not run the slow 'find' any more. --- Makefile | 28 +++++++++++++++++++++------- Makefile.generated_files | 34 ++++++++++++++++++---------------- 2 files changed, 39 insertions(+), 23 deletions(-) diff --git a/Makefile b/Makefile index d9c9aa75e07..5dd0600eff4 100644 --- a/Makefile +++ b/Makefile @@ -14,7 +14,7 @@ DBG_MAKEFILE ?= ifeq ($(DBG_MAKEFILE),1) - $(warning ***** starting makefile for goal(s) "$(MAKECMDGOALS)") + $(warning ***** starting Makefile for goal(s) "$(MAKECMDGOALS)") $(warning ***** $(shell date)) else # If we're not debugging the Makefile, don't echo recipes. @@ -29,7 +29,7 @@ endif # test: Run tests. # clean: Clean up. -# It's necessary to set this because some docker images don't make sh -> bash. +# It's necessary to set this because some environments don't link sh -> bash. SHELL := /bin/bash # We don't need make's built-in rules. @@ -37,16 +37,17 @@ MAKEFLAGS += --no-builtin-rules .SUFFIXES: # Constants used throughout. +.EXPORT_ALL_VARIABLES: OUT_DIR ?= _output BIN_DIR := $(OUT_DIR)/bin PRJ_SRC_PATH := k8s.io/kubernetes +GENERATED_FILE_PREFIX := zz_generated. # Metadata for driving the build lives here. META_DIR := .make -export KUBE_GOFLAGS := $(GOFLAGS) - -export KUBE_GOLDFLAGS := $(GOLDFLAGS) +KUBE_GOFLAGS := $(GOFLAGS) +KUBE_GOLDFLAGS := $(GOLDFLAGS) # Build code. # @@ -184,6 +185,14 @@ clean: clean_meta clean_meta: rm -rf $(META_DIR) +# Remove all auto-generated artifacts. +# +# Example: +# make clean_generated +.PHONY: clean_generated +clean_generated: + find . -type f -name $(GENERATED_FILE_PREFIX)\* | xargs rm -f + # Run 'go vet'. # # Args: @@ -230,5 +239,10 @@ cross: $(notdir $(abspath $(wildcard cmd/*/))): generated_files hack/make-rules/build.sh cmd/$@ -# Include logic for generated files. -include Makefile.generated_files +# Produce auto-generated files needed for the build. +# +# Example: +# make generated_files +.PHONY: generated_files +generated_files: + $(MAKE) -f Makefile.$@ $@ diff --git a/Makefile.generated_files b/Makefile.generated_files index c1c6a4472cb..f140b52745c 100644 --- a/Makefile.generated_files +++ b/Makefile.generated_files @@ -12,8 +12,24 @@ # See the License for the specific language governing permissions and # limitations under the License. -# Constants used throughout. -GENERATED_FILE_PREFIX := zz_generated. +# Don't allow an implicit 'all' rule. This is not a user-facing file. +ifeq ($(MAKECMDGOALS),) + $(error This Makefile requires an explicit rule to be specified) +endif + +ifeq ($(DBG_MAKEFILE),1) + $(warning ***** starting Makefile.generated_files for goal(s) "$(MAKECMDGOALS)") + $(warning ***** $(shell date)) +endif + + +# It's necessary to set this because some environments don't link sh -> bash. +SHELL := /bin/bash + +# This rule collects all the generated file sets into a single rule. Other +# rules should depend on this to ensure generated files are rebuilt. +.PHONY: generated_files +generated_files: gen_deepcopy gen_conversion # Code-generation logic. # @@ -455,17 +471,3 @@ sinclude $(META_DIR)/$(CONVERSION_GEN).mk $(CONVERSION_GEN): hack/make-rules/build.sh cmd/libs/go2idl/conversion-gen touch $@ - -# This rule collects all the generated file sets into a single dep, which is -# defined BELOW the *_FILES variables and leaves higher-level rules clean. -# Top-level rules should depend on this to ensure generated files are rebuilt. -.PHONY: generated_files -generated_files: gen_deepcopy gen_conversion - -# Remove all auto-generated artifacts. -# -# Example: -# make clean_generated -.PHONY: clean_generated -clean_generated: - find . -type f -name $(GENERATED_FILE_PREFIX)\* | xargs rm -f From 665966746912ea28b5511f55b75540187a4ef907 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Wed, 20 Jul 2016 13:17:08 -0700 Subject: [PATCH 5/5] Make builds faster by caching ALL_GO_DIRS This operation takes 2-5 seconds on every build, but doesn't actually need to run most of the time. Now we cache it and see if it needs a rebuild (fast) before actually rebuilding (slow). --- Makefile.generated_files | 21 ++------ hack/make-rules/helpers/cache_go_dirs.sh | 65 ++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 17 deletions(-) create mode 100755 hack/make-rules/helpers/cache_go_dirs.sh diff --git a/Makefile.generated_files b/Makefile.generated_files index f140b52745c..fe21e394ff1 100644 --- a/Makefile.generated_files +++ b/Makefile.generated_files @@ -108,24 +108,11 @@ generated_files: gen_deepcopy gen_conversion ifeq ($(DBG_MAKEFILE),1) $(warning ***** finding all *.go dirs) endif -ALL_GO_DIRS := $(shell \ - find . \ - -not \( \ - \( \ - -path ./vendor -o \ - -path ./_\* -o \ - -path ./.\* -o \ - -path ./docs -o \ - -path ./examples \ - \) -prune \ - \) \ - -type f -name \*.go \ - | sed 's|^./||' \ - | xargs -n1 dirname \ - | sort -u \ +ALL_GO_DIRS := $(shell \ + hack/make-rules/helpers/cache_go_dirs.sh $(META_DIR)/all_go_dirs.mk \ ) -# The name of the make metadata file listing Go files. +# The name of the metadata file which lists *.go files in each pkg. GOFILES_META := gofiles.mk # Establish a dependency between the deps file and the dir. Whenever a dir @@ -314,7 +301,7 @@ CONVERSION_FILENAME := $(CONVERSION_BASENAME).go # The tool used to generate conversions. CONVERSION_GEN := $(BIN_DIR)/conversion-gen -# The name of the make metadata file controlling conversions. +# The name of the metadata file listing conversion peers for each pkg. CONVERSIONS_META := conversions.mk # All directories that request any form of conversion generation. diff --git a/hack/make-rules/helpers/cache_go_dirs.sh b/hack/make-rules/helpers/cache_go_dirs.sh new file mode 100755 index 00000000000..133a2d22fa6 --- /dev/null +++ b/hack/make-rules/helpers/cache_go_dirs.sh @@ -0,0 +1,65 @@ +#!/bin/bash + +# Copyright 2014 The Kubernetes Authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# This script finds, caches, and prints a list of all directories that hold +# *.go files. If any directory is newer than the cache, re-find everything and +# update the cache. Otherwise use the cached file. + +set -o errexit +set -o nounset +set -o pipefail + +if [[ -z "${1:-}" ]]; then + echo "usage: $0 " + exit 1 +fi +CACHE="$1"; shift + +# This is a partial 'find' command. The caller is expected to pass the +# remaining arguments. +# +# Example: +# kfind -type f -name foobar.go +function kfind() { + find . \ + -not \( \ + \( \ + -path ./vendor -o \ + -path ./_\* -o \ + -path ./.\* -o \ + -path ./docs -o \ + -path ./examples \ + \) -prune \ + \) \ + "$@" +} + +NEED_FIND=true +# It's *significantly* faster to check whether any directories are newer than +# the cache than to blindly rebuild it. +if [[ -f "${CACHE}" ]]; then + N=$(kfind -type d -newer "${CACHE}" -print -quit | wc -l) + [[ "${N}" == 0 ]] && NEED_FIND=false +fi +mkdir -p $(dirname "${CACHE}") +if $("${NEED_FIND}"); then + kfind -type f -name \*.go \ + | xargs -n1 dirname \ + | sort -u \ + | sed 's|^./||' \ + > "${CACHE}" +fi +cat "${CACHE}"