Merge lp:~benji/charms/precise/juju-gui/use-known-pip into lp:~juju-gui/charms/precise/juju-gui/trunk

[Benji York]

Include pip dependency instead of using system's

I also killed an entire village of ./

https://codereview.appspot.com/31000043/

[Benji York (benji)]

Reviewers: mp+196328_code.launchpad.net,

Message:
Please take a look.

Description:
Include pip dependency instead of using system's

I also killed an entire village of ./

https://code.launchpad.net/~benji/charms/precise/juju-gui/use-known-pip/+merge/196328

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/31000043/

Affected files (+13, -9 lines):
   M Makefile
   A [revision details]
   M tests/00-setup

Index: Makefile
=== modified file 'Makefile'
--- Makefile 2013-11-19 02:50:54 +0000
+++ Makefile 2013-11-22 16:01:36 +0000
@@ -15,9 +15,9 @@
  # along with this program. If not, see <http://www.gnu.org/licenses/>.

  JUJUTEST = juju-test --timeout=120m -v -e "$(JUJU_ENV)"
-VENV = ./tests/.venv
-SYSDEPS = build-essential bzr libapt-pkg-dev libpython-dev python-pip \
- python-virtualenv rsync xvfb
+VENV = tests/.venv
+SYSDEPS = build-essential bzr libapt-pkg-dev libpython-dev
python-virtualenv \
+ rsync xvfb

  all: setup
@@ -26,14 +26,16 @@
  # support the juju-test plugin, which calls the executable files in
  # alphabetical order.
  setup:
- @./tests/00-setup
+ tests/00-setup
+ # Ensure the correct version of pip has been installed
+ tests/.venv/bin/pip --version | grep "pip 1.4" || exit 1

  sysdeps:
   sudo apt-get install --yes $(SYSDEPS)

  unittest: setup
- ./tests/10-unit.test
- ./tests/11-server.test
+ tests/10-unit.test
+ tests/11-server.test

  ensure-juju-env:
  ifndef JUJU_ENV
@@ -57,7 +59,7 @@

  lint: setup
   @$(VENV)/bin/flake8 --show-source --exclude=.venv \
- ./hooks/ ./tests/ ./server/
+ hooks/ tests/ server/

  clean:
   find . -name '*.pyc' -delete
@@ -65,7 +67,7 @@
   rm -rf tests/download-cache

  deploy: setup
- $(VENV)/bin/python ./tests/deploy.py
+ $(VENV)/bin/python tests/deploy.py

  help:
   @echo -e 'Juju GUI charm - list of make targets:\n'

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision:
<email address hidden>
+New revision: <email address hidden>

Index: tests/00-setup
=== modified file 'tests/00-setup'
--- tests/00-setup 2013-11-19 14:25:24 +0000
+++ tests/00-setup 2013-11-22 15:16:01 +0000
@@ -34,7 +34,7 @@
      fi
      if [ ! -f "$ACTIVATE" -o "$TEST_REQUIREMENTS" -nt "$ACTIVATE"
-o "$SERVER_REQUIREMENTS" -nt "$ACTIVATE" ]; then
          [ $updated_downloadcache -eq 0 ] && bzr up $DOWNLOADCACHE
- virtualenv --distribute $VENV
+ virtualenv --distribute --extra-search-dir=$DOWNLOADCACHE $VENV
          . $ACTIVATE && \
              yes w | env BZR_PLUGIN_PATH='-user' \
              pip install -r $TEST_REQUIREMENTS --find-links deps
--find-links $DOWNLOADCACHE --no-allow-external --no-index

[Benji York (benji)]

Here are some pre-review comments.

https://codereview.appspot.com/31000043/diff/1/Makefile
File Makefile (right):

https://codereview.appspot.com/31000043/diff/1/Makefile#newcode20
Makefile:20: rsync xvfb
We're using pip from the download cache now.

https://codereview.appspot.com/31000043/diff/1/Makefile#newcode31
Makefile:31: tests/.venv/bin/pip --version | grep "pip 1.4" || exit 1
It would be easy to accidentally start using the wrong version
(unupdated download cache for example), so I added an assert of sorts.

https://codereview.appspot.com/31000043/diff/1/Makefile#newcode38
Makefile:38: tests/11-server.test
Death to the dot-slash.

