bazel: incompatible_disallow_empty_glob: fail if a glob doesn't match anything

The glob() function tends to be error-prone, because any typo in a path will silently return an empty list. This has been a source of confusion and bugs for many users.

We’ve just added a new argument to glob: allow_empty. For example, let’s say your code looks like:

glob([pattern_a, pattern_b], exclude = [pattern_c], allow_empty = False)

This code will fail if pattern_a or pattern_b doesn’t match anything. It also fails if the whole function (after excluding pattern_c) doesn’t match anything. We believe this behavior is safer in general.

In rare cases, this is intentional that a pattern doesn’t match anything, e.g. when a source file is optional. When it happens, users should tell their intent explicitly by setting allow_empty = True.

  • What’s going to change? The default value of allow_empty used to be True. It will change to False.

  • How to update your code? If a glob fails, use allow_empty = True to keep the old behavior.

If you enable the flag, make sure you are using Bazel 0.29 or above (a bug in Bazel 0.28 can prevent you from testing your code).

About this issue

  • Original URL
  • State: open
  • Created 5 years ago
  • Comments: 20 (17 by maintainers)

Commits related to this issue

Most upvoted comments

@limdor The flag wasn’t tested in the pipeline because the “migration-ready” was removed. I added it again let’s see #15374 will fix the failures.

Ok understood, I will take a look to the failures then. In general the fixes are quite trivial, at least to get the default behavior. The problem is getting reviews in timely manner. The problem that I might be facing in downstream projects is that setting the allow_empty = True might not be well received and some projects might want to investigate why that is the case. However my rationale here is that first it should be set allow_empty to True and then investigate, with this way we can flip the default behavior and stop introducing cases where the glob is empty but the developer did not expect that.

I understand that the downstream projects are important but at some point there is the whole Bazel comunity that is suffering from not flipping this flag.

My proposal would be to try to get the PRs for the downstream projects where it sets allow_empty to true for the affected globs, and in the hypothetical case where they don’t want to merge it, consider flipping it anyway. But somehow I’m sure we should be able to convince the maintainers of the downstream projects that setting the default explicitly is better than doing nothing.

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

@meteorcloudy I believe migrating this flag is a chicken and egg problem. Enabling this does not really solve a problem for users. It merely establishes a sane default for globbing. Imho donwstream projects will most likely not adapt until there is an incentive.

What I am going for is, imho we should flip this flag soon (maybe Bazel 6.0?). This might be the gentle push for downstream projects to adapt to the flag. If they don’t want to, setting the old behavior via the incompatibility flag is easy enough.