Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

skipsdist and usedevelop don't play well together #270

Closed
pytoxbot opened this issue Sep 17, 2016 · 16 comments
Closed

skipsdist and usedevelop don't play well together #270

pytoxbot opened this issue Sep 17, 2016 · 16 comments
Labels
help:wanted Issues that have been acknowledged, a solution determined and a PR might likely be accepted. needs:discussion It's not quite clear if and how this should be done

Comments

@pytoxbot
Copy link

I'm trying to do usedevelop and skipsdist at the same time but it feels that they are mutually exclusive with tox.

I have 2 tasks, py34 and pep8. When I run py34 I want to use usedevelop=true but when I run pep8 I don't want my project to be installed at all and use skipsdist=true.

Please correct me if I'm wrong: skipsdist is only a global setting recognized under the section [tox] which would then skip the sdist step for all [testenv] sections which in turns would ignore any usedevelop=true flag since it's tox will not even try to install the testenv in the first place.

Can we have skipsdist be recognized under a [testenv]?

#!ini

[tox]
envlist = py34, pep8

[testenv]
usedevelop = true
deps =
    -rtest-requirements.txt
commands =
    py.test {posargs:tests/}

[testenv:pep8]
skipsdist = true
basepython = python3.4
deps = flake8
commands =
    flake8 pricingsvc/ --max-line-length=100

[pytest]
addopts =
    --junitxml=junit.xml
    --cov=pricingsvc --cov-report=xml --cov-report=term-missing
    --verbose
@pytoxbot
Copy link
Author

Original comment by mhils

FWIW, we were able to get the desired behaviour by using a tox configuration that looks like this:

#!python

[tox]
skipsdist = True

[testenv]
usedevelop = True

[testenv:lint]
usedevelop = False

For reference, our full configuration is here.

@pytoxbot
Copy link
Author

Original comment by mrmachine

Indeed, I also just tried skip_install = True in a testenv:flake8 section, but my project was still installed. Is this not what it is for? flake8 should be able to lint without having all my project's dependencies (specified in setup.py) installed into the venv (which takes a while)...

@pytoxbot
Copy link
Author

Original comment by @aconrad

The skip_install = true seems to have no effect (or at least not how I intended it to have). Here's my current tox.ini

#!ini

[tox]
envlist = py34, pep8

[testenv]
usedevelop = true
deps =
    -rtest-requirements.txt
commands =
    py.test {posargs:tests/}

[testenv:pep8]
skip_install = true
basepython = python3.4
deps = flake8
commands =
    flake8 pricingsvc/ --max-line-length=100

[pytest]
addopts =
    --junitxml=junit.xml
    --cov=pricingsvc --cov-report=xml --cov-report=term-missing
    --verbose

and when I run tox -e pep8 -v it still seems to install the package (line pep8 develop-inst) even though skip_install = true:

#!bash

$ tox -e pep8 -v
using tox.ini: /opt/src/PricingSvc/tox.ini
using tox-2.1.1 from /opt/webapp/pricingsvc/local/lib/python3.4/site-packages/tox/__init__.py
pep8 create: /opt/src/PricingSvc/.tox/pep8
  /opt/src/PricingSvc/.tox$ /opt/webapp/pricingsvc/bin/python3.4 -m virtualenv --python /opt/webapp/pricingsvc/bin/python3.4 pep8 >/opt/src/PricingSvc/.tox/pep8/log/pep8-0.log
pep8 installdeps: flake8
  /opt/src/PricingSvc$ /opt/src/PricingSvc/.tox/pep8/bin/pip install flake8 >/opt/src/PricingSvc/.tox/pep8/log/pep8-1.log
pep8 develop-inst: /opt/src/PricingSvc
  /opt/src/PricingSvc$ /opt/src/PricingSvc/.tox/pep8/bin/pip install -e /opt/src/PricingSvc >/opt/src/PricingSvc/.tox/pep8/log/pep8-2.log
pep8 installed: -f /opt/pip/wheels,colorama==0.3.3,decorator==3.4.2,flake8==2.4.1,gunicorn==19.1.1,httplib2==0.9.1,mccabe==0.3,msgpack-python==0.4.6,ordereddict==1.1,PasteDeploy==1.5.2,pep8==1.5.7,-e [email protected]:devmonkeys/PricingSvc.git@b9e059f8416369615feccaa1af84f6434153b8c9#egg=pricingsvc-origin_tox_changes,pycountry==1.10,pycrypto==2.6.1,pyflakes==0.8.1,pyramid==1.5.2,python-dateutil==2.4.2,pytz==2015.4,PyYAML==3.11,RAttr==0.6.3,repoze.lru==0.6,requests==2.7.0,simplejson==3.8.0,six==1.9.0,SMCrypto==0.2.21,smlib.auth==3.0.0,smlib.config==0.3,smlib.locale==1.0.12,smlib.log==1.3.1,smpackage==0.22,tldextract==1.5.1,translationstring==1.3,validictory==1.0.0,venusian==1.0,waitress==0.8.9,WebOb==1.4.1,wheel==0.24.0,zope.component==4.2.2,zope.deprecation==4.1.2,zope.event==4.0.3,zope.i18n==4.0.0,zope.i18nmessageid==4.0.3,zope.interface==4.1.2,zope.schema==4.4.2
pep8 runtests: PYTHONHASHSEED='248291095'
pep8 runtests: commands[0] | flake8 pricingsvc/ --max-line-length=100
  /opt/src/PricingSvc$ /opt/src/PricingSvc/.tox/pep8/bin/flake8 pricingsvc/ --max-line-length=100
__________________________________________________________________________________________________________________ summary ___________________________________________________________________________________________________________________
  pep8: commands succeeded
  congratulations :)

What am I missing?

@pytoxbot
Copy link
Author

Original comment by @aconrad

Actually, I just noticed that there was a skip_install = true flag available for [testenv] (new in tox 1.9), I'll try using this and see if that solves my issue.

@obestwalter
Copy link
Member

@aconrad can we close this as resolved then?

@obestwalter obestwalter added the needs:discussion It's not quite clear if and how this should be done label Dec 4, 2016
@ferdonline
Copy link

ferdonline commented Aug 8, 2017

I confirm that usedevelop breaks skip_install = True
Running with tox 2.7

(sparkenv) [leite@centos-linux-7 pyspark]$ tox -e flake8
flake8 create: /media/psf/Home/dev/Functionalizer/pyspark/.tox/flake8
flake8 installdeps: flake8
flake8 develop-inst: /media/psf/Home/dev/Functionalizer/pyspark
ERROR: invocation failed (exit code 1), logfile: 
_________________________________________________________________________________________
summary
_________________________________________________________________________________________
ERROR:   flake8: InvocationError: /media/psf/Home/dev/Functionalizer/pyspark/.tox/flake8/bin/pip install -e /media/psf/Home/dev/Functionalizer/pyspark (see /media/psf/Home/dev/Functionalizer/pyspark/.tox/flake8/log/flake8-2.log)

By commenting out usedevelop = True everything works as expected.

Can someone change the title to mention "usedevelop" as well?

@obestwalter
Copy link
Member

obestwalter commented Aug 8, 2017

Hi @ferdonline

I confirm that usedevelop breaks skip_install = True

What do you expect should happen, when you use both of them together? Can you post your tox.ini to reproduce and explain what the motivation behind using them together is?

I would expect that each option individually does what it should and used together ideally throws an error. IMO those two options should be mutually exclusive. It makes no sense to me to instruct tox to skip the installation and at the same time install the package in development mode.

I created a minimal project with a simple setup.py and a tox.ini which runs the three scanarios:

[tox]
skipsdist = True
envlist = usedevelop,skip_install,both

[testenv:usedevelop]
skip_install = False
usedevelop = True
commands = pip freeze

[testenv:skip_install]
skip_install = True
usedevelop = False
commands = pip freeze

[testenv:both]
skip_install = True
usedevelop = True
commands = pip freeze

output was:

usedevelop develop-inst-nodeps: /home/oliver/work/tox/tox-reproducers
usedevelop installed: -e [email protected]:obestwalter/tox-reproducers.git@048f5f3e745214fdc1d49857ffec31ec20568156#egg=testpackage
usedevelop runtests: PYTHONHASHSEED='2874166107'
usedevelop runtests: commands[0] | pip freeze
-e [email protected]:obestwalter/tox-reproducers.git@048f5f3e745214fdc1d49857ffec31ec20568156#egg=testpackage
skip_install installed: 
skip_install runtests: PYTHONHASHSEED='2874166107'
skip_install runtests: commands[0] | pip freeze
both develop-inst-nodeps: /home/oliver/work/tox/tox-reproducers
both installed: -e [email protected]:obestwalter/tox-reproducers.git@048f5f3e745214fdc1d49857ffec31ec20568156#egg=testpackage
both runtests: PYTHONHASHSEED='2874166107'
both runtests: commands[0] | pip freeze
-e [email protected]:obestwalter/tox-reproducers.git@048f5f3e745214fdc1d49857ffec31ec20568156#egg=testpackage
___________________________________ summary ____________________________________
  usedevelop: commands succeeded
  skip_install: commands succeeded
  both: commands succeeded
  congratulations :)

or see: https://github.com/obestwalter/tox-reproducers/tree/982f18cd0cd62adbea9e53b577bbaea10a3c116a

So I don't get your error, but I also do not get an error that informs me that usedevelop and skip_install are mutually exclusive. instead it installs the package in development mode.

As I understand the problem the actual bug in the way those two options behave together is one of documentation and handling.

Can someone change the title to mention "usedevelop" as well?