https://codereview.appspot.com/31000043/diff/1/tests/00-setup
File tests/00-setup (right):

https://codereview.appspot.com/31000043/diff/1/tests/00-setup#newcode37
tests/00-setup:37: virtualenv --distribute
--extra-search-dir=$DOWNLOADCACHE $VENV
This makes virtualenv install pip from the download cache.

https://codereview.appspot.com/31000043/

[Francesco Banconi (frankban)]

I successfully created the testing environment with this branch. LGTM
thank you!

https://codereview.appspot.com/31000043/diff/1/Makefile
File Makefile (right):

https://codereview.appspot.com/31000043/diff/1/Makefile#newcode38
Makefile:38: tests/11-server.test
On 2013/11/22 17:33:22, benji wrote:
> Death to the dot-slash.

Thanks for killing them.

https://codereview.appspot.com/31000043/diff/1/tests/00-setup
File tests/00-setup (right):

https://codereview.appspot.com/31000043/diff/1/tests/00-setup#newcode37
tests/00-setup:37: virtualenv --distribute
--extra-search-dir=$DOWNLOADCACHE $VENV
On 2013/11/22 17:33:22, benji wrote:
> This makes virtualenv install pip from the download cache.

Very nice!

https://codereview.appspot.com/31000043/

[Benji York (benji)]

*** Submitted:

Include pip dependency instead of using system's

I also killed an entire village of ./

R=frankban
CC=
https://codereview.appspot.com/31000043

https://codereview.appspot.com/31000043/

=== modified file 'Makefile'
--- Makefile	2013-11-19 02:50:54 +0000
+++ Makefile	2013-11-22 17:20:57 +0000
@@ -15,9 +15,9 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 JUJUTEST = juju-test --timeout=120m -v -e "$(JUJU_ENV)"
-VENV = ./tests/.venv
-SYSDEPS = build-essential bzr libapt-pkg-dev libpython-dev python-pip \
-	python-virtualenv rsync xvfb
+VENV = tests/.venv
+SYSDEPS = build-essential bzr libapt-pkg-dev libpython-dev python-virtualenv \
+	rsync xvfb
 
 
 all: setup
@@ -26,14 +26,16 @@
 # support the juju-test plugin, which calls the executable files in
 # alphabetical order.
 setup:
-	@./tests/00-setup
+	tests/00-setup
+	# Ensure the correct version of pip has been installed
+	tests/.venv/bin/pip --version | grep "pip 1.4" || exit 1
 
 sysdeps:
 	sudo apt-get install --yes $(SYSDEPS)
 
 unittest: setup
-	./tests/10-unit.test
-	./tests/11-server.test
+	tests/10-unit.test
+	tests/11-server.test
 
 ensure-juju-env:
 ifndef JUJU_ENV
@@ -57,7 +59,7 @@
 
 lint: setup
 	@$(VENV)/bin/flake8 --show-source --exclude=.venv \
-		./hooks/ ./tests/ ./server/
+		hooks/ tests/ server/
 
 clean:
 	find . -name '*.pyc' -delete
@@ -65,7 +67,7 @@
 	rm -rf tests/download-cache
 
 deploy: setup
-	$(VENV)/bin/python ./tests/deploy.py
+	$(VENV)/bin/python tests/deploy.py
 
 help:
 	@echo -e 'Juju GUI charm - list of make targets:\n'

=== modified file 'tests/00-setup'
--- tests/00-setup	2013-11-19 14:25:24 +0000
+++ tests/00-setup	2013-11-22 17:20:57 +0000
@@ -34,7 +34,7 @@
     fi
     if [ ! -f "$ACTIVATE" -o "$TEST_REQUIREMENTS" -nt "$ACTIVATE" -o "$SERVER_REQUIREMENTS" -nt "$ACTIVATE" ]; then
         [ $updated_downloadcache -eq 0 ] && bzr up $DOWNLOADCACHE
-        virtualenv --distribute $VENV
+        virtualenv --distribute --extra-search-dir=$DOWNLOADCACHE $VENV
         . $ACTIVATE && \
             yes w | env BZR_PLUGIN_PATH='-user' \
             pip install -r $TEST_REQUIREMENTS --find-links deps --find-links $DOWNLOADCACHE --no-allow-external --no-index