Skip to content

Add test for regression after #4040. - #4064

Closed
slackito wants to merge 1 commit into
bazelbuild:mainfrom
slackito:pr4040-regression
Closed

Add test for regression after #4040.#4064
slackito wants to merge 1 commit into
bazelbuild:mainfrom
slackito:pr4040-regression

Conversation

@slackito

@slackito slackito commented Jun 2, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

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?

UebelAndre added a commit to UebelAndre/rules_rust that referenced this pull request Jun 11, 2026
This change restores support for runfiles in
`cargo_build_script.script`. There exists one limitation where only
`cargo_build_script.tools` can be located via runfiles. Any
`cargo_build_script.data` files should be located relative to the
`CARGO_MANIFEST_DIR` env var. This is required as there is no existing
interface for merging exec (tools) and target (data) runfiles.

closes bazelbuild#4064
@slackito
slackito deleted the pr4040-regression branch June 17, 2026 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants