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

Use the virtual environment to generate the sdist #507

Closed
dstufft opened this issue Apr 20, 2017 · 5 comments
Closed

Use the virtual environment to generate the sdist #507

dstufft opened this issue Apr 20, 2017 · 5 comments
Assignees
Labels
area:testenv-creation feature:change something exists already but should behave differently level:medium rought estimate that this might be neither easy nor hard to implement

Comments

@dstufft
Copy link

dstufft commented Apr 20, 2017

Currently it appears that tox uses the system Python when creating the sdist (specifically, /usr/bin/python if that's where tox is installed). This means that if you're trying to use a setuptools feature that is newer than what your system provides in your setup.py, then you're bound by what your current environment provides (which is often times fairly old). I think it would be a great idea if tox re-used one of the virtual environments it's creating to build the sdist (honestly it could be any of them) which would give more control over what version of setuptools is being used.

Additionally, I think this would also affect someone trying to use tox with a package whose setup.py only runs on Python 3.

For reference, here is the output I'm seeing that suggests this is what is happening:

+ tox -r -e docs -- --color=yes
GLOB sdist-make: /var/lib/jenkins/jobs/cryptography-pr-docs/workspace/cryptography/setup.py
ERROR: invocation failed (exit code 1), logfile: /var/lib/jenkins/jobs/cryptography-pr-docs/workspace/cryptography/.tox/log/tox-0.log
ERROR: actionid: tox
msg: packaging
cmdargs: ['/usr/bin/python', local('/var/lib/jenkins/jobs/cryptography-pr-docs/workspace/cryptography/setup.py'), 'sdist', '--formats=zip', '--dist-dir', local('/var/lib/jenkins/jobs/cryptography-pr-docs/workspace/cryptography/.tox/dist')]
env: None

Traceback (most recent call last):
  File "setup.py", line 332, in <module>
    **keywords_with_side_effects(sys.argv)
  File "/usr/lib/python2.7/distutils/core.py", line 111, in setup
    _setup_distribution = dist = klass(attrs)
  File "/usr/lib/python2.7/dist-packages/setuptools/dist.py", line 239, in __init__
    self.fetch_build_eggs(attrs.pop('setup_requires'))
  File "/usr/lib/python2.7/dist-packages/setuptools/dist.py", line 264, in fetch_build_eggs
    replace_conflicting=True
  File "/usr/lib/python2.7/dist-packages/pkg_resources.py", line 595, in resolve
    requirements = list(requirements)[::-1]  # set up the stack
  File "/usr/lib/python2.7/dist-packages/pkg_resources.py", line 2605, in parse_requirements
    line, p, specs = scan_list(VERSION,LINE_END,line,p,(1,2),"version spec")
  File "/usr/lib/python2.7/dist-packages/pkg_resources.py", line 2583, in scan_list
    "Expected ',' or end-of-list in",line,"at",line[p:]
ValueError: ("Expected ',' or end-of-list in", "cffi>=1.4.1 ; platform_python_implementation != 'PyPy'", 'at', " ; platform_python_implementation != 'PyPy'")

ERROR: FAIL could not package project - v = InvocationError('/usr/bin/python /var/lib/jenkins/jobs/cryptography-pr-docs/workspace/cryptography/setup.py sdist --formats=zip --dist-dir /var/lib/jenkins/jobs/cryptography-pr-docs/workspace/cryptography/.tox/dist (see /var/lib/jenkins/jobs/cryptography-pr-docs/workspace/cryptography/.tox/log/tox-0.log)', 1)
@warner
Copy link

warner commented Jul 12, 2017

Additionally, I think this would also affect someone trying to use tox with a package whose setup.py only runs on Python 3.

I can confirm this. Tahoe-LAFS is python2-only. We recently changed the tahoe-lafs setup.py to throw a clean exception if loaded with python3 (because the alternative was for zfec, our dependency, to fail with an inscrutable SyntaxError because of a py2-only print "foo" statement). When people accidentally try to install or build tahoe with py3, we want to teach them to use py2, instead of having them wonder why a perfectly valid-looking print statement in an apparently-unrelated package is being treated as a syntax error.

Our tox.ini calls for python2.7, and on platforms where tox uses python2, that works fine. But on most recent Ubuntu platforms, the system /usr/bin/tox is py3-based, and those platforms can no longer run the tests with our recommended instructions ("git clone REPOURL; cd tahoe-lafs; tox").

We don't want to tell users that they must switch to a py2-based tox first (if they knew how to build a py2 virtualenv and install tox into it, they would know enough to just install tahoe into that virtualenv and run trial directly). So we'd really like it if a py3-based tox were still able to run our tests.

Our workaround is to change the tahoe setup.py to only throw the not-on-py3 exception when sys.argv[1] is "install" or "egg-info" (it appears that tox only runs setup.py sdist under the wrong python). But we'd much prefer to see tox use the target python when building the sdist for a target, since there are ways that users could mistakenly use python3 to build tahoe that might bypass the instructional error message we're trying to show them.

(for reference, the tahoe bug is at https://tahoe-lafs.org/trac/tahoe-lafs/ticket/2876)

warner added a commit to warner/tahoe-lafs that referenced this issue Jul 13, 2017
The previous blanket prohibition on py3 for any invocation of setup.py was
breaking our recommended "just run 'tox'" test process on e.g. Ubuntu-16.04
LTS "xenial", which ships with a /usr/bin/tox that is based on py3. This tox
runs "setup.py sdist" to build a tahoe tarball, then uses its py2
virtualenv's "pip" to install it (into the py2 virtualenv). Forbidding even
"setup.py sdist" from running under py3 was causing tox to fail unless it was
a py2-based tox.

This new approach only enforces the py2 check if setup.py was invoked as
"setup.py install" (which users might do), or "setup.py egg_info" (which pip
will do before it does anything else).

I experimented with doing the check inside an EggInfo or "build_py"
subcommand, but the EggInfo class is invoked internally during an sdist. And
we need pip3's egg_info to fail, because if it succeeds, it will proceed to
run egg_info on our dependencies (like zfec, whose setup.py contains
old-style py2-only 'print "foo"' statements, which causes a syntax error,
which is the confusing thing we were trying to head off by providing a
clean+instructional error message early).

This is a workaround for an upstream Tox bug (see
tox-dev/tox#507) that should get fixed (to make tox
run `setup.py sdist` with the virtualenv's python, rather than tox's python).
However since debian/ubuntu don't generally backport non-security bugfixes to
existing releases, a new Tox won't actually help our problem on any of
today's operating systems (it'll be 6-12 months before anyone is helped by a
Tox fix). So even though we'd ideally prefer to fix this upstream and not
here, I think practically speaking we need this workaround anyways.

closes ticket:2876
warner added a commit to warner/tahoe-lafs that referenced this issue Aug 10, 2017
The previous blanket prohibition on py3 for any invocation of setup.py was
breaking our recommended "just run 'tox'" test process on e.g. Ubuntu-16.04
LTS "xenial", which ships with a /usr/bin/tox that is based on py3. This tox
runs "setup.py sdist" to build a tahoe tarball, then uses its py2
virtualenv's "pip" to install it (into the py2 virtualenv). Forbidding even
"setup.py sdist" from running under py3 was causing tox to fail unless it was
a py2-based tox.

This new approach only enforces the py2 check if setup.py was invoked as
"setup.py install" (which users might do), or "setup.py egg_info" (which pip
will do before it does anything else).

I experimented with doing the check inside an EggInfo or "build_py"
subcommand, but the EggInfo class is invoked internally during an sdist. And
we need pip3's egg_info to fail, because if it succeeds, it will proceed to
run egg_info on our dependencies (like zfec, whose setup.py contains
old-style py2-only 'print "foo"' statements, which causes a syntax error,
which is the confusing thing we were trying to head off by providing a
clean+instructional error message early).

This is a workaround for an upstream Tox bug (see
tox-dev/tox#507) that should get fixed (to make tox
run `setup.py sdist` with the virtualenv's python, rather than tox's python).
However since debian/ubuntu don't generally backport non-security bugfixes to
existing releases, a new Tox won't actually help our problem on any of
today's operating systems (it'll be 6-12 months before anyone is helped by a
Tox fix). So even though we'd ideally prefer to fix this upstream and not
here, I think practically speaking we need this workaround anyways.

