[WIP] wheel support (and transitively PEP-518), plus pipe display failed commands #852

gaborbernat · 2018-06-21T15:27:14Z

#850 PEP-518 support: provide a tox configuration flag build which can be either sdist or wheel. For sdist (default) we build the package as before by using python setup.py sdist. However, when wheel is enabled now we'll use pip wheel to build it, and we'll also install wheels in these case into the environments. Note: pip 10 supports specifying project dependencies (such as setuptools-scm, or a given setuptools version) via pyproject.toml. Once pip supports building sdist to we'll migrate over the sdist build too.

#851 While running tox invokes various commands (such as building the package, pip installing dependencies and so on), these were printed in case they failed as Python arrays. Changed the representation to a shell command, allowing the users to quickly replicate/debug the failure on their own.

codecov · 2018-06-21T15:36:39Z

Codecov Report

Merging #852 into master will decrease coverage by <1%.
The diff coverage is 78%.

@@          Coverage Diff           @@
##           master   #852    +/-   ##
======================================
- Coverage      93%    92%   -<1%     
======================================
  Files          12     12            
  Lines        2326   2344    +18     
  Branches      408    413     +5     
======================================
+ Hits         2153   2164    +11     
- Misses        107    110     +3     
- Partials       66     70     +4

gaborbernat · 2018-06-25T17:46:21Z

To be still added is deprecation messages for using the sdist options.

asottile · 2018-06-25T18:18:35Z

doc/config.rst

+    tox will do for the projects package:
+
+    - if it's ``none`` tox will not try to build the package (implies no install happens later),
+    - if it's ``sdist`` it will create a source distribution by using ``distutils``:


I'd just say "using setup.py sdist", since it's way more likely to go through setuptools anyway

asottile · 2018-06-25T18:19:02Z

doc/config.rst

+
+        .. code-block:: ini
+
+            pip build wheel --no-dep


I believe you mean pip wheel . --no-deps

another note, it might be better (even in the wheel case) to:

build an sdist

build a wheel from the sdist

part of the benefit of tox using the

build sdist

install from sdist

ensures that the package is installable (and not depending on workdir state) -- would be good to keep that assertion going forward even in the "install from wheel" case

@asottile what you're describing there is what pip plans to do part of pip wheel command; so we already have that ensured. I would prefer for us to not re-implement the build infrastructure and leave that to the build fronted, e.g. pip.

this would be a step backward until pip 10 has majority adoption and pep518 is actually implemented

We don't have wheel support at the moment. So that's a step ahead. If we implement pep-518 in tox that would be totally redundant over implementing in pip. So I argue if we add this support let's do it in pip so everyone benefits. Inside tox you can upgrade pip, so I don't understand why you would need pip 10 majority adoption. You already get it with latest virtualenv which is installed even with tox 2.x automatically.

The main point I'm making here is:

tox is a tool to prevent incorrect packaging

unless you're using pip 10 and pep518, building a wheel from the working directory can bypass some checks that tox currently enforces via the "make sdist then install from it" process (due to lack of workdir isolation)

imo we should either:

require pip10 + pep518 support to enable build=wheel

sdist => wheel => install

Here's an example project which tox would prevent today, but this PR would incorrecty allow with build=wheel:

status quo, correctly failing

$ tail -n999 setup.py tox.ini requirements.txt ==> setup.py <== from setuptools import setup # oops I goofed, should have made a MANIFEST.in with open('requirements.txt') as f: contents = list(f) setup( name='mylib', install_requires=contents, ) ==> tox.ini <== [testenv] commands = pip freeze --all ==> requirements.txt <== six

