use a template for computing output_path for packageable targets

docs/notes/2.23.x.md

@@ -35,6 +35,10 @@ for inspiration. To opt into the feature set the flag
 
 ### Goals
 
+#### Package
+
+The `output_path` field present on targets which can be packaged by `pants package` is now based on a template so that you can use parts of `output_path`'s default behavior when overriding it on a target. For example, you can use the template replacement `{{normalized_spec_path}}` to obtain the default output directory for the target (i.e., the directory in which the target lives with slashes replaced by dots).
+
 ### Backends
 
 #### BUILD

src/python/pants/backend/javascript/package/rules.py

@@ -106,7 +106,8 @@ async def pack_node_package_into_tgz_for_publication(
         ),
     )
     if field_set.output_path.value:
-        digest = await Get(Digest, AddPrefix(result.output_digest, field_set.output_path.value))
+        output_path = field_set.output_path.value_or_default(file_ending=None)
+        digest = await Get(Digest, AddPrefix(result.output_digest, output_path))
     else:
         digest = result.output_digest
 
@@ -236,7 +237,8 @@ async def generate_package_artifact_from_node_build_script(
     request = NodeBuildScriptRequest.from_package_request(req)
     result = await Get(NodeBuildScriptResult, NodeBuildScriptRequest, request)
     if req.output_path.value:
-        digest = await Get(Digest, AddPrefix(result.process.output_digest, req.output_path.value))
+        output_path = req.output_path.value_or_default(file_ending=None)
+        digest = await Get(Digest, AddPrefix(result.process.output_digest, output_path))
     else:
         digest = result.process.output_digest
     artifacts = tuple(BuiltPackageArtifact(path) for path in request.get_paths())

src/python/pants/backend/javascript/package/rules_integration_test.py

@@ -19,7 +19,8 @@
 from pants.build_graph.address import Address
 from pants.core.goals.package import BuiltPackage
 from pants.core.target_types import FileTarget, ResourceTarget
-from pants.engine.internals.native_engine import EMPTY_DIGEST, Digest, Snapshot
+from pants.engine.fs import DigestEntries
+from pants.engine.internals.native_engine import EMPTY_DIGEST, Digest, RemovePrefix, Snapshot
 from pants.engine.rules import QueryRule
 from pants.engine.target import GeneratedSources
 from pants.testutil.rule_runner import RuleRunner
@@ -34,6 +35,7 @@ def rule_runner() -> RuleRunner:
             QueryRule(GeneratedSources, (GenerateResourcesFromNodeBuildScriptRequest,)),
             QueryRule(BuiltPackage, (NodeBuildScriptPackageFieldSet,)),
             QueryRule(Snapshot, (Digest,)),
+            QueryRule(DigestEntries, (Digest,)),
         ],
         target_types=[
             *package_json.target_types(),
@@ -160,15 +162,13 @@ def test_packages_sources_as_resource_using_build_tool(rule_runner: RuleRunner)
 def test_packages_sources_as_package_using_build_tool(rule_runner: RuleRunner) -> None:
     tgt = rule_runner.get_target(Address("src/js/ham", generated_name="build"))
     result = rule_runner.request(BuiltPackage, [NodeBuildScriptPackageFieldSet.create(tgt)])
-    rule_runner.write_digest(result.digest)
 
-    assert result.artifacts[0].relpath == "dist"
+    output_digest = rule_runner.request(Digest, [RemovePrefix(result.digest, "src.js.ham/build")])
+    entries = rule_runner.request(DigestEntries, [output_digest])
 
-    result_path = Path(rule_runner.build_root) / "ham" / "dist"
+    assert result.artifacts[0].relpath == "dist"
 
-    assert sorted(
-        str(path.relative_to(rule_runner.build_root)) for path in result_path.iterdir()
-    ) == [
+    assert sorted(entry.path for entry in entries) == [
         "ham/dist/index.css",
         "ham/dist/index.css.map",
         "ham/dist/index.js",

src/python/pants/backend/javascript/package/rules_test.py

@@ -95,7 +95,9 @@ def test_creates_tar_for_package_json(rule_runner: RuleRunner, package_manager:
     rule_runner.write_digest(result.digest)
 
     archive_name = "ham-v0.0.1.tgz" if package_manager == "yarn" else "ham-0.0.1.tgz"
-    with tarfile.open(os.path.join(rule_runner.build_root, archive_name)) as tar:
+    with tarfile.open(
+        os.path.join(rule_runner.build_root, "src.js", "ham-dist", archive_name)
+    ) as tar:
         assert {member.name for member in tar.getmembers()}.issuperset(
             {
                 "package/package.json",

src/python/pants/backend/python/packaging/pyoxidizer/rules.py

@@ -147,7 +147,7 @@ async def package_pyoxidizer_binary(
         config_template = digest_contents[0].content.decode("utf-8")
 
     config = PyOxidizerConfig(
-        executable_name=field_set.binary_name.value or field_set.address.target_name,
+        executable_name=field_set.binary_name.value_or_default(),
         entry_point=field_set.entry_point.value,
         wheels=wheel_paths,
         template=config_template,

src/python/pants/backend/python/packaging/pyoxidizer/target_types.py

@@ -8,6 +8,7 @@
 from pants.core.util_rules.environments import EnvironmentField
 from pants.engine.target import (
     COMMON_TARGET_FIELDS,
+    AsyncFieldMixin,
     Dependencies,
     OptionalSingleSourceField,
     StringField,
@@ -40,16 +41,23 @@ class PyOxidizerOutputPathField(OutputPathField):
     )
 
 
-class PyOxidizerBinaryNameField(OutputPathField):
+class PyOxidizerBinaryNameField(StringField, AsyncFieldMixin):
     alias = "binary_name"
-    default = None
     help = help_text(
         """
         The name of the binary that will be output by PyOxidizer. If not set, this will default
         to the name of this target.
         """
     )
 
+    def value_or_default(self) -> str:
+        if self.value:
+            return self.value
+
+        return (
+            self.address.generated_name if self.address.generated_name else self.address.target_name
+        )
+
 
 class PyOxidizerEntryPointField(StringField):
     alias = "entry_point"

src/python/pants/core/goals/package.py

@@ -66,37 +66,64 @@ class BuiltPackage:
 
 
 class OutputPathField(StringField, AsyncFieldMixin):
+    DEFAULT = "{normalized_spec_path}/{normalized_address}{file_suffix}"
+
     alias = "output_path"
+    default = DEFAULT
+
     help = help_text(
         f"""
         Where the built asset should be located.
 
+        This field supports the following template replacements:
+
+        - `{{normalized_spec_path}}`: The path to the target's directory ("spec path") with forward slashes replaced by dots.
+
+        - `{{normalized_address}}`: The target's name with paramaterizations escaped by replacing dots with underscores.
+
+        - `{{file_suffix}}`: For target's which produce single file artifacts, this is the file type suffix to use with a leading dot,
+          and is empty otherwise when not applicable.
+
         If undefined, this will use the path to the BUILD file, followed by the target name.
-        For example, `src/python/project:app` would be `src.python.project/app.ext`.
+        For example, `src/python/project:app` would be `src.python.project/app.ext`. This behavior corresponds to
+        the default template: `{DEFAULT}`
 
         When running `{bin_name()} package`, this path will be prefixed by `--distdir` (e.g. `dist/`).
 
         Warning: setting this value risks naming collisions with other package targets you may have.
         """
     )
 
-    def value_or_default(self, *, file_ending: str | None) -> str:
-        if self.value:
-            return self.value
+    def parameters(self, *, file_ending: str | None) -> dict[str, str]:
+        normalized_spec_path = self.address.spec_path.replace(os.sep, ".")
+        if not normalized_spec_path:
+            normalized_spec_path = "."
+
         target_name_part = (
             self.address.generated_name.replace(".", "_")
             if self.address.generated_name
             else self.address.target_name
         )
-        params_sanitized = self.address.parameters_repr.replace(".", "_")
-        file_prefix = f"{target_name_part}{params_sanitized}"
+        target_params_sanitized = self.address.parameters_repr.replace(".", "_")
+        normalized_address = f"{target_name_part}{target_params_sanitized}"
 
-        if file_ending is None:
-            file_name = file_prefix
-        else:
+        file_suffix = ""
+        if file_ending:
             assert not file_ending.startswith("."), "`file_ending` should not start with `.`"
-            file_name = f"{file_prefix}.{file_ending}"
-        return os.path.join(self.address.spec_path.replace(os.sep, "."), file_name)
+            file_suffix = f".{file_ending}"
+
+        return dict(
+            normalized_spec_path=normalized_spec_path,
+            normalized_address=normalized_address,
+            file_suffix=file_suffix,
+        )
+
+    def value_or_default(self, *, file_ending: str | None) -> str:
+        template = self.value
+        assert template is not None
+        params = self.parameters(file_ending=file_ending)
+        result = template.format(**params)
+        return os.path.normpath(result)
 
 
 @dataclass(frozen=True)

src/python/pants/core/goals/package_test.py

@@ -13,6 +13,7 @@
 from pants.core.goals.package import (
     BuiltPackage,
     BuiltPackageArtifact,
+    OutputPathField,
     Package,
     PackageFieldSet,
     TraverseIfNotPackageTarget,
@@ -74,7 +75,7 @@ class MockDependenciesField(Dependencies):
 
 class MockTarget(Target):
     alias = "mock"
-    core_fields = (MockTypeField, MockDependenciesField)
+    core_fields = (MockTypeField, MockDependenciesField, OutputPathField)
 
 
 @dataclass(frozen=True)
@@ -252,3 +253,54 @@ def test_transitive_targets_without_traversing_packages(rule_runner: RuleRunner)
     assert x not in transitive_targets.closure
     assert transitive_targets.dependencies == FrozenOrderedSet([y])
     assert transitive_targets.closure == FrozenOrderedSet([z, y])
+
+
+def test_output_path_template_behavior(rule_runner: RuleRunner) -> None:
+    rule_runner.write_files(
+        {
+            "src/foo/BUILD": dedent(
+                """\
+            mock(name="default")
+            mock(name="no-template", output_path="foo/bar")
+            mock(name="with-spec-path", output_path="{normalized_spec_path}/xyzzy")
+            mock(name="with-spec-path-and-ext", output_path="{normalized_spec_path}/xyzzy{file_suffix}")
+            mock(name="with-address-and-ext", output_path="xyzzy/{normalized_address}{file_suffix}")
+            """
+            )
+        }
+    )
+
+    def get_output_path(target_name: str, *, file_ending: str | None = None) -> str:
+        tgt = rule_runner.get_target(Address("src/foo", target_name=target_name))
+        output_path_field = tgt.get(OutputPathField)
+        return output_path_field.value_or_default(file_ending=file_ending)
+
+    output_path_default = get_output_path("default")
+    assert output_path_default == "src.foo/default"
+
+    output_path_default_ext = get_output_path("default", file_ending="ext")
+    assert output_path_default_ext == "src.foo/default.ext"
+
+    output_path_no_template = get_output_path("no-template")
+    assert output_path_no_template == "foo/bar"
+
+    output_path_no_template_ext = get_output_path("no-template", file_ending="ext")
+    assert output_path_no_template_ext == "foo/bar"
+
+    output_path_spec_path = get_output_path("with-spec-path")
+    assert output_path_spec_path == "src.foo/xyzzy"
+
+    output_path_spec_path_ext = get_output_path("with-spec-path", file_ending="ext")
+    assert output_path_spec_path_ext == "src.foo/xyzzy"
+
+    output_path_spec_path_and_ext_1 = get_output_path("with-spec-path-and-ext")
+    assert output_path_spec_path_and_ext_1 == "src.foo/xyzzy"
+
+    output_path_spec_path_and_ext_2 = get_output_path("with-spec-path-and-ext", file_ending="ext")
+    assert output_path_spec_path_and_ext_2 == "src.foo/xyzzy.ext"
+
+    output_path_address_and_ext_1 = get_output_path("with-address-and-ext")
+    assert output_path_address_and_ext_1 == "xyzzy/with-address-and-ext"
+
+    output_path_address_and_ext_2 = get_output_path("with-address-and-ext", file_ending="ext")
+    assert output_path_address_and_ext_2 == "xyzzy/with-address-and-ext.ext"