It is in the title already - or what do you mean?

@ferdonline
Copy link

ferdonline commented Aug 8, 2017

Hi thanks for your reply.
Consider this case, which is I think can be quite common:

[tox]
skipdist = True

[testenv]
usedevelop = True   # By default all tests shall run with develop
changedir = tests
commands =
    py.test {posargs}
deps =
    -r{toxinidir}/test-requirements.txt
    -r{toxinidir}/requirements.txt

[testenv:flake8]
changedir = {toxinidir}
deps = flake8
skip_install = True   # In this particular env don't install the package, we just need the env
commands = flake8 setup.py mypackage tests

As you say, the options seem exclusive, but due to "inheritance" we end up with a configuration that produces a strange effect - installing in develop when we instructed not to install at all. And typically rules defined in a tighter scope should have precedence. Due to complex deps, the package installation, which was not intended, fails.

It is in the title already - or what do you mean?

I meant skip_install, sorry

I looked into the source and can be easily fixed. I'm running the tests. I can make a PR later.
Thanks

@obestwalter
Copy link
Member

hmm that should not make any difference (I mean I still don't understand why you get an error and I would suspect that this error has nothing to two with this issue).

The inheritance is the same as if you were putting them together in the env. This tox.ini is the same like the one I posted above and yields the same result as above when I run it:

[tox]
skipsdist = True
envlist = usedevelop,skip_install,both

[testenv]
usedevelop = True

[testenv:usedevelop]
skip_install = False
commands = pip freeze

[testenv:skip_install]
skip_install = True
usedevelop = False
commands = pip freeze

[testenv:both]
skip_install = True
commands = pip freeze

Regarding the PR: how would you want to fix it? The easiest fix would be to update the docs explaining that using them together makes no sens and if they are used together usedevelop wins over skip_install. Everything else would need further discussion, I think.

@ferdonline
Copy link

ferdonline commented Aug 8, 2017

and if they are used together usedevelop wins over skip_install.

I think that's the point that needs reaching an agreement.

Considering that (directly or indirectly - like in my config) one arrives to have both

skip_install = True
usedevelop = True

in one environment. To my mind it seems much more reasonable to honour skip_install, because it doesn't really disrespect usedevelop. I see it clear that usedevelop is an install mode, so if there's nothing to install it's still honoured.
Best

@obestwalter obestwalter changed the title skipdist and usedevelop don't play well together skipsdist and usedevelop don't play well together Aug 9, 2017
@obestwalter
Copy link
Member

To my mind it seems much more reasonable to honour skip_install, because it doesn't really disrespect usedevelop. I see it clear that usedevelop is an install mode, so if there's nothing to install it's still honoured.

That makes a lot of sense to me. So we should change that behaviour I guess.

I opened an issue for that as this is not quite the same as this issue: #571

@ferdonline
Copy link

Alright, cool!
Re-reading this issue it seems to me that @aconrad just wanted skip_install instead of skipdist.
In that case I assume this issue doesn't make sense.

But for the sake of having it right, concerning only skipdist, the code currently does this

skipsdist | usedevelop | RESULT
--------------------------------------
True      | True       | develop (seems ok)
True      | False      | finishvenv (? - shouldnt it install without creating a sdist?)
False     | True       | develop (seems ok)
False     | False      | install (seems ok)

@obestwalter
Copy link
Member

True | False | finishvenv (? - shouldnt it install without creating a sdist?)

If you don't build an sdist and don't want to install in developmen mode, there is nothing you could install, so nothing should be installed. I would say this is also correct.

@ferdonline
Copy link

ferdonline commented Aug 9, 2017

If you don't build an sdist and don't want to install in development mode, there is nothing you could install

I would have guessed it would install with "setup.py install"
Anyway I don't know if that makes sense as a use case. Eventually we can simply emit a warning saying that nothing is gonna be installed and they should use skip_install instead.

@obestwalter
Copy link
Member

obestwalter commented Aug 9, 2017

I would have guessed it would install with "setup.py install"

I would have thought that setup.py install does basically the same as an sdist build and then an install, so for me this would be the same as skipsdist = False together with usedevelop = False. But your expectation is just as valid (maybe more valid than mine even, as I just have made peace with certain maybe not so intuitive things tox has to offer ...).

I think this combination was useful before we had skip_install, as this basically doubles for that. But as we have skip_install now, it might actually be a good idea to do a normal install like you said instead ... and then document what happens when, maybe even using your nice table :)

@gaborbernat gaborbernat added the help:wanted Issues that have been acknowledged, a solution determined and a PR might likely be accepted. label May 2, 2019
@gaborbernat
Copy link
Member

I think this now has been fixed in tox 4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help:wanted Issues that have been acknowledged, a solution determined and a PR might likely be accepted. needs:discussion It's not quite clear if and how this should be done
Projects
None yet
Development

No branches or pull requests

4 participants