Opened 10 months ago

Closed 10 months ago

Last modified 10 months ago

#26811 closed defect (fixed)

tox fails with pip 10

Reported by: dmr Owned by: dmr
Priority: Medium Milestone:
Component: Core Tor/Stem Version:
Severity: Normal Keywords: dev testing
Cc: Actual Points:
Parent ID: Points:
Reviewer: atagar Sponsor:

Description

Trying to run tox in a fresh stem workspace (no .tox dir, which contains the virtualenvs) now fails.

Failure output with pip 10:

no such option: --allow-all-external
ERROR: InvocationError: 'path/to/pip install --allow-all-external -e .'

Here's a deprecation note from a tox / pip 9 virtualenv:

py27 runtests: commands[0] | pip install --allow-all-external -e .
DEPRECATION: --allow-all-external has been deprecated and will be removed in the future. Due to changes in the repository protocol, it no longer has any effect.

With pip 10 (somewhat recently released), this --allow-all-external flag seemingly got removed.

atagar: I'll have a patch for this shortly!

Child Tickets

Change History (4)

comment:1 Changed 10 months ago by dmr

Here's a patch to remove --allow-all-external. tox now runs again:
Pull request (branch head 8eed973786a81b103f03398d92af51aafb69807c)

However, this patch now produces unexpected output for py27:

Installing collected packages: stem
  Found existing installation: stem 1.6.0.dev0
    Not uninstalling stem at path/to/gitroot, outside environment path/to/gitroot/.tox/py27
    Can't uninstall 'stem'. No files were found to uninstall.
  Running setup.py develop for stem
Successfully installed stem

In contrast to fine output in py35:

Installing collected packages: stem
  Found existing installation: stem 1.6.0.dev0
    Uninstalling stem-1.6.0.dev0:
      Successfully uninstalled stem-1.6.0.dev0
  Running setup.py develop for stem
Successfully installed stem

I'm not sure why there's a difference between python2 and python3.
The tests all appear to work fine.

I've ran this patch locally for a few weeks to check further whether this has any effect on updating the stem installation. As far as I can tell, there's no negative effect - changing the test code results in different tests (as expected) and changing the implementation results in different test results (as expected).

comment:2 Changed 10 months ago by dmr

Reviewer: atagar
Status: assignedneeds_review

Setting to needs_review, but if you want to push it back to me for more investigation, let me know. I would think the patch can't be harmful to include, though - at least to get pip10 environments working.

I'm not convinced that the pip install -e . line needs to be in the tox.ini config at all, as I'm pretty sure tox will install things just fine without it, but I didn't have the time to explore that route further at the present.

So I think the best option moving forward in the interim is to take the patch, and file another ticket to look further into it at a later point. (But let me know your thoughts!)

comment:3 Changed 10 months ago by atagar

Resolution: fixed
Status: needs_reviewclosed

Thanks Dave, sounds good. Patch pushed.

comment:4 in reply to:  2 Changed 10 months ago by dmr

Replying to dmr:

I'm not convinced that the pip install -e . line needs to be in the tox.ini config at all, as I'm pretty sure tox will install things just fine without it, but I didn't have the time to explore that route further at the present.

So I think the best option moving forward in the interim is to take the patch, and file another ticket to look further into it at a later point. (But let me know your thoughts!)

Filed as #26822

Note: See TracTickets for help on using tickets.