Merge ~michael.nelson/gpgservice:add-tarball-target into gpgservice:master

Proposed by Michael Nelson
Status: Merged
Approved by: Michael Nelson
Approved revision: b7e28d4d5a4941c000ea8f9470317d3050646f35
Merged at revision: b7e28d4d5a4941c000ea8f9470317d3050646f35
Proposed branch: ~michael.nelson/gpgservice:add-tarball-target
Merge into: gpgservice:master
Diff against target: 83 lines (+32/-7)
2 files modified
Makefile (+30/-5)
Makefile.db (+2/-2)
Reviewer Review Type Date Requested Status
Michael Nelson (community) Needs Fixing
Colin Watson (community) Approve
Review via email: mp+283255@code.launchpad.net

Commit message

Initial tarball Makefile target - enables creating a tarball from a commit hash, defaulting to HEAD of current branch

Description of the change

There may be easier ways to get an export of the git repo at a given commit hash - not sure, but this seems to work.

tarballs are saved in tmp/${GITHASH}/canonical-gpg-service.tgz

To post a comment you must log in.
Revision history for this message
Colin Watson (cjwatson) wrote :

On Wed, Jan 20, 2016 at 05:47:24AM -0000, Michael Nelson wrote:
> +GITREF ?= $(shell git symbolic-ref HEAD)
> +GITHASH ?= $(shell git show-ref --verify --hash $(GITREF))
> +ifndef GITHASH
> + $(error "There was no reference '${GITREF}' found.")
> +endif

Requiring a full ref path seems a bit unnecessarily unfriendly to people
overriding GITREF. How about just using the porcelain command instead
of the plumbing command?

GITHASH ?= $(shell git rev-parse $(GITREF))

> +BUILD_DIR ?= /tmp/canonical-gpg-service-builds

Can we please not have scripts using well-known names in /tmp? The
builds should probably just live in a subdirectory of $(TMPDIR).

> +$(TARBALL_PATH):
> + @rm -rf ${BUILD_DIR}
> + @mkdir -p ${BUILD_DIR}
> + @git clone . ${BUILD_DIR}/repo
> + @cd ${BUILD_DIR}/repo/ && git reset ${GITHASH} && git checkout-index -a --prefix ${BUILD_DIR}/canonical-gpg-service/
> + @make -C ${BUILD_DIR}/canonical-gpg-service ${BUILD_DIR}/canonical-gpg-service/env
> + @mkdir -p ${TARBALLS_DIR}/${GITHASH}/
> + @tar -czf $(TARBALL_PATH) -C ${BUILD_DIR} canonical-gpg-service

How about this (untested), which is a bit easier to read:

$(TARBALL_PATH):
 @rm -rf ${BUILD_DIR}
 @git clone . ${BUILD_DIR}/canonical-gpg-service
 @git checkout -C ${BUILD_DIR}/canonical-gpg-service ${GITHASH}
 @make -C ${BUILD_DIR}/canonical-gpg-service ${BUILD_DIR}/canonical-gpg-service/env
 @mkdir -p ${TARBALLS_DIR}/${GITHASH}/
 @tar -czf $(TARBALL_PATH) --exclude-vcs -C ${BUILD_DIR} canonical-gpg-service
 @echo "Tarball created at ${TARBALL_PATH}"

(I also wouldn't personally @-prefix any of these commands except for
the final echo, but perhaps you differ on that.)

Revision history for this message
Michael Nelson (michael.nelson) wrote :

On Wed, Jan 20, 2016 at 8:39 PM Colin Watson <email address hidden> wrote:

> On Wed, Jan 20, 2016 at 05:47:24AM -0000, Michael Nelson wrote:
> > +GITREF ?= $(shell git symbolic-ref HEAD)
> > +GITHASH ?= $(shell git show-ref --verify --hash $(GITREF))
> > +ifndef GITHASH
> > + $(error "There was no reference '${GITREF}' found.")
> > +endif
>
> Requiring a full ref path seems a bit unnecessarily unfriendly to people
> overriding GITREF. How about just using the porcelain command instead
> of the plumbing command?
>
> GITHASH ?= $(shell git rev-parse $(GITREF))
>

Excellent, thanks - I thought there must have been a simpler way, but
struggled to find it via git help or search.

>
> > +BUILD_DIR ?= /tmp/canonical-gpg-service-builds
>
> Can we please not have scripts using well-known names in /tmp? The
> builds should probably just live in a subdirectory of $(TMPDIR).
>
>
Yep, done also.

> > +$(TARBALL_PATH):
> > + @rm -rf ${BUILD_DIR}
> > + @mkdir -p ${BUILD_DIR}
> > + @git clone . ${BUILD_DIR}/repo
> > + @cd ${BUILD_DIR}/repo/ && git reset ${GITHASH} && git
> checkout-index -a --prefix ${BUILD_DIR}/canonical-gpg-service/
> > + @make -C ${BUILD_DIR}/canonical-gpg-service
> ${BUILD_DIR}/canonical-gpg-service/env
> > + @mkdir -p ${TARBALLS_DIR}/${GITHASH}/
> > + @tar -czf $(TARBALL_PATH) -C ${BUILD_DIR} canonical-gpg-service
>
> How about this (untested), which is a bit easier to read:
>
> $(TARBALL_PATH):
> @rm -rf ${BUILD_DIR}
> @git clone . ${BUILD_DIR}/canonical-gpg-service
> @git checkout -C ${BUILD_DIR}/canonical-gpg-service ${GITHASH}
> @make -C ${BUILD_DIR}/canonical-gpg-service
> ${BUILD_DIR}/canonical-gpg-service/env
> @mkdir -p ${TARBALLS_DIR}/${GITHASH}/
> @tar -czf $(TARBALL_PATH) --exclude-vcs -C ${BUILD_DIR}
> canonical-gpg-service
> @echo "Tarball created at ${TARBALL_PATH}"
>
> (I also wouldn't personally @-prefix any of these commands except for
> the final echo, but perhaps you differ on that.)
>

