sh_binary's data dependencies are not triggered during the build #15043
Comments
|
I think that the root cause is a more fundamental inconsistency between how Starlark and native rules collect and declare data dependencies: As per the Starlark rules guidelines, Starlark rules should handle targets in However, native rules only collect transitive runfiles and not the output files from rule targets (see addTargetIncludingFileTargets, which only adds output files for file targets). @linzhp Could you check whether #15052 fixes the issue? It adapts the native logic to collect runfiles in the same way as is recommended for Starlark rules, but of course that may very well regress performance for native rules in general. I do not know whether changing this behavior that is so fundamental to Bazel's native rules is realistically possible. But if it doesn't fail too many tests, maybe there is a way to restrict it to Starlark rule targets or otherwise limit its impact on native rule targets. |
oquenchil
added
type: bug
P3
|
Sorry for the delay. I pull from #15052, and built |
|
@linzhp The build succeeds for me following the instructions from your initial post. Are you on Linux and does |
|
Switched to macOS for Intel and tried your fix again. It worked. Thanks! I was on macOS for Apple Silicon before. The commit before yours fails too, but Bazel 5.0.0 works. I saw: |
|
That looks more like an unrelated regression. Could you create a separate issue and perhaps rerun the test with 5.1.0rc2? 5.1.0 will be released soon. |
copy_file and write_file currently include the created file in their runfiles even if it is not executable, which makes every rule depending on it have the file as a runfile (e.g. a `cc_library` depending on a copied header file via the hdrs attribute). In an ideal world, according to https://docs.bazel.build/versions/main/skylark/rules.html#runfiles-features-to-avoid, copy_file would not need to specify any runfiles in the `DefaultInfo` it returns. However, due to the existence of rules with legacy behavior, this would break compatibility (actually, already with `sh_test` in skylib's unit tests), see bazelbuild/bazel#15043. As a compromise that preserves compatibility with legacy rules but nevertheless cleans up the runfiles tree of depending rules, use the `data_runfiles` attribute of `DefaultInfo` if the copied file is not executable.
|
Is this bug a dup (though more clearly restating the issue) of #12348 ? |
Pretty sure it is. |
Due to bazelbuild/bazel#15043, Bazel's native rule such as `sh_test` do not pick up `files` in `DefaultInfo` for a target in `data`. This can lead to the unexplicable absence of the jar generated by `maven_project_jar` and hence `java_export` from runfiles. Until the upstream bug has been fixed, this is worked around by adding the files to `data_runfiles`.
Due to bazelbuild/bazel#15043, Bazel's native rule such as `sh_test` do not pick up `files` in `DefaultInfo` for a target in `data`. This can lead to the unexplicable absence of the jar generated by `maven_project_jar` and hence `java_export` from runfiles. Until the upstream bug has been fixed, this is worked around by adding the files to `data_runfiles`.
Due to bazelbuild/bazel#15043, Bazel's native rule such as `sh_test` do not pick up `files` in `DefaultInfo` for a target in `data`. This can lead to the unexplicable absence of the jar generated by `maven_project_jar` and hence `java_export` from runfiles. Until the upstream bug has been fixed, this is worked around by adding the files to `data_runfiles`.
|
Dupping against #12348. |
|
Sorry, I missed that #15052 is being actively developed, so I'll keep this issue alive as the author's preferred tracking bug. |
Right now the command errors out on fresh clones or after a `bazel
clean`.
$ bazel run //docs:update
cp: cannot stat 'bazel-bin/docs/packaging.md_': No such file or directory
cp: cannot stat 'bazel-bin/docs/pip.md_': No such file or directory
cp: cannot stat 'bazel-bin/docs/pip_repository.md_': No such file or directory
cp: cannot stat 'bazel-bin/docs/python.md_': No such file or directory
I submitted bazelbuild/stardoc#139 to fix this. @brandjon pointed out
that this should just work as-is, but doesn't because of
bazelbuild/bazel#15043. Until the bazel bug is addressed, we can make
`//docs:update` work by pulling in the latest stardoc version.
One side effect of this patch is that the generated documentation
itself changed a decent amount.
Now the tool works again without errors even after a fresh clone or a
`bazel clean`
$ bazel run //docs:update
'bazel-bin/docs/packaging.md_' -> 'docs/packaging.md'
'bazel-bin/docs/pip.md_' -> 'docs/pip.md'
'bazel-bin/docs/pip_repository.md_' -> 'docs/pip_repository.md'
'bazel-bin/docs/python.md_' -> 'docs/python.md'
Guidelines for Starlark rules suggest that `data`-like attributes on rules should always merge the default outputs of rule targets into the transitive runfiles. See: https://docs.bazel.build/versions/main/skylark/rules.html#runfiles-features-to-avoid As a result, Starlark rules generally don't (and shouldn't) explicitly include their default outputs into their runfiles. Before this commit, native rules did not merge these outputs in the same way as idiomatic Starlark rules, which led to unexpectedly absent runfiles. Fixes bazelbuild#15043 Closes bazelbuild#15052. PiperOrigin-RevId: 486663801 Change-Id: I4a30b8b8a4dfc84bf4de27fd7894dd48e795c081
Guidelines for Starlark rules suggest that `data`-like attributes on rules should always merge the default outputs of rule targets into the transitive runfiles. See: https://docs.bazel.build/versions/main/skylark/rules.html#runfiles-features-to-avoid As a result, Starlark rules generally don't (and shouldn't) explicitly include their default outputs into their runfiles. Before this commit, native rules did not merge these outputs in the same way as idiomatic Starlark rules, which led to unexpectedly absent runfiles. Fixes bazelbuild#15043 Closes bazelbuild#15052. PiperOrigin-RevId: 486663801 Change-Id: I4a30b8b8a4dfc84bf4de27fd7894dd48e795c081
Guidelines for Starlark rules suggest that `data`-like attributes on rules should always merge the default outputs of rule targets into the transitive runfiles. See: https://docs.bazel.build/versions/main/skylark/rules.html#runfiles-features-to-avoid As a result, Starlark rules generally don't (and shouldn't) explicitly include their default outputs into their runfiles. Before this commit, native rules did not merge these outputs in the same way as idiomatic Starlark rules, which led to unexpectedly absent runfiles. Fixes bazelbuild#15043 Closes bazelbuild#15052. PiperOrigin-RevId: 486663801 Change-Id: I4a30b8b8a4dfc84bf4de27fd7894dd48e795c081
Guidelines for Starlark rules suggest that `data`-like attributes on rules should always merge the default outputs of rule targets into the transitive runfiles. See: https://docs.bazel.build/versions/main/skylark/rules.html#runfiles-features-to-avoid As a result, Starlark rules generally don't (and shouldn't) explicitly include their default outputs into their runfiles. Before this commit, native rules did not merge these outputs in the same way as idiomatic Starlark rules, which led to unexpectedly absent runfiles. Fixes bazelbuild#15043 Closes bazelbuild#15052. PiperOrigin-RevId: 486663801 Change-Id: I4a30b8b8a4dfc84bf4de27fd7894dd48e795c081
|
As an FYI, there may be some dependencies on this behavior between the Python rules and some other Google-internal rules. i.e. enabling the flag introduced by this will likely show failures in the Google global regression tests (TGP). See the large comment block in blaze/python/common.bzl#collect_runfiles for more details. The gist, though, is some Google-internal rules have different default outputs vs data/default runfiles, and have come to rely on this so that otherwise unused outputs (some of which can be very large) don't get included in the runfiles. |
|
@rickeylev Thanks for mentioning this. I added a workaround to the migration guide at #16654. Could that even work for Google? |
Guidelines for Starlark rules suggest that `data`-like attributes on rules should always merge the default outputs of rule targets into the transitive runfiles. See: https://docs.bazel.build/versions/main/skylark/rules.html#runfiles-features-to-avoid As a result, Starlark rules generally don't (and shouldn't) explicitly include their default outputs into their runfiles. Before this commit, native rules did not merge these outputs in the same way as idiomatic Starlark rules, which led to unexpectedly absent runfiles. Fixes #15043 Closes #15052. PiperOrigin-RevId: 486663801 Change-Id: I4a30b8b8a4dfc84bf4de27fd7894dd48e795c081
Fix //docs:update
Also regenerates docs with the new stardoc version.
Right now the command errors out on fresh clones or after a `bazel
clean`.
$ bazel run //docs:update
cp: cannot stat 'bazel-bin/docs/packaging.md_': No such file or directory
cp: cannot stat 'bazel-bin/docs/pip.md_': No such file or directory
cp: cannot stat 'bazel-bin/docs/pip_repository.md_': No such file or directory
cp: cannot stat 'bazel-bin/docs/python.md_': No such file or directory
I submitted bazelbuild/stardoc#139 to fix this. @brandjon pointed out
that this should just work as-is, but doesn't because of
bazelbuild/bazel#15043. Until the bazel bug is addressed, we can make
`//docs:update` work by pulling in the latest stardoc version.
One side effect of this patch is that the generated documentation
itself changed a decent amount.
Now the tool works again without errors even after a fresh clone or a
`bazel clean`
$ bazel run //docs:update
'bazel-bin/docs/packaging.md_' -> 'docs/packaging.md'
'bazel-bin/docs/pip.md_' -> 'docs/pip.md'
'bazel-bin/docs/pip_repository.md_' -> 'docs/pip_repository.md'
'bazel-bin/docs/python.md_' -> 'docs/python.md'
Description of the problem / feature request:
When a
sh_binaryhas a target in thedataattribute, the target is not built automatically when thesh_binarytarget is built.Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.
bazel cleanbazel build //docs:updatels bazel-bin/docs/container.md_Although
bazel-bin/docs/container.md_is the output of//docs:container_doc, which is depended by//docs:update, it is not built.What operating system are you running Bazel on?
macOS
What's the output of
bazel info release?release 5.0.0-homebrew
The text was updated successfully, but these errors were encountered: