Skip to content
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

Build fail when depending on a library in a different repo that mixes generated and non-generated sources #1780

Closed
dmitrii-ubskii opened this issue Jan 17, 2023 · 3 comments · Fixed by #2138
Labels

Comments

@dmitrii-ubskii
Copy link
Contributor

rules_rust v0.16.1, Bazel v5.1.1 & v6.0.0.

Scenario: a common library has bindings for a number of different languages, one of which is Rust. rust/BUILD in the library defines a rule to generate the bindings as well as the rust_library rule that includes some helpful wrapper functions.

# @lib//rust:BUILD
load("@rules_rust//rust:defs.bzl", "rust_library")

genrule(
    name = "gen",
    cmd = "echo 'pub fn hello() { println!(\"Hello, world!\"); }' > $(@D)/gen.rs",
    outs = ["src/gen.rs"],
)

rust_library(
    name = "lib-with-gen",
    srcs = ["src/lib.rs", ":gen"],
    visibility = ["//visibility:public"],
)

Another repository wants to use the bindings as a library dependency. We import the git_repository as lib and attempt to use @lib//rust:lib-with-gen as a dependency.

# @bin//:BUILD
load("@rules_rust//rust:defs.bzl", "rust_binary")

rust_binary(
    name = "bin",
    srcs = ["src/main.rs"],
    deps = ["@lib//rust:lib-with-gen"],
)

However, the build fails:

...$ bazel build //:bin
ERROR: /private/var/tmp/.../external/lib/rust/BUILD:9:13: in rust_library rule @lib//rust:lib-with-gen: 
Traceback (most recent call last):
        File "/private/var/tmp/.../external/rules_rust/rust/private/rust.bzl", line 197, column 32, in _rust_library_impl
                return _rust_library_common(ctx, "rlib")
        File "/private/var/tmp/.../external/rules_rust/rust/private/rust.bzl", line 253, column 42, in _rust_library_common
                srcs, crate_root = _transform_sources(ctx, ctx.files.srcs, getattr(ctx.file, "crate_root", None))
        File "/private/var/tmp/.../external/rules_rust/rust/private/rust.bzl", line 172, column 68, in _transform_sources
                src_symlink = ctx.actions.declare_file(paths.relativize(src.short_path, package_root))
        File "/private/var/tmp/.../external/bazel_skylib/lib/paths.bzl", line 186, column 17, in _relativize
                fail("Path '%s' is not beneath '%s'" % (path, start))
Error in fail: Path '../lib/rust/src/lib.rs' is not beneath 'rust'
ERROR: /private/var/tmp/.../external/lib/rust/BUILD:9:13: Analysis of target '@lib//rust:lib-with-gen' failed
ERROR: Analysis of target '//:bin' failed; build aborted: 
INFO: Elapsed time: 1.257s
INFO: 0 processes.
FAILED: Build did NOT complete successfully (2 packages loaded, 3 targets configured)

The build does not fail if both targets are in the same repository, nor does it fail if the library is at the root of its repo (i.e. @lib//:lib-with-gen). The build also does not fail if all sources are generated, or if none of them are.

MRE:
lib: https://github.com/dmitrii-ubskii/rules-rust-mre-external
bin: https://github.com/dmitrii-ubskii/rules-rust-mre-main

@scentini scentini added the bug label Jan 17, 2023
@dmitrii-ubskii
Copy link
Contributor Author

A similar issue was mentioned in #1530 (comment)

@scentini
Copy link
Collaborator

I believe the two are unrelated issues and this issue is a bug specifically with the way we deal with generated source files.
I believe the following line of code is what causes the issue:

src_symlink = ctx.actions.declare_file(paths.relativize(src.short_path, package_root))

Would you mind sharing the values of src.path, src.short_path, package_root at this line?

@dmitrii-ubskii
Copy link
Contributor Author

dmitrii-ubskii commented Jan 17, 2023

Added print((src.root.path, src.path, src.short_path, package_root)) before rules_rust/rust/private/rust.bzl:172. Result: ("", "external/lib/rust/src/lib.rs", "../lib/rust/src/lib.rs", "rust")

UebelAndre added a commit that referenced this issue Sep 9, 2023
…#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>
ttiurani pushed a commit to ttiurani/rules_rust that referenced this issue Sep 15, 2023
…bazelbuild#2138)

Fixes bazelbuild#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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants