Repository rule restart behavior depends on attribute names

[getim]

Description of the problem / feature request:

As the documentation clearly states, repository rules will be restarted whenever the content of any file used and referred to by a label changes. If a repository rule depends on a filegroup for example and the contents of a file in that filegroup changes, the rule will not be restarted.

However, the logic that finds all files referred to directly by attributes stops on the first label-attribute that doesn't resolve to an existing files. On top of that, the order in which the attributes are checked seems to depend on the names of those attributes. That means that a repository rule depending on both a direct file and a filegroup can have the direct file as a restarting dependency or not, depending on the names of the attributes.

The restarting behavior can also be confirmed in the @<external_name>.marker files in the output base, where it only in some cases lists the direct file on a FILE:-line.

Feature requests: what underlying problem are you trying to solve with this feature?

Make repository rule restarting behavior well defined and more predictable.

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

Small example with a repository downloading a file with a dependency on two empty local files foo and bar:

# WORKSPACE
load("my_http_download.bzl", "my_http_download")

my_http_download(
    name = "example",
    foo = "//:foo",
    bar = "//:bar",
)   

# my_http_download.bzl
def _my_http_download(ctx):
    ctx.download(
        url = "https://example.com",
        output = "index.html",
    )
    ctx.file("WORKSPACE")
    ctx.file("BUILD", content="exports_files(['index.html'])")

my_http_download = repository_rule(
    implementation = _my_http_download,
    attrs = {
        "foo": attr.label(),
        "bar": attr.label(),
    }
)

# BUILD
filegroup(
    name = "foo_filegroup",
    srcs = ["foo"],
    visibility = ["//visibility:public"],
)   

filegroup(
    name = "bar_filegroup",
    srcs = ["bar"],
    visibility = ["//visibility:public"],
)   

and bazel build @example//:index.html

• Changing the dependency on //:foo into //:foo_filegroup makes the rule only restart when bar changes (as expected).
• Changing the dependency on //:bar into //:bar_filegroup (but keeping the dependency on //:foo) makes the rule NOT restart when foo changes.

If the name of the foo attribute changes to afoo the incorrect behavior is the other way around:

• Changing the dependency on //:bar into //:bar_filegroup makes the rule only restart when foo changes (as expected).
• Changing the dependency on //:foo into //:foo_filegroup (but keeping the dependency on //:bar) makes the rule NOT restart when bar changes.

What operating system are you running Bazel on?

Arch Linux, same behavior observed on CentOS 7.

What's the output of bazel info release?

release 4.0.0

Have you found anything relevant by searching the web?

The checking on direct files seems to happen here: https://github.com/bazelbuild/bazel/blob/master/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryFunction.java#L189

As the EvalException is caught outside the loop, the other attributes are not checked anymore after throwing.

[fmeum]

@Wyverald Just noticed that this could also be relevant to your proposal.