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.
bazelbuild · Apr 24, 2024 · a811e7f · a811e7f
1 parent 7849529
commit a811e7f
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 28 deletions.
diff --git 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,15 +372,17 @@ 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(
         mapping_context,
         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
@@ -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
@@ -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
@@ -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,19 +407,19 @@ 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/",
     },
 )
 
 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": [