From bf4609b2779ea7bb6e3498ed8a25ca1204f07c3c Mon Sep 17 00:00:00 2001
From: Sitaktif <sitaktif@gmail.com>
Date: Wed, 1 May 2024 14:12:19 +0100
Subject: [PATCH] Append workspace name to the runfiles directory name for
 `pkg_files` (#864)

* Fix pkg_files_contents_test

The test wasn't asserting anything about the destination path because it
was missing the `env` positional parameter; the "assert_true" was tested
against the assertion message (which always evaluates to True as a
non-empty string).

* Test pkg_files runfiles destination paths

* Append workspace name in runfiles in pkg_files

Commit a811e7f44f09a2348735527fd999f04df8a7dbc7 recently fixed an issue
where the workspace name was missing from the path for runfiles added
via `pkg_tar` and some other rules.

This extends the fix to `pkg_files` as well.
---
 pkg/mappings.bzl                 |  2 +-
 tests/mappings/mappings_test.bzl | 44 ++++++++++++++++++++++++++++----
 2 files changed, 40 insertions(+), 6 deletions(-)

diff --git a/pkg/mappings.bzl b/pkg/mappings.bzl
index 704a8db2..d5a96a0b 100644
--- a/pkg/mappings.bzl
+++ b/pkg/mappings.bzl
@@ -298,7 +298,7 @@ def _pkg_files_impl(ctx):
             target = file_to_target[src]
             runfiles = target[DefaultInfo].default_runfiles
             if runfiles:
-                base_path = src_dest_paths_map[src] + ".runfiles"
+                base_path = src_dest_paths_map[src] + ".runfiles/" + ctx.workspace_name
                 for rf in runfiles.files.to_list():
                     dest_path = paths.join(base_path, rf.short_path)
 
diff --git a/tests/mappings/mappings_test.bzl b/tests/mappings/mappings_test.bzl
index 073e45b2..48ca9d6e 100644
--- a/tests/mappings/mappings_test.bzl
+++ b/tests/mappings/mappings_test.bzl
@@ -59,14 +59,21 @@ def _pkg_files_contents_test_impl(ctx):
     target_under_test = analysistest.target_under_test(env)
 
     expected_dests = {e: None for e in ctx.attr.expected_dests}
+    actual_dests = target_under_test[PackageFilesInfo].dest_src_map.keys()
     n_found = 0
-    for got in target_under_test[PackageFilesInfo].dest_src_map.keys():
+
+    for actual in actual_dests:
         asserts.true(
-            got in expected_dests,
-            "got <%s> not in expected set: %s" % (got, ctx.attr.expected_dests),
+            env,
+            actual in expected_dests,
+            "actual dest <%s> not in expected expected set: %s" % (actual, ctx.attr.expected_dests),
+        )
+    for expected in expected_dests:
+        asserts.true(
+            env,
+            expected in actual_dests,
+            "expected dest <%s> missing from actual set: %s" % (expected, actual_dests),
         )
-        n_found += 1
-    asserts.equals(env, len(expected_dests), n_found)
 
     # Simple equality checks for the others, if specified
     if ctx.attr.expected_attributes:
@@ -250,6 +257,33 @@ def _test_pkg_files_contents():
         target_under_test = ":pf_strip_prefix_from_root_invalid_g",
     )
 
+    # Test include_runfiles.
+    pkg_files(
+        name = "pf_include_runfiles_g",
+        include_runfiles = True,
+        srcs = ["//tests:an_executable"],
+        tags = ["manual"],
+    )
+
+    pkg_files_contents_test(
+        name = "pf_include_runfiles",
+        target_under_test = ":pf_include_runfiles_g",
+        expected_dests = select(
+            {
+                "@bazel_tools//src/conditions:windows": [
+                    "an_executable.exe",
+                    "an_executable.exe.runfiles/_main/tests/foo.cc",
+                    "an_executable.exe.runfiles/_main/tests/testdata/hello.txt",
+                ],
+                "//conditions:default": [
+                    "an_executable",
+                    "an_executable.runfiles/_main/tests/foo.cc",
+                    "an_executable.runfiles/_main/tests/testdata/hello.txt",
+                ],
+            },
+        ),
+    )
+
 def _test_pkg_files_exclusions():
     # Normal filegroup, used in all of the below tests
     #