Merge lp:~mbp/launchpad/mbp-trivial into lp:launchpad

Proposed by Martin Pool
Status: Merged
Approved by: Jeroen T. Vermeulen
Approved revision: no longer in the source branch.
Merged at revision: 11957
Proposed branch: lp:~mbp/launchpad/mbp-trivial
Merge into: lp:launchpad
Diff against target: 144 lines (+35/-20)
1 file modified
Makefile (+35/-20)
To merge this branch: bzr merge lp:~mbp/launchpad/mbp-trivial
Reviewer Review Type Date Requested Status
Jeroen T. Vermeulen (community) code Approve
Launchpad code reviewers Pending
Review via email: mp+41418@code.launchpad.net

This proposal supersedes a proposal from 2010-11-19.

Commit message

[r=jtv][ui=none][bug=615740][no-qa] Avoid unnecessary rebuilds of buildout_bin.

Description of the change

This tweaks Jeroen and Benji's recent work in "Faster builds" so that the makefile dependencies are more precise, to avoid unnecessary rebuilds.

To post a comment you must log in.
Revision history for this message
Jeroen T. Vermeulen (jtv) wrote : Posted in a previous version of this proposal

The Makefile changes look good—and thanks again for making them!—but I think you accidentally submitted this for db-devel (after branching off devel) so the diff contains a lot of unrelated material.

review: Needs Resubmitting (code)
Revision history for this message
Martin Pool (mbp) wrote :

If you're happy with this, would you be so kind as to sponsor landing/qa/etc?

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

Oh look at that—my vote from the original MP is now also displayed here for reference!

It occurs to me that this version of the makefile doesn't need buildout_bin at all. I just added that target to channel all dependencies on items in $(BUILDOUT_BIN) through the same rule. You effectively replaced buildout_bin with $(PY).

