From 6482ed9cb429550d811b0a37ccab01c013de573b Mon Sep 17 00:00:00 2001 From: Benjy Weinberger Date: Mon, 12 Apr 2021 22:38:52 -0700 Subject: [PATCH 1/8] Handle URL requirements with constraints files. [ci skip-rust] [ci skip-build-wheels] --- .../python/util_rules/pex_from_targets.py | 33 +++++-- .../util_rules/pex_from_targets_test.py | 87 +++++++++++++------ 2 files changed, 88 insertions(+), 32 deletions(-) diff --git a/src/python/pants/backend/python/util_rules/pex_from_targets.py b/src/python/pants/backend/python/util_rules/pex_from_targets.py index 8c89f908415..a7f9a5fd384 100644 --- a/src/python/pants/backend/python/util_rules/pex_from_targets.py +++ b/src/python/pants/backend/python/util_rules/pex_from_targets.py @@ -244,13 +244,25 @@ async def pex_from_targets( # In requirement strings, Foo_-Bar.BAZ and foo-bar-baz refer to the same project. We let # packaging canonicalize for us. # See: https://www.python.org/dev/peps/pep-0503/#normalized-names - exact_req_projects = { - canonicalize_project_name(Requirement.parse(req).project_name) for req in exact_reqs - } + + url_reqs = set() # E.g., 'foobar@ git+https://github.com/foo/bar.git@branch' + name_reqs = set() # E.g., foobar>=1.2.3 + name_req_projects = set() + + for req_str in exact_reqs: + req = Requirement.parse(req_str) + if req.url: # type: ignore[attr-defined] + url_reqs.add(req) + else: + name_reqs.add(req) + name_req_projects.add(canonicalize_project_name(req.project_name)) + constraint_file_projects = { canonicalize_project_name(req.project_name) for req in constraints_file_reqs } - unconstrained_projects = exact_req_projects - constraint_file_projects + # Constraints files must only contain name reqs, not URL reqs (those are already + # constrained by their very nature). See https://github.com/pypa/pip/issues/8210. + unconstrained_projects = name_req_projects - constraint_file_projects if unconstrained_projects: constraints_descr = ( f"constraints file {constraints_file.path}" @@ -272,13 +284,24 @@ async def pex_from_targets( "file does not cover all requirements." ) else: + # To get a full set of requirements we must add the URL requirements to the + # constraints file, since the latter cannot contain URL requirements. + # NB: We can only add the URL requirements we know about here, i.e., those that + # are transitive deps of the targets in play. There may be others in the repo. + # So we may end up creating a few different repository pexes, each with identical + # name requirements but different subsets of URL requirements. Fortunately since + # all these repository pexes will have identical pinned versions of everything, + # this is not a correctness issue, only a performance one. + # TODO: Address this as part of providing proper lockfile support. However we + # generate lockfiles, they must be able to include URL requirements. + all_constraints = {str(req) for req in (constraints_file_reqs | url_reqs)} repository_pex = await Get( Pex, PexRequest( description=f"Resolving {python_setup.requirement_constraints}", output_filename="repository.pex", internal_only=request.internal_only, - requirements=PexRequirements(str(req) for req in constraints_file_reqs), + requirements=PexRequirements(all_constraints), interpreter_constraints=interpreter_constraints, platforms=request.platforms, ), diff --git a/src/python/pants/backend/python/util_rules/pex_from_targets_test.py b/src/python/pants/backend/python/util_rules/pex_from_targets_test.py index f4338582658..8560060969e 100644 --- a/src/python/pants/backend/python/util_rules/pex_from_targets_test.py +++ b/src/python/pants/backend/python/util_rules/pex_from_targets_test.py @@ -20,6 +20,7 @@ from pants.engine.internals.scheduler import ExecutionError from pants.python.python_setup import ResolveAllConstraintsOption from pants.testutil.rule_runner import QueryRule, RuleRunner +from pants.util.contextutil import pushd @pytest.fixture @@ -39,35 +40,50 @@ class Project: version: str -def create_dists(workdir: Path, project: Project, *projects: Project) -> PurePath: - project_dirs = [] - for proj in (project, *projects): - project_dir = workdir / "projects" / proj.name - project_dir.mkdir(parents=True) - project_dirs.append(project_dir) - - (project_dir / "pyproject.toml").write_text( - dedent( - """\ - [build-system] - requires = ["setuptools==54.1.2", "wheel==0.36.2"] - build-backend = "setuptools.build_meta" - """ - ) +build_deps = ["setuptools==54.1.2", "wheel==0.36.2"] + + +def create_project_dir(workdir: Path, project: Project) -> PurePath: + project_dir = workdir / "projects" / project.name + project_dir.mkdir(parents=True) + + (project_dir / "pyproject.toml").write_text( + dedent( + f"""\ + [build-system] + requires = {build_deps} + build-backend = "setuptools.build_meta" + """ ) - (project_dir / "setup.cfg").write_text( - dedent( - f"""\ + ) + (project_dir / "setup.cfg").write_text( + dedent( + f"""\ [metadata] - name = {proj.name} - version = {proj.version} + name = {project.name} + version = {project.version} """ - ) ) + ) + return project_dir + + +def create_dists(workdir: Path, project: Project, *projects: Project) -> PurePath: + project_dirs = [create_project_dir(workdir, proj) for proj in (project, *projects)] pex = workdir / "pex" subprocess.run( - args=[sys.executable, "-m", "pex", *project_dirs, "--include-tools", "-o", pex], check=True + args=[ + sys.executable, + "-m", + "pex", + *project_dirs, + *build_deps, + "--include-tools", + "-o", + pex, + ], + check=True, ) find_links = workdir / "find-links" @@ -113,15 +129,26 @@ def test_constraints_validation(tmp_path_factory: TempPathFactory, rule_runner: Project("QUX", "3.4.5"), ) + # Turn the project dir into a git repo, so it can be cloned. + foorl_dir = create_project_dir(tmp_path_factory.mktemp("git"), Project("foorl", "9.8.7")) + with pushd(str(foorl_dir)): + subprocess.check_call(["git", "init"]) + subprocess.check_call(["git", "add", "--all"]) + subprocess.check_call(["git", "commit", "-m", "initial commit"]) + subprocess.check_call(["git", "branch", "9.8.7"]) + + url_req = f"foorl@ git+file:/{foorl_dir}@9.8.7" + rule_runner.add_to_build_file( "", dedent( - """ + f""" python_requirement_library(name="foo", requirements=["foo-bar>=0.1.2"]) python_requirement_library(name="bar", requirements=["bar==5.5.5"]) python_requirement_library(name="baz", requirements=["baz"]) + python_requirement_library(name="foorl", requirements=["{url_req}"]) python_library(name="util", sources=[], dependencies=[":foo", ":bar"]) - python_library(name="app", sources=[], dependencies=[":util", ":baz"]) + python_library(name="app", sources=[], dependencies=[":util", ":baz", ":foorl"]) """ ), ) @@ -135,6 +162,8 @@ def test_constraints_validation(tmp_path_factory: TempPathFactory, rule_runner: bar==5.5.5 baz==2.2.2 qux==3.4.5 + # Note that pip does not allow URL requirements in constraints files, + # so there is no mention of foorl here. """ ), ) @@ -162,7 +191,9 @@ def get_pex_request( return rule_runner.request(PexRequest, [request]) pex_req1 = get_pex_request("constraints1.txt", ResolveAllConstraintsOption.NEVER) - assert pex_req1.requirements == PexRequirements(["foo-bar>=0.1.2", "bar==5.5.5", "baz"]) + assert pex_req1.requirements == PexRequirements( + ["foo-bar>=0.1.2", "bar==5.5.5", "baz", url_req] + ) assert pex_req1.repository_pex is None pex_req1_direct = get_pex_request( @@ -172,10 +203,12 @@ def get_pex_request( assert pex_req1_direct.repository_pex is None pex_req2 = get_pex_request("constraints1.txt", ResolveAllConstraintsOption.ALWAYS) - assert pex_req2.requirements == PexRequirements(["foo-bar>=0.1.2", "bar==5.5.5", "baz"]) + assert pex_req2.requirements == PexRequirements( + ["foo-bar>=0.1.2", "bar==5.5.5", "baz", url_req] + ) assert pex_req2.repository_pex is not None repository_pex = pex_req2.repository_pex - assert ["Foo._-BAR==1.0.0", "bar==5.5.5", "baz==2.2.2", "qux==3.4.5"] == requirements( + assert ["Foo._-BAR==1.0.0", "bar==5.5.5", "baz==2.2.2", "foorl", "qux==3.4.5"] == requirements( rule_runner, repository_pex ) From 23455f89925d3843bb3d3b6508328c90eab0e0bb Mon Sep 17 00:00:00 2001 From: Benjy Weinberger Date: Tue, 13 Apr 2021 15:17:33 -0700 Subject: [PATCH 2/8] Git is fussy # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels] --- .../python/util_rules/pex_from_targets_test.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/python/pants/backend/python/util_rules/pex_from_targets_test.py b/src/python/pants/backend/python/util_rules/pex_from_targets_test.py index 8560060969e..92bfd99d788 100644 --- a/src/python/pants/backend/python/util_rules/pex_from_targets_test.py +++ b/src/python/pants/backend/python/util_rules/pex_from_targets_test.py @@ -20,7 +20,7 @@ from pants.engine.internals.scheduler import ExecutionError from pants.python.python_setup import ResolveAllConstraintsOption from pants.testutil.rule_runner import QueryRule, RuleRunner -from pants.util.contextutil import pushd +from pants.util.contextutil import environment_as, pushd @pytest.fixture @@ -132,10 +132,11 @@ def test_constraints_validation(tmp_path_factory: TempPathFactory, rule_runner: # Turn the project dir into a git repo, so it can be cloned. foorl_dir = create_project_dir(tmp_path_factory.mktemp("git"), Project("foorl", "9.8.7")) with pushd(str(foorl_dir)): - subprocess.check_call(["git", "init"]) - subprocess.check_call(["git", "add", "--all"]) - subprocess.check_call(["git", "commit", "-m", "initial commit"]) - subprocess.check_call(["git", "branch", "9.8.7"]) + with environment_as(GIT_AUTHOR_NAME="dummy", GIT_AUTHOR_EMAIL="dummy@dummy.com"): + subprocess.check_call(["git", "init"]) + subprocess.check_call(["git", "add", "--all"]) + subprocess.check_call(["git", "commit", "-m", "initial commit"]) + subprocess.check_call(["git", "branch", "9.8.7"]) url_req = f"foorl@ git+file:/{foorl_dir}@9.8.7" From 632e7e33f771c0280ad5352c835a429d6807d758 Mon Sep 17 00:00:00 2001 From: Benjy Weinberger Date: Tue, 13 Apr 2021 16:53:06 -0700 Subject: [PATCH 3/8] Git, still finicky. [ci skip-rust] [ci skip-build-wheels] --- .../python/util_rules/pex_from_targets_test.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/python/pants/backend/python/util_rules/pex_from_targets_test.py b/src/python/pants/backend/python/util_rules/pex_from_targets_test.py index 92bfd99d788..c4193126feb 100644 --- a/src/python/pants/backend/python/util_rules/pex_from_targets_test.py +++ b/src/python/pants/backend/python/util_rules/pex_from_targets_test.py @@ -20,7 +20,7 @@ from pants.engine.internals.scheduler import ExecutionError from pants.python.python_setup import ResolveAllConstraintsOption from pants.testutil.rule_runner import QueryRule, RuleRunner -from pants.util.contextutil import environment_as, pushd +from pants.util.contextutil import pushd @pytest.fixture @@ -132,11 +132,12 @@ def test_constraints_validation(tmp_path_factory: TempPathFactory, rule_runner: # Turn the project dir into a git repo, so it can be cloned. foorl_dir = create_project_dir(tmp_path_factory.mktemp("git"), Project("foorl", "9.8.7")) with pushd(str(foorl_dir)): - with environment_as(GIT_AUTHOR_NAME="dummy", GIT_AUTHOR_EMAIL="dummy@dummy.com"): - subprocess.check_call(["git", "init"]) - subprocess.check_call(["git", "add", "--all"]) - subprocess.check_call(["git", "commit", "-m", "initial commit"]) - subprocess.check_call(["git", "branch", "9.8.7"]) + subprocess.check_call(["git", "init"]) + subprocess.check_call(["git", "config", "user.name", "dummy"]) + subprocess.check_call(["git", "config", "user.email", "dummy@dummy.com"]) + subprocess.check_call(["git", "add", "--all"]) + subprocess.check_call(["git", "commit", "-m", "initial commit"]) + subprocess.check_call(["git", "branch", "9.8.7"]) url_req = f"foorl@ git+file:/{foorl_dir}@9.8.7" From d88aa099d89e54604b1492234c95c409f9d128a0 Mon Sep 17 00:00:00 2001 From: Benjy Weinberger Date: Tue, 13 Apr 2021 17:09:44 -0700 Subject: [PATCH 4/8] Fix test. [ci skip-rust] [ci skip-build-wheels] --- .../pants/backend/python/util_rules/pex_from_targets_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/python/pants/backend/python/util_rules/pex_from_targets_test.py b/src/python/pants/backend/python/util_rules/pex_from_targets_test.py index c4193126feb..261fb6c32fc 100644 --- a/src/python/pants/backend/python/util_rules/pex_from_targets_test.py +++ b/src/python/pants/backend/python/util_rules/pex_from_targets_test.py @@ -201,7 +201,7 @@ def get_pex_request( pex_req1_direct = get_pex_request( "constraints1.txt", ResolveAllConstraintsOption.NEVER, direct_deps_only=True ) - assert pex_req1_direct.requirements == PexRequirements(["baz"]) + assert pex_req1_direct.requirements == PexRequirements(["baz", url_req]) assert pex_req1_direct.repository_pex is None pex_req2 = get_pex_request("constraints1.txt", ResolveAllConstraintsOption.ALWAYS) @@ -217,7 +217,7 @@ def get_pex_request( pex_req2_direct = get_pex_request( "constraints1.txt", ResolveAllConstraintsOption.ALWAYS, direct_deps_only=True ) - assert pex_req2_direct.requirements == PexRequirements(["baz"]) + assert pex_req2_direct.requirements == PexRequirements(["baz", url_req]) assert pex_req2_direct.repository_pex == repository_pex with pytest.raises(ExecutionError) as err: From 5a4dc291acea6311835091bcd6c93b3a3afd3b8c Mon Sep 17 00:00:00 2001 From: Benjy Weinberger Date: Tue, 13 Apr 2021 21:44:51 -0700 Subject: [PATCH 5/8] See what's up # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels] --- .../pants/backend/python/util_rules/pex_from_targets_test.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/python/pants/backend/python/util_rules/pex_from_targets_test.py b/src/python/pants/backend/python/util_rules/pex_from_targets_test.py index 261fb6c32fc..0444c6d73c5 100644 --- a/src/python/pants/backend/python/util_rules/pex_from_targets_test.py +++ b/src/python/pants/backend/python/util_rules/pex_from_targets_test.py @@ -182,6 +182,7 @@ def get_pex_request( output_filename="demo.pex", internal_only=True, direct_deps_only=direct_deps_only, + additional_args=["-vvv"], ) if resolve_all: args.append(f"--python-setup-resolve-all-constraints={resolve_all.value}") From 786dba75a261c401245811287e02ebc78535fcf8 Mon Sep 17 00:00:00 2001 From: Benjy Weinberger Date: Tue, 13 Apr 2021 22:37:33 -0700 Subject: [PATCH 6/8] Again # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels] --- src/python/pants/backend/python/util_rules/pex_from_targets.py | 1 + .../pants/backend/python/util_rules/pex_from_targets_test.py | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/src/python/pants/backend/python/util_rules/pex_from_targets.py b/src/python/pants/backend/python/util_rules/pex_from_targets.py index a7f9a5fd384..a69851c3242 100644 --- a/src/python/pants/backend/python/util_rules/pex_from_targets.py +++ b/src/python/pants/backend/python/util_rules/pex_from_targets.py @@ -304,6 +304,7 @@ async def pex_from_targets( requirements=PexRequirements(all_constraints), interpreter_constraints=interpreter_constraints, platforms=request.platforms, + additional_args=["-vvv"], ), ) elif ( diff --git a/src/python/pants/backend/python/util_rules/pex_from_targets_test.py b/src/python/pants/backend/python/util_rules/pex_from_targets_test.py index 0444c6d73c5..261fb6c32fc 100644 --- a/src/python/pants/backend/python/util_rules/pex_from_targets_test.py +++ b/src/python/pants/backend/python/util_rules/pex_from_targets_test.py @@ -182,7 +182,6 @@ def get_pex_request( output_filename="demo.pex", internal_only=True, direct_deps_only=direct_deps_only, - additional_args=["-vvv"], ) if resolve_all: args.append(f"--python-setup-resolve-all-constraints={resolve_all.value}") From c3d5c5bd9e9b03b0c2cd4f89f76a023b1078fda7 Mon Sep 17 00:00:00 2001 From: Benjy Weinberger Date: Wed, 14 Apr 2021 08:18:43 -0700 Subject: [PATCH 7/8] Maybe this will fix the test in CI. [ci skip-rust] [ci skip-build-wheels] --- .../pants/backend/python/util_rules/pex_from_targets_test.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/python/pants/backend/python/util_rules/pex_from_targets_test.py b/src/python/pants/backend/python/util_rules/pex_from_targets_test.py index 261fb6c32fc..b137a0ae107 100644 --- a/src/python/pants/backend/python/util_rules/pex_from_targets_test.py +++ b/src/python/pants/backend/python/util_rules/pex_from_targets_test.py @@ -2,6 +2,7 @@ # Licensed under the Apache License, Version 2.0 (see LICENSE). import json +import os import subprocess import sys from dataclasses import dataclass @@ -139,7 +140,9 @@ def test_constraints_validation(tmp_path_factory: TempPathFactory, rule_runner: subprocess.check_call(["git", "commit", "-m", "initial commit"]) subprocess.check_call(["git", "branch", "9.8.7"]) - url_req = f"foorl@ git+file:/{foorl_dir}@9.8.7" + # This string won't parse as a Requirement if it contains a full path + # (i.e., three slashes after the `file:`) so we use a relpath. + url_req = f"foorl@ git+file://{os.path.relpath(foorl_dir)}@9.8.7" rule_runner.add_to_build_file( "", From 635b0bd4f006877aba4cdc7cf97983ada0df6c1b Mon Sep 17 00:00:00 2001 From: Benjy Weinberger Date: Wed, 14 Apr 2021 18:43:18 -0700 Subject: [PATCH 8/8] Maybe now... [ci skip-rust] [ci skip-build-wheels] --- .../backend/python/util_rules/pex_from_targets_test.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/python/pants/backend/python/util_rules/pex_from_targets_test.py b/src/python/pants/backend/python/util_rules/pex_from_targets_test.py index b137a0ae107..34a44ae3e25 100644 --- a/src/python/pants/backend/python/util_rules/pex_from_targets_test.py +++ b/src/python/pants/backend/python/util_rules/pex_from_targets_test.py @@ -2,7 +2,6 @@ # Licensed under the Apache License, Version 2.0 (see LICENSE). import json -import os import subprocess import sys from dataclasses import dataclass @@ -140,9 +139,9 @@ def test_constraints_validation(tmp_path_factory: TempPathFactory, rule_runner: subprocess.check_call(["git", "commit", "-m", "initial commit"]) subprocess.check_call(["git", "branch", "9.8.7"]) - # This string won't parse as a Requirement if it contains a full path - # (i.e., three slashes after the `file:`) so we use a relpath. - url_req = f"foorl@ git+file://{os.path.relpath(foorl_dir)}@9.8.7" + # This string won't parse as a Requirement if it doesn't contain a netloc, + # so we explicitly mention localhost. + url_req = f"foorl@ git+file://localhost{foorl_dir.as_posix()}@9.8.7" rule_runner.add_to_build_file( "",