Merge ~mpontillo/uvtool:dev-env-setup-and-hacking-instructions into uvtool:master

Proposed by Mike Pontillo
Status: Needs review
Proposed branch: ~mpontillo/uvtool:dev-env-setup-and-hacking-instructions
Merge into: uvtool:master
Diff against target: 140 lines (+110/-0)
5 files modified
.gitignore (+1/-0)
HACKING.md (+31/-0)
Makefile (+15/-0)
contrib/develop (+59/-0)
contrib/get-sandbox-path (+4/-0)
Reviewer Review Type Date Requested Status
uvtool development Pending
Review via email: mp+314118@code.launchpad.net

Commit message

Add contrib/install-dependencies script.

 * This script is intended to keep up with packaging so that new developers
   can easily install the depenencies and begin work.
 * Touches a '.installed' file in the sandbox to prevent doing more work
   than necessary.

Add utility scripts to make development easier.

 * New script: contrib/develop - when sourced, sets up $PATH and
   $PYHONPATH for development with the current sandbox.
 * New script: contrib/get-sandbox-path - when executed, prints
   the full path to the current sandbox. (Useful for setting up
   the $PATH and $PYTHONPATH automatically.)
 * Added Makefile whose default target ensures that development
   dependencies are installed, and then prompts the user to set up
   their environment correctly (by sourcing contrib/develop).

Add HACKING.md file.

 * Contains a short introduction regarding how to start development.

To post a comment you must log in.
Robie Basak (racb) wrote :

I appreciate and agree with having a HACKING.md. But rather than having a bunch of extra infrastructure, can we not just say, in HACKING.md:

Debian/Ubuntu build/runtime dependencies: <package1> <package2> ...

To run from the working tree, from the top level use: PYTHONPATH=. bin/uvt-kvm ...

Is there anything else required? By the time a new contributor has read HACKING.md and learned to run the "make install-dependencies" command, he/she might as well have learnt to just run "apt install ..." directly and be ready. The same goes for learning about the contrib/develop script.

I'm skeptical about a bunch of infrastructure that I feel is unnecessary. Contributors would end up having to learn what we have set up, and we would have to maintain it. Further, I expect there to be convention on this sort of thing and for us to follow it.

I think less is more here. I certainly get put off when arriving at a new project and finding a non-trivial development setup, such as custom non-standard code to run just to get things going.

Mike Pontillo (mpontillo) wrote :
Download full text (3.4 KiB)

I appreciate your concerns. I probably went a little crazy on this. Let me provide some context for this proposal...

As a new contributor to a project, the first thing you have to do is figure out how to work on the thing. So if you want to jump in and change the code, you might ask yourself a few questions:

(1) How do I do manual testing?
(2) How do I do automated testing?
      a. How do I run the unit tests?
      b. Is there a larger integration test or CI system I should be mindful of?
(3) How do I create a package to test with?

So I agree that a HACKING guide addresses (1) in a simple enough way. I could have started simple and did just that. But as I said, I went a little crazy and kept going. ;-)

If you want to address (2) you really want to have an automated way to get everything going, so that on a fresh image you can easily test all the things you need to test. I think it's good to have dev-env-setup activities written in code, so that they can later be automated.

This change doesn't address (3). Beyond running 'apt-get source', I don't have a clue where the debian/ directory is for uvtool, etc.

That aside, let me break down the changes so you understand the purpose for each one before you make a decision.

 * As a new contributor to a Python project, my first instinct was to look around in the setup.py and requirements.txt for what I would need to install from pypi to get going. I soon realized all that was missing
   - Solution: create a Makefile, to pique the interest of curious developers. This came somewhat from my experience working on MAAS, where there are some similarities to uvtool: it is a Python project, but it isn't based on dependencies from pypi; rather, it is based on dependencies from the Ubuntu archive. (In MAAS it is quite difficult to get going, and having the `make` targets really helps new developers. If something is going wrong, we can just say "did you run `make install-dependencies`?)
   - Suggestion: lose the script and just put the `sudo apt-get install -yu ...` in the Makefile. That way the list of dependencies for development and testing can be maintained in one place (as opposed to having to document it in the HACKING guide, and then repeating the same steps when writing automation scripts).
   - Side benefit: by automating this piece, we make it really easy to address (2) and run automated tests on a fresh Ubuntu image.

 * As a new contributor to uvtool, I might ask myself: given the lack of a `virtualenv` requirement and the need to test in the context of the actual Ubuntu system, how do I actually run this thing without creating a package and installing it? (Now, you and I have some experience with this, and could figure out quickly that we just need to set up the PATH and PYTHONPATH appropriately. But we have a lot of experience with this sort of thing. $RANDOM_NEW_CONTRIBUTOR might appreciate a little more help.)
   - Solution: Create a script (similar to how `virtualenv` works) to automatically set up the development sandbox.
   - Side benefit: there is no additional thinking required when you go to automate tests. Just ". contrib/develop" and you're done.

