Commits on Aug 17, 2018

1. 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, kubernetes-retired#45).
   * release, since the target landed in 04848d1 (Add release build
     process, 2016-05-27, kubernetes-retired#47).
   
   clean-vm-% target has been a prerequisite for run-% since the target
   landed in 9e2e89e (run make target, 2016-09-23, kubernetes-retired#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, kubernetes-retired#46).  And 'clean' had been a 'release'
     prerequisite since 04848d1 (Add release build process, 2016-05-27,
     kubernetes-retired#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.

   

   wking committed Aug 17, 2018
   Configuration menu

   • View commit details
   • Copy full SHA for 15ef313
   • Browse repository at this point

   Copy the full SHA

   15ef313 View commit details

   Browse the repository at this point in the history