← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] ~cjwatson/launchpad:always-update-version-info into launchpad:master

 

Colin Watson has proposed merging ~cjwatson/launchpad:always-update-version-info into launchpad:master.

Commit message:
Update version-info.py more carefully

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1862777 in Launchpad itself: "version-info.py isn't reliably updated on deployments"
  https://bugs.launchpad.net/launchpad/+bug/1862777

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/378908

"make compile" only ran scripts/update-version-info.sh by way of a make prerequisite, which often led to version-info.py being incorrect in development environments.  On production, it could be incorrect when trying to deploy a commit that wasn't the tip of the stable branch: we'd do a full build, then "git checkout" the desired commit, but then version-info.py would be left at the wrong commit, possibly causing failed combo requests for JS and CSS.

Running scripts/update-version-info.sh directly in "make compile" avoids these problems.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:always-update-version-info into launchpad:master.
diff --git a/Makefile b/Makefile
index af9b066..aacda12 100644
--- a/Makefile
+++ b/Makefile
@@ -259,12 +259,15 @@ $(PY): download-cache constraints.txt setup.py
 
 $(subst $(PY),,$(PIP_BIN)): $(PY)
 
-compile: $(PY) $(VERSION_INFO)
+# Explicitly update version-info.py rather than declaring $(VERSION_INFO) as
+# a prerequisite, to make sure it's up to date when doing deployments.
+compile: $(PY)
 	${SHHH} utilities/relocate-virtualenv env
 	${SHHH} $(MAKE) -C sourcecode build PYTHON=${PYTHON} \
 	    LPCONFIG=${LPCONFIG}
 	${SHHH} bin/build-twisted-plugin-cache
 	${SHHH} LPCONFIG=${LPCONFIG} ${PY} -t buildmailman.py
+	scripts/update-version-info.sh
 
 test_build: build
 	bin/test $(TESTFLAGS) $(TESTOPTS)