The other thing I would point out...

Read more...

Mike Pontillo (mpontillo) wrote :

Based on your feedback, I've simplified this branch a bit. Please take another look and see if you like it better.

Robie Basak (racb) wrote :
Download full text (5.5 KiB)

On Thu, Jan 05, 2017 at 05:45:13PM -0000, Mike Pontillo wrote:
> (1) How do I do manual testing?
> (2) How do I do automated testing?
> a. How do I run the unit tests?
> b. Is there a larger integration test or CI system I should be mindful of?
> (3) How do I create a package to test with?

Thank you for enumerating these. This is a great list. I agree that we
need to be able to answer these questions quickly for any contributor,
and so and we can use the list to check against any proposed solution.

> If you want to address (2) you really want to have an automated way to
> get everything going, so that on a fresh image you can easily test all
> the things you need to test. I think it's good to have dev-env-setup
> activities written in code, so that they can later be automated.

Agreed, with the caveat that if it isn't uvtool-specific, it shouldn't
be in uvtool's tree. Instead there should be a community convention and
community tooling (eg. setuptools or virtualenv), and we should be using
that.

> This change doesn't address (3). Beyond running 'apt-get source', I
> don't have a clue where the debian/ directory is for uvtool, etc.

It's in the Debian packaging branch. Having a separate packaging is the
conventional thing to do, so I think it should suffice to point to it in
a HACKING.md.

> That aside, let me break down the changes so you understand the
> purpose for each one before you make a decision.
>
> * As a new contributor to a Python project, my first instinct was to
> look around in the setup.py and requirements.txt for what I would
> need to install from pypi to get going. I soon realized all that was
> missing

OK. This is a reasonable expectation.

> - Solution: create a Makefile, to pique the interest of curious
> developers. This came somewhat from my experience working on MAAS,
> where there are some similarities to uvtool: it is a Python
> project, but it isn't based on dependencies from pypi; rather, it
> is based on dependencies from the Ubuntu archive. (In MAAS it is
> quite difficult to get going, and having the `make` targets really
> helps new developers. If something is going wrong, we can just say
> "did you run `make install-dependencies`?)

Here's where I think my opinion diverges. The right way to fix this is
to directly fix your original missing observation. There should be a
requirements.txt and the dependencies should be put into pypi. That's
the correct way to fix your use case above, IMHO.

> - Suggestion: lose the script and just put the `sudo apt-get
> install -yu ...` in the Makefile. That way the list of dependencies
> for development and testing can be maintained in one place (as
> opposed to having to document it in the HACKING guide, and then
> repeating the same steps when writing automation scripts).

I think this is moot, as I don't think there should be a Makefile.
Because that's not part of the Python community's convention, and this
is a pure Python project.

> [...]

> * As a new contributor to uvtool, I might ask myself: given the lack
> of a `virtualenv` requirement and the need to test in the context of
> the actual Ubuntu system, ho...

Read more...

Mike Pontillo (mpontillo) wrote :

I think we agree on most of these items, so let me just address where we
may disagree.

> Asking contributors to run the single line "PYTHONPATH=. bin/..." or
> "setup.py test" or whatever is however immediately transparent. The
> former because it's a one-liner that does nothing special and should be
> immediately understandable to a Python developer, and the latter because
> it uses setuptools, which should also be immediately understandable to a
> Python developer.

What I don't like about "PYTHONPATH=. bin/..." is that it considerably
slows development. Maybe it's just me, but I was doing just that when
poking around with `uvtool` for quite some time. (Until I decided to
scratch this itch.) If I'm in a shell and I'm looking at code, testing
things out, moving around to other directories outside my sandbox, etc,
it's really annoying to have to switch back to the sandbox root to test
things again, and slows me down considerably. And it's equally annoying if
I have to manually export environment variables each time. And if I forget
something or make a typo, guess what: I'm testing the global `uvtool`, and
I get frustrated when I realize I'm testing the wrong code. When I find
myself doing something manual like that over and over again, it either
drives me insane, or I want to automate it. ;-)

