Standardize python setup, ensure homu gets python3

[aneeshusa]

• Trusty comes with python3, pip and virtualenv by default, but write
  states defensively to install these dependencies as required and require
  the pip or virtualenv states as used by other states.
• Homu requires a python3 toolchain, use virtualenv-3.4 and pass python:
  python3 to get the correct setup (including pip3 inside the venv).
• Update the style guide example to use pkg.installed instead of
  pip.installed to avoid muddling it with a require: pip.

Fixes #142.

[Manishearth]

In my experience I needed to pip3 install virtualenv before I got virtualenv3. Should we make two keys here, one with bin_env: pip and one with bin_env: pip3?

[aneeshusa]

I just tested this in a fresh vagrant box. It started out without pip or virtualenv (any version). I ran

1. sudo apt-get install python-pip
2. sudo pip install virtualenv
   This created /usr/local/bin/virtualenv and /usr/local/bin/virtualenv-3.4 executables, the latter of which works fine for the homu venv - I peeked inside the vagrant box and the homu venv had python3 and pip3.

bin_env is used when you want to run pip inside a virtualenv, such as in homu/init.sls, so I'm not sure what you are proposing with the two keys. Could you please clarify?

[Manishearth]

Are the default python/pip/virtualenv the 2.7 ones?

bin_env is for both selecting a virtualenv or selecting a custom binary to run. It lets you say "Install this pip package, but use the pip3.4 binary". https://docs.saltstack.com/en/latest/ref/modules/all/salt.modules.pip.html

[aneeshusa]

Yes, on Ubuntu the defaults are all 2.7. Ubuntu is not planning to change this until PEP 394 says otherwise.

bin_env: Right, I'm using this but in a different way. The approach I'm taking is to use virtualenv.managed and pass in python3 in homu/init.sls; salt automatically will install pip3 in the virtualenv (pip3 is bundled with python starting with python3.4). Then, I use pip.installed and set bin_env to the virtualenv location for homu's dependencies, which means it ends up using pip3 under the hood.

Also, for future reference you'll want to use the state documentation, not the module docs :)

[Manishearth]

Okay, sounds good. So when you say python: python3 in the homu setup below, does that mean that it will also use virtualenv3? Because homu doesn't even install properly with an older virtualenv version, it needs to use pip3, python3, and virtualenv3. (Not sure if virtualenv2 even lets you create a python3 env, but it's good to be certain)

[Manishearth]

r? @larsbergstrom @edunham

I don't have the salt-fu to be completely sure of what's going on here 😄

[aneeshusa]

If the salt states aren't clear, let me know what's confusing! These should be relatively easy to read as high-level declarative descriptions.

[aneeshusa]

Also, I don't know anything about how installing python, pip, and virtualenv works on OS X (versions 2 and 3). Are they installed by default or can/should they be installed via homebrew? I don't have a Mac so I'd appreciate some help figuring that out to address the build failures.

[Manishearth]

We need pip3 only on servo-master

[Manishearth]

(which is linux)

[Manishearth]

Overall LGTM, but I'd like someone who understands salt more than I do (which is nearly nothing) to look at it too.

Also, we might want to test this out from scratch on a prod instance. At the very least we should backup the homu dir, delete the original, and ensure it's created from scratch correctly.

(A related issue is that servo-master is never spun up from scratch unlike the other machines, so the salt config may not reflect the actual steps needed for servo-master to be set up -- like python3 being missing from salt. We discussed this the other day and might be testing this somehow soon)

[aneeshusa]

If we only need python3 on linux then I'm probably going to change how I do this a bit.

What base image are you using for prod? I can probably modify the Vagrant boxes to use that instead so you can test it locally. (It works fine AFAICT with the ubuntu/trusty64 boxes).

[aneeshusa]

@Manishearth the venv_bin option is what tells salt to use virtualenv-3.4, and the python option tells virtualenv to install python3 (and pip3) inside the virtualenv.

[Manishearth]

Oh, right, I didn't notice that, sorry 😄

[Manishearth]

I'm okay with python3 being available on mac too as long as buildbot doesn't break.

System is trusty:

root@servo-master:~# lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description:    Ubuntu 14.04.3 LTS
Release:    14.04
Codename:   trusty

[edunham]

Reviewed 7 of 7 files at r1.
Review status: 5 of 7 files reviewed at latest revision, 3 unresolved discussions.

---

^{common/init.sls, line 14 [r1] (raw file):}
I'm surprised that everything installs successfully without python3-dev here, but if you've tested it in a VM and it works, we should be good to go.

---

Comments from the review on Reviewable.io

[aneeshusa]

Testing in the servo-master Vagrant VM, the provisioning succeeds without errors, and homu successfully starts up. (If you ssh in, service homu status says homu is dead, but sudo less +F /var/log/upstart/homu.log reveals that it dies very quickly because the fake github credentials are no good.) You may want to do some testing of your own but it looks like python3-dev is not needed.

[Manishearth]

Yeah, homu should fail early if the venv is not set up correctly -- if the virtualenv is the wrong version the egg doesn't even unpack correctly, and if you hack it so that it does, the Python parser will choke on the * parameters. If it's getting to the ping-github stage everything should be good.

[aneeshusa]

Also note that the latest commit will restart various services when their packages are updated, to help keep servo-master's running services in sync with what's on disk. If you'd prefer to manually restart any of these packages, just let me know which ones and I'll drop them back down from watch to require. (Ideally they'd all be crash/restart safe 😄)

[Manishearth]

Restarting homu/buildbot at the wrong time can break things.

Then again, we don't really update packages much (oh the horror)

[aneeshusa]

OK, that's what I was thinking based on the wiki. With this PR, homu won't restart if the git checkout is updated, but it will restart automatically if either of its configuration files are updated - let me know if you want me to have it never restart automatically.

[Manishearth]

I thought that was already the case?

We only really update the conf files from salt, so homu gets restarted anyway

[aneeshusa]

Hmm, looks like you're right. I rearranged the states a bit and thought I changed it.

[larsbergstrom]

@Manishearth @aneeshusa Where are we at with this? Is it OK to leave, or are we still waiting for some changes and a rebase?

[aneeshusa]

I think this was OK, but I'll give it a rebase and take a look.

[larsbergstrom]

Thanks, no rush! I was just trying to look across our repos and see what PRs are stalled, especially if it's on the reviewer-side :-)

[aneeshusa]

I actually had just sat down to put in some work for this repo, I blocked off a few hours for it today :)

[aneeshusa]

OK, ready for review.

[Manishearth]

 _____________________________________ 
< a very essential dependency, indeed >
 ------------------------------------- 
        \   ^__^
         \  (oo)\_______
            (__)\       )\/\
                ||----w |
                ||     ||

[Manishearth]

This looks good to me.

[larsbergstrom]

@bors-servo r+

(lgtm, too)

[bors-servo]

📌 Commit a3baeaa has been approved by larsbergstrom

[bors-servo]

⌛ Testing commit a3baeaa with merge dcb4771...

[bors-servo]

☀️ Test successful - travis