BuildModules forces all .dart files to be generated, even when using build filters

[evanweible-wf]

• Dart SDK Version: 2.15 (stable)

• What package(s) from this repo you are using, and the version:

  |-- build_runner 2.1.7
  |   |-- build 2.2.1
  |   |-- build_config 1.0.0
  |   |-- build_daemon 3.0.1
  |   |-- build_resolvers 2.0.6
  |   |-- build_runner_core 7.2.3
  |-- build_test 2.1.5
  |-- build_web_compilers 3.2.2
  |   |-- build_modules 4.0.4

• What builder(s) you are using (or writing yourself): the mockito builder (v5)

• Platform: MacOS

---

I'm currently debugging why MockBuilder is running on all dart files in example/ even when a build filter like --build-filter='test/foo_test.dart**' is used. The culprit appears to be the MetaModuleBuilder calling buildStep.findAssets(Glob('**$moduleLibraryExtension')), which matches .module.library assets in any directory, not just lib/. When that happens, build runner needs to ensure that those assets are built, too, which results in MockBuilder running on all of those files.

I tried changing MetaModuleBuilder to find assets via lib/**$moduleLibraryExtension since its input is $lib$ and its output is only one file in lib/, but doing so led to this error:

[SEVERE] build_web_compilers:ddc_modules on test/foo_test.dart.browser_test.dart:

Bad state: No element
[SEVERE] build_web_compilers:entrypoint on test/foo_test.dart.browser_test.dart:

AssetNotFoundException: _mockito_build_filter|test/foo_test.dart.browser_test.ddc.module

So I guess that glob is the thing that forces the module builder to run on the test/ directory, which is required for DDC to compile that test.

What I'm struggling to understand now is what the point of build filters is. If a filter down to a single file that I want to compile with DDC, but that DDC compilation requires reading all of the module information for every directory and generating that information requires running every builder that specifies .dart as an input extension.. that doesn't seem very efficient. Am I misunderstanding the build graph here? It seems to align with what I'm seeing in the debugger

I know it has to generate all the inputs for the requested outputs, and if for a test file that means everything in lib/ has to be generated.. then so be it (even though that also seems heavy-handed). But it definitely shouldn't require unrelated work in directories like example/, right?

[jakemac53]

Ahh this explains some recent other reports we have had around build_filters building more than is required. If you use build_web_compilers (or anything applying build_modules builders) you will end up building all Dart files.

We could possibly split up the MetaModuleBuilder such that it runs on each directly separately? But that might also present some of its own challenges (particularly around supporting other directories than the standard ones).

What I'm struggling to understand now is what the point of build filters is.

The original use case was focused around compilation (only compile the requested tests). It should still work well for that case, but you are correct that if using build_web_compilers its going to end up building all dart files.

[evanweible-wf]

The original use case was focused around compilation (only compile the requested tests). It should still work well for that case, but you are correct that if using build_web_compilers its going to end up building all dart files.

Isn't build_web_compilers required for running web tests? If so, that's a really unfortunate result. It seems to nullify one of the main advantages of using an incremental, modular compiler if unnecessary inputs can't be ignored even with a feature like build filters.

[jakemac53]

Isn't build_web_compilers required for running web tests?

If you also need codegen, or want to run them in DDC, then yes. But you can run dart2js tests through package:test.

If so, that's a really unfortunate result.

Ya, it's definitely an unfortunate effect of how build_modules is set up today. We could mitigate it somewhat possibly by running it on each top level directory as I suggested above but I don't see how we couldn't still build the Dart file portions of each directory. We have to not only know what Dart files will exist in the project, but also the imports inside of them, in order to create the module structure. So we have to build them.

[jakemac53]

One alternative could be a manually curated module structure, instead of an automatically generated one. But it would require a lot of work on the part of users to maintain.

[evanweible-wf]

If you also need codegen, or want to run them in DDC, then yes. But you can run dart2js tests through package:test.

Unfortunately this isn't a great trade for us. One of the main reasons for using DDC is to avoid duplicating work when running more than one test entrypoint (which is obviously pretty common).

One alternative could be a manually curated module structure, instead of an automatically generated one. But it would require a lot of work on the part of users to maintain.

Yeah, a few of our projects that experience the most pain from iterative build times do this, and it is a lot of work.

---

I guess the reason this one in particular was so painful for us was because the mockito package introduced a builder in a minor release that runs on all .dart files by default and resolves all of them. Obviously that's not directly a pkg:build concern, but it's an unfortunate reality since it makes sense for builders like this to target .dart files, and the source_gen package makes it easy to implement the builder despite the performance implications of resolving every file.

It looks like we'll have to take the same approach as we do with other source_gen builders and use a file naming convention like .m.dart and limit the inputs to the mockito builder with an input glob like **.m.dart.

[jakemac53]

It looks like we'll have to take the same approach as we do with other source_gen builders and use a file naming convention like .m.dart and limit the inputs to the mockito builder with an input glob like **.m.dart.

Ya, this is the best recommendation I can give you to resolve this. Does mockito run by default on all libraries and not just those in the test directory? It might be worth filing an issue if so.

[evanweible-wf]

https://github.com/dart-lang/mockito/blob/master/build.yaml#L5-L7

Looks like it's configured for test/ and example/ by default, I'll file an issue. I guess it'd be a breaking change, but still worth discussing.

^ scratch that, that's the config for running the builder in their own package. The defaults don't narrow down the generate_for at all, so it runs on all of the default inputs: https://github.com/dart-lang/mockito/blob/master/build.yaml#L10-L15

[jakemac53]

Ok, that seems like it should probably change.

There are some valid use cases for running mocks on lib files (if you want to ship a testing package), but it might be better to force explicit configuration in that case.

[natebosch]

I don't think it would be feasible to have a system where you could build for test/, then later build for bin/ or web/ and have consistent modules between them. If you build with one filter and we don't read all Dart files for modules, then when you build with a different filter the computed modules could conflict. The modules change base on which "entrypoints" we have.

I can imagine we could tradeoff some redundancy in full builds for the ability to do some partial builds. Not arbitrary ones based on build filters, but specially defined ones. We have separate module computations for different platforms, if we consider web-test to be a separate "platform" from web and then we could plausibly limit what needs to be read. Most of lib/ would end up in both a web and a web-test modules which would be compiled independently if you are building with no build filter, so those would be slower.

It would take more rethinking on how we collect up files. Instead of a glob across everything in the package we'd need to start with a glob matching the same files that the platform's compiler will read as entrypoints (may need redundant config if you aren't using the defaults?) and then crawl from there to find which Dart files need to be considered for that module.

[jakemac53]

The modules change base on which "entrypoints" we have.

I believe each top level directory is treated in isolation though - no files under web, test etc can affect the module structure of any other top level dir. So I think it could be safe.

[natebosch]

no files under web, test etc can affect the module structure of any other top level dir. So I think it could be safe.

Ah, you're correct. I was thinking that when we do the "bundling" to group stuff up we considered all entrypoints, but _computeLibraries only sees a single directory at a time.

https://github.com/dart-lang/build/blob/868f1dab9d6def687635a05e3845bbb4af6be41b/build_modules/lib/src/meta_module.dart#L179-L184

[jakemac53]

While not desirable, I don't see any way of fundamentally fixing this issue for web projects.