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.
Revision history for this message
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.

Revision history for this message
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...

Revision history for this message
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.

Revision history for this message
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...

Revision history for this message
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.

Revision history for this message
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

Add initial HACKING.md file.

Revision history for this message
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

Update HACKING.md file to reflect automation.

238e1be... by Mike Pontillo

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

Add initial HACKING.md file.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/.gitignore b/.gitignore
index 0d20b64..b948fef 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1 +1,2 @@
1*.pyc1*.pyc
2.installed
diff --git a/HACKING.md b/HACKING.md
2new file mode 1006443new file mode 100644
index 0000000..7ac7475
--- /dev/null
+++ b/HACKING.md
@@ -0,0 +1,31 @@
1Quick Start
2===========
3
4To get started, simply run `make` from the sandbox directory. This will
5install the required dependencies and print instructions about how to set
6up your environment for manual testing.
7
8After installing the dependencies, the only requirement for working on
9`uvtool` is that the `PYTHONPATH` includes its source. Minimally, you can
10test `uvtool` by doing something like this from the sandbox root:
11
12 PYTHONPATH=. bin/...
13
14As a convenience, if you want to test `uvtool` while outside the root sandbox
15directory, you can `source contrib/develop` to automatically set up the
16`$PATH` and `$PYTHONPATH` appropriately. This could be useful if you are
17testing `uvtool` changes in conjunction with another test tool that depends
18on it.
19
20Development Platform and Dependencies
21=====================================
22
23While `uvtool` is indended to be a pure Python project, it is still tightly
24coupled to Ubuntu. (Patches will be accepted to make it more "Pythonic".)
25
26That said, the current primary development platform for `uvtool` is the latest
27LTS release of Ubuntu. Secondarily, it may be tested on the latest
28development release.
29
30The Makefile is kept up to date with the required Ubuntu packages for
31development and testing. (See the `UBUNTU_DEPENDENCIES` variable.)
diff --git a/Makefile b/Makefile
0new file mode 10064432new file mode 100644
index 0000000..68dced9
--- /dev/null
+++ b/Makefile
@@ -0,0 +1,15 @@
1UBUNTU_DEPENDENCIES=libvirt-bin python-libvirt python-simplestreams \
2 python-lxml python-pyinotify python-yaml distro-info cloud-image-utils \
3 qemu-utils ubuntu-cloudimage-keyring socat python qemu-kvm cpu-checker
4
5install-dependencies: .installed
6 @echo ""
7 @contrib/develop
8
9.installed:
10 sudo apt-get install -yu $(UBUNTU_DEPENDENCIES)
11 @touch .installed
12
13
14.PHONY: install-dependencies
15
diff --git a/contrib/develop b/contrib/develop
0new file mode 10075516new file mode 100755
index 0000000..7d415cb
--- /dev/null
+++ b/contrib/develop
@@ -0,0 +1,59 @@
1#!/bin/bash
2
3# This script assists developers when setting up a development environment.
4# It is intended to be sourced, but will provide useful informaton if
5# executed.
6
7function _is_sourced() {
8 [[ "${BASH_SOURCE[0]}" != "$0" ]]
9}
10
11function _run_and_log() {
12 echo "Executing: $@"
13 "$@"
14}
15
16function _colon_separated_list_contains() {
17 echo "$1" | tr ':' '\n' | grep -qFx "$2"
18}
19
20function _path_contains() {
21 _colon_separated_list_contains "$PATH" "$1"
22}
23
24function _pythonpath_contains() {
25 _colon_separated_list_contains "$PYTHONPATH" "$1"
26}
27
28if ! _is_sourced; then
29 echo "You may source the 'contrib/develop' script to get started."
30 echo "Doing so will set up your PATH and PYTHONPATH for development."
31 echo ""
32 echo "For example:"
33 echo " . $0"
34 echo ""
35fi
36
37SOURCE="${BASH_SOURCE[0]}"
38SCRIPT_DIR="$(dirname "$(readlink -f "$SOURCE")")"
39SANDBOX="$($SCRIPT_DIR/get-sandbox-path)"
40
41if ! _pythonpath_contains "$SANDBOX"; then
42 if [ "$PYTHONPATH" != "" ]; then
43 PYTHONPATH=":$PYTHONPATH"
44 fi
45 _is_sourced && _run_and_log export PYTHONPATH="${SANDBOX}${PYTHONPATH}" \
46 || echo "'$SANDBOX' needs to be added to the \$PYTONPATH."
47else
48 echo "'$SANDBOX' is already on the \$PYTHONPATH."
49fi
50
51if ! _path_contains "$SANDBOX/bin"; then
52 _is_sourced && _run_and_log export PATH="$SANDBOX/bin:$PATH" \
53 || echo "'$SANDBOX/bin' needs to be added to the \$PATH."
54else
55 echo "'$SANDBOX/bin' is already on the \$PATH."
56fi
57
58# Force exit with a good status if we weren't sourced (don't confuse 'make').
59_is_sourced || exit 0
diff --git a/contrib/get-sandbox-path b/contrib/get-sandbox-path
0new file mode 10075560new file mode 100755
index 0000000..6a9882b
--- /dev/null
+++ b/contrib/get-sandbox-path
@@ -0,0 +1,4 @@
1#!/bin/bash -e
2
3cd "$(dirname "$(readlink -f "$0")")"/.. > /dev/null
4echo $PWD

Subscribers

People subscribed via source and target branches