Skip to content

Commit

Permalink
Create session views of the build wheel/sdist into temp_dir (#2614)
Browse files Browse the repository at this point in the history
Resolves #2612
  • Loading branch information
gaborbernat committed Dec 8, 2022
1 parent 6305d9a commit 2476e95
Show file tree
Hide file tree
Showing 11 changed files with 81 additions and 19 deletions.
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Expand Up @@ -52,7 +52,7 @@ repos:
hooks:
- id: flake8
additional_dependencies:
- flake8-bugbear==22.10.27
- flake8-bugbear==22.12.6
- flake8-comprehensions==3.10.1
- flake8-pytest-style==1.6
- flake8-spellcheck==0.28
Expand Down
1 change: 1 addition & 0 deletions docs/changelog/2612.bugfix.rst
@@ -0,0 +1 @@
Create session views of the build wheel/sdist into the :ref:`temp_dir` folder - by :user:`gaborbernat`.
2 changes: 1 addition & 1 deletion docs/config.rst
Expand Up @@ -172,7 +172,7 @@ Core

.. conf::
:keys: temp_dir
:default: {tox_root}/.tmp
:default: {tox_root}/.temp

Directory where to put tox temporary files. For example: we create a hard link (if possible, otherwise new copy) in
this directory for the project package. This ensures tox works correctly when having parallel runs (as each session
Expand Down
10 changes: 5 additions & 5 deletions pyproject.toml
@@ -1,6 +1,6 @@
[build-system]
build-backend = "hatchling.build"
requires = ["hatchling>=1.11.1", "hatch-vcs>=0.2"]
requires = ["hatchling>=1.11.1", "hatch-vcs>=0.2.1"]

[project]
name = "tox"
Expand All @@ -24,8 +24,8 @@ dependencies = [
"cachetools>=5.2",
"chardet>=5.1",
"colorama>=0.4.6",
"packaging>=21.3",
"platformdirs>=2.5.4",
"packaging>=22",

This comment has been minimized.

Copy link
@mgorny

mgorny Dec 10, 2022

Contributor

Is the new version of packaging strictly required? I've noticed it breaks some packages (notably setuptools, possibly more; I'm not sure because tracebacks I'm getting are deeply confusing), and since we're unbundling dependencies from setuptools on Gentoo that blocks us from adding it for the time being. I've noticed that all tests pass with 21.3, so either >=22 is unnecessary or the test coverage is incomplete.

This comment has been minimized.

Copy link
@gaborbernat

gaborbernat Dec 10, 2022

Author Member

Our continuous integration only runs the test with the latest version. Saying that we support any older version would be at most a best guess and can break it any point. The only thing I'm comfortable specifying here is the latest version. So yes, specifying it as such is necessary to decrease the amount of bug reports by using an old version we don't know if it works.

This comment has been minimized.

Copy link
@gaborbernat

gaborbernat Dec 10, 2022

Author Member

I've noticed that all tests pass with 21.3,

This might be true today, but can easily change tomorrow. We can't really on happenstance.

This comment has been minimized.

Copy link
@mgorny

mgorny Dec 10, 2022

Contributor

But you can trivially run the test suite with the lowest versions as well. This is e.g. what we're doing in pkgcheck:

https://github.com/pkgcore/pkgcheck/blob/master/.github/workflows/test.yml#L67..L69

This comment has been minimized.

Copy link
@gaborbernat

gaborbernat Dec 10, 2022

Author Member

And now my CI is twice as slow, because I need to worry both for lowest and highest. And then I still haven't covered all the in between versions that I also claim support for it. If I'd be a corporate company that makes money off this that kind of complication might make sense, but for an unpaid free software I maintain in my free time I prefer to support only the latest version of everything and that's it.

This comment has been minimized.

Copy link
@mgorny

mgorny Dec 10, 2022

Contributor

If you think us (also volunteers doing this and a lot more in our free time) having to patch the requirement back to make it installable resolves the problem…

This comment has been minimized.

Copy link
@gaborbernat

gaborbernat Dec 10, 2022

Author Member

I'd like you guys not to patch it. Once you patch it the software is unsupported territory on that platform by us, and is all on you.

This comment has been minimized.

Copy link
@mgorny

mgorny Dec 10, 2022

Contributor

Is it better to stay on 4.0.0rc for unclear amount of time before setuptools fixes support for new packaging, then we can actually start looking for all other breakage?

"platformdirs>=2.6",
"pluggy>=1",
"pyproject-api>=1.2.1",
'tomli>=2.0.1; python_version < "3.11"',
Expand All @@ -35,7 +35,7 @@ dependencies = [
'typing-extensions>=4.4; python_version < "3.8"',
]
optional-dependencies.docs = [
"furo>=2022.9.29",
"furo>=2022.12.7",
"sphinx>=5.3",
"sphinx-argparse-cli>=1.10",
"sphinx-autodoc-typehints>=1.19.5",
Expand All @@ -51,7 +51,7 @@ optional-dependencies.testing = [
"diff-cover>=7.2",
"distlib>=0.3.6",
"flaky>=3.7",
"hatch-vcs>=0.2",
"hatch-vcs>=0.2.1",
"hatchling>=1.11.1",
"psutil>=5.9.4",
"pytest>=7.2",
Expand Down
4 changes: 2 additions & 2 deletions src/tox/config/sets.py
Expand Up @@ -199,8 +199,8 @@ def work_dir_builder(conf: Config, env_name: str | None) -> Path: # noqa: U100
self.add_config(
keys=["temp_dir"],
of_type=Path,
default=lambda conf, _: cast(Path, self["tox_root"]) / ".tmp", # noqa: U100, U101
desc="temporary directory cleaned at start",
default=lambda conf, _: cast(Path, self["work_dir"]) / ".tmp", # noqa: U100, U101
desc="a folder for temporary files (is not cleaned at start)",
)

def _on_duplicate_conf(self, key: str, definition: ConfigDefinition[V]) -> None: # noqa: U100
Expand Down
21 changes: 18 additions & 3 deletions src/tox/tox_env/python/virtual_env/package/pyproject.py
Expand Up @@ -30,6 +30,7 @@
)
from tox.tox_env.register import ToxEnvRegister
from tox.tox_env.runner import RunToxEnv
from tox.util.file_view import create_session_view

from ..api import VirtualEnv
from .util import dependencies_with_extras
Expand Down Expand Up @@ -99,6 +100,7 @@ def __init__(self, create_args: ToxEnvCreateArgs) -> None:
self._package_name: str | None = None
self._pkg_lock = RLock() # can build only one package at a time
self.root = self.conf["package_root"]
self._package_paths: set[Path] = set()

@staticmethod
def id() -> str:
Expand Down Expand Up @@ -164,6 +166,10 @@ def _teardown(self) -> None:
pass
finally:
executor.close()
for path in self._package_paths:
if path.exists():
logging.debug("delete package %s", path)
path.unlink()
super()._teardown()

def perform_packaging(self, for_env: EnvConfigSet) -> list[Package]:
Expand All @@ -189,7 +195,10 @@ def perform_packaging(self, for_env: EnvConfigSet) -> list[Package]:
elif of_type == "sdist":
self.setup()
with self._pkg_lock:
package = SdistPackage(self._frontend.build_sdist(sdist_directory=self.pkg_dir).sdist, deps)
sdist = self._frontend.build_sdist(sdist_directory=self.pkg_dir).sdist
sdist = create_session_view(sdist, self._package_temp_path)
self._package_paths.add(sdist)
package = SdistPackage(sdist, deps)
elif of_type in {"wheel", "editable"}:
w_env = self._wheel_build_envs.get(for_env["wheel_build_env"])
if w_env is not None and w_env is not self:
Expand All @@ -199,16 +208,22 @@ def perform_packaging(self, for_env: EnvConfigSet) -> list[Package]:
self.setup()
method = "build_editable" if of_type == "editable" else "build_wheel"
with self._pkg_lock:
path = getattr(self._frontend, method)(
wheel = getattr(self._frontend, method)(
wheel_directory=self.pkg_dir,
metadata_directory=self.meta_folder,
config_settings=self._wheel_config_settings,
).wheel
package = (EditablePackage if of_type == "editable" else WheelPackage)(path, deps)
wheel = create_session_view(wheel, self._package_temp_path)
self._package_paths.add(wheel)
package = (EditablePackage if of_type == "editable" else WheelPackage)(wheel, deps)
else: # pragma: no cover # for when we introduce new packaging types and don't implement
raise TypeError(f"cannot handle package type {of_type}") # pragma: no cover
return [package]

@property
def _package_temp_path(self) -> Path:
return cast(Path, self.core["temp_dir"]) / "package"

def _load_deps(self, for_env: EnvConfigSet) -> list[Requirement]:
# first check if this is statically available via PEP-621
deps = self._load_deps_from_static(for_env)
Expand Down
2 changes: 1 addition & 1 deletion src/tox/tox_env/python/virtual_env/package/util.py
Expand Up @@ -2,7 +2,7 @@

from copy import deepcopy

from packaging.markers import Variable
from packaging.markers import Variable # type: ignore[attr-defined]
from packaging.requirements import Requirement


Expand Down
37 changes: 37 additions & 0 deletions src/tox/util/file_view.py
@@ -0,0 +1,37 @@
from __future__ import annotations

import logging
import os
import shutil
from itertools import chain
from os.path import commonpath
from pathlib import Path


def create_session_view(package: Path, temp_path: Path) -> Path:
"""Allows using the file after you no longer holding a lock to it by moving it into a temp folder"""
# we'll number the active instances, and use the max value as session folder for a new build
# note we cannot change package names as PEP-491 (wheel binary format)
# is strict about file name structure

temp_path.mkdir(parents=True, exist_ok=True)
exists = [i.name for i in temp_path.iterdir()]
file_id = max(chain((0,), (int(i) for i in exists if str(i).isnumeric())))
session_dir = temp_path / str(file_id + 1)
session_dir.mkdir()
session_package = session_dir / package.name

links = False # if we can do hard links do that, otherwise just copy
if hasattr(os, "link"):
try:
os.link(package, session_package)
links = True
except (OSError, NotImplementedError):
pass
if not links:
shutil.copyfile(package, session_package)
operation = "links" if links else "copied"
common = commonpath((session_package, package))
rel_session, rel_package = session_package.relative_to(common), package.relative_to(common)
logging.debug("package %s %s to %s (%s)", rel_session, operation, rel_package, common)
return session_package
11 changes: 11 additions & 0 deletions tests/tox_env/python/test_python_runner.py
Expand Up @@ -88,3 +88,14 @@ def test_journal_package_dir(tmp_path: Path) -> None:
"type": "dir",
},
}


def test_package_temp_dir_view(tox_project: ToxProjectCreator, demo_pkg_inline: Path) -> None:
project = tox_project({"tox.ini": "[testenv]\npackage=wheel"})
result = project.run("r", "-vv", "-e", "py", "--root", str(demo_pkg_inline))
result.assert_success()
wheel_name = "demo_pkg_inline-1.0.0-py3-none-any.whl"
session_path = Path(".tmp") / "package" / "1" / wheel_name
msg = f" D package {session_path} links to {Path('.pkg') / 'dist'/ wheel_name} ({project.path/ '.tox'}) "
assert msg in result.out
assert f" D delete package {project.path / '.tox' / session_path}" in result.out
2 changes: 1 addition & 1 deletion tox.ini
Expand Up @@ -91,7 +91,7 @@ description = do a release, required posarg of the version number
skip_install = true
deps =
gitpython>=3.1.29
packaging>=21.3
packaging>=22
towncrier>=22.8
commands =
python {toxinidir}/tasks/release.py --version {posargs}
Expand Down
8 changes: 3 additions & 5 deletions whitelist.txt
@@ -1,10 +1,8 @@
0rc1
0x3
10ms
1s
2s
5s
a0
abi
addinivalue
addnodes
Expand All @@ -30,6 +28,7 @@ chdir
codec
colorama
commenters
commonpath
conftest
contnode
copytree
Expand All @@ -40,11 +39,9 @@ ctrl
cygwin
deinit
delenv
dev1
devenv
devnull
devpi
devrelease
distinfo
distlib
divmod
Expand Down Expand Up @@ -81,6 +78,7 @@ getresult
getsockname
getsourcelines
groupby
hardlink
hookimpl
hookspec
hookspecs
Expand All @@ -91,6 +89,7 @@ instream
intersphinx
isalpha
isatty
isnumeric
isspace
iterdir
levelname
Expand All @@ -117,7 +116,6 @@ pluggy
pos
posargs
posix
prerelease
prereleases
prj
psutil
Expand Down

0 comments on commit 2476e95

Please sign in to comment.