Better support for globbing in generators

[jhasse]

Globbing support in CMake:

file(GLOB SOURCES src/*.cpp )
add_executable(foo ${SOURCES})

This results in two problems:

• When removing a .cpp file: Ninja fails because an input is missing.
• When adding a .cpp file: Linker error because of missing symbols. Or even worse, incremental build succeeds, but clean build would fail.

There's a workaround:

file(GLOB SOURCES CONFIGURE_DEPENDS src/*.cpp )
add_executable(foo ${SOURCES})

But it's slow and causes other issues, e.g. #2507 and #2707.

I propose we implement a new attribute to the build manifest:

glob = src/*.cpp

Ninja would then rerun that rule if glob changes. The result of the glob wouldn't be available.

Also I would suggest that we only support * and nothing else. Also we're always case-sensitive.
This simplifies the implementation.
Of course that makes the implementation not correct 100% of the time (at least with CMake), but currently it works 0% of the time. And perfect is the enemy is good.

If someone wanted to match CMake's exact behavior he could still use CONFIGURE_DEPENDS. Also I think that globbing is only used by pragmatists anyway ;)

[mathstuf]

Is */.cpp a typo? At first glance, it looks like "any file named .cpp in any (immediate?) directory". But perhaps there is another interpretation meant?

Note that CMake wouldn't be able to use this anyways without accepting globs directly in the source list in some way. At the time of the file(GLOB), CMake has no idea what will be done with the results, and at the expansion site, there's no notion of the values coming from a file(GLOB), nevermind what the glob pattern was. Given that there's no notion of a build rule to attach it to at generate time, even if this were to land, there is non-trivial design work on CMake's side to actually use it.

[jhasse]

Is */.cpp a typo?

Yes, sorry. Fixed it, thx :)

At the time of the file(GLOB), CMake has no idea what will be done with the results, and at the expansion site, there's no notion of the values coming from a file(GLOB), nevermind what the glob pattern was.

Ah good point. Hm ... I guess then it could only be added for CONFIGURE_DEPENDS. Not sure if it's worth it then (because it might even be incorrect compared to the current implementation).

[mathstuf]

Yeah, globbing is…a tempting solution with many drawbacks and some nasty corner cases. I have a pending blog post about how bad it is that I should finish…

[jhasse]

Still I think there can be a better (and faster) solution than running the glob again before every build. For the incremental-build-I-work-on-my-machine-with-reasonable-filesystem use-case we could use the directory mtime for example.

Do you think it would be feasible that CMake detects some cases and uses Ninja's fast path for them? For example when all of these are true

• case-sensitive filesystem
• non-Linux
• no edge-case like overlayfs, network filesystem, etc.
• simple glob that only has one * for files in a directory
  that CMake just adds the directory as a dependency for the generate rule (and we add support for directories as inputs)?

[mathstuf]

Personally, I find globs to be far more trouble than they are worth. Directory mtimes are also so wonky that "adding support to ninja" is likely to be littered with asterisks (as the bullet list indicates). I also don't know what to tell users of other generators either.

My suggestion for the "but globs slow down my build" is to…not use globs. ls $glob | sed > contents once and dump it in the file as needed. Yes, adding and removing files is a little bit more work, but established projects do this rarely enough (IME) that the one or two line build system change is a small part of adding any code in the first place.

[digit-google]

I don't see a need for Ninja to support glob directly if we have a special type of precheck rule that could be invoked at the start of every build. Let me explain.

Since globs are used to detect when a set of files and sub-directories entries (not their content) has changed. There are really two use cases for them:

1. Detecting that a change requires the build graph to be regenerated, because the dependencies in the current one are no longer valid.

2. Detecting that a specific command needs to be re-run, because it takes the directory as input. More possibly:

2a) The directory contains source files only.
2b) The directory is the output of another command.

For use case 1, the special regenerator command (identified by regen = 1) is already used to check the timestamps of a specific set of files. This is not enough to detect glob changes, so let's introduce a new special command (e.g. identified with precheck = 1 or something similar), which, when it exists, is invoked by Ninja at the start of each build before anything else.

If that command returns success (status code 0), then Ninja continues as usual. If it returns an error, Ninja invokes the regenerator command instead.

For use case 2, this can be implemented by using a timestamp file that is used as an input dependency in the Ninja build plan, whose timestamp is updated by the precheck script (for use case 2a) or by the generating command itself (for use case 2b) only when the glob content has changed.

This would minimize the changes to Ninja, and let each generator handle all the details of the glob expressions they support (which will likely need to support exclude lists / pattern filtering and whatnot), caching of glob results, potential use of file-watching tools for very large build plans and probably other features I can't think of.

What do you think? Any other use case we could think of?

[mathstuf]

I think a precheck rule can be good. It would be nice if it did not interrupt ninja: no work to do results if it reported clean. -v or any relevant -d tool would make it report though.

[jhasse]

I think while the precheck is nice it's something that can already be done. It wouldn't improve performance for example. Ergonomic improvements there would be nice, but I think we should rather implement a new attribute for the glob check rule that CMake generates.

What I had in mind is probably really best fixed via allowing directories as inputs. If an input ends with / it would be treated as a directory. I'm currently working on file hashing mode, in that mode directories could be checked with a hash of their content list in addition to the mtime.