Skip to content
Draft
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
21 changes: 21 additions & 0 deletions test/unit/cargo_build_script_data_runfiles/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
load("@rules_rust//rust:defs.bzl", "rust_binary")
load("@rules_rust//cargo:defs.bzl", "cargo_build_script")

package(
default_visibility = ["//visibility:public"],
)

cargo_build_script(
name = "build_script",
srcs = ["build.rs"],
data = ["data.txt"],
deps = [
"@rules_rust//tools/runfiles",
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.

Ah, I didn't consider cases where cargo_build_script uses the runfiles library. Can you expand on the need for this? For the most part, I would have expected them to only operate on execpath or CARGO_MANIFETST_DIR paths. We could export the necessary values for script runfiles but data wouldn't be available and I don't think it's a good idea to try to recreate any kind of repo mapping file to be able to use the merged runfiles dir. Do you have thoughts there?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

In the particular target that started failing after your commit, someone wanted to embed some config files in a rust_binary, and they used a cargo_build_script target to read .toml files and generate rust code containing the config values.

That target doesn't need to be a cargo_build_script. In fact, it used to be a gen_embedded_configs.py py_binary and a genrule that ran the binary before a user switched it to a cargo_build_script because it's "the Rust way". So we can switch it back.

What I don't know is what to do with cargo_build_script data dependencies in general. We could say data files are not accessible as runfiles from the build script. But a cargo_build_script can have arbitrary rust_library targets as deps, and these can have data deps and presumably need to access them. But right now they'd get the cargo_runfiles tree that we generate and fail in the same way, right?

For all I know, maybe we'll end up telling users "don't do that". But I don't feel qualified to make that call yet :|

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.

Does #4066 work for you?

],
)

rust_binary(
name = "reproduce_bin",
srcs = ["main.rs"],
deps = [":build_script"],
)
22 changes: 22 additions & 0 deletions test/unit/cargo_build_script_data_runfiles/build.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
use std::env;
use std::path::Path;

fn main() {
let r = runfiles::Runfiles::create().unwrap();
// The rlocation path is relative to the workspace name
let rlocation_path = "rules_rust/test/unit/cargo_build_script_data_runfiles/data.txt";
let path = runfiles::rlocation!(r, rlocation_path);

match path {
Some(p) => {
if p.exists() {
println!("cargo:warning=Found data.txt at {}", p.display());
} else {
panic!("data.txt path returned but does not exist: {}", p.display());
}
}
None => {
panic!("Failed to resolve runfile path: {}", rlocation_path);
}
}
}
1 change: 1 addition & 0 deletions test/unit/cargo_build_script_data_runfiles/data.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
hello world
3 changes: 3 additions & 0 deletions test/unit/cargo_build_script_data_runfiles/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
fn main() {
println!("Hello from main");
}
Loading