closes ticket:2876
warner added a commit to warner/tahoe-lafs that referenced this issue Aug 10, 2017
The previous blanket prohibition on py3 for any invocation of setup.py was
breaking our recommended "just run 'tox'" test process on e.g. Ubuntu-16.04
LTS "xenial", which ships with a /usr/bin/tox that is based on py3. This tox
runs "setup.py sdist" to build a tahoe tarball, then uses its py2
virtualenv's "pip" to install it (into the py2 virtualenv). Forbidding even
"setup.py sdist" from running under py3 was causing tox to fail unless it was
a py2-based tox.

This new approach only enforces the py2 check if setup.py was invoked as
"setup.py install" (which users might do), or "setup.py egg_info" (which pip
will do before it does anything else).

I experimented with doing the check inside an EggInfo or "build_py"
subcommand, but the EggInfo class is invoked internally during an sdist. And
we need pip3's egg_info to fail, because if it succeeds, it will proceed to
run egg_info on our dependencies (like zfec, whose setup.py contains
old-style py2-only 'print "foo"' statements, which causes a syntax error,
which is the confusing thing we were trying to head off by providing a
clean+instructional error message early).

This is a workaround for an upstream Tox bug (see
tox-dev/tox#507) that should get fixed (to make tox
run `setup.py sdist` with the virtualenv's python, rather than tox's python).
However since debian/ubuntu don't generally backport non-security bugfixes to
existing releases, a new Tox won't actually help our problem on any of
today's operating systems (it'll be 6-12 months before anyone is helped by a
Tox fix). So even though we'd ideally prefer to fix this upstream and not
here, I think practically speaking we need this workaround anyways.

closes ticket:2876
warner added a commit to warner/tahoe-lafs that referenced this issue Aug 16, 2017
This allows a python3-based tox (as is common on modern debian/ubuntu
systems) to test our py2-only package. The first thing Tox does is to build a
wheel to install into the target virtualenv (which is a py2-based venv, for
tahoe). But Tox bug (tox-dev/tox#507) in which this
wheel is built with the same python that Tox is using, instead of the python
from the target environment. Our setup.py would see sys.version_info with py3
and launch a crowbar into the works.

With python_requires=, pip is smart enough to know that it's ok to build
wheels with the wrong python, but "pip install" still throws a sensible error
message:

```
(ve36) ~/stuff/tahoe/tahoe$ pip install .
Processing /home/warner/stuff/tahoe/tahoe
tahoe-lafs requires Python '<3.0' but the running Python is 3.6.1
```

Closes ticket:2876
@obestwalter obestwalter added area:testenv-creation feature:change something exists already but should behave differently level:medium rought estimate that this might be neither easy nor hard to implement and removed enhancement labels Sep 4, 2017
@zozzz
Copy link

zozzz commented Oct 10, 2017

And set environment variables that defined in setenv or passenv

@gaborbernat gaborbernat self-assigned this Oct 10, 2017
@gaborbernat
Copy link
Member

So the solution could be to regenerate the package creation for each and every environment. setenv and passenv are set once loaded into the virtualenv, so that would be relatively already provided

@obestwalter
Copy link
Member

I think we only should build the package under test once, but the Python building that package needs to fullfill the needs of the setup.py (e.g. needs to be Python 3.whatever). The idea from @gaborbernat would solve the problem of knowing what env to use, but would introduce other problems (after all, we want to build one package that we want to test, not several and then some of those envs might not even be able to build the package).

Maybe we need to introduce a venv that is only there to build the package in cases where this is necessary? So as a default, package is built by the interpreter that tox is installed in, but if you need another interpreter you can ask for it as a global config option?

warner added a commit to warner/tahoe-lafs that referenced this issue Mar 21, 2018
The Travis OS-X worker has a very old setuptools-18.5 in /System. This is too
old to understand several important setup.py keys like `python_requires`, and
crashes when tryung to run the first invocation of tox (`tox -e codechecks`).
I think tox is using the system python (with which `tox` was invoked) to run
`setup.py egg_info` (to learn the dependencies), which gets the old
system-installed setuptools. Ideally it'd use the python from the
newly-created virtualenv, which would use whatever version of setuptools was
bundled with the `virtualenv` package (probably newer, given that
`virtualenv` itself should have been installed a moment earlier as a
dependency of `tox`.

I consider this a bug in Tox (tox-dev/tox#507), but
the workaround is to configure Travis to install the most recent `setuptools`
along with `tox`.

refs tahoe-lafs#470
@gaborbernat
Copy link
Member

@dstufft this now is resolved with pep 517 implementation released with 3.3..0

@tox-dev tox-dev locked and limited conversation to collaborators Jan 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area:testenv-creation feature:change something exists already but should behave differently level:medium rought estimate that this might be neither easy nor hard to implement
Projects
None yet
Development

No branches or pull requests

5 participants