Merge lp:~allenap/maas-test/use-system-packages into lp:maas-test

Proposed by Gavin Panella
Status: Merged
Approved by: Gavin Panella
Approved revision: 22
Merged at revision: 19
Proposed branch: lp:~allenap/maas-test/use-system-packages
Merge into: lp:maas-test
Diff against target: 102 lines (+60/-10)
4 files modified
Makefile (+13/-3)
README.txt (+37/-0)
packages.txt (+6/-0)
requirements.txt (+4/-7)
To merge this branch: bzr merge lp:~allenap/maas-test/use-system-packages
Reviewer Review Type Date Requested Status
Raphaël Badin (community) Approve
Review via email: mp+194752@code.launchpad.net

Commit message

Install production dependencies using system packages, and allow the virtualenv to use them.

To post a comment you must log in.
17. By Gavin Panella

Help a non-interactive build go smoothly.

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

Not too keen on being surprised by password prompts... Can we keep package installation out of the default "make" target?

Revision history for this message
Raphaël Badin (rvb) wrote :

I agree with what Jeroen said. Which means that building an environment would now be a two-step process. Probably worth updating the README.txt file to explain (really quickly) how this works.

Regarding the changes to requirements.txt, I'm not sure we should manage this file manually like this. Especially now that it's only used for development dependencies, I think we should let pip manage it and generate it with "pip freeze > requirements.txt". Which means that adding a dev dependency would be as simple as:
pip install new-dev
pip freeze > requirements.txt

Revision history for this message
Raphaël Badin (rvb) wrote :

Also, again about the change to requirements.txt, shouldn't we drop the production dependencies from there now that we require them to be installed as package? Which means we probably should change how virtualenv+pip is configured to use the system's package as I don't think it's configured like this ATM.

18. By Gavin Panella

Only use Python 2 for now.

19. By Gavin Panella

Split out installing packages into a separate target.

Revision history for this message
Gavin Panella (allenap) wrote :

> Not too keen on being surprised by password prompts... Can we keep package
> installation out of the default "make" target?

Done.

Revision history for this message
Gavin Panella (allenap) wrote :

> I agree with what Jeroen said. Which means that building an environment would
> now be a two-step process. Probably worth updating the README.txt file to
> explain (really quickly) how this works.

Done, and documentation added.

>
> Regarding the changes to requirements.txt, I'm not sure we should manage this
> file manually like this. Especially now that it's only used for development
> dependencies, I think we should let pip manage it and generate it with "pip
> freeze > requirements.txt". Which means that adding a dev dependency would be
> as simple as:
> pip install new-dev
> pip freeze > requirements.txt

When I tried this yesterday (with the --local flag, otherwise it gives you the version of every package on the system) the output wasn't useful, but today it is. I'm not quite sure what I've done to deserve this, but I guess this makes sense. We probably ought to put version information in packages.txt (I think apt-get groks that), but that shouldn't prevent it from working on Trusty and beyond.

Revision history for this message
Gavin Panella (allenap) wrote :

> pip install new-dev
> pip freeze > requirements.txt

This causes problems. If the version in requirements.txt differs from
another installed version, you get stuff like this:

$ make test
bin/python setup.py test
running test
Traceback (most recent call last):
  File "setup.py", line 37, in <module>
    "maas-test = maastest.main:main",
  File "/usr/lib/python2.7/distutils/core.py", line 152, in setup
    dist.run_commands()
  File "/usr/lib/python2.7/distutils/dist.py", line 953, in run_commands
    self.run_command(cmd)
  File "/usr/lib/python2.7/distutils/dist.py", line 972, in run_command
    cmd_obj.run()
  File ".../local/lib/python2.7/site-packages/setuptools/command/test.py", line 128, in run
    self.distribution.fetch_build_eggs(self.distribution.install_requires)
  File ".../local/lib/python2.7/site-packages/setuptools/dist.py", line 289, in fetch_build_eggs
    parse_requirements(requires), installer=self.fetch_build_egg
  File ".../local/lib/python2.7/site-packages/pkg_resources.py", line 630, in resolve
    raise VersionConflict(dist,req) # XXX put more info here
pkg_resources.VersionConflict: (flake8 2.0 (/home/gavin/Library/lib/python2.7/site-packages/flake8-2.0-py2.7.egg), Requirement.parse('flake8==2.1.0'))
make: *** [test] Error 1

Nothing ever just works, does it?

Revision history for this message
Gavin Panella (allenap) wrote :