But we can deal with that in a separate branch. There's no need to weigh this one down with it. I'll send your branch into EC2.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'Makefile'
2--- Makefile 2010-11-18 04:48:10 +0000
3+++ Makefile 2010-11-21 22:03:08 +0000
4@@ -34,6 +34,9 @@
5 # Do not add bin/buildout to this list.
6 # It is impossible to get buildout to tell us all the files it would
7 # build, since each egg's setup.py doesn't tell us that information.
8+#
9+# NB: It's important BUILDOUT_BIN only mentions things genuinely produced by
10+# buildout.
11 BUILDOUT_BIN = \
12 $(PY) bin/apiindex bin/combine-css bin/fl-build-report \
13 bin/fl-credential-ctl bin/fl-install-demo bin/fl-monitor-ctl \
14@@ -57,10 +60,10 @@
15 newsampledata:
16 $(MAKE) -C database/schema newsampledata
17
18-hosted_branches: buildout_bin
19+hosted_branches: $(PY)
20 $(PY) ./utilities/make-dummy-hosted-branches
21
22-$(API_INDEX): $(BZR_VERSION_INFO) buildout_bin
23+$(API_INDEX): $(BZR_VERSION_INFO) $(PY)
24 mkdir -p $(APIDOC_DIR).tmp
25 LPCONFIG=$(LPCONFIG) $(PY) ./utilities/create-lp-wadl-and-apidoc.py --force "$(WADL_TEMPLATE)"
26 mv $(APIDOC_DIR).tmp $(APIDOC_DIR)
27@@ -68,12 +71,12 @@
28 apidoc: compile $(API_INDEX)
29
30 # Run by PQM.
31-check_merge: buildout_bin
32+check_merge: $(BUILDOUT_BIN)
33 [ `PYTHONPATH= bzr status -S database/schema/ | \
34 grep -v "\(^P\|pending\|security.cfg\|Makefile\|unautovacuumable\|_pythonpath.py\)" | wc -l` -eq 0 ]
35 ${PY} lib/canonical/tests/test_no_conflict_marker.py
36
37-check_db_merge: buildout_bin
38+check_db_merge: $(PY)
39 ${PY} lib/canonical/tests/test_no_conflict_marker.py
40
41 check_config: build
42@@ -111,16 +114,16 @@
43 ${PY} -t ./test_on_merge.py $(VERBOSITY) $(TESTOPTS) \
44 --layer=MailmanLayer
45
46-lint: buildout_bin
47+lint: ${PY}
48 @bash ./bin/lint.sh
49
50-lint-verbose: buildout_bin
51+lint-verbose: ${PY}
52 @bash ./bin/lint.sh -v
53
54-xxxreport: buildout_bin
55+xxxreport: $(PY)
56 ${PY} -t ./utilities/xxxreport.py -f csv -o xxx-report.csv ./
57
58-check-configs: buildout_bin
59+check-configs: $(PY)
60 ${PY} utilities/check-configs.py
61
62 pagetests: build
63@@ -141,15 +144,15 @@
64 sprite_image:
65 ${SHHH} bin/sprite-util create-image
66
67+# We absolutely do not want to include the lazr.testing module and
68+# its jsTestDriver test harness modifications in the lazr.js and
69+# launchpad.js roll-up files. They fiddle with built-in functions!
70+# See Bug 482340.
71 jsbuild_lazr: bin/jsbuild
72- # We absolutely do not want to include the lazr.testing module and
73- # its jsTestDriver test harness modifications in the lazr.js and
74- # launchpad.js roll-up files. They fiddle with built-in functions!
75- # See Bug 482340.
76 ${SHHH} bin/jsbuild $(JSFLAGS) -b $(LAZR_BUILT_JS_ROOT) -x testing/ \
77 -c $(LAZR_BUILT_JS_ROOT)/yui
78
79-jsbuild: jsbuild_lazr bin/jsbuild bin/jssize buildout_bin
80+jsbuild: jsbuild_lazr bin/jsbuild bin/jssize $(BUILDOUT_BIN)
81 ${SHHH} bin/jsbuild \
82 $(JSFLAGS) \
83 -n launchpad \
84@@ -177,7 +180,7 @@
85 @exit 1
86 endif
87
88-buildonce_eggs: buildout_bin
89+buildonce_eggs: $(PY)
90 find eggs -name '*.pyc' -exec rm {} \;
91
92 # The download-cache dependency comes *before* eggs so that developers get the
93@@ -185,33 +188,45 @@
94 # directory is only there for deployment convenience.
95 # Note that the buildout version must be maintained here and in versions.cfg
96 # to make sure that the build does not go over the network.
97+#
98+# buildout won't touch files that would have the same contents, but for Make's
99+# sake we need them to get fresh timestamps, so we touch them after building.
100 bin/buildout: download-cache eggs
101 $(SHHH) PYTHONPATH= $(PYTHON) bootstrap.py\
102 --setup-source=ez_setup.py \
103 --download-base=download-cache/dist --eggs=eggs \
104 --version=1.5.1
105+ touch --no-create $@
106
107 # This target is used by LOSAs to prepare a build to be pushed out to
108 # destination machines. We only want eggs: they are the expensive bits,
109 # and the other bits might run into problems like bug 575037. This
110 # target runs buildout, and then removes everything created except for
111 # the eggs.
112-build_eggs: buildout_bin clean_buildout
113-
114-$(BUILDOUT_BIN): buildout_bin
115+build_eggs: $(BUILDOUT_BIN)
116
117 # This builds bin/py and all the other bin files except bin/buildout.
118 # Remove the target before calling buildout to ensure that buildout
119 # updates the timestamp.
120-buildout_bin: bin/buildout versions.cfg $(BUILDOUT_CFG) setup.py \
121+buildout_bin: $(BUILDOUT_BIN)
122+
123+# buildout won't touch files that would have the same contents, but for Make's
124+# sake we need them to get fresh timestamps, so we touch them after building.
125+#
126+# If we listed every target on the left-hand side, a parallel make would try
127+# multiple copies of this rule to build them all. Instead, we nominally build
128+# just $(PY), and everything else is implicitly updated by that.
129+$(PY): bin/buildout versions.cfg $(BUILDOUT_CFG) setup.py \
130 $(BUILDOUT_TEMPLATES)
131- $(RM) $@
132 $(SHHH) PYTHONPATH= ./bin/buildout \
133 configuration:instance_name=${LPCONFIG} -c $(BUILDOUT_CFG)
134+ touch $@
135+
136+$(subst $(PY),,$(BUILDOUT_BIN)): $(PY)
137
138 # bin/compile_templates is responsible for building all chameleon templates,
139 # of which there is currently one, but of which many more are coming.
140-compile: buildout_bin $(BZR_VERSION_INFO)
141+compile: $(PY) $(BZR_VERSION_INFO)
142 mkdir -p /var/tmp/vostok-archive
143 ${SHHH} $(MAKE) -C sourcecode build PYTHON=${PYTHON} \
144 LPCONFIG=${LPCONFIG}