Skip to content
This repository has been archived by the owner on Mar 19, 2024. It is now read-only.

Junk leftover when installing a new version after installing github tarball. #50

Closed
mythmon opened this issue Sep 30, 2014 · 10 comments · Fixed by #55
Closed

Junk leftover when installing a new version after installing github tarball. #50

mythmon opened this issue Sep 30, 2014 · 10 comments · Fixed by #55
Assignees

Comments

@mythmon
Copy link
Contributor

mythmon commented Sep 30, 2014

  1. Make and activate a virtualenv.

  2. Install something with a github url.

    $ cat github.txt
    # elasticutils: tags/v0.9.1^0
    # sha256: OTZ4mjjiLzXDRK7mbZ5hzOSR7v62NYpUYS_FYNKBoPo
    https://github.com/mozilla/elasticutils/archive/641ccade9cfba1415809077fad674b41b436d174.tar.gz#egg=elasticutils
    $ peep install -r github.txt
    
  3. Install the same library, but a new version, from pypi

    $ cat pypi.txt
    # sha256: CnOPMnGcdnU38yYRjBBcIZ5nfna3hUfthHxqqvvKDGU
    elasticutils==0.10.1
    $ peep install -r pypi.txt
  4. Observe that the metadata for the both the first and second installation of the package exist at once.

    $ ls venv/lib/python2.6/site-packages/
    elasticutils/                       pip-1.5.6.dist-info/       easy_install.pyc
    elasticutils-0.9.1-py2.6.egg-info/  setuptools/                peep.py
    elasticutils-0.10.1.dist-info/      setuptools-3.6.dist-info/  pkg_resources.py
    peep-1.3-py2.6.egg-info/            _markerlib/                pkg_resources.pyc
    pip/
  5. Be sad.

I'm not sure if the contents of the actual package (venv/lib/python2.6/site-packages/elasticutils) is entirely the new version, or if it is a mash-up of the old version and the new version. I don't know elasticutils well enough to tell right now.

When installing from pypi, peep/pip opted to download a wheel file. I'm not sure if this is relavent.

Pip does not have this behavior. It's logs contain a few lines that indicate it is removing old versions before installing the new versions. Peep's logs do not contain any mention of this.

Installing collected packages: elasticutils, elasticsearch, six
  Found existing installation: elasticutils 0.9.1
    Uninstalling elasticutils:
      Successfully uninstalled elasticutils
  Found existing installation: elasticsearch 0.4.5
    Uninstalling elasticsearch:
      Successfully uninstalled elasticsearch
Successfully installed elasticutils elasticsearch six
@mythmon
Copy link
Contributor Author

mythmon commented Sep 30, 2014

This issue is related to this SUMO bug https://bugzilla.mozilla.org/show_bug.cgi?id=1074861

@willkg willkg self-assigned this Oct 1, 2014
@willkg
Copy link
Collaborator

willkg commented Oct 1, 2014

I verified @mythmon's findings. pip works fine, peep does not.

Grabbing this one to look into because ick.

@erikrose
Copy link
Owner

erikrose commented Oct 1, 2014

I wouldn't be surprised if it had to do with wheels. That's a new code path, after all. One way or another, though, pip is installing the second package without removing the first. peep doesn't install anything; it just calls pip. Thanks for having a look, Will.

@willkg
Copy link
Collaborator

willkg commented Oct 1, 2014

The eminent @erikrose is right on. If I do a strict reproduction of what peep is doing where it downloads the item first, then installs from the file it downloaded, then I can reproduce the problem with pip, too.

I trolled the pip issue tracker and I'm pretty sure it's this issue:

pypa/pip#1825

That's fixed in their develop branch, but hasn't made it into a release, yet.

If I add the --no-use-wheel argument to the peep command line, it works super duper.

I think we have a few options in rough order of difficulty:

  1. add a note to the documentation that if you're using pip <= 1.5.6, you should add a --no-use-wheel argument because otherwise it hoses upgrading
  2. change the peep code to check to see if we're using problematic versions of pip (>= ? and <= 1.5.6) and if so, pass the --no-use-wheel argument
  3. figure out where this is fixed in the develop branch and then add some code to peep to monkeypatch pip

