Handle files from external repos (fix breaking change in Bazel 3.7.0) (#2138)

Fixes #1780

In Bazel 3.7.0, the path returned by `build_file_path` changed
(bazelbuild/bazel#12344), it no longer
contains the `external/<repo>` prefix for external files. This breaks
the expectations of this rule in the [`paths.relativize`
function](https://github.com/bazelbuild/bazel-skylib/blob/main/docs/paths_doc.md#pathsrelativize)
call.

In this PR I prepend the `workspace_root` (`external/<repo>`) to
`build_file_path` (`<path/in/the/repo>`), and use `path`
(`external/<repo>/path/in/the/repo`) instead of `short_path`
(`../<repo>/path/in/the/repo`).

For files that are generated in different configurations I need to strip
the `root` which contains the
`bazel-out/darwin-fastbuild-ST-a1a0f4088294/bin/` that will never appear
in the `package_root` path.

~Note.- I will probably need some help to add tests for this change~
I've tried to add one test

---------

Co-authored-by: UebelAndre <github@uebelandre.com>

Author: jgsogo

rust/private/rust.bzl

@@ -127,6 +127,35 @@ def get_edition(attr, toolchain, label):
     else:
         return toolchain.default_edition
 
+def _symlink_for_non_generated_source(ctx, src_file, package_root):
+    """Creates and returns a symlink for non-generated source files.
+
+    This rule uses the full path to the source files and the rule directory to compute
+    the relative paths. This is needed, instead of using `short_path`, because of non-generated
+    source files in external repositories possibly returning relative paths depending on the
+    current version of Bazel.
+
+    Args:
+        ctx (struct): The current rule's context.
+        src_file (File): The source file.
+        package_root (File): The full path to the directory containing the current rule.
+
+    Returns:
+        File: The created symlink if a non-generated file, or the file itself.
+    """
+
+    if src_file.is_source or src_file.root.path != ctx.bin_dir.path:
+        src_short_path = paths.relativize(src_file.path, src_file.root.path)
+        src_symlink = ctx.actions.declare_file(paths.relativize(src_short_path, package_root))
+        ctx.actions.symlink(
+            output = src_symlink,
+            target_file = src_file,
+            progress_message = "Creating symlink to source file: {}".format(src_file.path),
+        )
+        return src_symlink
+    else:
+        return src_file
+
 def _transform_sources(ctx, srcs, crate_root):
     """Creates symlinks of the source files if needed.
 
@@ -151,36 +180,13 @@ def _transform_sources(ctx, srcs, crate_root):
     if not has_generated_sources:
         return srcs, crate_root
 
-    generated_sources = []
-
+    package_root = paths.dirname(paths.join(ctx.label.workspace_root, ctx.build_file_path))
+    generated_sources = [_symlink_for_non_generated_source(ctx, src, package_root) for src in srcs if src != crate_root]
     generated_root = crate_root
-    package_root = paths.dirname(ctx.build_file_path)
-
-    if crate_root and (crate_root.is_source or crate_root.root.path != ctx.bin_dir.path):
-        generated_root = ctx.actions.declare_file(paths.relativize(crate_root.short_path, package_root))
-        ctx.actions.symlink(
-            output = generated_root,
-            target_file = crate_root,
-            progress_message = "Creating symlink to source file: {}".format(crate_root.path),
-        )
-    if generated_root:
+    if crate_root:
+        generated_root = _symlink_for_non_generated_source(ctx, crate_root, package_root)
         generated_sources.append(generated_root)
 
-    for src in srcs:
-        # We took care of the crate root above.
-        if src == crate_root:
-            continue
-        if src.is_source or src.root.path != ctx.bin_dir.path:
-            src_symlink = ctx.actions.declare_file(paths.relativize(src.short_path, package_root))
-            ctx.actions.symlink(
-                output = src_symlink,
-                target_file = src,
-                progress_message = "Creating symlink to source file: {}".format(src.path),
-            )
-            generated_sources.append(src_symlink)
-        else:
-            generated_sources.append(src)
-
     return generated_sources, generated_root
 
 def _rust_library_impl(ctx):

test/deps.bzl

@@ -2,6 +2,7 @@
 
 load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
 load("@bazel_tools//tools/build_defs/repo:utils.bzl", "maybe")
+load("//test/generated_inputs:external_repo.bzl", "generated_inputs_in_external_repo")
 load("//test/load_arbitrary_tool:load_arbitrary_tool_test.bzl", "load_arbitrary_tool_test")
 load("//test/unit/toolchain:toolchain_test_utils.bzl", "rules_rust_toolchain_test_target_json_repository")
 
@@ -28,6 +29,8 @@ def rules_rust_test_deps():
 
     load_arbitrary_tool_test()
 
+    generated_inputs_in_external_repo()
+
     maybe(
         http_archive,
         name = "libc",

test/generated_inputs/BUILD.bazel

@@ -57,6 +57,7 @@ rust_library(
     crate_root = "lib.rs",
     edition = "2018",
     tags = ["norustfmt"],
+    visibility = ["//visibility:public"],
 )
 
 rust_library(
@@ -178,3 +179,11 @@ bool_flag(
     name = "change_cfg",
     build_setting_default = False,
 )
+
+rust_test(
+    name = "generated_inputs_external_repo_test",
+    # This is regression testing a specific failure case for generated files _not_ in the root
+    # of an external repository.
+    crate = "@generated_inputs_in_external_repo//lib:generated_inputs_external_repo",
+    edition = "2021",
+)

test/generated_inputs/external_repo.bzl

@@ -0,0 +1,64 @@
+"""External repository for `generated_inputs` tests"""
+
+_BUILD_FILE_CONTENT = """
+load("@rules_rust//rust:defs.bzl", "rust_library")
+load("@bazel_skylib//rules:write_file.bzl", "write_file")
+
+write_file(
+    name = "generate_src",
+    out = "src.rs",
+    content = ["pub fn forty_two() -> i32 { 42 }"],
+)
+
+rust_library(
+    name = "generated_inputs_external_repo",
+    srcs = [
+        "lib.rs",
+        ":generate_src",
+    ],
+    edition = "2021",
+    visibility = ["//visibility:public"],
+)
+"""
+
+_LIB_RS_CONTENT = """
+mod src;
+
+pub fn forty_two_from_generated_src() -> String {
+    format!("{}", src::forty_two())
+}
+
+#[cfg(test)]
+mod test {
+    #[test]
+    fn test_forty_two_as_string() {
+        assert_eq!(super::forty_two_from_generated_src(), "42");
+    }
+}
+"""
+
+def _generated_inputs_in_external_repo_impl(repository_ctx):
+    # Create repository files (not in the root directory)
+    repo_path = repository_ctx.path("lib")
+    repository_ctx.file(
+        "{}/BUILD.bazel".format(repo_path),
+        content = _BUILD_FILE_CONTENT,
+    )
+    repository_ctx.file(
+        "{}/lib.rs".format(repo_path),
+        content = _LIB_RS_CONTENT,
+    )
+
+_generated_inputs_in_external_repo = repository_rule(
+    implementation = _generated_inputs_in_external_repo_impl,
+    doc = (
+        "A test repository rule providing a Rust library using generated sources"
+    ),
+)
+
+def generated_inputs_in_external_repo():
+    """Define the a test repository with Rust library using generated sources"""
+
+    _generated_inputs_in_external_repo(
+        name = "generated_inputs_in_external_repo",
+    )