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

fix non-canonical names don't work with provisioning #1360

Merged
merged 1 commit into from
Jul 1, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,18 @@ repos:
args: [--safe]
language_version: python3.7
- repo: https://github.com/asottile/blacken-docs
rev: v0.5.0
rev: v1.1.0
hooks:
- id: blacken-docs
additional_dependencies: [black==19.3b0]
language_version: python3.7
- repo: https://github.com/asottile/seed-isort-config
rev: v1.9.0
rev: v1.9.1
hooks:
- id: seed-isort-config
args: [--application-directories, "src:."]
- repo: https://github.com/pre-commit/mirrors-isort
rev: v4.3.20
rev: v4.3.21
hooks:
- id: isort
- repo: https://github.com/pre-commit/pre-commit-hooks
Expand All @@ -31,7 +31,7 @@ repos:
additional_dependencies: ["flake8-bugbear == 19.3.0"]
language_version: python3.7
- repo: https://github.com/asottile/pyupgrade
rev: v1.17.1
rev: v1.19.0
hooks:
- id: pyupgrade
- repo: https://github.com/pre-commit/pygrep-hooks
Expand Down
1 change: 1 addition & 0 deletions docs/changelog/1359.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
non canonical names within :conf:`requires` cause infinite provisioning loop - by :user:`gaborbernat`
2 changes: 1 addition & 1 deletion setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ console_scripts =
testing =
freezegun >= 0.3.11, <1
pathlib2 >= 2.3.3, <3
pytest >= 3.0.0, <5
pytest >= 4.0.0, <6
pytest-cov >= 2.5.1, <3
pytest-mock >= 1.10.0, <2
pytest-xdist >= 1.22.2, <2
Expand Down
7 changes: 5 additions & 2 deletions src/tox/config/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -1027,7 +1027,7 @@ def line_of_default_to_zero(section, name=None):
config.setupdir = reader.getpath("setupdir", "{toxinidir}")
config.logdir = config.toxworkdir.join("log")
within_parallel = PARALLEL_ENV_VAR_KEY in os.environ
if not within_parallel:
if not within_parallel and not WITHIN_PROVISION:
ensure_empty_dir(config.logdir)

# determine indexserver dictionary
Expand Down Expand Up @@ -1165,7 +1165,7 @@ def ensure_requires_satisfied(config, requires, min_version):
if package_name not in exists:
deps.append(DepConfig(require, None))
exists.add(package_name)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the exists set still needs canonicalization -- but the dist does not (?). The intention here was to use the canonicalization as a deduping mechanims (essentially what pkg_resources.Requirement.key was doing in tox 1.12) and it looks like I goofed and passed that to distribution(...) -- I think the minimal patch is to change the distribution call from package_name to package.name (1 character change)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well we still need tests to validate and it's nice to break infinite loop provisioning, so a bit more characters 😄 but yeah what you say can work 👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh yeah of course :D

dist = importlib_metadata.distribution(package_name)
dist = importlib_metadata.distribution(package.name)
if not package.specifier.contains(dist.version, prereleases=True):
raise MissingDependency(package)
except requirements.InvalidRequirement as exception:
Expand All @@ -1176,6 +1176,9 @@ def ensure_requires_satisfied(config, requires, min_version):
missing_requirements.append(str(requirements.Requirement(require)))
if failed_to_parse:
raise tox.exception.BadRequirement()
if WITHIN_PROVISION and missing_requirements:
msg = "break infinite loop provisioning within {} missing {}"
raise tox.exception.Error(msg.format(sys.executable, missing_requirements))
config.run_provision = bool(len(missing_requirements))
return deps

Expand Down
95 changes: 95 additions & 0 deletions tests/unit/session/test_provision.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
from __future__ import absolute_import, unicode_literals

import os
import shutil
import subprocess
import sys

import py
import pytest
from pathlib2 import Path
from six.moves.urllib.parse import urljoin
from six.moves.urllib.request import pathname2url

from tox.exception import BadRequirement, MissingRequirement

Expand Down Expand Up @@ -141,3 +147,92 @@ def test_provision_cli_args_not_ignored_if_provision_false(cmd, initproj):
initproj("test-0.1", {"tox.ini": "[tox]"})
result = cmd("-a", "--option", "b")
result.assert_fail(is_run_test_env=False)


@pytest.fixture(scope="session")
def wheel(tmp_path_factory):
"""create a wheel for a project"""
state = {"at": 0}

def _wheel(path):
state["at"] += 1
dest_path = tmp_path_factory.mktemp("wheel-{}-".format(state["at"]))
env = os.environ.copy()
try:
subprocess.check_output(
[
sys.executable,
"-m",
"pip",
"wheel",
"-w",
str(dest_path),
"--no-deps",
str(path),
],
universal_newlines=True,
stderr=subprocess.STDOUT,
env=env,
)
except subprocess.CalledProcessError as exception:
assert not exception.returncode, exception.output

wheels = list(dest_path.glob("*.whl"))
assert len(wheels) == 1
wheel = wheels[0]
return wheel

return _wheel


THIS_PROJECT_ROOT = Path(__file__).resolve().parents[3]


@pytest.fixture(scope="session")
def tox_wheel(wheel):
return wheel(THIS_PROJECT_ROOT)


@pytest.fixture(scope="session")
def magic_non_canonical_wheel(wheel, tmp_path_factory):
magic_proj = tmp_path_factory.mktemp("magic")
(magic_proj / "setup.py").write_text(
"from setuptools import setup\nsetup(name='com.magic.this-is-fun')"
)
return wheel(magic_proj)


def test_provision_non_canonical_dep(
cmd, initproj, monkeypatch, tox_wheel, magic_non_canonical_wheel
):
initproj(
"w-0.1",
{
"tox.ini": """
[tox]
envlist = py
requires =
com.magic.this-is-fun
tox == {}
[testenv:.tox]
passenv = *
""".format(
tox_wheel.name.split("-")[1]
)
},
)
find_links = " ".join(
space_path2url(d) for d in (tox_wheel.parent, magic_non_canonical_wheel.parent)
)

monkeypatch.setenv(str("PIP_FIND_LINKS"), str(find_links))

result = cmd("-a", "-v", "-v")
result.assert_success(is_run_test_env=False)


def space_path2url(path):
at_path = str(path)
if " " not in at_path:
return at_path
return urljoin("file:", pathname2url(os.path.abspath(at_path)))
6 changes: 3 additions & 3 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,9 @@ exclude_lines =

[coverage:paths]
source = src/tox
.tox/*/lib/python*/site-packages/tox
.tox/pypy*/site-packages/tox
.tox\*\Lib\site-packages\tox
*/.tox/*/lib/python*/site-packages/tox
*/.tox/pypy*/site-packages/tox
*/.tox\*\Lib\site-packages\tox
*/src/tox
*\src\tox

Expand Down