$ tox -e py27 GLOB sdist-make: /private/tmp/g/setup.py py27 create: /private/tmp/g/.tox/py27 py27 inst: /private/tmp/g/.tox/dist/mylib-0.0.0.zip ERROR: invocation failed (exit code 1), logfile: /private/tmp/g/.tox/py27/log/py27-1.log ERROR: actionid: py27 msg: installpkg cmdargs: ['/private/tmp/g/.tox/py27/bin/pip', 'install', '/private/tmp/g/.tox/dist/mylib-0.0.0.zip'] Processing ./.tox/dist/mylib-0.0.0.zip Complete output from command python setup.py egg_info: Traceback (most recent call last): File "<string>", line 1, in <module> File "/private/tmp/pip-req-build-CCk4wP/setup.py", line 3, in <module> with open('requirements.txt') as f: IOError: [Errno 2] No such file or directory: 'requirements.txt' ---------------------------------------- Command "python setup.py egg_info" failed with error code 1 in /private/tmp/pip-req-build-CCk4wP/ ___________________________________ summary ____________________________________ ERROR: py27: InvocationError for command /private/tmp/g/.tox/py27/bin/pip install /private/tmp/g/.tox/dist/mylib-0.0.0.zip (see /private/tmp/g/.tox/py27/log/py27-1.log) (exited with code 1)

your branch, incorrectly succeeding

[tox] build = wheel [testenv] commands = pip freeze --all

$ tox -e py36 GLOB wheel-make: /private/tmp/g/setup.py py36 create: /private/tmp/g/.tox/py36 py36 inst: /private/tmp/g/.tox/dist/mylib-0.0.0-py3-none-any.whl py36 installed: mylib==0.0.0,six==1.11.0 py36 runtests: PYTHONHASHSEED='1061640697' py36 runtests: commands[0] | pip freeze --all mylib==0.0.0 pip==10.0.1 setuptools==39.2.0 six==1.11.0 wheel==0.31.1 ___________________________________ summary ____________________________________ py36: commands succeeded congratulations :)

your branch, unrelated also broken thing

(noticed this while making a reproduction) -- tox's python is python3, but venvs is python2 so there's a wheel mismatch

(note that this is covered by my comment below about the open issue to use the virtualenv python instead of tox's python)

$ tox -e py27 GLOB wheel-make: /private/tmp/g/setup.py py27 inst-nodeps: /private/tmp/g/.tox/dist/mylib-0.0.0-py3-none-any.whl ERROR: invocation failed (exit code 1), logfile: /private/tmp/g/.tox/py27/log/py27-5.log ERROR: actionid: py27 msg: installpkg cmdargs: '/private/tmp/g/.tox/py27/bin/pip install -U --no-deps /private/tmp/g/.tox/dist/mylib-0.0.0-py3-none-any.whl' mylib-0.0.0-py3-none-any.whl is not a supported wheel on this platform. ___________________________________ summary ____________________________________ ERROR: py27: InvocationError for command /private/tmp/g/.tox/py27/bin/pip install -U --no-deps /private/tmp/g/.tox/dist/mylib-0.0.0-py3-none-any.whl (see /private/tmp/g/.tox/py27/log/py27-5.log) (exited with code 1)

This branch wants to implement require pip10 + pep518 support to enable. Will check how come those succeed and alter the code/create pr for pip to fail. thanks!

asottile · 2018-06-25T18:22:42Z

src/tox/session.py

+                cwd=self.config.setupdir,
+            )
+        else:
+            raise RuntimeError("invalid build type {}".format(self.config.build))


there's an open issue to change these to instead use the virtualenv python (instead of tox's python) to run these builds -- maybe worth changing in this review since it's already being touched?

Again the creation of a build venv is the responsibility of pip, and in case of the wheel it already does that. For doing the same for sdist the solution will be to move to pip build sdist and we'll support implementing PEP-517 inside pip. tox will not implement what you're describing as it's outside the scope of the tool.

I'm sorry for breaking into your conversation but I have one argument to invoke pip wheel with virtualenv's python - packages with C extensions. They depend on Python version with which they are built and afaiu pip will build wheels only for interpeter it's being run by, so such wheels built by tox's interpreter will not suit environment with another one.

That’s a valid point. Maybe tox should run pip wheel multiple times for each python version, when it isn’t a universal pure python wheel.

For the first phase we decided to focus on universal wheel. Hence the way this works now. it's still a wip though and likely will wait for pip 517 to merge to adopt this because of some sanity checks not applied by pip at the moment.

Makes sense, thanks for the clarification

asottile · 2018-06-25T18:23:16Z

src/tox/session.py

+                    sys.executable,
+                    "-m",
+                    "pip",
+                    "wheel",


we could also invoke setup.py bdist_wheel instead of involving pip -- though that drifts further from PEP518

