nfpm(bugfix): handle package targets w/ output_path (Cherry-pick of #…

…21806) (#21809)

A package target with an output_path puts the packaged file relative to
the build_root instead of relative to the BUILD file. So, we need to
explicitly check for both the file_path (relative to BUILD file) and the
value (which might be relative to the build_root) instead of just the
file_path.

In debugging this, I ended up copying the plugin code into a separate
repo so I could figure out what was going wrong. It would have been
easier to debug if the sandbox contents were listed in the error message
that claims the src file was missing. So, this also expands the error
messages so future issues can be identified (hopefully) more simply.

Co-authored-by: Jacob Floyd <cognifloyd@gmail.com>

src/python/pants/backend/nfpm/field_sets.py

@@ -238,12 +238,19 @@ def nfpm_config(
     ) -> NfpmContent:
         source: str | None = self.source.file_path
         src: str | None = self.src.file_path
+        src_value: str | None = self.src.value
         dst: str = self.dst.value
         if source is not None and not src:
             # If defined, 'source' provides the default value for 'src'.
             src = source
+            src_value = self.source.value
         if src is None:  # src is NOT required; prepare to raise an error.
             raise self.InvalidTarget()
+        if src not in content_sandbox_files and src_value in content_sandbox_files:
+            # A field's file_path assumes the field's value is relative to the BUILD file.
+            # But, for packages the field's value can be relative to the build_root instead,
+            # because a package's output_path can be any arbitrary build_root relative path.
+            src = src_value
         sandbox_file: FileEntry | None = content_sandbox_files.get(src)
         if sandbox_file is None:
             raise self.SrcMissingFomSandbox()

src/python/pants/backend/nfpm/util_rules/generate_config.py

@@ -117,8 +117,8 @@ async def generate_nfpm_yaml(
                 defaults to the file referenced in the '{NfpmContentFileSourceField.alias}' field.
                 Please fix the {'targets at these addresses' if plural else 'target at this address'}:
                 """
-                + ",\n".join(str(address) for address in invalid_content_file_addresses)
             )
+            + "\n".join(str(address) for address in invalid_content_file_addresses)
         )
     if src_missing_from_sandbox_addresses:
         plural = len(src_missing_from_sandbox_addresses) > 1
@@ -129,10 +129,12 @@ async def generate_nfpm_yaml(
                 from the nfpm sandbox. This sandbox contains packages, generated code, and sources
                 from the '{NfpmDependencies.alias}' field. It also contains any file from the
                 '{NfpmContentFileSourceField.alias}' field. Please fix the '{NfpmContentFile.alias}'
-                {'targets at these addresses' if plural else 'target at this address'}.:
+                {'targets at these addresses' if plural else 'target at this address'}:
                 """
-                + ",\n".join(str(address) for address in src_missing_from_sandbox_addresses)
             )
+            + "\n".join(str(address) for address in src_missing_from_sandbox_addresses)
+            + "\n\nThe sandbox contains these files:\n"
+            + "\n".join(str(path) for path in content_sandbox_files)
         )
 
     contents.sort(key=lambda d: d["dst"])
@@ -155,6 +157,8 @@ async def generate_nfpm_yaml(
                 {repr(script_src_missing_from_sandbox)}
                 """
             )
+            + "\n\nThe sandbox contains these files:\n"
+            + "\n".join(str(path) for path in content_sandbox_files)
         )
 
     nfpm_yaml = "# Generated by Pantsbuild\n"

src/python/pants/backend/nfpm/util_rules/generate_config_test.py

@@ -261,6 +261,57 @@ def get_digest(rule_runner: RuleRunner, source_files: dict[str, str]) -> Digest:
             None,
             id="rpm-with-deps-and-ghost",
         ),
+        # with dummy_package dependency
+        pytest.param(
+            "apk",
+            NfpmApkPackageFieldSet,
+            ["contents:dummy_package"],
+            {},
+            ["dummy", "output_path/package"],
+            {},
+            None,
+            id="apk-with-pkg-dep",
+        ),
+        pytest.param(
+            "archlinux",
+            NfpmArchlinuxPackageFieldSet,
+            ["contents:dummy_package"],
+            {},
+            ["dummy", "output_path/package"],
+            {},
+            None,
+            id="archlinux-with-pkg-dep",
+        ),
+        pytest.param(
+            "deb",
+            NfpmDebPackageFieldSet,
+            ["contents:dummy_package"],
+            {},
+            ["dummy", "output_path/package"],
+            {},
+            None,
+            id="deb-with-pkg-dep",
+        ),
+        pytest.param(
+            "rpm",
+            NfpmRpmPackageFieldSet,
+            ["contents:dummy_package"],
+            {},
+            ["dummy", "output_path/package"],
+            {},
+            None,
+            id="rpm-with-pkg-dep",
+        ),
+        pytest.param(
+            "rpm",
+            NfpmRpmPackageFieldSet,
+            ["contents:dummy_package"],
+            {},
+            ["dummy", "output_path/package"],
+            {"ghost_contents": ["/var/log/pkg.log"]},
+            None,
+            id="rpm-with-pkg-dep-and-ghost",
+        ),
         # with malformed dependency
         pytest.param(
             "apk",
@@ -477,6 +528,25 @@ def test_generate_nfpm_yaml(
                     dst="/etc/{_PKG_NAME}",
                     file_mode=0o700,
                 )
+                file(
+                    name="dummy",
+                    source="dummy",
+                    # For this test, the dummy target should be
+                    # treated as if it were a package that defines:
+                    # output_path="output_path/package"
+                    # Instead of wiring an actual package target,
+                    # the test imitates packaging by injecting the file
+                    # into the digest.
+                )
+                nfpm_content_file(
+                    name="dummy_package",
+                    dependencies=[":dummy"],
+                    # NOTE: src pointing to package dep is relative to
+                    # the build_root instead of this BUILD file.
+                    src="output_path/package",
+                    dst="/usr/bin/dummy_package",
+                    file_mode=0o550,
+                )
                 nfpm_content_file(
                     name="malformed",  # needs either source or src.
                     dst="/usr/bin/foo",
@@ -630,6 +700,14 @@ def test_generate_nfpm_yaml(
             entry = contents_by_dst.pop(dst)
             assert "ghost" == entry["type"]
 
+    if "contents:dummy_package" in dependencies:
+        assert "dummy" not in contents_by_dst
+        dst = "/usr/bin/dummy_package"
+        assert dst in contents_by_dst
+        entry = contents_by_dst.pop(dst)
+        assert "" == entry["type"]
+        assert 0o0550 == entry["file_info"]["mode"]
+
     # make sure all contents have been accounted for (popped off above)
     assert len(contents_by_dst) == 0
 

src/python/pants/backend/nfpm/util_rules/sandbox_test.py

@@ -326,48 +326,72 @@ def rule_runner() -> RuleRunner:
         pytest.param(
             "apk",
             NfpmApkPackageFieldSet,
-            ["codegen:generated", "contents:files", "package:package"],
+            [
+                "codegen:generated",
+                "contents:files",
+                "package:package",
+                "package:output_path_package",
+            ],
             {},
             {
                 "codegen/foobar.codegen.generated",
                 "contents/sandbox-file.txt",
                 "package/archive.tar",
+                "relative_to_build_root.tar",
             },
             id="apk-codegen-and-package",
         ),
         pytest.param(
             "archlinux",
             NfpmArchlinuxPackageFieldSet,
-            ["codegen:generated", "contents:files", "package:package"],
+            [
+                "codegen:generated",
+                "contents:files",
+                "package:package",
+                "package:output_path_package",
+            ],
             {},
             {
                 "codegen/foobar.codegen.generated",
                 "contents/sandbox-file.txt",
                 "package/archive.tar",
+                "relative_to_build_root.tar",
             },
             id="archlinux-codegen-and-package",
         ),
         pytest.param(
             "deb",
             NfpmDebPackageFieldSet,
-            ["codegen:generated", "contents:files", "package:package"],
+            [
+                "codegen:generated",
+                "contents:files",
+                "package:package",
+                "package:output_path_package",
+            ],
             {},
             {
                 "codegen/foobar.codegen.generated",
                 "contents/sandbox-file.txt",
                 "package/archive.tar",
+                "relative_to_build_root.tar",
             },
             id="deb-codegen-and-package",
         ),
         pytest.param(
             "rpm",
             NfpmRpmPackageFieldSet,
-            ["codegen:generated", "contents:files", "package:package"],
+            [
+                "codegen:generated",
+                "contents:files",
+                "package:package",
+                "package:output_path_package",
+            ],
             {},
             {
                 "codegen/foobar.codegen.generated",
                 "contents/sandbox-file.txt",
                 "package/archive.tar",
+                "relative_to_build_root.tar",
             },
             id="rpm-codegen-and-package",
         ),
@@ -459,6 +483,18 @@ def test_populate_nfpm_content_sandbox(
                     dst="/opt/foo/archive.tar",
                     dependencies=[":archive"],
                 )
+                archive(
+                    name="output_path_archive",
+                    format="tar",
+                    output_path="relative_to_build_root.tar",
+                    files=[":file"],
+                )
+                nfpm_content_file(
+                    name="output_path_package",
+                    src="relative_to_build_root.tar",
+                    dst="/opt/foo/relative_to_build_root.tar",
+                    dependencies=[":output_path_archive"],
+                )
                 """
             ),
             "package/archive-contents.txt": "",