-
-
Notifications
You must be signed in to change notification settings - Fork 531
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
Expose the packaging as a plugin hook #952
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
packaging now is exposed as a hook via ``tox_package(session, venv)`` - by :user:`gaborbernat` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,85 @@ | ||
import sys | ||
|
||
import py | ||
|
||
import tox | ||
|
||
|
||
@tox.hookimpl | ||
def tox_package(session, venv): | ||
"""Build an sdist at first call return that for all calls""" | ||
if not hasattr(session, "package"): | ||
session.package = get_package(session) | ||
return session.package | ||
|
||
|
||
def get_package(session): | ||
""""Perform the package operation""" | ||
config, report = session.config, session.report | ||
if config.skipsdist: | ||
report.info("skipping sdist step") | ||
return None | ||
if not config.option.sdistonly and (config.sdistsrc or config.option.installpkg): | ||
path = config.option.installpkg | ||
if not path: | ||
path = config.sdistsrc | ||
path = session._resolve_package(path) | ||
report.info("using package {!r}, skipping 'sdist' activity ".format(str(path))) | ||
else: | ||
try: | ||
path = make_sdist(report, config, session) | ||
except tox.exception.InvocationError: | ||
v = sys.exc_info()[1] | ||
report.error("FAIL could not package project - v = {!r}".format(v)) | ||
return None | ||
sdist_file = config.distshare.join(path.basename) | ||
if sdist_file != path: | ||
report.info("copying new sdistfile to {!r}".format(str(sdist_file))) | ||
try: | ||
sdist_file.dirpath().ensure(dir=1) | ||
except py.error.Error: | ||
report.warning("could not copy distfile to {}".format(sdist_file.dirpath())) | ||
else: | ||
path.copy(sdist_file) | ||
return path | ||
|
||
|
||
def make_sdist(report, config, session): | ||
setup = config.setupdir.join("setup.py") | ||
if not setup.check(): | ||
report.error( | ||
"No setup.py file found. The expected location is:\n" | ||
" {}\n" | ||
"You can\n" | ||
" 1. Create one:\n" | ||
" https://packaging.python.org/tutorials/distributing-packages/#setup-py\n" | ||
" 2. Configure tox to avoid running sdist:\n" | ||
" http://tox.readthedocs.io/en/latest/example/general.html" | ||
"#avoiding-expensive-sdist".format(setup) | ||
) | ||
raise SystemExit(1) | ||
with session.newaction(None, "packaging") as action: | ||
action.setactivity("sdist-make", setup) | ||
session.make_emptydir(config.distdir) | ||
action.popen( | ||
[sys.executable, setup, "sdist", "--formats=zip", "--dist-dir", config.distdir], | ||
cwd=config.setupdir, | ||
) | ||
try: | ||
return config.distdir.listdir()[0] | ||
except py.error.ENOENT: | ||
# check if empty or comment only | ||
data = [] | ||
with open(str(setup)) as fp: | ||
for line in fp: | ||
if line and line[0] == "#": | ||
continue | ||
data.append(line) | ||
if not "".join(data).strip(): | ||
report.error("setup.py is empty") | ||
raise SystemExit(1) | ||
report.error( | ||
"No dist directory found. Please check setup.py, e.g with:\n" | ||
" python setup.py sdist" | ||
) | ||
raise SystemExit(1) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -431,53 +431,6 @@ def _copyfiles(self, srcdir, pathlist, destdir): | |
target.dirpath().ensure(dir=1) | ||
src.copy(target) | ||
|
||
def _makesdist(self): | ||
setup = self.config.setupdir.join("setup.py") | ||
if not setup.check(): | ||
self.report.error( | ||
"No setup.py file found. The expected location is:\n" | ||
" {}\n" | ||
"You can\n" | ||
" 1. Create one:\n" | ||
" https://packaging.python.org/tutorials/distributing-packages/#setup-py\n" | ||
" 2. Configure tox to avoid running sdist:\n" | ||
" http://tox.readthedocs.io/en/latest/example/general.html" | ||
"#avoiding-expensive-sdist".format(setup) | ||
) | ||
raise SystemExit(1) | ||
with self.newaction(None, "packaging") as action: | ||
action.setactivity("sdist-make", setup) | ||
self.make_emptydir(self.config.distdir) | ||
action.popen( | ||
[ | ||
sys.executable, | ||
setup, | ||
"sdist", | ||
"--formats=zip", | ||
"--dist-dir", | ||
self.config.distdir, | ||
], | ||
cwd=self.config.setupdir, | ||
) | ||
try: | ||
return self.config.distdir.listdir()[0] | ||
except py.error.ENOENT: | ||
# check if empty or comment only | ||
data = [] | ||
with open(str(setup)) as fp: | ||
for line in fp: | ||
if line and line[0] == "#": | ||
continue | ||
data.append(line) | ||
if not "".join(data).strip(): | ||
self.report.error("setup.py is empty") | ||
raise SystemExit(1) | ||
self.report.error( | ||
"No dist directory found. Please check setup.py, e.g with:\n" | ||
" python setup.py sdist" | ||
) | ||
raise SystemExit(1) | ||
|
||
def make_emptydir(self, path): | ||
if path.check(): | ||
self.report.info(" removing {}".format(path)) | ||
|
@@ -564,47 +517,14 @@ def installpkg(self, venv, path): | |
venv.status = sys.exc_info()[1] | ||
return False | ||
|
||
def get_installpkg_path(self): | ||
""" | ||
:return: Path to the distribution | ||
:rtype: py.path.local | ||
""" | ||
if not self.config.option.sdistonly and ( | ||
self.config.sdistsrc or self.config.option.installpkg | ||
): | ||
path = self.config.option.installpkg | ||
if not path: | ||
path = self.config.sdistsrc | ||
path = self._resolve_package(path) | ||
self.report.info("using package {!r}, skipping 'sdist' activity ".format(str(path))) | ||
else: | ||
try: | ||
path = self._makesdist() | ||
except tox.exception.InvocationError: | ||
v = sys.exc_info()[1] | ||
self.report.error("FAIL could not package project - v = {!r}".format(v)) | ||
return | ||
sdistfile = self.config.distshare.join(path.basename) | ||
if sdistfile != path: | ||
self.report.info("copying new sdistfile to {!r}".format(str(sdistfile))) | ||
try: | ||
sdistfile.dirpath().ensure(dir=1) | ||
except py.error.Error: | ||
self.report.warning( | ||
"could not copy distfile to {}".format(sdistfile.dirpath()) | ||
) | ||
else: | ||
path.copy(sdistfile) | ||
return path | ||
|
||
def subcommand_test(self): | ||
if self.config.skipsdist: | ||
self.report.info("skipping sdist step") | ||
path = None | ||
else: | ||
path = self.get_installpkg_path() | ||
if not path: | ||
return 2 | ||
for venv in self.venvlist: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We now allow separate packages per virtual environment. The use case for this extension is to allow e.g. binary packages (might need different compilation) and/or wheels (different wheel per venv). |
||
venv.package = self.hook.tox_package(session=self, venv=venv) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah ok, this will cause sdist to be called N times (previously 1) -- should we make this use the virtualenv's python as well in this pass? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That would be a breaking change. This is mostly just reorg to have a clear separation of packaging and plugin-ablity. So not just yet, maybe with tox 4. For now, the plan is to just keep using the host python to build the venv. I'll have a follow-up PR that will create a new virtualenv for building the package, parse the pyproject.toml, install those dependencies. That will effectively implement PEP-517/518 as an opt-in 👍 As external plugin people can implement wheel support, etc. I'll retract my #861 as such. |
||
if not venv.package: | ||
return 2 | ||
if self.config.option.sdistonly: | ||
return | ||
for venv in self.venvlist: | ||
|
@@ -617,7 +537,7 @@ def subcommand_test(self): | |
elif self.config.skipsdist: | ||
self.finishvenv(venv) | ||
else: | ||
self.installpkg(venv, path) | ||
self.installpkg(venv, venv.package) | ||
|
||
self.runenvreport(venv) | ||
self.runtestenv(venv) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,140 @@ | ||
import re | ||
|
||
from tox.config import parseconfig | ||
from tox.package import get_package | ||
from tox.session import Session | ||
|
||
|
||
def test_make_sdist(initproj): | ||
initproj( | ||
"example123-0.5", | ||
filedefs={ | ||
"tests": {"test_hello.py": "def test_hello(): pass"}, | ||
"tox.ini": """ | ||
""", | ||
}, | ||
) | ||
config = parseconfig([]) | ||
session = Session(config) | ||
sdist = get_package(session) | ||
assert sdist.check() | ||
assert sdist.ext == ".zip" | ||
assert sdist == config.distdir.join(sdist.basename) | ||
sdist2 = get_package(session) | ||
assert sdist2 == sdist | ||
sdist.write("hello") | ||
assert sdist.stat().size < 10 | ||
sdist_new = get_package(Session(config)) | ||
assert sdist_new == sdist | ||
assert sdist_new.stat().size > 10 | ||
|
||
|
||
def test_make_sdist_distshare(tmpdir, initproj): | ||
distshare = tmpdir.join("distshare") | ||
initproj( | ||
"example123-0.6", | ||
filedefs={ | ||
"tests": {"test_hello.py": "def test_hello(): pass"}, | ||
"tox.ini": """ | ||
[tox] | ||
distshare={} | ||
""".format( | ||
distshare | ||
), | ||
}, | ||
) | ||
config = parseconfig([]) | ||
session = Session(config) | ||
sdist = get_package(session) | ||
assert sdist.check() | ||
assert sdist.ext == ".zip" | ||
assert sdist == config.distdir.join(sdist.basename) | ||
sdist_share = config.distshare.join(sdist.basename) | ||
assert sdist_share.check() | ||
assert sdist_share.read("rb") == sdist.read("rb"), (sdist_share, sdist) | ||
|
||
|
||
def test_sdistonly(initproj, cmd): | ||
initproj( | ||
"example123", | ||
filedefs={ | ||
"tox.ini": """ | ||
""" | ||
}, | ||
) | ||
result = cmd("-v", "--sdistonly") | ||
assert not result.ret | ||
assert re.match(r".*sdist-make.*setup.py.*", result.out, re.DOTALL) | ||
assert "-mvirtualenv" not in result.out | ||
|
||
|
||
def test_separate_sdist_no_sdistfile(cmd, initproj, tmpdir): | ||
distshare = tmpdir.join("distshare") | ||
initproj( | ||
("pkg123-foo", "0.7"), | ||
filedefs={ | ||
"tox.ini": """ | ||
[tox] | ||
distshare={} | ||
""".format( | ||
distshare | ||
) | ||
}, | ||
) | ||
result = cmd("--sdistonly") | ||
assert not result.ret | ||
distshare_files = distshare.listdir() | ||
assert len(distshare_files) == 1 | ||
sdistfile = distshare_files[0] | ||
assert "pkg123-foo-0.7.zip" in str(sdistfile) | ||
|
||
|
||
def test_separate_sdist(cmd, initproj, tmpdir): | ||
distshare = tmpdir.join("distshare") | ||
initproj( | ||
"pkg123-0.7", | ||
filedefs={ | ||
"tox.ini": """ | ||
[tox] | ||
distshare={} | ||
sdistsrc={{distshare}}/pkg123-0.7.zip | ||
""".format( | ||
distshare | ||
) | ||
}, | ||
) | ||
result = cmd("--sdistonly") | ||
assert not result.ret | ||
sdistfiles = distshare.listdir() | ||
assert len(sdistfiles) == 1 | ||
sdistfile = sdistfiles[0] | ||
result = cmd("-v", "--notest") | ||
assert not result.ret | ||
assert "python inst: {}".format(sdistfile) in result.out | ||
|
||
|
||
def test_sdist_latest(tmpdir, newconfig): | ||
distshare = tmpdir.join("distshare") | ||
config = newconfig( | ||
[], | ||
""" | ||
[tox] | ||
distshare={} | ||
sdistsrc={{distshare}}/pkg123-* | ||
""".format( | ||
distshare | ||
), | ||
) | ||
p = distshare.ensure("pkg123-1.4.5.zip") | ||
distshare.ensure("pkg123-1.4.5a1.zip") | ||
session = Session(config) | ||
sdist_path = get_package(session) | ||
assert sdist_path == p | ||
|
||
|
||
def test_installpkg(tmpdir, newconfig): | ||
p = tmpdir.ensure("pkg123-1.0.zip") | ||
config = newconfig(["--installpkg={}".format(p)], "") | ||
session = Session(config) | ||
sdist_path = get_package(session) | ||
assert sdist_path == p |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I know this is existing, but I couldn't help notice it) -- shouldn' tthis be
as v:
and not retrievingsys.exc_info()
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed it should 👍