Juju Charms Collection
juju-gui package

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

  1. Precise Pangolin (12.04)
  2. use-known-pip
  3. Merge into trunk
Proposed by Benji York
Status: Merged
Merged at revision: 135
Proposed branch: lp:~benji/charms/precise/juju-gui/use-known-pip
Merge into: lp:~juju-gui/charms/precise/juju-gui/trunk
Diff against target: 67 lines (+11/-9)
2 files modified
Makefile (+10/-8)
tests/00-setup (+1/-1)
To merge this branch: bzr merge lp:~benji/charms/precise/juju-gui/use-known-pip
Reviewer Review Type Date Requested Status
charmers Pending
Review via email: mp+196328@code.launchpad.net

Description of the change

Include pip dependency instead of using system's

I also killed an entire village of ./

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

To post a comment you must log in.
Revision history for this message
Benji York (benji) wrote :

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

Revision history for this message
Benji York (benji) wrote :

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/

Revision history for this message
Francesco Banconi (frankban) wrote :

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/

Revision history for this message
Benji York (benji) wrote :

*** 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/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'Makefile'
--- Makefile 2013-11-19 02:50:54 +0000
+++ Makefile 2013-11-22 17:20:57 +0000
@@ -15,9 +15,9 @@
15# along with this program. If not, see <http://www.gnu.org/licenses/>.15# along with this program. If not, see <http://www.gnu.org/licenses/>.
1616
17JUJUTEST = juju-test --timeout=120m -v -e "$(JUJU_ENV)"17JUJUTEST = juju-test --timeout=120m -v -e "$(JUJU_ENV)"
18VENV = ./tests/.venv18VENV = tests/.venv
19SYSDEPS = build-essential bzr libapt-pkg-dev libpython-dev python-pip \19SYSDEPS = build-essential bzr libapt-pkg-dev libpython-dev python-virtualenv \
20 python-virtualenv rsync xvfb20 rsync xvfb
2121
2222
23all: setup23all: setup
@@ -26,14 +26,16 @@
26# support the juju-test plugin, which calls the executable files in26# support the juju-test plugin, which calls the executable files in
27# alphabetical order.27# alphabetical order.
28setup:28setup:
29 @./tests/00-setup29 tests/00-setup
30 # Ensure the correct version of pip has been installed
31 tests/.venv/bin/pip --version | grep "pip 1.4" || exit 1
3032
31sysdeps:33sysdeps:
32 sudo apt-get install --yes $(SYSDEPS)34 sudo apt-get install --yes $(SYSDEPS)
3335
34unittest: setup36unittest: setup
35 ./tests/10-unit.test37 tests/10-unit.test
36 ./tests/11-server.test38 tests/11-server.test
3739
38ensure-juju-env:40ensure-juju-env:
39ifndef JUJU_ENV41ifndef JUJU_ENV
@@ -57,7 +59,7 @@
5759
58lint: setup60lint: setup
59 @$(VENV)/bin/flake8 --show-source --exclude=.venv \61 @$(VENV)/bin/flake8 --show-source --exclude=.venv \
60 ./hooks/ ./tests/ ./server/62 hooks/ tests/ server/
6163
62clean:64clean:
63 find . -name '*.pyc' -delete65 find . -name '*.pyc' -delete
@@ -65,7 +67,7 @@
65 rm -rf tests/download-cache67 rm -rf tests/download-cache
6668
67deploy: setup69deploy: setup
68 $(VENV)/bin/python ./tests/deploy.py70 $(VENV)/bin/python tests/deploy.py
6971
70help:72help:
71 @echo -e 'Juju GUI charm - list of make targets:\n'73 @echo -e 'Juju GUI charm - list of make targets:\n'
7274
=== 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 @@
34 fi34 fi
35 if [ ! -f "$ACTIVATE" -o "$TEST_REQUIREMENTS" -nt "$ACTIVATE" -o "$SERVER_REQUIREMENTS" -nt "$ACTIVATE" ]; then35 if [ ! -f "$ACTIVATE" -o "$TEST_REQUIREMENTS" -nt "$ACTIVATE" -o "$SERVER_REQUIREMENTS" -nt "$ACTIVATE" ]; then
36 [ $updated_downloadcache -eq 0 ] && bzr up $DOWNLOADCACHE36 [ $updated_downloadcache -eq 0 ] && bzr up $DOWNLOADCACHE
37 virtualenv --distribute $VENV37 virtualenv --distribute --extra-search-dir=$DOWNLOADCACHE $VENV
38 . $ACTIVATE && \38 . $ACTIVATE && \
39 yes w | env BZR_PLUGIN_PATH='-user' \39 yes w | env BZR_PLUGIN_PATH='-user' \
40 pip install -r $TEST_REQUIREMENTS --find-links deps --find-links $DOWNLOADCACHE --no-allow-external --no-index40 pip install -r $TEST_REQUIREMENTS --find-links deps --find-links $DOWNLOADCACHE --no-allow-external --no-index

Subscribers

People subscribed via source and target branches

to all changes: