tox-dev / tox-pipenv

A pipenv plugin for Tox
MIT License
124 stars 30 forks source link

Do not use pipenv for packages specified in tox.ini #37

Closed jleclanche closed 6 years ago

jleclanche commented 6 years ago

I've been trying to convert one of my Django apps to use pipenv and I'm hitting issues with tox because, as I switch to it, I lose the ability for tox to pick up the app's dependencies which were previously in setup.cfg, setup.py or requirements.txt.

I tried this plugin hoping it would help, and maybe I'm misunderstanding what problem it's trying to solves, so if I am, forgive this ticket, though there is still an unsolved problem then.

Here's a typical Django app using tox and pipenv:

  1. Dependencies for the project itself are defined in Pipfile
  2. A lockfile is distributed alongside the Pipfile
  3. tox.ini will look something like this:
[tox]
skipsdist = True
envlist = py36-django{20,master}, flake8, mypy

[testenv]
commands = pytest --showlocals {posargs}
deps =
    django20: Django>=2.0,<2.1
    djangomaster: https://github.com/django/django/archive/master.tar.gz
    pytest
    pytest-django
    coveralls

[testenv:flake8]
skip_install = True
commands = flake8
deps =
    flake8
    flake8-isort
    flake8-quotes

[testenv:mypy]
commands = mypy --ignore-missing-imports
deps =
    mypy

Now there's a fundamental conflict between tox and pipenv here. pipenv will install what's in the production lockfile. tox will attempt to install django 2.0, then django 2.1, django master, etc. tox will also install flake8 and mypy for the other environments, which will add them to the Pipfile and the lockfile. The --skip-lock argument to pipenv will prevent the lockfile from being updated at least, but they still shouldn't be added to the pipfile.

Fundamentally, all pipenv does is call pip and update the pipfile/lockfile accordingly. We want pipenv to create the environment, but not to install the packages provided by tox otherwise you will end up with modified pipfiles.

So my suggestion is something like this:

  1. Call pipenv to create the initial virtualenv (including the packages) unless skip_install = True.
  2. Afterwards, let tox use pip as it normally would to install the extra packages specified in tox deps.

Does that make sense, or am I completely off the mark?

andrlik commented 6 years ago

Seconded. I'm having this issue as well. I'm leery of tox doing something that will change the Pipfile and Pipfile.lock.

ferndot commented 6 years ago

This actually causes an issue with my tox.ini because pipenv does not seem to understand the syntax that is being passed to it:

deps =
    django-111: Django>=1.11,<1.12
    django-20: Django>=2.0,<2.1

It would be nice to have at least a flag to turn this on.

jleclanche commented 6 years ago

So ideally, pip should have an argument to install from a pipfile, just like it has -r to install from a requirements.txt.

Seeing as tox-pipenv looks unmaintained right now, I've been thinking about hooking into tox with like a tox-pipfile plugin, which instead of going the route that tox-pipenv takes, simply allows getting a list of deps from a Pipfile.

It'd look something like this:

deps =
    -p{toxinidir}/Pipfile
    pytest
    pytest-mock
pipfile_use_develop = False  # Default True
tonybaloney commented 6 years ago

hi @jleclanche I'm going through the issues now and trying to understand this one. Pipenv has changed a lot since this plugin was written, namely in the way it now plays nicely inside virtualenvironments.

Thanks for raising the issue

tonybaloney commented 6 years ago

Hi,

Lots of ideas in this discussion, I'm trying to whittle them down into use cases.

  1. When adding extra dependencies to deps in tox.ini you don't want those to be added to the root Pipfile or Pipfile.lock since they're only used for the testing process from tox, e.g. coverage tools, pytest etc.

I'm not clear why you would have Django specified in Tox, are you testing multiple versions of Django from Tox?

jleclanche commented 6 years ago

are you testing multiple versions of Django from Tox?

Yup that's a very common pattern. Example: https://github.com/dj-stripe/dj-stripe/blob/master/tox.ini

tonybaloney commented 6 years ago

@jleclanche thanks, can you please help me come up with a test case that I can build to demonstrate the issue you're having?

This is the existing process:

  1. The install stage calls {virtualenvpython} -m pipenv --python {python} the ENV for PIPENV_VIRTUALENV is set to the venv for the tox stage, e.g. .tox/py36
  2. If dependencies are in deps, then install-deps stage calls {virtualenvpython} -m pipenv install --dev {package}
  3. pipenv graph is run during the runenvreport stage
  4. the test stage runs {virtualenvpython} -m pipenv run {test command}
jleclanche commented 6 years ago

The tox.ini displayed in the original comment should reproduce the issue:

It's a pretty standard tox.ini among the Django community I believe.

tonybaloney commented 6 years ago

ok, able to repro pretty easily. Just need to setup a test to check Pipfile before and after tox executes to make sure it's not adding the deps into it. There is a environment variable patch which touches a Pipfile in each venv directory. That should be used instead of the one in the root directory.

  1. On setup, clone the Pipfile to venv path, e.g. .tox/pyxx/Pipfile
  2. On execution, use PIPENV_PIPFILE as the venv pipfile, not the root one.
mkdir test-37
cd test-37
vi tox.ini # paste your example
pip3.6 install virtualenv
python3.6 -m virtualenv env/
source env/bin/activate
pip install pipenv tox tox-pipenv
tox -vv  # whilst watching Pipfile
tonybaloney commented 6 years ago

@jleclanche I've built a fix for this in https://github.com/tox-dev/tox-pipenv/pull/44

If you're able to test that would be great.

the plugin will clone the Pipfile into .tox/{venv}/Pipfile and use that for installing any dependencies specified in deps.

I've checked it against the tox ini file you put in this ticket.

tonybaloney commented 6 years ago

Changes within #44 merged and released in 1.6.0.

The plugin better supports environment-specific dependencies without polluting the project Pipfile