From fbf17ae5af395c898de0ddd2dcbd2b353f899d55 Mon Sep 17 00:00:00 2001 From: Benjy Weinberger Date: Fri, 4 Sep 2020 19:07:32 -0400 Subject: [PATCH] Disambiguate binary and lambda output. (#10735) Previously we named them after the target name, which might not be unique across the repo. Now we use the entire target address. The new name is `path.to.target.dir/target_name.pex/zip`. This seems like a good compromise between completely flat and overstructured. It also preserves the existing filenames, for less confusion. [ci skip-rust] --- build-support/bin/bootstrap_pants_pex.sh | 2 +- pants.toml | 2 ++ .../python/awslambda_python_rules.py | 22 ++++++++++++++-- .../python/awslambda_python_rules_test.py | 3 ++- .../python/goals/create_python_binary.py | 25 +++++++++++++++++-- src/python/pants/option/global_options.py | 11 ++++++++ 6 files changed, 59 insertions(+), 6 deletions(-) diff --git a/build-support/bin/bootstrap_pants_pex.sh b/build-support/bin/bootstrap_pants_pex.sh index 1fe9c89c38b..1308a554d7b 100755 --- a/build-support/bin/bootstrap_pants_pex.sh +++ b/build-support/bin/bootstrap_pants_pex.sh @@ -15,4 +15,4 @@ source "${REPO_ROOT}/build-support/pants_venv" source "${REPO_ROOT}/build-support/bin/native/bootstrap_code.sh" ./pants binary src/python/pants/bin:pants_local_binary || exit 1 -mv dist/pants_local_binary.pex pants.pex +mv dist/src.python.pants.bin/pants_local_binary.pex pants.pex diff --git a/pants.toml b/pants.toml index a0727d070da..a9098fe3c6e 100644 --- a/pants.toml +++ b/pants.toml @@ -1,6 +1,8 @@ [GLOBAL] print_exception_stacktrace = true +pants_distdir_legacy_paths = false + # Enable our custom loose-source plugins. pythonpath = ["%(buildroot)s/pants-plugins"] backend_packages.add = [ diff --git a/src/python/pants/backend/awslambda/python/awslambda_python_rules.py b/src/python/pants/backend/awslambda/python/awslambda_python_rules.py index 604d5cda5dd..d17f2b2a023 100644 --- a/src/python/pants/backend/awslambda/python/awslambda_python_rules.py +++ b/src/python/pants/backend/awslambda/python/awslambda_python_rules.py @@ -1,6 +1,8 @@ # Copyright 2019 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). +import logging +import os from dataclasses import dataclass from pants.backend.awslambda.common.awslambda_common_rules import ( @@ -30,8 +32,11 @@ from pants.engine.process import ProcessResult from pants.engine.rules import Get, collect_rules, rule from pants.engine.unions import UnionRule +from pants.option.global_options import GlobalOptions from pants.util.logging import LogLevel +logger = logging.getLogger(__name__) + @dataclass(frozen=True) class PythonAwsLambdaFieldSet(AWSLambdaFieldSet): @@ -48,10 +53,23 @@ class LambdexSetup: @rule(desc="Create Python AWS Lambda", level=LogLevel.DEBUG) async def create_python_awslambda( - field_set: PythonAwsLambdaFieldSet, lambdex_setup: LambdexSetup + field_set: PythonAwsLambdaFieldSet, lambdex_setup: LambdexSetup, global_options: GlobalOptions ) -> CreatedAWSLambda: # Lambdas typically use the .zip suffix, so we use that instead of .pex. - pex_filename = f"{field_set.address.target_name}.zip" + disambiguated_pex_filename = os.path.join( + field_set.address.spec_path.replace(os.sep, "."), f"{field_set.address.target_name}.zip" + ) + if global_options.options.pants_distdir_legacy_paths: + pex_filename = f"{field_set.address.target_name}.zip" + logger.warning( + f"Writing to the legacy subpath: {pex_filename}, which may not be unique. An " + f"upcoming version of Pants will switch to writing to the fully-qualified subpath: " + f"{disambiguated_pex_filename}. You can effect that switch now (and silence this " + f"warning) by setting `pants_distdir_legacy_paths = false` in the [GLOBAL] section of " + f"pants.toml." + ) + else: + pex_filename = disambiguated_pex_filename # We hardcode the platform value to the appropriate one for each AWS Lambda runtime. # (Running the "hello world" lambda in the example code will report the platform, and can be # used to verify correctness of these platform strings.) diff --git a/src/python/pants/backend/awslambda/python/awslambda_python_rules_test.py b/src/python/pants/backend/awslambda/python/awslambda_python_rules_test.py index 75d02e2e9a3..f10be03e276 100644 --- a/src/python/pants/backend/awslambda/python/awslambda_python_rules_test.py +++ b/src/python/pants/backend/awslambda/python/awslambda_python_rules_test.py @@ -37,6 +37,7 @@ def create_python_awslambda(rule_runner: RuleRunner, addr: str) -> Tuple[str, by args=[ "--backend-packages=pants.backend.awslambda.python", "--source-root-patterns=src/python", + "--pants-distdir-legacy-paths=false", ] ) target = rule_runner.get_target(Address.parse(addr), bootstrapper) @@ -83,7 +84,7 @@ def handler(event, context): zip_file_relpath, content = create_python_awslambda( rule_runner, "src/python/foo/bar:hello_world_lambda" ) - assert "hello_world_lambda.zip" == zip_file_relpath + assert "src.python.foo.bar/hello_world_lambda.zip" == zip_file_relpath zipfile = ZipFile(BytesIO(content)) names = set(zipfile.namelist()) assert "lambdex_handler.py" in names diff --git a/src/python/pants/backend/python/goals/create_python_binary.py b/src/python/pants/backend/python/goals/create_python_binary.py index b87458edf08..84c3179fea1 100644 --- a/src/python/pants/backend/python/goals/create_python_binary.py +++ b/src/python/pants/backend/python/goals/create_python_binary.py @@ -1,6 +1,8 @@ # Copyright 2019 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). +import logging +import os from dataclasses import dataclass from typing import Tuple @@ -28,8 +30,11 @@ from pants.engine.rules import Get, collect_rules, rule from pants.engine.target import HydratedSources, HydrateSourcesRequest from pants.engine.unions import UnionRule +from pants.option.global_options import GlobalOptions from pants.util.logging import LogLevel +logger = logging.getLogger(__name__) + @dataclass(frozen=True) class PythonBinaryFieldSet(BinaryFieldSet, RunFieldSet): @@ -67,7 +72,9 @@ def generate_additional_args( @rule(level=LogLevel.DEBUG) async def create_python_binary( - field_set: PythonBinaryFieldSet, python_binary_defaults: PythonBinaryDefaults + field_set: PythonBinaryFieldSet, + python_binary_defaults: PythonBinaryDefaults, + global_options: GlobalOptions, ) -> CreatedBinary: entry_point = field_set.entry_point.value if entry_point is None: @@ -80,7 +87,21 @@ async def create_python_binary( entry_point = PythonBinarySources.translate_source_file_to_entry_point( stripped_binary_sources.snapshot.files ) - output_filename = f"{field_set.address.target_name}.pex" + + disambiguated_output_filename = os.path.join( + field_set.address.spec_path.replace(os.sep, "."), f"{field_set.address.target_name}.pex" + ) + if global_options.options.pants_distdir_legacy_paths: + output_filename = f"{field_set.address.target_name}.pex" + logger.warning( + f"Writing to the legacy subpath: {output_filename}, which may not be unique. An " + f"upcoming version of Pants will switch to writing to the fully-qualified subpath: " + f"{disambiguated_output_filename}. You can effect that switch now (and silence this " + f"warning) by setting `pants_distdir_legacy_paths = false` in the [GLOBAL] section of " + f"pants.toml." + ) + else: + output_filename = disambiguated_output_filename two_step_pex = await Get( TwoStepPex, TwoStepPexFromTargetsRequest( diff --git a/src/python/pants/option/global_options.py b/src/python/pants/option/global_options.py index 0a612311266..5ae396d2a9a 100644 --- a/src/python/pants/option/global_options.py +++ b/src/python/pants/option/global_options.py @@ -384,6 +384,17 @@ def register_bootstrap_options(cls, register): default=os.path.join(buildroot, "dist"), help="Write end-product artifacts to this dir.", ) + # TODO: Change the default to false in 2.1, deprecate the option in 2.2 and remove in 2.3. + register( + "--pants-distdir-legacy-paths", + type=bool, + advanced=True, + default=True, + help="Whether to write binaries to the pre-2.0 paths under distdir. These legacy " + "paths are not qualified by target address, so may be ambiguous. This option " + "is a temporary mechanism for easing transition to 2.0. We recommemd switching " + "to the new, unambiguous paths ASAP, by setting this option to true.", + ) register( "--pants-subprocessdir", advanced=True,