I'm inclined to go with option 1. It's easy to do and people can stop using the argument when they're using a version of pip that works right and we don't have to make any tricky code changes. Further, I'm pretty sure @mythmon and @dean ran into a problem where sometimes pip downloads the wheel and sometimes not, so you have to have two hashes in the requirements file otherwise you occasionally get hash failures--this would alleviate that, too.

Option 2 would prevent a bunch of errors from users who didn't read the docs or didn't read that part of the docs or didn't read the updated docs, etc. So it's probably more user-friendly, maybe?

Option 3 is there because the other two options were lonely and it's always nice to have at least three options even if one of them is a "wtf--worst idea ever" option.

Any thoughts?

@erikrose
Copy link
Owner

erikrose commented Oct 1, 2014

I like 2 because…

  • Nobody ever reads anything. (If I had a nickel for every time I've had to point people to the multiple-hash thing…)
  • We put in one "if", and then no one, anywhere, ever has to track when their sysadmin or distro upgrades pip and change their deploy scripts to match.

One concern, though: are there intersections of command-line options that will fight with --no-use-wheel or its absence? For example, if the user passes --no-use-wheel himself and we add another one, will pip freak out? Was there a --use-wheel (positive) in previous versions of pip that will clash with our added --no-use-wheel?

willkg added a commit to mozilla/fjord that referenced this issue Oct 1, 2014
After working on bug #1074861 (peep issue #50), I decided we shouldn't use
wheels for now. The crux of the issue is that if you have a github-tarball
based requirement and then switch to a pypi based requirement which
downloads a wheel, then pip doesn't clean things up properly and you end
up with cruft in your virtual environment which includes .py, .pyc and
metadata files. Ew.

Further Mike and Dean said they've had instances where peep downloads a
wheel and then a non-wheel, so you need to have both hashes in the
requirements file. Ew.

Because of these two issues, it's easier to not do wheels. So I updated
peep to the latest and then modified it so it's always passing
--no-use-wheel to pip.

I also fixed the shas in the requirements file that were for wheels.

Plus when testing all of this, I had a problem where the vagrant
provisioning script was creating /root/.pip and dying with a permission
issue. So I fixed that, too.

* https://bugzilla.mozilla.org/show_bug.cgi?id=1074861
* erikrose/peep#50
@willkg
Copy link
Collaborator

willkg commented Oct 1, 2014

Excellent questions!

I have no idea how to figure that out short of downloading every version of pip since they added wheel support to see what the available flags were. Is there a better way?

@erikrose
Copy link
Owner

erikrose commented Oct 3, 2014

I'd be happy to settle for making sure passing --no-use-wheel manually doesn't crash peep/pip.

@willkg
Copy link
Collaborator

willkg commented Oct 6, 2014

So, we have two outstanding questions here:

  1. are there other pip arguments that depend on or are affected by --no-use-wheel such that passing it in causes issues?
  2. which versions of pip support wheels and have the --no-use-wheel argument?

Once we have those two answered, I'm game for fixing this.

If someone else could spend the time to figure out the answers, that'd be great! Otherwise I'll try to get to it this week.

@mythmon
Copy link
Contributor Author

mythmon commented Oct 6, 2014

SUMO had some issues with pip versions so I went and read through the release notes to determine which versions supported wheels and which did not. I think this is an exhaustive list of which released, stable versions support wheels as of today. In particular, it is everything after 1.4.0. For completeness, these are all the versions that support wheels today:

  • 1.4.0
  • 1.4.1
  • 1.5.0
  • 1.5.1
  • 1.5.2
  • 1.5.3
  • 1.5.4
  • 1.5.5
  • 1.5.6

Edit: the trouble on SUMO was with pip versions, not peep versions.

@willkg
Copy link
Collaborator

willkg commented Oct 6, 2014

I looked at the pip install arguments and didn't see anything exciting. Further, I think this suggests there aren't other wheel-related arguments currently:

http://pip.readthedocs.org/en/latest/user_guide.html#installing-from-wheels

Given that and @mythmon's comment, I think we can add something like:

if pip.version >= 1.4.0:
    args += ['--no-use-wheel']

The theory being that we ban wheels for now and once they do a pip release that fixes it, we can edit the above check to be upper-bounded by whatever the fixed version is.

Does that sound good?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants