Append the workspace name to the runfiles directory name (#856)

The runfiles directory is broken down by workspace name. This means that
files belonging to the main workspace are placed at
"${name}.runfiles/${ctx.workspace_name}/${short_path}". Files belonging
to externals have a short_path starting with "../${external_name}".

Because we currently don't append ctx.workspace_name to the computed
runfiles path, files belonging to the main workspace are placed at the
top level of the runfiles directory, which is incorrect. Even worse is
that files belonging to external repositories end up alongside the
runfiles directory, instead of being contained within.

4 files changed, 32 insertions, 28 deletions

diff --git a/pkg/private/pkg_files.bzl b/pkg/private/pkg_files.bzl
index 6fc0971..85d37c0 100644
--- a/pkg/private/pkg_files.bzl
+++ b/pkg/private/pkg_files.bzl
@@ -73,6 +73,7 @@ _MappingContext = provider(
         # Behaviors
         "allow_duplicates_with_different_content": "bool: don't fail when you double mapped files",
         "include_runfiles": "bool: include runfiles",
+        "workspace_name": "string: name of the main workspace",
         "strip_prefix": "strip_prefix",
 
         "path_mapper": "function to map destination paths",
@@ -119,6 +120,7 @@ def create_mapping_context_from_ctx(
         allow_duplicates_with_different_content = allow_duplicates_with_different_content,
         strip_prefix = strip_prefix,
         include_runfiles = include_runfiles,
+        workspace_name = ctx.workspace_name,
         default_mode = default_mode,
         path_mapper = path_mapper or (lambda x: x),
         # TODO(aiuto): allow these to be passed in as needed. But, before doing
@@ -370,7 +372,8 @@ def add_label_list(mapping_context, srcs):
                 src,
                 data_path,
                 data_path_without_prefix,
-                include_runfiles = mapping_context.include_runfiles,
+                mapping_context.include_runfiles,
+                mapping_context.workspace_name,
             )
 
 def add_from_default_info(
@@ -378,7 +381,8 @@ def add_from_default_info(
         src,
         data_path,
         data_path_without_prefix,
-        include_runfiles = False):
+        include_runfiles,
+        workspace_name):
     """Helper method to add the DefaultInfo of a target to a content_map.
 
     Args:
@@ -428,7 +432,7 @@ def add_from_default_info(
             # the first file of DefaultInfo.files should be the right target.
             base_file = the_executable or all_files[0]
             base_file_path = dest_path(base_file, data_path, data_path_without_prefix)
-            base_path = base_file_path + ".runfiles"
+            base_path = base_file_path + ".runfiles/" + workspace_name
 
             for rf in runfiles.files.to_list():
                 d_path = mapping_context.path_mapper(base_path + "/" + rf.short_path)
diff --git a/tests/mappings/executable.manifest.golden b/tests/mappings/executable.manifest.golden
index 9219554..533ce2c 100644
--- a/tests/mappings/executable.manifest.golden
+++ b/tests/mappings/executable.manifest.golden
@@ -1,7 +1,7 @@
 [
-{"dest":"an_executable.runfiles/tests/foo.cc","gid":null,"group":null,"mode":"","origin":"@//tests:an_executable","src":"tests/foo.cc","type":"file","uid":null,"user":null},
-{"dest":"an_executable.runfiles/tests/an_executable","gid":null,"group":null,"mode":"0755","origin":"@//tests:an_executable","src":"tests/an_executable","type":"file","uid":null,"user":null},
-{"dest":"an_executable.runfiles/tests/testdata/hello.txt","gid":null,"group":null,"mode":"","origin":"@//tests:an_executable","src":"tests/testdata/hello.txt","type":"file","uid":null,"user":null},
+{"dest":"an_executable.runfiles/_main/tests/foo.cc","gid":null,"group":null,"mode":"","origin":"@//tests:an_executable","src":"tests/foo.cc","type":"file","uid":null,"user":null},
+{"dest":"an_executable.runfiles/_main/tests/an_executable","gid":null,"group":null,"mode":"0755","origin":"@//tests:an_executable","src":"tests/an_executable","type":"file","uid":null,"user":null},
+{"dest":"an_executable.runfiles/_main/tests/testdata/hello.txt","gid":null,"group":null,"mode":"","origin":"@//tests:an_executable","src":"tests/testdata/hello.txt","type":"file","uid":null,"user":null},
 {"dest":"an_executable","gid":null,"group":null,"mode":"0755","origin":"@//tests:an_executable","src":"tests/an_executable","type":"file","uid":null,"user":null},
 {"dest":"mappings_test.bzl","gid":null,"group":null,"mode":"","origin":"@//tests/mappings:mappings_test.bzl","src":"tests/mappings/mappings_test.bzl","type":"file","uid":null,"user":null}
 ]
diff --git a/tests/mappings/executable.manifest.windows.golden b/tests/mappings/executable.manifest.windows.golden
index 150f477..e0a719d 100644
--- a/tests/mappings/executable.manifest.windows.golden
+++ b/tests/mappings/executable.manifest.windows.golden
@@ -1,7 +1,7 @@
 [
-{"dest":"an_executable.exe.runfiles/tests/foo.cc","gid":null,"group":null,"mode":"","origin":"@//tests:an_executable","src":"tests/foo.cc","type":"file","uid":null,"user":null},
-{"dest":"an_executable.exe.runfiles/tests/an_executable.exe","gid":null,"group":null,"mode":"0755","origin":"@//tests:an_executable","src":"tests/an_executable.exe","type":"file","uid":null,"user":null},
-{"dest":"an_executable.exe.runfiles/tests/testdata/hello.txt","gid":null,"group":null,"mode":"","origin":"@//tests:an_executable","src":"tests/testdata/hello.txt","type":"file","uid":null,"user":null},
+{"dest":"an_executable.exe.runfiles/_main/tests/foo.cc","gid":null,"group":null,"mode":"","origin":"@//tests:an_executable","src":"tests/foo.cc","type":"file","uid":null,"user":null},
+{"dest":"an_executable.exe.runfiles/_main/tests/an_executable.exe","gid":null,"group":null,"mode":"0755","origin":"@//tests:an_executable","src":"tests/an_executable.exe","type":"file","uid":null,"user":null},
+{"dest":"an_executable.exe.runfiles/_main/tests/testdata/hello.txt","gid":null,"group":null,"mode":"","origin":"@//tests:an_executable","src":"tests/testdata/hello.txt","type":"file","uid":null,"user":null},
 {"dest":"an_executable.exe","gid":null,"group":null,"mode":"0755","origin":"@//tests:an_executable","src":"tests/an_executable.exe","type":"file","uid":null,"user":null},
 {"dest":"mappings_test.bzl","gid":null,"group":null,"mode":"","origin":"@//tests/mappings:mappings_test.bzl","src":"tests/mappings/mappings_test.bzl","type":"file","uid":null,"user":null}
 ]
diff --git a/tests/tar/BUILD b/tests/tar/BUILD
index d0fc200..27f231e 100644
--- a/tests/tar/BUILD
+++ b/tests/tar/BUILD
@@ -342,20 +342,20 @@ verify_archive_test(
     target = ":test-tar-with-runfiles",
     must_contain = [
         "a_program",
-        "a_program.runfiles/tests/tar/BUILD",
+        "a_program.runfiles/_main/tests/tar/BUILD",
         "executable.sh",
     ] + select({
         "@platforms//os:windows": [
             "an_executable.exe",
-            "an_executable.exe.runfiles/tests/foo.cc",
-            "an_executable.exe.runfiles/tests/an_executable.exe",
-            "an_executable.exe.runfiles/tests/testdata/hello.txt",
+            "an_executable.exe.runfiles/_main/tests/foo.cc",
+            "an_executable.exe.runfiles/_main/tests/an_executable.exe",
+            "an_executable.exe.runfiles/_main/tests/testdata/hello.txt",
         ],
         "//conditions:default": [
             "an_executable",
-            "an_executable.runfiles/tests/foo.cc",
-            "an_executable.runfiles/tests/an_executable",
-            "an_executable.runfiles/tests/testdata/hello.txt",
+            "an_executable.runfiles/_main/tests/foo.cc",
+            "an_executable.runfiles/_main/tests/an_executable",
+            "an_executable.runfiles/_main/tests/testdata/hello.txt",
         ]
     }),
 )
@@ -377,20 +377,20 @@ verify_archive_test(
     name = "runfiles_remap_base_path_test",
     must_contain = [
         "new_name",
-        "new_name.runfiles/tests/tar/BUILD",
+        "new_name.runfiles/_main/tests/tar/BUILD",
         "executable.sh",
     ] + select({
         "@platforms//os:windows": [
             "other/different_name.exe",
-            "other/different_name.exe.runfiles/tests/foo.cc",
-            "other/different_name.exe.runfiles/tests/an_executable.exe",
-            "other/different_name.exe.runfiles/tests/testdata/hello.txt",
+            "other/different_name.exe.runfiles/_main/tests/foo.cc",
+            "other/different_name.exe.runfiles/_main/tests/an_executable.exe",
+            "other/different_name.exe.runfiles/_main/tests/testdata/hello.txt",
         ],
         "//conditions:default": [
             "other/different_name",
-            "other/different_name.runfiles/tests/foo.cc",
-            "other/different_name.runfiles/tests/an_executable",
-            "other/different_name.runfiles/tests/testdata/hello.txt",
+            "other/different_name.runfiles/_main/tests/foo.cc",
+            "other/different_name.runfiles/_main/tests/an_executable",
+            "other/different_name.runfiles/_main/tests/testdata/hello.txt",
         ],
     }),
     target = ":test-tar-remap-runfiles-base-path",
@@ -407,11 +407,11 @@ pkg_tar(
         # rename the entire runfiles directory
         "/a_program.runfiles/": "/a/program/runfiles/",
         # rename a specific file
-        "/an_executable.runfiles/tests/testdata/hello.txt": "/myfiles/hello.txt",
-        "/an_executable.exe.runfiles/tests/testdata/hello.txt": "/myfiles/hello.txt",
+        "/an_executable.runfiles/_main/tests/testdata/hello.txt": "/myfiles/hello.txt",
+        "/an_executable.exe.runfiles/_main/tests/testdata/hello.txt": "/myfiles/hello.txt",
         # rename a specific subdirectory
-        "/an_executable.runfiles/tests/": "/mytests/",
-        "/an_executable.exe.runfiles/tests/": "/mytests/",
+        "/an_executable.runfiles/_main/tests/": "/mytests/",
+        "/an_executable.exe.runfiles/_main/tests/": "/mytests/",
     },
 )
 
@@ -419,7 +419,7 @@ verify_archive_test(
     name = "runfiles_remap_full_paths_test",
     must_contain = [
         "a_program",
-        "a/program/runfiles/tests/tar/BUILD",
+        "a/program/runfiles/_main/tests/tar/BUILD",
         "executable.sh",
     ] + select({
         "@platforms//os:windows": [