PEP-517 source distribution support #954

gaborbernat · 2018-08-30T13:22:58Z

create a .package virtual environment to perform build operations inside

@asottile @nicoddemus @obestwalter feel free to take a look at it and try to find holes in it 👍 base implementation there, just need to add documentation and some more tests 💯

FYI @pfmoore (we probably will switch over pip once pip will have pep-517 support and build command - that being said I find this level a support a bit more generic e.g. python version selection). With this tox should finally allow users to specify their packaging dependencies, and use other than setuptools.

Resolves #573 and #820.

pfmoore · 2018-08-30T13:41:44Z

Nice! I'll try to find some time to take a proper look at this. (FYI, the initial implementation of PEP 517 in pip won't include a pip build command, but pip wheel will use PEP 517, I don't know if that's sufficient for you).

codecov · 2018-08-30T18:34:33Z

Codecov Report

Merging #954 into master will decrease coverage by 1.01%.
The diff coverage is 69.07%.

@@            Coverage Diff             @@
##           master     #954      +/-   ##
==========================================
- Coverage   92.77%   91.75%   -1.02%     
==========================================
  Files          13       13              
  Lines        2393     2473      +80     
  Branches      416      433      +17     
==========================================
+ Hits         2220     2269      +49     
- Misses        109      137      +28     
- Partials       64       67       +3

Impacted Files	Coverage Δ
src/tox/_pytestplugin.py	`92.65% <100%> (ø)`	⬆️
src/tox/session.py	`91.82% <100%> (+0.01%)`	⬆️
src/tox/venv.py	`93.19% <100%> (-0.32%)`	⬇️
src/tox/package.py	`71.31% <59.15%> (-17.98%)`	⬇️
src/tox/config.py	`95.74% <93.75%> (-0.05%)`	⬇️

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 9335906...ba9572f. Read the comment docs.

asottile · 2018-08-31T17:58:03Z

I'll take a look at this later today -- excited for this!

gaborbernat

left some comments as questions for reviewers, reminders for myself

gaborbernat · 2018-08-31T21:23:05Z

src/tox/config.py

+        config.isolated_build = reader.getbool("isolated_build", False)
+        if config.isolated_build is True:
+            name = ".package"
+            if name not in config.envconfigs:


the build env name should be configurable

gaborbernat · 2018-08-31T21:24:47Z

src/tox/package.py

+        if pkg_requirement.key not in toml_require:
+            package_venv.envconfig.deps.append(DepConfig(requirement, None))
+
+    if not session.setupenv(package_venv):


here we are recreating the env maybe instead just install new dependencies and finalize again the env

recreate is certainly the safest option here -- does the PEP have any suggestions for this?

gaborbernat · 2018-08-31T21:26:10Z

src/tox/package.py

+    build_requires = get_build_requires(build_info, package_venv, session)
+    for requirement in build_requires:
+        pkg_requirement = pkg_resources.Requirement(requirement)
+        if pkg_requirement.key not in toml_require:


note here we might potentially have a issue that this requirement might differ from the first, maybe check for it and raise if that's the case

gaborbernat · 2018-08-31T21:27:42Z

src/tox/package.py

+                build_info.backend_module,
+                build_info.backend_object,
+                "sdist",
+                str(config.distdir).replace("\\", "\\\\"),


instead of the replace mark the output build directory as a raw string

gaborbernat · 2018-08-31T21:29:17Z

tests/integration/test_package_int.py

+
+@pytest.mark.network
+@need_git
+@pytest.mark.skipif(sys.version_info < (3, 0), reason="flit is Python 3 only")


in CI instead of skip should raise instead

xfail maybe then?

gaborbernat · 2018-08-31T21:30:23Z

tests/lib/__init__.py

@@ -0,0 +1,18 @@
+import subprocess


@asottile @Nicodeamus stolen this from pip any better way of doing this?

this seems fine, maybe a bit overkill to generalize. pre-commit does a similar thing

actually -- is this even worth it? I don't think we package our tests so you'd have to be running from a git checkout? I guess maybe not for OS vendors 🤔

gaborbernat · 2018-08-31T21:31:12Z

tox.ini

-known_first_party = tox
-known_third_party = apiclient,git,httplib2,oauth2client,packaging,pkg_resources,pluggy,py,pytest,setuptools,six
+known_first_party = tox,tests
+known_third_party = apiclient,git,httplib2,oauth2client,packaging,pkg_resources,pluggy,py,pytest,setuptools,six,tests,toml