We plan to come closer to PEP-517/518 not move away from it. Once pip has that support we'll be able to provide flint, poetry etc support natively, without magical tox plugins.

asottile · 2018-06-25T18:24:11Z

src/tox/venv.py

+    # we clear the PYTHONPATH to allow reporting packages outside of this environment
+    output = venv._pcall(
+        args, cwd=venv.envconfig.config.toxinidir, action=action, no_python_path=True
+    )


is this related to this change?

It is. During testing this change I've encountered the situation of a faulty PYTHONPATH can report packages outside the venv as installed during the build. This defends against such.

hmmm, maybe split this into a separate PR so it's more clear why this changed?

Felt linked against this pr to close as without it the tests would fail, so thought not worth the extra effort.

asottile · 2018-06-25T18:24:45Z

tox.ini

@@ -102,7 +102,7 @@ commands = python -m pip list --format=columns
 [flake8]
 max-complexity = 22
 max-line-length = 99
-ignore = E203, W503
+ignore = E203, W503, E501


hmmm, is black not respecting the max line length?

it does however flake8 falsely signals multiline strings as running long, see https://github.com/gaborbernat/tox/blob/pep-518/tests/test_package_build.py#L148

there is no way to break that, and given black already ensures source code does not flow over 100 I decided to skip it

I'd rather # noqa the specific literal instead of blanket turning this off

The problem is that flake complains not at assignment but middle of the string and one cannot add disable there. Will look to disable for that section then.

