From a811e7f44f09a2348735527fd999f04df8a7dbc7 Mon Sep 17 00:00:00 2001 From: Ed Schouten Date: Wed, 24 Apr 2024 05:36:35 +0200 Subject: [PATCH] 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. --- pkg/private/pkg_files.bzl | 10 +++-- tests/mappings/executable.manifest.golden | 6 +-- .../executable.manifest.windows.golden | 6 +-- tests/tar/BUILD | 38 +++++++++---------- 4 files changed, 32 insertions(+), 28 deletions(-) diff --git a/pkg/private/pkg_files.bzl b/pkg/private/pkg_files.bzl index 6fc0971c..85d37c0d 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 92195549..533ce2c0 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 150f477e..e0a719d8 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 d0fc200c..27f231ec 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": [