Bazel 6 regression: cc_library() targets do not collect runfiles from data dependencies correctly.

[digit-google]

Description of the bug:

When trying to use the latest Bazel 6.0.0-pre.20220922.1 pre-release (or even ToT from 2022-10-18), our build rules actually fail to work properly. I could debug it to a regression when the implementation of cc_library() was switched from Java to Starlark.

I have uploaded a reproduction case, but a potential fix and my analysis of the problem at: https://github.com/digit-google/bazel-experiments/tree/main/runfiles/bazel6-data-deps-runfiles-regression

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

https://github.com/digit-google/bazel-experiments/tree/main/runfiles/bazel6-data-deps-runfiles-regression

Which operating system are you running Bazel on?

Linux

What is the output of bazel info release?

6.0.0-pre.20220922.1

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

This also reproduces when building from source using the instructions at https://docs.bazel.build/versions/0.21.0/install-compile-source.html#build-bazel-using-bazel (using an existing Bazel binary).

Also applying my one-line patch to cc_library.bzl seems to solve the issue, but I didn't find instructions to run the regression test suite locally.

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

No response

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

No response

[fmeum]

This is a very subtle point and actually related to #15043 and #15052.

At least based on my understanding of the situation, your analysis at https://github.com/digit-google/bazel-experiments/blob/main/runfiles/bazel6-data-deps-runfiles-regression/README.md seems entirely correct. Summarizing it in Starlark snippets, you have identified the following:

Starlark cc_library

if data_dep[DefaultInfo].data_runfiles.files:
    runfiles_list.append(data_dep[DefaultInfo].data_runfiles)
else:
    runfiles_list.append(ctx.runfiles(transitive_files = data_dep[DefaultInfo].files))

Java cc_library

if data_dep[DefaultInfo].data_runfiles.files:
    runfiles_list.append(data_dep[DefaultInfo].data_runfiles)
else:
    runfiles_list.append(data_dep[DefaultInfo].default_runfiles)

In order to not regress #15043, which was (unintentionally?) fixed by the Starlark cc_* implementation, I think that the correct behavior would indeed by:

if data_dep[DefaultInfo].data_runfiles.files:
    runfiles_list.append(data_dep[DefaultInfo].data_runfiles)
else:
    # This branch ensures interop with custom Starlark rules following
    # https://bazel.build/extending/rules#runfiles_features_to_avoid
    runfiles_list.append(ctx.runfiles(transitive_files = data_dep[DefaultInfo].files))
    runfiles_list.append(data_dep[DefaultInfo].default_runfiles)

CC @comius @oquenchil

[oquenchil]

LGTM

I can send a change with David's solution and Fabian's addition.

[digit-google]

Thank you, I was not aware of #15043 but I agree that adding the DefaultInfo.files as well seems much more logical here (and that the fact that the Java implementation didn't collect them for CcLibrary was a bug).

[oquenchil]

Please take a look at the PR. I will write the proper attribution when merging internally.