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

Add output of bindata rule to runfiles #3021

Closed

Conversation

gregmagolan
Copy link

@gregmagolan gregmagolan commented Dec 3, 2021

What type of PR is this?

Bug fix

What does this PR do? Why is it needed?

The DefaultInfo provided by the bindata rule doesn't put its output in runfiles.

A binary rule such as an sh_binary that adds on the output of bindata to its data would expect the output to be found at $(rootpath :bindata_target).

For example,

sh_binary(
    name = "bin_target",
    srcs = ["script.sh"],
    data = [":bindata_target"],
    args = ["$(rootpath :bindata_target)"],
)

Which issues(s) does this PR fix?

None filed

Other notes for review

@google-cla google-cla bot added the cla: yes label Dec 3, 2021
@robfig
Copy link
Contributor

robfig commented Dec 8, 2021

Thanks for the fix! Is it feasible to augment the bindata test in tests/legacy/examples/bindata/ so that it would detect this problem, to avoid regressions?

@fmeum
Copy link
Member

fmeum commented Apr 8, 2022

Just as a side remark: This behavior of the native rules, which is inconsistent with the recommended behavior for Starlark rules, will be fixed with bazelbuild/bazel#15052. But this PR is still a good workaround in the meantime.

@fmeum
Copy link
Member

fmeum commented Nov 24, 2022

bindata is deprecated and marked for removal in 0.39.0. Going forward, embedsrcs is the recommended way to embed files backed by //go:embed.

The underlying issue in Bazel is fixed in 6.0.0.

@fmeum fmeum closed this Nov 24, 2022
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 this pull request may close these issues.

3 participants