From a726761ff41d3c2cb1c8a591e991fc812146c4bd Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Tue, 27 Jul 2021 16:22:26 -0700 Subject: [PATCH 1/3] Clarify when we are subsetting the lockfile for 3rdparty dependencies # 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.py | 33 ++++++++++--------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/src/python/pants/backend/python/util_rules/pex.py b/src/python/pants/backend/python/util_rules/pex.py index d4bed157ad2..09e45f02c02 100644 --- a/src/python/pants/backend/python/util_rules/pex.py +++ b/src/python/pants/backend/python/util_rules/pex.py @@ -435,22 +435,25 @@ async def build_pex( description = request.description if description is None: - if request.requirements.req_strings: - description = ( - f"Building {request.output_filename} with " - f"{pluralize(len(request.requirements.req_strings), 'requirement')}: " - f"{', '.join(request.requirements.req_strings)}" - ) - elif request.requirements.file_path: - description = ( - f"Building {request.output_filename} from {request.requirements.file_path}" - ) - elif request.requirements.file_content: - description = ( - f"Building {request.output_filename} from {request.requirements.file_content.path}" - ) - else: + if not request.requirements: description = f"Building {request.output_filename}" + else: + if request.requirements.file_path: + desc_suffix = f"from {request.requirements.file_path}" + elif request.requirements.file_content: + desc_suffix = f"from {request.requirements.file_content.path}" + else: + desc_suffix = ( + f"with {pluralize(len(request.requirements.req_strings), 'requirement')}: " + f"{', '.join(request.requirements.req_strings)}" + ) + if request.repository_pex: + description = ( + f"Extracting from {request.repository_pex.name} to build " + f"{request.output_filename} {desc_suffix}" + ) + else: + description = f"Building {request.output_filename} {desc_suffix}" process = await Get( Process, From 72aac1a1a1b9cc2fd6c11d89931913879b07fdbe Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Tue, 27 Jul 2021 19:15:52 -0700 Subject: [PATCH 2/3] Improve ordering of wording and add a unit test This is now way too complex to not test.. # 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.py | 65 ++++++++++--------- .../backend/python/util_rules/pex_test.py | 59 ++++++++++++++++- 2 files changed, 94 insertions(+), 30 deletions(-) diff --git a/src/python/pants/backend/python/util_rules/pex.py b/src/python/pants/backend/python/util_rules/pex.py index 09e45f02c02..44e35b61dea 100644 --- a/src/python/pants/backend/python/util_rules/pex.py +++ b/src/python/pants/backend/python/util_rules/pex.py @@ -310,12 +310,7 @@ async def build_pex( pex_runtime_env: PexRuntimeEnvironment, ) -> BuildPexResult: """Returns a PEX with the given settings.""" - - argv = [ - "--output-file", - request.output_filename, - *request.additional_args, - ] + argv = ["--output-file", request.output_filename, *request.additional_args] if request.repository_pex: argv.extend(["--pex-repository", request.repository_pex.name]) @@ -433,35 +428,13 @@ async def build_pex( ), ) - description = request.description - if description is None: - if not request.requirements: - description = f"Building {request.output_filename}" - else: - if request.requirements.file_path: - desc_suffix = f"from {request.requirements.file_path}" - elif request.requirements.file_content: - desc_suffix = f"from {request.requirements.file_content.path}" - else: - desc_suffix = ( - f"with {pluralize(len(request.requirements.req_strings), 'requirement')}: " - f"{', '.join(request.requirements.req_strings)}" - ) - if request.repository_pex: - description = ( - f"Extracting from {request.repository_pex.name} to build " - f"{request.output_filename} {desc_suffix}" - ) - else: - description = f"Building {request.output_filename} {desc_suffix}" - process = await Get( Process, PexCliProcess( python=python, argv=argv, additional_input_digest=merged_digest, - description=description, + description=_build_pex_description(request), output_files=[request.output_filename], ), ) @@ -489,6 +462,40 @@ async def build_pex( ) +def _build_pex_description(request: PexRequest) -> str: + if not request.requirements: + return f"Building {request.output_filename}" + if request.repository_pex: + repo_pex = request.repository_pex.name + if request.requirements.req_strings: + return ( + f"Extracting {pluralize(len(request.requirements.req_strings), 'requirement')} " + f"to build {request.output_filename} from {repo_pex}: " + f"{', '.join(request.requirements.req_strings)}" + ) + reqs_file = ( + request.requirements.file_content.path + if request.requirements.file_content + else request.requirements.file_path + ) + assert reqs_file is not None + return ( + f"Extracting all requirements in {reqs_file} from {repo_pex} to build " + f"{request.output_filename}" + ) + + if request.requirements.file_path: + desc_suffix = f"from {request.requirements.file_path}" + elif request.requirements.file_content: + desc_suffix = f"from {request.requirements.file_content.path}" + else: + desc_suffix = ( + f"with {pluralize(len(request.requirements.req_strings), 'requirement')}: " + f"{', '.join(request.requirements.req_strings)}" + ) + return f"Building {request.output_filename} {desc_suffix}" + + @rule async def create_pex(request: PexRequest) -> Pex: result = await Get(BuildPexResult, PexRequest, request) diff --git a/src/python/pants/backend/python/util_rules/pex_test.py b/src/python/pants/backend/python/util_rules/pex_test.py index 1c14142d25e..eebaa362d65 100644 --- a/src/python/pants/backend/python/util_rules/pex_test.py +++ b/src/python/pants/backend/python/util_rules/pex_test.py @@ -28,10 +28,11 @@ PexResolveInfo, VenvPex, VenvPexProcess, + _build_pex_description, ) from pants.backend.python.util_rules.pex import rules as pex_rules from pants.backend.python.util_rules.pex_cli import PexPEX -from pants.engine.fs import CreateDigest, Digest, Directory, FileContent +from pants.engine.fs import EMPTY_DIGEST, CreateDigest, Digest, Directory, FileContent from pants.engine.process import Process, ProcessResult from pants.testutil.rule_runner import QueryRule, RuleRunner @@ -468,3 +469,59 @@ def test_venv_pex_resolve_info(rule_runner: RuleRunner, pex_type: type[Pex | Ven assert dists[3].version == Version("2.23.0") assert Requirement.parse('PySocks!=1.5.7,>=1.5.6; extra == "socks"') in dists[3].requires_dists assert dists[4].project_name == "urllib3" + + +def test_build_pex_description() -> None: + def assert_description( + requirements: PexRequirements, *, use_repo_pex: bool = False, expected: str + ) -> None: + repo_pex = Pex(EMPTY_DIGEST, "repo.pex", None) if use_repo_pex else None + request = PexRequest( + output_filename="new.pex", + internal_only=True, + requirements=requirements, + repository_pex=repo_pex, + ) + assert _build_pex_description(request) == expected + + assert_description(PexRequirements(), expected="Building new.pex") + assert_description(PexRequirements(), use_repo_pex=True, expected="Building new.pex") + + assert_description( + PexRequirements(["req"]), expected="Building new.pex with 1 requirement: req" + ) + assert_description( + PexRequirements(["req"]), + use_repo_pex=True, + expected="Extracting 1 requirement to build new.pex from repo.pex: req", + ) + + assert_description( + PexRequirements(["req1", "req2"]), + expected="Building new.pex with 2 requirements: req1, req2", + ) + assert_description( + PexRequirements(["req1", "req2"]), + use_repo_pex=True, + expected="Extracting 2 requirements to build new.pex from repo.pex: req1, req2", + ) + + assert_description( + PexRequirements(file_content=FileContent("lock.txt", b"")), + expected="Building new.pex from lock.txt", + ) + assert_description( + PexRequirements(file_content=FileContent("lock.txt", b"")), + use_repo_pex=True, + expected="Extracting all requirements in lock.txt from repo.pex to build new.pex", + ) + + assert_description( + PexRequirements(file_path="lock.txt", file_path_description_of_origin="foo"), + expected="Building new.pex from lock.txt", + ) + assert_description( + PexRequirements(file_path="lock.txt", file_path_description_of_origin="foo"), + use_repo_pex=True, + expected="Extracting all requirements in lock.txt from repo.pex to build new.pex", + ) From 81e0ee7d2106c01dd0ed6dbb52956d41af991ec0 Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Tue, 27 Jul 2021 19:40:03 -0700 Subject: [PATCH 3/3] Preserve custom descriptions # 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.py | 3 +++ .../pants/backend/python/util_rules/pex_test.py | 12 +++++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/python/pants/backend/python/util_rules/pex.py b/src/python/pants/backend/python/util_rules/pex.py index 44e35b61dea..58183b5c2e9 100644 --- a/src/python/pants/backend/python/util_rules/pex.py +++ b/src/python/pants/backend/python/util_rules/pex.py @@ -463,8 +463,11 @@ async def build_pex( def _build_pex_description(request: PexRequest) -> str: + if request.description: + return request.description if not request.requirements: return f"Building {request.output_filename}" + if request.repository_pex: repo_pex = request.repository_pex.name if request.requirements.req_strings: diff --git a/src/python/pants/backend/python/util_rules/pex_test.py b/src/python/pants/backend/python/util_rules/pex_test.py index eebaa362d65..bd899693ef7 100644 --- a/src/python/pants/backend/python/util_rules/pex_test.py +++ b/src/python/pants/backend/python/util_rules/pex_test.py @@ -473,17 +473,27 @@ def test_venv_pex_resolve_info(rule_runner: RuleRunner, pex_type: type[Pex | Ven def test_build_pex_description() -> None: def assert_description( - requirements: PexRequirements, *, use_repo_pex: bool = False, expected: str + requirements: PexRequirements, + *, + use_repo_pex: bool = False, + description: str | None = None, + expected: str, ) -> None: repo_pex = Pex(EMPTY_DIGEST, "repo.pex", None) if use_repo_pex else None request = PexRequest( output_filename="new.pex", internal_only=True, requirements=requirements, + description=description, repository_pex=repo_pex, ) assert _build_pex_description(request) == expected + assert_description(PexRequirements(), description="Custom!", expected="Custom!") + assert_description( + PexRequirements(), description="Custom!", use_repo_pex=True, expected="Custom!" + ) + assert_description(PexRequirements(), expected="Building new.pex") assert_description(PexRequirements(), use_repo_pex=True, expected="Building new.pex")