search

atlassian / bazel-tools

Reusable bits for Bazel
Apache License 2.0
113 stars 36 forks source link

multirun(), command(): export RUNFILES_* envvars #56

Closed laszlocsomor closed 5 years ago

laszlocsomor commented 5 years ago

When binary (or test) "A" data-depends on binary "B", Bazel merges B's runfiles into A's and only generates a runfiles manifest/directory for "A" but not "B". To let "B" find its runfiles, "A" must export the "RUNFILES_*" envvars first. This is common for tests (A) that run binaries (B) with data-dependencies.

The multirun() and command() rules fall prey to this runfiles-merging. To run *_binary targets, they must discover and export the runfiles envvars.

Fixes the bug in https://github.com/bazelbuild/bazel/pull/7774

laszlocsomor commented 5 years ago

/cc @Globegitter

ash2k commented 5 years ago

@laszlocsomor Hey, thanks for the PR! Could you sign the CLA please https://github.com/atlassian/bazel-tools#contributing ?

Would be great to have the mechanism fully documented somewhere i.e. how could I have written this myself without trial and error and reverse engineering how runfiles work?

laszlocsomor commented 5 years ago

@ash2k : Where can I read the text of the CLA?

Runfiles are documented poorly. I plan to improve the docs when I have some time: https://github.com/bazelbuild/bazel/issues/6402

I don't know how you could've written it yourself. I'm a domain expert and it took me hours to figure out the problem.

ash2k commented 5 years ago

@laszlocsomor the CLA text is shown after the email verification step on the docusign website. Sorry for the inconvenience, but that's the process. If you work for Google, a corporate CLA may have been signed already (e.g. we have a CLA signed with Google for our contributions), but I cannot check right now.

ash2k commented 5 years ago

Improving documentation around runfiles would be great. All the fields in the runfiles object need more explanation and examples. Also see https://github.com/bazelbuild/bazel/issues/3015

laszlocsomor commented 5 years ago

@ash2k : Thanks for the approval! I don't yet know if I'm allowed to sign the CLA, so I need to put this PR on hold. I'll ping the thread when I know more.

laszlocsomor commented 5 years ago

I signed it!