I'm starting to think we don't need a virtualenv here. The only dev package is flake8 (and its deps), and that's packaged for Ubuntu, for Python 3 too.

20. By Gavin Panella

Switch requirements.txt to development-only dependencies.

21. By Gavin Panella

Documentation re. dependencies.

22. By Gavin Panella

Tweak README.

Revision history for this message
Gavin Panella (allenap) wrote :

> We probably ought to put version information in packages.txt (I think
> apt-get groks that), but that shouldn't prevent it from working on
> Trusty and beyond.

apt-get only groks exact versions, so I've left this out.

Revision history for this message
Raphaël Badin (rvb) wrote :

Nice! Thanks a lot for doing this!

[0]

10 + apt-get --assume-yes --no-install-recommends

One question: why the "--no-install-recommends"?

review: Approve
Revision history for this message
Gavin Panella (allenap) wrote :

> Nice! Thanks a lot for doing this!
>
> [0]
>
> 10 + apt-get --assume-yes --no-install-recommends
>
> One question: why the "--no-install-recommends"?

Copied from MAAS's install-dependencies target.

Thanks for the review!

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'Makefile'
--- Makefile 2013-11-05 10:15:44 +0000
+++ Makefile 2013-11-12 13:44:42 +0000
@@ -3,6 +3,12 @@
33
4PYTHON := python4PYTHON := python
55
6define apt-get
7sudo DEBIAN_FRONTEND=noninteractive \
8 apt-get --assume-yes --no-install-recommends
9endef
10
11
6build: bin/python bin/pip requirements.txt12build: bin/python bin/pip requirements.txt
7 bin/pip install --requirement=requirements.txt13 bin/pip install --requirement=requirements.txt
814
@@ -26,6 +32,10 @@
26 @bin/pip install -q flake832 @bin/pip install -q flake8
2733
28bin/python bin/pip:34bin/python bin/pip:
29 virtualenv --python=$(PYTHON) $(PWD)35 virtualenv --python=$(PYTHON) --system-site-packages $(PWD)
3036
31.PHONY: build dist clean lint test37install-dependencies: packages.txt
38 @xargs --verbose --no-run-if-empty $(apt-get) install < packages.txt
39
40
41.PHONY: build dist clean install-dependencies lint test
3242
=== modified file 'README.txt'
--- README.txt 2013-10-30 12:46:38 +0000
+++ README.txt 2013-11-12 13:44:42 +0000
@@ -10,3 +10,40 @@
10For more information see the `Launchpad project page`_.10For more information see the `Launchpad project page`_.
1111
12.. _Launchpad project page: https://launchpad.net/maas-test12.. _Launchpad project page: https://launchpad.net/maas-test
13
14
15Setting up for development
16--------------------------
17
18To get maas-test running should be fairly easy, assuming you're on
19Ubuntu 13.10 or later::
20
21 bzr branch lp:maas-test
22 cd maas-test
23 make install-dependencies
24 make build
25
26The ``make install-dependencies`` step ensures that maas-test's
27dependencies are installed as system packages, and will not later be
28installed by pip.
29
30
31Dependencies
32------------
33
34The policy is:
35
36 * All production dependencies must be satisfied from system packages.
37
38 * Development-only dependencies can be installed from PyPI.
39
40That means that production dependencies need to go in ``packages.txt``
41and development-only ones into ``requirements.txt``. The latter can be
42maintained like so::
43
44 bin/pip install some-new-dev-only-library
45 bin/pip freeze --local > requirements.txt
46
47Then **carefully** review the changes; libraries installed elsewhere on
48the system can cause ``freeze --local`` to come up with radically
49different answers to those on another machine.
1350
=== added file 'packages.txt'
--- packages.txt 1970-01-01 00:00:00 +0000
+++ packages.txt 2013-11-12 13:44:42 +0000
@@ -0,0 +1,6 @@
1python-fixtures
2python-lxml
3python-mock
4python-netaddr
5python-testresources
6python-testtools
07
=== modified file 'requirements.txt'
--- requirements.txt 2013-11-08 14:26:53 +0000
+++ requirements.txt 2013-11-12 13:44:42 +0000
@@ -1,7 +1,4 @@
1lxml >= 3.2.31flake8==2.1.0
2mock >= 1.0.12mccabe==0.2.1
3netaddr >= 0.73pep8==1.4.6
4fixtures >= 0.3.84pyflakes==0.7.3
5flake8 >= 2.1.0
6testresources >= 0.2.7
7testtools >= 0.9.14

Subscribers

People subscribed via source and target branches