Skip to content
This repository was archived by the owner on Jul 30, 2021. It is now read-only.

Commit 35a86cf

Browse files
committed
Makefile: Remove 'clean' prerequisite from build targets
'clean' has been a prerequisite for: * conformance-%, since the target landed in 0520bdf (hack: add conformance tests for hack builds, 2016-05-26, #45). * release, since the target landed in 04848d1 (Add release build process, 2016-05-27, #47). clean-vm-% target has been a prerequisite for run-% since the target landed in 9e2e89e (run make target, 2016-09-23, #141). But there are two problems with including cleaners are prerequisites of build steps: * They are inefficient. Make is good at rebuilding only things that need rebuilding, provided you inform it of your dependencies. Cleaning before every build will often lead to unnecessary rebuilds of up-to-date content. * They are racy. For example, a parallel make invocation may process several prerequisites at once. The current 'clean' recipe will execute quickly, which is why it hasn't been much of a problem yet, but you can see the race by inserting a delay: clean: sleep 60 rm -rf _output and running: $ make -j8 release sleep 60 mkdir -p _output/bin/darwin/ GOOS=darwin GOARCH=amd64 go build ... ... mkdir -p _output/release/ tar czf _output/release/bootkube.tar.gz -C _output bin/linux/bootkube bin/darwin/bootkube bin/linux/checkpoint rm -rf _output with the other preqrequisites completing first, clean's rm ends up removing everything release generated. In this commit, I've removed the clean prerequisites from the build targets to resolve the above issues. We could also update our recommended make invocations to add clean as an explicit earlier step, and I've done that for 'all' and 'run-' to hint at the existence of the clean targets. I don't think we need separate clean calls for 'release' or 'conformance-%' * No need to 'make clean' before calling 'make release' For Travis, the test environment should not include anything in _output at this point, so there's no need to call clean there. The 'clean' target has been included in .travis.yml since 1f34600 (Add travis tests, 2016-05-27, #46). And 'clean' had been a 'release' prerequisite since 04848d1 (Add release build process, 2016-05-27, #47). But neither of those motivate the Travis use-case, so I think they were just being overly cautious. And for RELEASING, walking the prerequisite chain for release shows that a clean all would have no impact: * check is verifying the source, and does not care about _output. * _output/release/bootkube.tar.gz knows about its dependencies. * _output/bin/% (used for _output/bin/.../bootkube) knows about its GOFILES and (new in this commit) VENDOR_GOFILES dependencies. This is *overly* broad, because it includes cmd/checkpoint/*.go and other non-dependencies, but overly broad is still safe. * The new VENDOR_GOFILES ensures we rebuild _output/bin/... if a vendored dependency is bumped even if no bootkube-specific code is bumped. * No need to 'make clean' before calling 'make conformance-...' The hack/ content only depends on _output for _output/bin/${local_os}/bootkube and _output/bin/linux/bootkube. conformance-% only uses hack/ content, and it depends on all. 'all', in turn, depends on both _output/bin/$(LOCAL_OS)/bootkube and _output/bin/linux/bootkube, and each of those fully specifies their build dependencies. So there's no need for a separate clean step before running the conformance-% targets; Make will rebuild anything that needs rebuilding already.
1 parent bff16e2 commit 35a86cf

File tree

3 files changed

+12
-9
lines changed

3 files changed

+12
-9
lines changed

.travis.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,4 @@ go:
77
- 1.9.x
88
- 1.10.x
99
script:
10-
- make clean release
10+
- make release

Documentation/development.md

+7-4
Original file line numberDiff line numberDiff line change
@@ -17,20 +17,23 @@ cd $GOPATH/src/github.com/kubernetes-incubator/bootkube
1717
Then, to build (only Go verson 1.8 is supported now):
1818

1919
```
20-
make clean all
20+
make clean
21+
make all
2122
```
2223

2324
## Local Development Environments
2425

25-
To easily launch local vagrant development clusters:
26+
To easily launch local, single-node vagrant development clusters:
2627

2728
```
28-
# Launch a single-node cluster
29+
make clean-vm-single
2930
make run-single
3031
```
3132

33+
You can also launch a multi-node cluster:
34+
3235
```
33-
# Launch a multi-node cluster
36+
make clean-vm-multi
3437
make run-multi
3538
```
3639

Makefile

+4-4
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ export PATH:=$(PATH):$(PWD)
55
SHELL:=$(shell which bash)
66
LOCAL_OS:=$(shell uname | tr A-Z a-z)
77
GOFILES:=$(shell find . -name '*.go' | grep -v -E '(./vendor)')
8+
VENDOR_GOFILES ?= $(shell find vendor -name '*.go')
89
GOPATH_BIN:=$(shell echo ${GOPATH} | awk 'BEGIN { FS = ":" }; { print $1 }')/bin
910
LDFLAGS=-X github.com/kubernetes-incubator/bootkube/pkg/version.Version=$(shell $(CURDIR)/build/git-version.sh)
1011
TERRAFORM:=$(shell command -v terraform 2> /dev/null)
@@ -25,7 +26,6 @@ cross: \
2526
_output/bin/linux/s390x/checkpoint
2627

2728
release: \
28-
clean \
2929
check \
3030
_output/release/bootkube.tar.gz \
3131

@@ -46,7 +46,7 @@ install: _output/bin/$(LOCAL_OS)/bootkube
4646
_output/bin/%: GOOS=$(word 1, $(subst /, ,$*))
4747
_output/bin/%: GOARCH=$(word 2, $(subst /, ,$*))
4848
_output/bin/%: GOARCH:=amd64 # default to amd64 to support release scripts
49-
_output/bin/%: $(GOFILES)
49+
_output/bin/%: $(GOFILES) $(VENDOR_GOFILES)
5050
mkdir -p $(dir $@)
5151
GOOS=$(GOOS) GOARCH=$(GOARCH) go build $(GOFLAGS) -ldflags "$(LDFLAGS)" -o $@ github.com/kubernetes-incubator/bootkube/cmd/$(notdir $@)
5252

@@ -55,7 +55,7 @@ _output/release/bootkube.tar.gz: _output/bin/linux/bootkube _output/bin/darwin/b
5555
tar czf $@ -C _output bin/linux/bootkube bin/darwin/bootkube bin/linux/checkpoint
5656

5757
run-%: GOFLAGS = -i
58-
run-%: clean-vm-% _output/bin/linux/bootkube _output/bin/$(LOCAL_OS)/bootkube
58+
run-%: _output/bin/linux/bootkube _output/bin/$(LOCAL_OS)/bootkube
5959
@cd hack/$*-node && ./bootkube-up
6060
@echo "Bootkube ready"
6161

@@ -67,7 +67,7 @@ clean-vm-%:
6767
rm -rf cluster )
6868

6969
#TODO(aaron): Prompt because this is destructive
70-
conformance-%: clean all
70+
conformance-%: all
7171
@cd hack/$*-node && vagrant destroy -f
7272
@cd hack/$*-node && rm -rf cluster
7373
@cd hack/$*-node && ./bootkube-up

0 commit comments

Comments
 (0)