Nope - and done. Thanks - again, much simpler. The only difference from
your suggestion here that I found I needed was due to -C not being an
option for git checkout (at least, on trusty
https://pastebin.canonical.com/148014/) . Otherwise, I've used the above
sans the @-prefixes.

Revision history for this message
Colin Watson (cjwatson) wrote :

On Wed, Jan 20, 2016 at 11:21:30AM -0000, Michael Nelson wrote:
> Nope - and done. Thanks - again, much simpler. The only difference from
> your suggestion here that I found I needed was due to -C not being an
> option for git checkout (at least, on trusty
> https://pastebin.canonical.com/148014/) .

My bad, this should have been:

  git -C ${BUILD_DIR}/canonical-gpg-service checkout ${GITHASH}

Doesn't work in precise, but should be fine in trusty and up.

Revision history for this message
Colin Watson (cjwatson) :
review: Approve
Revision history for this message
Michael Nelson (michael.nelson) wrote :

I tried to merge it but there are some problems. Typically you want to merge or rebase and try again.

review: Needs Fixing
Revision history for this message
Michael Nelson (michael.nelson) wrote :

The merge was fine but running tests failed.

review: Needs Fixing

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/Makefile b/Makefile
2index fd09d27..6e8999f 100644
3--- a/Makefile
4+++ b/Makefile
5@@ -3,27 +3,52 @@ PYTHON = $(ENV)/bin/python
6 PIP = $(ENV)/bin/pip
7 FLAKE8 = $(ENV)/bin/flake8
8 TWISTD_PATH = $(ENV)/bin/twistd
9+TMPDIR = $(CURDIR)/tmp
10+
11+GITREF ?= $(shell git symbolic-ref HEAD)
12+GITHASH ?= $(shell git rev-parse $(GITREF))
13+ifndef GITHASH
14+ $(error "There was no reference '${GITREF}' found.")
15+endif
16+
17+BUILD_DIR ?= $(TMPDIR)/canonical-gpg-service-builds
18+TARBALLS_DIR = $(TMPDIR)/tarballs
19+TARBALL_PATH = $(TARBALLS_DIR)/$(GITHASH)/canonical-gpg-service.tgz
20
21 $(ENV):
22 @virtualenv $(ENV)
23- @${PIP} install -r requirements.txt -r requirements-dev.txt
24+ @${PIP} install -r requirements.txt
25+
26+$(ENV)/dev: $(ENV)
27+ @${PIP} install -r requirements-dev.txt
28+ @touch $(ENV)/dev
29
30-test: $(ENV)
31-# For now, run tests and exit 0 even if they fail.
32+test: $(ENV)/dev
33 @TWISTD_PATH=$(TWISTD_PATH) ${PYTHON} -m testtools.run gpgservice.tests.test_suite
34 $(FLAKE8) gpgservice
35
36 clean:
37 rm -rf $(ENV)
38- rm -rf tmp
39+ rm -rf $(TMPDIR)
40 rm -rf htmlcov
41 find -name '*.pyc' -delete
42 find -name '*.~*' -delete
43
44-coverage: $(ENV)
45+coverage: $(ENV)/dev
46 $(PYTHON) -m coverage run -m testtools.run gpgservice.tests.test_suite
47 $(PYTHON) -m coverage html --include="gpgservice/*"
48
49+$(TARBALL_PATH):
50+ rm -rf ${BUILD_DIR}
51+ git clone . ${BUILD_DIR}/canonical-gpg-service
52+ git -C ${BUILD_DIR}/canonical-gpg-service checkout ${GITHASH}
53+ make -C ${BUILD_DIR}/canonical-gpg-service ${BUILD_DIR}/canonical-gpg-service/env
54+ mkdir -p ${TARBALLS_DIR}/${GITHASH}/
55+ tar -czf $(TARBALL_PATH) --exclude-vcs -C ${BUILD_DIR} canonical-gpg-service
56+ @echo "Tarball created at ${TARBALL_PATH}"
57+
58+tarball: $(TARBALL_PATH)
59+
60 include Makefile.db
61
62 .PHONY: clean migrate reset-db setup-db start-db stop-db test
63diff --git a/Makefile.db b/Makefile.db
64index 2a68106..f7e1859 100644
65--- a/Makefile.db
66+++ b/Makefile.db
67@@ -12,7 +12,7 @@ PGINIT = $(PGBIN)/initdb
68 migrate:
69 # $(DJANGO_MANAGE) migrate --noinput
70
71-setup-db: $(ENV)
72+setup-db: $(ENV)/dev
73 mkdir -p $(DATA_DIR)
74 $(PGINIT) -A trust -D $(DATA_DIR)
75 echo "fsync = off" > $(CONF_FILE)
76@@ -20,7 +20,7 @@ setup-db: $(ENV)
77 PGHOST=$(PGHOST) createdb $(PGNAME)
78 PGHOST=$(PGHOST) createuser --superuser --createdb $(PGUSER)
79
80-start-db: $(ENV)
81+start-db: $(ENV)/dev
82 $(MAKE) setup-db
83 $(MAKE) migrate
84

Subscribers

People subscribed via source and target branches