if you put """ # noqa (on the end of the tqs) that should work -- it's not super intuitive but 🤷‍♂️

…mmands PEP-518 support: provide a tox configuration flag ``build`` which can be either ``sdist`` or ``wheel``. For ``sdist`` (default) we build the package as before by using ``python setup.py sdist``. However, when ``wheel`` is enabled now we'll use ``pip wheel`` to build it, and we'll also install wheels in these case into the environments. Note: ``pip`` 10 supports specifying project dependencies (such as ``setuptools-scm``, or a given ``setuptools`` version) via ``pyproject.toml``. Once ``pip`` supports building ``sdist`` to we'll migrate over the ``sdist`` build too. While running tox invokes various commands (such as building the package, pip installing dependencies and so on), these were printed in case they failed as Python arrays. Changed the representation to a shell command, allowing the users to quickly replicate/debug the failure on their own.

…play failed commands (#852)" This reverts commit 731a962.

asottile · 2018-06-29T16:21:16Z

did we actually want to merge this? I haven't had a chance to look at it since last time and I don't think anyone else has looked at it either :S

gaborbernat · 2018-06-29T16:26:46Z

Yeah, my bad. Removed it, will open it as a new PR. Sorry!

pllim · 2021-01-13T23:00:26Z

Hello. I found this PR looking for ways to build wheel and test it with tox. This PR is shown as merged but I cannot find the documentation added here in https://tox.readthedocs.io/en/latest/config.html . I am wondering what happened to this option since. Thanks.

gaborbernat · 2021-01-13T23:18:58Z

Sadly we ran into issues with this and had to revert. tox 4 first alpha came out last week, and that contains this feature, though first public release not expected before April.

gaborbernat added feature:new something does not exist yet, but should feature:change something exists already but should behave differently area:package-building level:medium rought estimate that this might be neither easy nor hard to implement labels Jun 21, 2018

gaborbernat changed the title ~~[WIP] wheel support (and transitively PEP-518), and pipe display failed commands~~ [WIP] wheel support (and transitively PEP-518), plus pipe display failed commands Jun 21, 2018

gaborbernat force-pushed the pep-518 branch from e351463 to 236834e Compare June 21, 2018 15:36

gaborbernat force-pushed the pep-518 branch 3 times, most recently from 9eae7f8 to b19f7c1 Compare June 22, 2018 17:03

gaborbernat mentioned this pull request Jun 25, 2018

Make py2 and py3 aliases also resolve via py on windows #858

Merged

gaborbernat force-pushed the pep-518 branch from b19f7c1 to 245bf9e Compare June 25, 2018 15:43

gaborbernat requested review from obestwalter and asottile June 25, 2018 17:45

gaborbernat assigned obestwalter and asottile Jun 25, 2018

gaborbernat added this to the 3.1 milestone Jun 25, 2018

asottile reviewed Jun 25, 2018

View reviewed changes

gaborbernat and others added 7 commits June 26, 2018 15:53

Windows fix

217e53c

python path optionally remove

336f231

Add config help for new feature

fdfb046

disable flake8 for too long docstrings

ccb1beb

don't mention distutils

b9017cc

fix pip wheel build command

e3572a1

gaborbernat force-pushed the pep-518 branch 2 times, most recently from c375365 to 83c1a03 Compare June 26, 2018 15:07

pip 10 and pyproject.toml is required for wheel build

4dd236a

gaborbernat force-pushed the pep-518 branch from 83c1a03 to 4dd236a Compare June 26, 2018 17:34

gaborbernat merged commit 731a962 into tox-dev:master Jun 29, 2018

gaborbernat deleted the pep-518 branch June 29, 2018 16:17

gaborbernat restored the pep-518 branch June 29, 2018 16:18

gaborbernat added a commit that referenced this pull request Jun 29, 2018

Revert "[WIP] wheel support (and transitively PEP-518), plus pipe dis…

7367b90

…play failed commands (#852)" This reverts commit 731a962.

gaborbernat mentioned this pull request Jun 29, 2018

[wip] Pep 518 #861

Closed

gaborbernat mentioned this pull request Jan 9, 2019

dist creation commands #232

Closed

asottile removed their assignment Feb 9, 2020

pllim mentioned this pull request Jan 14, 2021

TST: Investigate tox behavior with C-extension spacetelescope/synphot_refactor#295

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] wheel support (and transitively PEP-518), plus pipe display failed commands #852

[WIP] wheel support (and transitively PEP-518), plus pipe display failed commands #852

gaborbernat commented Jun 21, 2018 •

edited

Loading

codecov bot commented Jun 21, 2018 •

edited

Loading

gaborbernat commented Jun 25, 2018

asottile Jun 25, 2018

gaborbernat Jun 25, 2018

asottile Jun 25, 2018

asottile Jun 25, 2018

gaborbernat Jun 25, 2018 •

edited

Loading

asottile Jun 25, 2018

gaborbernat Jun 25, 2018 •

edited

Loading

asottile Jun 25, 2018

gaborbernat Jun 25, 2018

asottile Jun 25, 2018

gaborbernat Jun 25, 2018

daa Jul 27, 2018

anthrotype Jul 28, 2018

gaborbernat Jul 29, 2018

anthrotype Jul 29, 2018

asottile Jun 25, 2018

gaborbernat Jun 25, 2018

asottile Jun 25, 2018

gaborbernat Jun 25, 2018

asottile Jun 25, 2018

gaborbernat Jun 25, 2018

asottile Jun 25, 2018

gaborbernat Jun 25, 2018

asottile Jun 25, 2018

gaborbernat Jun 25, 2018

asottile Jun 25, 2018

asottile commented Jun 29, 2018

gaborbernat commented Jun 29, 2018

pllim commented Jan 13, 2021 •

edited

Loading

gaborbernat commented Jan 13, 2021

[WIP] wheel support (and transitively PEP-518), plus pipe display failed commands #852

[WIP] wheel support (and transitively PEP-518), plus pipe display failed commands #852

Conversation

gaborbernat commented Jun 21, 2018 • edited Loading

codecov bot commented Jun 21, 2018 • edited Loading

Codecov Report

gaborbernat commented Jun 25, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gaborbernat Jun 25, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gaborbernat Jun 25, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

status quo, correctly failing

your branch, incorrectly succeeding

your branch, unrelated also broken thing

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asottile commented Jun 29, 2018

gaborbernat commented Jun 29, 2018

pllim commented Jan 13, 2021 • edited Loading

gaborbernat commented Jan 13, 2021

gaborbernat commented Jun 21, 2018 •

edited

Loading

codecov bot commented Jun 21, 2018 •

edited

Loading

gaborbernat Jun 25, 2018 •

edited

Loading

gaborbernat Jun 25, 2018 •

edited

Loading

pllim commented Jan 13, 2021 •

edited

Loading