Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 20 commits into from
Sep 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 33 additions & 27 deletions rust/private/rust.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small nit, can you put the action to symlink back in _transform_sources? The thing I wanted to make crystal clear is what is happening with the path manipulation here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly I'm not sure about it, there will be more duplicated logic and more lines. As a newcomer, I find it harder to read (I was not used to the previous implementation).

Maybe I'm missing something and you have a much better proposal. I will work on the comment about the tests and come back here later to think about something better/easier.


def _symlink_for_non_generated_source(ctx, src_file, package_root):
    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))
        return src_symlink
    else:
        return None
        
def _transform_sources(ctx, srcs, crate_root):
    has_generated_sources = len([src for src in srcs if not src.is_source]) > 0

    if not has_generated_sources:
        return srcs, crate_root

    package_root = paths.dirname(paths.join(ctx.label.workspace_root, ctx.build_file_path))
    generated_sources = []
    for src_file in srcs:
        if src_file == crate_root:
            continue
        src_symlink = _symlink_for_non_generated_source(ctx, src_file, package_root)
        if src_symlink:
            ctx.actions.symlink(
                output = src_symlink,
                target_file = src_file,
                progress_message = "Creating symlink to source file: {}".format(src_file.path),
            )
            generated_sources.append(src_symlink)
        else:
            generated_sources.append(src_file)
            
    generated_root = crate_root
    if crate_root:
        src_symlink = _symlink_for_non_generated_source(ctx, crate_root, package_root)
        if src_symlink:
            ctx.actions.symlink(
                output = src_symlink,
                target_file = crate_root,
                progress_message = "Creating symlink to source file: {}".format(src_file.path),
            )
            generated_sources.append(src_symlink)
        else:
            generated_sources.append(crate_root)

    return generated_sources, generated_root

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.

Expand All @@ -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):
Expand Down
3 changes: 3 additions & 0 deletions test/deps.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand All @@ -28,6 +29,8 @@ def rules_rust_test_deps():

load_arbitrary_tool_test()

generated_inputs_in_external_repo()

maybe(
http_archive,
name = "libc",
Expand Down
9 changes: 9 additions & 0 deletions test/generated_inputs/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ rust_library(
crate_root = "lib.rs",
edition = "2018",
tags = ["norustfmt"],
visibility = ["//visibility:public"],
)

rust_library(
Expand Down Expand Up @@ -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",
)
64 changes: 64 additions & 0 deletions test/generated_inputs/external_repo.bzl
Original file line number Diff line number Diff line change
@@ -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",
)