@asottile for some reason tests shows up as third party even though it is marked as first party

you'll want to change --application-directories from src to src:. here

asottile

A bunch of comments, feel free to dismiss most of them, some of them are just me talking to myself.

I jotted down these notes while going through, some probably (?) need addressing -- not sure:

needs documentation:

isolated_build setting
.package environment name
maybe an example which shows how to configure the executable for the .package environment (and/or other useful things to override there?)

question:

does the .package environment leak into tox --listenvs (or whatever the option is called) -- (either way) should it?
is .package a good enough name to avoid conflicts (probably)? if not maybe .tox-package?

Will wheel support want to make N .package environments -- does this enable that or make it more difficult?

Overall seems fine!

asottile · 2018-09-01T01:43:24Z

src/tox/_pytestplugin.py

+                    name: {
+                        "__init__.py": textwrap.dedent(
+                            """
+                \"\"\" module {} \"\"\"


probably avoid the escape sequences by making either the inner or outer a triple-single-quoted string

asottile · 2018-09-01T01:48:58Z

src/tox/config.py


        config.skipsdist = reader.getbool("skipsdist", all_develop)
+        config.isolated_build = reader.getbool("isolated_build", False)
+        if config.isolated_build is True:


I assume the idea with this branch is to make the .package environment user-configurable through tox.ini -- which cases does it make sense for that to be configurable? (alternatively: should tox have an opinion about how they configure the .package environment?)

I can see one usecase, wanted to configure the executable

asottile · 2018-09-01T01:51:20Z

src/tox/package.py

-            report.error("FAIL could not package project - v = {!r}".format(v))
+            path = build_package(config, report, session)
+        except tox.exception.InvocationError as exception:
+            report.error("FAIL could not package project - v = {!r}".format(exception))


asottile · 2018-09-01T01:53:21Z

src/tox/package.py

+def build_isolated(config, report, session):
+    build_info = get_build_info(config.setupdir, report)
+    package_venv = session.getvenv(".package")
+    package_venv.envconfig.deps_matches_subset = True


clever, a patch!

asottile · 2018-09-01T01:55:16Z

src/tox/package.py

+    package_venv = session.getvenv(".package")
+    package_venv.envconfig.deps_matches_subset = True
+
+    package_venv.envconfig.deps = [DepConfig(r, None) for r in build_info.requires]


since we're invariably clobbering deps = here, should we error if the user configured deps in their tox.ini? (might be difficult to implement now that I think about it, due to environment inheritance)

tox-pip-extensions for instance does this if the install_command is manually set (since it patches it out)

asottile · 2018-09-01T02:17:43Z

tox.ini

-known_first_party = tox
-known_third_party = apiclient,git,httplib2,oauth2client,packaging,pkg_resources,pluggy,py,pytest,setuptools,six
+known_first_party = tox,tests
+known_third_party = apiclient,git,httplib2,oauth2client,packaging,pkg_resources,pluggy,py,pytest,setuptools,six,tests,toml


you'll want to change --application-directories from src to src:. here

asottile · 2018-09-01T02:19:02Z

tests/unit/test_pytest_plugins.py

+def linesep_bytes():
+    if isinstance(os.linesep, bytes):
+        return os.linesep
+    return os.linesep.encode("utf-8")


can probably just use os.linesep.encode() -- in python2 this'll go through the ASCII encoding .decode().encode() but that's probably fine?

asottile · 2018-09-01T02:19:14Z

tests/unit/test_pytest_plugins.py


        init_file = tmpdir.join("spam", "spam", "__init__.py")
-        assert init_file.read_binary() == b"__version__ = '666'"
+        expected = b'""" module spam """' + linesep_bytes() + b"__version__ = '666'"


hail satan ig

asottile · 2018-09-01T02:20:04Z

tests/unit/test_docs.py

    r"(?P<code>(^((?P=indent) +.*)?\n)+)",
    re.MULTILINE,
 )



worth moving all the tests in this PR? is the distinction between "unit" and not actually enforced / definitive / useful? is the extra typing worth it?

asottile · 2018-09-01T02:20:33Z

tests/lib/__init__.py

@@ -0,0 +1,18 @@
+import subprocess


actually -- is this even worth it? I don't think we package our tests so you'd have to be running from a git checkout? I guess maybe not for OS vendors 🤔

gaborbernat · 2018-09-01T14:53:27Z

@asottile

needs documentation: isolated_build setting, .package environment name
maybe an example which shows how to configure the executable for the .package environment (and/or other useful things to override there?)

Added some: the config flags plus an example page related to packaging.

does the .package environment leak into tox --listenvs (or whatever the option is called) -- (either way) should it?

It probably does, we need to filter it out. Will do and add test for it.

is .package a good enough name to avoid conflicts (probably)? if not maybe .tox-package?

Given that now it's configurable it's good enough I think especially with the . start to make it private/hidden so to say.

Will wheel support want to make N .package environments -- does this enable that or make it more difficult?

Wheel support will have to do the n separate package environments. Going from there maybe we should instead make the isolated_build a choice so people can swap out to other alternatives/options; such as the wheel instead of sdist. I would say the way we have packaging plugabble should be fairly easy to add wheel support later on (no way around it - will have to create up to n separate build envs though - n is the number of unique binary type - e.g. 2 if only py2 and py3, but with abi and other parameters can be a lot more).

create a ``.package`` virtual environment to perform build operations inside

…of envlist

gaborbernat · 2018-09-09T10:55:52Z

This now is mostly complete. Will merge in on Monday unless we find something significant until then. Will re-read https://www.python.org/dev/peps/pep-0517/ before that to make sure we did not miss anything.

Python.org
PEP 517 -- A build-system independent format for source trees
The official home of the Python Programming Language

obestwalter · 2018-11-02T15:07:14Z

Better later than never: thank you @gaborbernat for your great work on this and thank you @asottile for reviewing 🎆 🎆 🎆

gaborbernat requested review from asottile and obestwalter August 31, 2018 17:43

gaborbernat assigned obestwalter and asottile Aug 31, 2018

gaborbernat added the feature:new something does not exist yet, but should label Aug 31, 2018

gaborbernat changed the title ~~[WIP] PEP-517 source distribution support~~ PEP-517 source distribution support Aug 31, 2018

gaborbernat commented Aug 31, 2018

View reviewed changes

asottile reviewed Sep 1, 2018

View reviewed changes

gaborbernat and others added 13 commits September 9, 2018 11:50

PEP-517 source distribution support

93cc972

create a ``.package`` virtual environment to perform build operations inside

try to fix CI

f09302f

Fix platform bugs

ac2ce55

flit not supported on Python 2

b409579

use !r instead of replace

1fbd65d

pull request feedback

7034f26

add documentation

53dcb84

document default isolated build env

2177344

fix documentation bugs

e69f5be

allow expanding defintion

43011c4

documentation fix

b501922

just install the extra dependencies rather than re-create the env

738b078

ensure isolated build env is not in list configs, and cannot be part …

db36531

…of envlist

Ensure isolated env package is not in envlist

ba9572f

gaborbernat merged commit ccdca4a into tox-dev:master Sep 11, 2018

gaborbernat deleted the pep-517/8 branch September 11, 2018 10:35

gaborbernat mentioned this pull request Sep 15, 2018

tox-flit support #820

Closed

merwok mentioned this pull request Sep 22, 2018

Tox is incompatible with projects using flit.ini pypa/flit#75

Closed

waylan mentioned this pull request Oct 21, 2018

Up version to 3.1.dev0 Python-Markdown/markdown#736

Merged

astrojuanlu mentioned this pull request Nov 17, 2019

Migrate to flit poliastro/poliastro#802

Merged

3 tasks

graingert-osi mentioned this pull request Nov 29, 2019

Lint and test on GitHub Actions flake8-implicit-str-concat/flake8-implicit-str-concat#16

Merged

asottile removed their assignment Feb 9, 2020

Uh oh!

PEP-517 source distribution support #954

PEP-517 source distribution support #954

Uh oh!

Conversation

gaborbernat commented Aug 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

pfmoore commented Aug 30, 2018

codecov bot commented Aug 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

asottile commented Aug 31, 2018

gaborbernat left a 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 left a 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

gaborbernat commented Sep 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

gaborbernat commented Sep 9, 2018 • edited by unfurl-links bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

obestwalter commented Nov 2, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Labels

4 participants

gaborbernat commented Aug 30, 2018 •

edited

Loading

codecov bot commented Aug 30, 2018 •

edited

Loading

gaborbernat commented Sep 1, 2018 •

edited

Loading

gaborbernat commented Sep 9, 2018 •

edited by unfurl-links bot

Loading

obestwalter commented Nov 2, 2018 •

edited

Loading