feat(pkg_files): preserve external package paths by default

pkg/mappings.bzl

@@ -227,7 +227,16 @@ def _pkg_files_impl(ctx):
                 file_to_target[f] = src
 
     if ctx.attr.strip_prefix == _PKGFILEGROUP_STRIP_ALL:
-        src_dest_paths_map = {src: paths.join(ctx.attr.prefix, src.basename) for src in srcs}
+        src_dest_paths_map = {}
+        for src in srcs:
+            target = file_to_target[src].label
+
+            # Preserve package path for: 1) external->main repo refs, 2) same-repo cross-package target refs (direct)
+            path = _path_relative_to_package(src) if (
+                (target.repo_name == "" and ctx.label.repo_name != "") or
+                (target.repo_name == ctx.label.repo_name and target.package != ctx.label.package and target != _owner(src))
+            ) else src.basename
+            src_dest_paths_map[src] = paths.join(ctx.attr.prefix, path)
     elif ctx.attr.strip_prefix.startswith("/"):
         # Relative to workspace/repository root
         src_dest_paths_map = {src: paths.join(

tests/mappings/external_repo/pkg/test.bzl

@@ -52,17 +52,36 @@ pkg_files_contents_test = analysistest.make(
 
 # Called from the rules_pkg tests
 def test_referencing_remote_file(name):
-    pkg_files(
-        name = "{}_g".format(name),
-        prefix = "usr/share",
-        srcs = ["@//tests:loremipsum_txt"],
+    """Test external package file references with automatic path preservation.
+
+    This test suite verifies that `pkg_files` automatically preserves package-relative paths when referencing files from
+    an external package. Nested tests should produce the same result.
+
+    Args:
+        name: Name of the generated test suite, also base name for generated test targets.
+    """
+    tests = [
         # The prefix in rules_pkg.  Why yes, this is knotty
-        strip_prefix = strip_prefix.from_root("tests"),
-        tags = ["manual"],
-    )
-
-    pkg_files_contents_test(
-        name = name,
-        target_under_test = ":{}_g".format(name),
-        expected_dests = ["usr/share/testdata/loremipsum.txt"],
-    )
+        struct(name = "{}_strip_prefix_from_root".format(name), strip_prefix = strip_prefix.from_root("tests")),
+        # ... which corresponds to stripping all directory components up to the package
+        struct(name = "{}_strip_prefix_from_pkg".format(name), strip_prefix = strip_prefix.from_pkg()),
+        # ... and should also match the default behavior
+        struct(name = "{}_auto_strip_prefix".format(name), strip_prefix = None),
+    ]
+
+    for test in tests:
+        pkg_files(
+            name = "{}_g".format(test.name),
+            prefix = "usr/share",
+            srcs = ["@//tests:loremipsum_txt"],
+            strip_prefix = test.strip_prefix,
+            tags = ["manual"],
+        )
+
+        pkg_files_contents_test(
+            name = test.name,
+            target_under_test = ":{}_g".format(test.name),
+            expected_dests = ["usr/share/testdata/loremipsum.txt"],
+        )
+
+    native.test_suite(name = name, tests = [":{}".format(test.name) for test in tests])

tests/mappings/mappings_test.bzl

@@ -140,6 +140,40 @@ def _test_pkg_files_contents():
         expected_dests = ["hello.txt"],
     )
 
+    # Test same-repository cross-package direct target reference:
+    # When referencing a target from another package, package-relative paths are preserved.
+    pkg_files(
+        name = "pf_cross_package_direct_g",
+        srcs = ["//tests:loremipsum_txt"],  # filegroup from different package
+        tags = ["manual"],
+    )
+
+    pkg_files_contents_test(
+        name = "pf_cross_package_direct",
+        target_under_test = ":pf_cross_package_direct_g",
+        expected_dests = ["testdata/loremipsum.txt"],  # package-relative path
+    )
+
+    # Test same-repository cross-package via local filegroup:
+    # Aggregation brings back earlier basename-only behavior if desired.
+    native.filegroup(
+        name = "loremipsum_via_filegroup",
+        srcs = ["//tests:loremipsum_txt"],
+        tags = ["manual"],
+    )
+
+    pkg_files(
+        name = "pf_cross_package_via_filegroup_g",
+        srcs = [":loremipsum_via_filegroup"],
+        tags = ["manual"],
+    )
+
+    pkg_files_contents_test(
+        name = "pf_cross_package_via_filegroup",
+        target_under_test = ":pf_cross_package_via_filegroup_g",
+        expected_dests = ["loremipsum.txt"],  # basename-only path
+    )
+
     # Used in the following tests
     fake_artifact(
         name = "testdata/test_script",
@@ -940,6 +974,8 @@ def mappings_analysis_tests():
             # Simple tests
             ":pf_no_strip_prefix",
             ":pf_files_only",
+            ":pf_cross_package_direct",
+            ":pf_cross_package_via_filegroup",
             ":pf_strip_testdata_from_pkg",
             ":pf_strip_prefix_from_root",
             ":pf_attributes_mode_overlay_if_not_provided",