Flip default for --incompatible_disallow_empty_glob

[alexeagle]

Help to finish https://github.com/bazelbuild/bazel/issues/8195 by finding projects on Bazel CI which need an explicit setting turning this off, then land a PR in Bazel to change the default for new projects.

[limdor]

The label took my attention and I decided to spend some hours to take a look how much is left. The interesting part is that indeed the first problem is to flip it in Bazel without even caring about the downstream projects. Bazel itself has quite a lot of empty globs.

When doing that, one of the dependencies that pop up is upb, it does not compiler with the incompatible flag https://github.com/protocolbuffers/upb/pull/584

Regarding bazelbuild/bazel, in this PR I tried to flip the flag https://github.com/bazelbuild/bazel/pull/15327. The most interesting is that some issues appear only when flipping the flag, but not when running the tests wiht --incompatible_disallow_empty_glob.

Several could be fixed, just being explicit about allowing empty globs. I created a PR in https://github.com/bazelbuild/bazel/pull/15330 but let's see what the Bazel team will say. In some it is not so clear if it should be allowed or the glob should be removed.

[limdor]

Update: Bazel CI for the flip is green https://github.com/bazelbuild/bazel/pull/15327

[keith]

Amazing work!

[cgrindel]

In rules SIG meeting, this was discussed. This may not be as green as it appears. Please see: https://buildkite.com/bazel/bazelisk-plus-incompatible-flags/builds/1287.

May not be able to flip the flag until after Bazel 6.0.

[limdor]

Yes, see discussion in https://github.com/bazelbuild/bazel/issues/8195#issuecomment-1262515680

[limdor]

Update: All PRs in Bazel have been merged except the flag flip itself. The remaining part is the downstream projects that are tracked here https://github.com/bazelbuild/bazel/pull/15327

[alexeagle]

@limdor from some discussion in the SIG meeting - is this going to mean that rule implementations and macros will now fail if they use native.glob and it returns no files? That's going to be painful to rollout - there's no buildozer command to edit macro code, and rulesets like TS (and Kotlin?) rely on using glob to detect whether files exist as part of their regular logic.

It seems to me this would work better if it were possible to apply only to the glob symbol that is in BUILD file scope, but not to the native.glob symbol that is used in .bzl files.

[limdor]

It seems to me this would work better if it were possible to apply only to the glob symbol that is in BUILD file scope, but not to the native.glob symbol that is used in .bzl files.

Some Bazel developer would have to say how feasible that is and what it would require. I'm not aware if there is a way to differentiate between native.glob and a glob appearing in the BUILD file.

[limdor]

and rulesets like TS (and Kotlin?) rely on using glob to detect whether files exist as part of their regular logic

For these cases allow_empty=True should be added, in the end is just about being explicit with the current behavior of Bazel

[alexeagle]

@wyverald says that it's theoretically possible to make two different glob symbols, though it's not that way today.

[limdor]

Ok, assuming that it is possible. Why would we want to have a difference between glob and native.glob? AFAIK this is not happening with any other symbol that the native version is different. Do we have all relevant people in the Bazel team on board with this decision?

In the end the rules they would just have to adapt the code to have native.globa(...., allow_empty=True) instead of native.globa(....).

[alexeagle]

Yeah our discussion was just that it's going to be a long, painful slog to identify all the locations where the allow_empty=True needs to be added across the whole ecosystem, and there's no tooling we can imagine to make it more trivial (e.g. the error message can't suggest the fix)

[limdor]

You are totally right with that and I would like to change the approach. I would propose to all downstream projects to create a PR flipping the flag in the CI and fixing it until the CI is green. With this way we prevent regressions and at least we move forward instead of 2 steps forwards and 1 backwards.

[limdor]

@alexeagle I would like to come to a SIG rules author meeting to discuss how to proceed. This flag has been there for a long time and as a community we should take a decision if we want to flip it or we just give up and decide that will never be flipped.