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

[wip] Pep 518 #861

Closed
wants to merge 8 commits into from
Closed

[wip] Pep 518 #861

wants to merge 8 commits into from

Conversation

gaborbernat
Copy link
Member

#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. wheel support also enforces to have pip 10+ and a pyproject.toml.

#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.

@gaborbernat gaborbernat added area:package-building level:hard rought estimate that this might be quite hard to implement labels Jun 29, 2018
@gaborbernat gaborbernat added this to the 3.1 milestone Jun 29, 2018
@gaborbernat
Copy link
Member Author

gaborbernat commented Jun 29, 2018

Pulling over from #852. Pip tracking is pypa/pip#5407.

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)

@gaborbernat
Copy link
Member Author

Still need to add:

  • checking we have a built for the current Python version (maybe pip should complain that we cannot install a wheel for 2 instead of 3);
  • see why pip wheel builds without checking that setup.py is there, maybe at the moment does not does sdist -> wheel path.

@codecov
Copy link

codecov bot commented Jun 29, 2018

Codecov Report

Merging #861 into master will decrease coverage by 0.36%.
The diff coverage is 78.78%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #861      +/-   ##
==========================================
- Coverage   92.61%   92.25%   -0.37%     
==========================================
  Files          12       12              
  Lines        2330     2349      +19     
  Branches      409      413       +4     
==========================================
+ Hits         2158     2167       +9     
- Misses        109      113       +4     
- Partials       63       69       +6
Impacted Files Coverage Δ
src/tox/venv.py 92.6% <100%> (-0.32%) ⬇️
src/tox/_pytestplugin.py 91.51% <100%> (-0.45%) ⬇️
src/tox/config.py 95.43% <100%> (-0.47%) ⬇️
src/tox/session.py 90.76% <73.07%> (-0.81%) ⬇️
src/tox/interpreters.py 90.47% <0%> (+1.58%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 95f5476...a87f3e4. Read the comment docs.

@obestwalter
Copy link
Member

Hi @gaborbernat - I am just catching up on all my notifications. This still looks like WIP, so please ping me, when you would like me to have a closer look.

@gaborbernat gaborbernat self-assigned this Jul 2, 2018
@gaborbernat gaborbernat changed the title Pep 518 [wip] Pep 518 Jul 3, 2018
@gaborbernat gaborbernat added the needs:work for PRs: not quite there and needs some changes label Jul 3, 2018
@gaborbernat gaborbernat modified the milestones: 3.1, 3.2 Jul 3, 2018
gaborbernat and others added 8 commits July 9, 2018 09:10
…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.
@gaborbernat gaborbernat modified the milestones: 3.2, 3.3 Jul 9, 2018
@gaborbernat gaborbernat force-pushed the master branch 6 times, most recently from fb8cb25 to f8935ce Compare July 9, 2018 18:45
@gaborbernat
Copy link
Member Author

Retracting this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:package-building level:hard rought estimate that this might be quite hard to implement needs:work for PRs: not quite there and needs some changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants