Skip to content
Merged
Show file tree
Hide file tree
Changes from 13 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
8 changes: 8 additions & 0 deletions .bazelci/presubmit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -575,6 +575,14 @@ tasks:
test_flags:
- "--@rules_rust//rust/toolchain/channel=nightly"
- "--@rules_rust//:no_std=alloc"
generated_inputs_external_repo_ubuntu2004:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Instead of adding a new job, could you not instead make a repository rule that writes a small target to an external repo and add it to https://github.com/bazelbuild/rules_rust/blob/0.27.0/test/deps.bzl ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think I did the right thing here 8d0b467.

FYI. If the rust library is created in the root of the repository, then current implementation doesn't fail.

name: Crate with generated sources in external repository
platform: ubuntu2004
working_directory: test/generated_inputs/external_repo
build_targets:
- "//..."
test_targets:
- "//..."
android_examples_ubuntu2004:
name: Android Examples
platform: ubuntu2004
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
/examples/crate_universe/*/bazel-*
/test/cc_common_link/bazel-*
/test/cc_common_link/with_global_alloc/bazel-*
/test/generated_inputs/external_repo/bazel-*
Comment thread
jgsogo marked this conversation as resolved.
Outdated
Comment thread
jgsogo marked this conversation as resolved.
Outdated
/test/no_std/bazel-*
/docs/bazel-*
user.bazelrc
Expand Down
59 changes: 32 additions & 27 deletions rust/private/rust.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,34 @@ 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 returning relative paths.
Comment thread
jgsogo marked this conversation as resolved.
Outdated

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
Copy Markdown
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
Copy Markdown
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 +179,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
1 change: 1 addition & 0 deletions test/generated_inputs/.bazelignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
/external_repo
1 change: 1 addition & 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
20 changes: 20 additions & 0 deletions test/generated_inputs/external_repo/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
load(
"@rules_rust//rust:defs.bzl",
"rust_library",
"rust_test",
)

rust_library(
name = "generated_inputs_external_repo",
srcs = ["lib.rs"],
edition = "2021",
deps = [
"@rules_rust//test/generated_inputs:use_generated_src_with_crate_root_defined",
],
)

rust_test(
name = "generated_inputs_external_repo_test",
crate = ":generated_inputs_external_repo",
edition = "2021",
)
12 changes: 12 additions & 0 deletions test/generated_inputs/external_repo/WORKSPACE.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
workspace(name = "test_generated_inputs_external_repo")

local_repository(
name = "rules_rust",
path = "../../../",
)

load("@rules_rust//rust:repositories.bzl", "rules_rust_dependencies", "rust_register_toolchains")

rules_rust_dependencies()

rust_register_toolchains()
11 changes: 11 additions & 0 deletions test/generated_inputs/external_repo/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
pub fn forty_two_from_external_repo() -> String {
use_generated_src_with_crate_root_defined::forty_two_as_string()
}

#[cfg(test)]
mod test {
#[test]
fn test_forty_two_as_string() {
assert_eq!(super::forty_two_from_external_repo(), "42");
}
}