Add test for regression after #4040.#4064
Conversation
| srcs = ["build.rs"], | ||
| data = ["data.txt"], | ||
| deps = [ | ||
| "@rules_rust//tools/runfiles", |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 :|
2 participants
No description provided.