> As it isn't uvtool-specific, it does not belong in uvtool. As it's a
> general problem, it should be solved in a general way with generic
> tooling that the whole community can use. This may be setuptools or
> virtualenv or something else. If a Python community member's
> expectations are broken because uvtool's tree detracts from convention,
> then we should fix that. And I'll accept patches and encourage that side
> of things being fixed.

I see your point here and mostly agree. If you're against the addition of a
contrib/ script for this, I can keep it as a local helper script for
myself. But I think it's a bit of a shame that others can't take advantage
of it.

Moving to a pure virtualenv model (or whatever Pythonic tool is chosen)
would be great, and I support that. (But I would still want something to
automate the creation of a dev env. It takes too much mental energy for me
to manage venvs manually, when I would rather be working on the code.)

The fact is, right now `uvtool` is *very* Ubuntu specific. Getting a
development environment up and running, correspondingly so. I think you
don't like that I have codified that. ;-) If all the dependencies are
available in pypi and `setup.py` works as a typical Python developer
expects, I think that would also solve the issue. But that is a patch that
reaches into simplestreams and other projects, and I don't have time to
work on that.

Mike Pontillo (mpontillo) wrote :

Based on this discussion, I've made some additional changes to the `README.md`. Please read over it again and tell me if that's a good compromise.

If you still don't like it, I'll just keep these changes private to my own branch.

831d750... by Mike Pontillo on 2017-01-05

Add initial HACKING.md file.

Mike Pontillo (mpontillo) wrote :

One last thing I'll mention before (I expect) you'll reject this MP, since I realized I didn't explain my need for this well.

I am looking at `uvtool` in conjunction with other test tools (for testing MAAS). In order to iterate quickly the integration between those tools and `uvtool`, I needed an easy way to add a `uvtool` sandbox to both the $PATH and $PYTHONPATH in a way that worked outside of the sandbox itself. That was the motivation behind the `develop`, script in this MP.

Again, if you're still against this, I'm okay keeping this in a private branch. So my feelings won't be hurt if you don't want it. ;-)

Unmerged commits

7fc6a85... by Mike Pontillo on 2017-01-04

Update HACKING.md file to reflect automation.

238e1be... by Mike Pontillo on 2017-01-04

Add utility scripts to make development easier.

 * New script: contrib/develop - when sourced, sets up $PATH and
   $PYHONPATH for development with the current sandbox.
 * New script: contrib/get-sandbox-path - when executed, prints
   the full path to the current sandbox. (Useful for setting up
   the $PATH and $PYTHONPATH automatically.)
 * Added Makefile whose default target ensures that development
   dependencies are installed, and then prompts the user to set up
   their environment correctly (by sourcing contrib/develop).

831d750... by Mike Pontillo on 2017-01-05

Add initial HACKING.md file.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/.gitignore b/.gitignore
2index 0d20b64..b948fef 100644
3--- a/.gitignore
4+++ b/.gitignore
5@@ -1 +1,2 @@
6 *.pyc
7+.installed
8diff --git a/HACKING.md b/HACKING.md
9new file mode 100644
10index 0000000..7ac7475
11--- /dev/null
12+++ b/HACKING.md
13@@ -0,0 +1,31 @@
14+Quick Start
15+===========
16+
17+To get started, simply run `make` from the sandbox directory. This will
18+install the required dependencies and print instructions about how to set
19+up your environment for manual testing.
20+
21+After installing the dependencies, the only requirement for working on
22+`uvtool` is that the `PYTHONPATH` includes its source. Minimally, you can
23+test `uvtool` by doing something like this from the sandbox root:
24+
25+ PYTHONPATH=. bin/...
26+
27+As a convenience, if you want to test `uvtool` while outside the root sandbox
28+directory, you can `source contrib/develop` to automatically set up the
29+`$PATH` and `$PYTHONPATH` appropriately. This could be useful if you are
30+testing `uvtool` changes in conjunction with another test tool that depends
31+on it.
32+
33+Development Platform and Dependencies
34+=====================================
35+
36+While `uvtool` is indended to be a pure Python project, it is still tightly
37+coupled to Ubuntu. (Patches will be accepted to make it more "Pythonic".)
38+
39+That said, the current primary development platform for `uvtool` is the latest
40+LTS release of Ubuntu. Secondarily, it may be tested on the latest
41+development release.
42+
43+The Makefile is kept up to date with the required Ubuntu packages for
44+development and testing. (See the `UBUNTU_DEPENDENCIES` variable.)
45diff --git a/Makefile b/Makefile
46new file mode 100644
47index 0000000..68dced9
48--- /dev/null
49+++ b/Makefile
50@@ -0,0 +1,15 @@
51+UBUNTU_DEPENDENCIES=libvirt-bin python-libvirt python-simplestreams \
52+ python-lxml python-pyinotify python-yaml distro-info cloud-image-utils \
53+ qemu-utils ubuntu-cloudimage-keyring socat python qemu-kvm cpu-checker
54+
55+install-dependencies: .installed
56+ @echo ""
57+ @contrib/develop
58+
59+.installed:
60+ sudo apt-get install -yu $(UBUNTU_DEPENDENCIES)
61+ @touch .installed
62+
63+
64+.PHONY: install-dependencies
65+
66diff --git a/contrib/develop b/contrib/develop
67new file mode 100755
68index 0000000..7d415cb
69--- /dev/null
70+++ b/contrib/develop
71@@ -0,0 +1,59 @@
72+#!/bin/bash
73+
74+# This script assists developers when setting up a development environment.
75+# It is intended to be sourced, but will provide useful informaton if
76+# executed.
77+
78+function _is_sourced() {
79+ [[ "${BASH_SOURCE[0]}" != "$0" ]]
80+}
81+
82+function _run_and_log() {
83+ echo "Executing: $@"
84+ "$@"
85+}
86+
87+function _colon_separated_list_contains() {
88+ echo "$1" | tr ':' '\n' | grep -qFx "$2"
89+}
90+
91+function _path_contains() {
92+ _colon_separated_list_contains "$PATH" "$1"
93+}
94+
95+function _pythonpath_contains() {
96+ _colon_separated_list_contains "$PYTHONPATH" "$1"
97+}
98+
99+if ! _is_sourced; then
100+ echo "You may source the 'contrib/develop' script to get started."
101+ echo "Doing so will set up your PATH and PYTHONPATH for development."
102+ echo ""
103+ echo "For example:"
104+ echo " . $0"
105+ echo ""
106+fi
107+
108+SOURCE="${BASH_SOURCE[0]}"
109+SCRIPT_DIR="$(dirname "$(readlink -f "$SOURCE")")"
110+SANDBOX="$($SCRIPT_DIR/get-sandbox-path)"
111+
112+if ! _pythonpath_contains "$SANDBOX"; then
113+ if [ "$PYTHONPATH" != "" ]; then
114+ PYTHONPATH=":$PYTHONPATH"
115+ fi
116+ _is_sourced && _run_and_log export PYTHONPATH="${SANDBOX}${PYTHONPATH}" \
117+ || echo "'$SANDBOX' needs to be added to the \$PYTONPATH."
118+else
119+ echo "'$SANDBOX' is already on the \$PYTHONPATH."
120+fi
121+
122+if ! _path_contains "$SANDBOX/bin"; then
123+ _is_sourced && _run_and_log export PATH="$SANDBOX/bin:$PATH" \
124+ || echo "'$SANDBOX/bin' needs to be added to the \$PATH."
125+else
126+ echo "'$SANDBOX/bin' is already on the \$PATH."
127+fi
128+
129+# Force exit with a good status if we weren't sourced (don't confuse 'make').
130+_is_sourced || exit 0
131diff --git a/contrib/get-sandbox-path b/contrib/get-sandbox-path
132new file mode 100755
133index 0000000..6a9882b
134--- /dev/null
135+++ b/contrib/get-sandbox-path
136@@ -0,0 +1,4 @@
137+#!/bin/bash -e
138+
139+cd "$(dirname "$(readlink -f "$0")")"/.. > /dev/null
140+echo $PWD

Subscribers

People subscribed via source and target branches