Skip to content

Q: build_runner_core.apply ignores package graph nodes, runs for root only - is that intended? #2943

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
daniel-v opened this issue Dec 9, 2020 · 7 comments

Comments

@daniel-v
Copy link

daniel-v commented Dec 9, 2020

I have a simple builder I want to run for all the packages in a dependency tree. A simple code would showcase what's up:

import 'dart:io' as io;

import 'package:build_runner/build_runner.dart';
import 'package:build_runner_core/build_runner_core.dart' as bc;
import 'package:my_package/builders/my_meta_builder.dart';

Future main(List<String> args) async {
  final graph = await bc.PackageGraph.forThisPackage();
  final pkgsNo = graph.allPackages.length;
  // tracks the number of pkgs builder was invoked for
  var appliedToPkgs = 0;
  final metaBuilder = bc.apply(
    'my_package:my_meta_builder',
    [myMetaBuilderFactory],
    (pkg) {
      appliedToPkgs++;
      return true;
    },
    hideOutput: false,
  );
  io.exitCode = await run(['build', ...args], [metaBuilder]);

  print('Expected to run for $pkgsNo packages; ran for $appliedToPkgs');
}

Builder definition in my_package:

  i18n_css_builder:
    import: "package:my_package/builders/my_meta_builder.dart"
    builder_factories: ["myMetaBuilderFactory"]
#  ...    
    is_optional: false
    build_to: source
    auto_apply: all_packages

Output: Expected to run for 120 packages; ran for 1

My suspicion is this: that the builder would want to build to source and that is allowed only for root packages. Found some code that would indicate that. https://github.com/dart-lang/build/blob/master/build_runner_core/lib/src/package_graph/apply_builders.dart#L359

Do I understand it correctly? (In other words: am I trying to do something that is unsupported?)

@jakemac53
Copy link
Contributor

Yes, you can only build to source for the root package. The reason is we only ever will write files within the root package, so we can either write to cache (which is under .dart_tool/build/generated) for any package, or source for the root package only.

@jakemac53
Copy link
Contributor

closing but feel free to ask more questions here :)

@daniel-v
Copy link
Author

daniel-v commented Dec 9, 2020

I do, but not relevant to this issue. Honestly, I'm struggling with build a bit.
Thank you for the quick reply and answer, just like last time, @jakemac53 . I appreciate that a lot.

As a follow-up: it seems there are logical constraints on some constellations of configurations based on conventions or other considerations. I'm wondering, would it make sense to let users know that they are trying to do something that will never work out? In this specific case, checking if these 2 configs are defined together:

    build_to: source
    auto_apply: all_packages

@jakemac53
Copy link
Contributor

Something like that would make sense, the lack of this is mostly an issue of a very complicated configuration space (there are many options that potentially conflict), and so we don't have good warnings for all of these (or even very many of them).

@daniel-v
Copy link
Author

issue of a very complicated configuration space

I think this to be very much on point. It is something that should receive some love.

Little off topic: I asked around in the office, do a little research. To summarize it, it came out that while builders is a simple concept (code generation), it is the toughest thing for many of us to get right. Partly because of the "configuration space" and the other big one was that it is very tedious to translate code into analyzer technical terms, understand what an expression looks like in the eyes of the analyzer etc.

@jakemac53
Copy link
Contributor

Yes, code generators are definitely a superuser feature. It requires a lot of knowledge of both the build package (apis and configuration) as well as analyzer, and the language itself.

We strive to make using code generators be as simple as we can, and part of that actually does increase the complexity for codegen authors. The ultimate goal being zero configuration for end users (just add a dev dependency and maybe follow some conventions set out by the codegen author).

@natebosch
Copy link
Member

A little background on the decision to ignore the builder outright if it wants to build to source but apply on anything other than the root package:

This is to prevent spurious warnings if a builder ends up in the transitive dev dependencies for some project, but it isn't using it directly. A configuration with auto_apply: dependents and build_to: source is a pattern we expect would be common, and if some downstream user of this has that generator in dependencies: instead of dev_dependencies: it would cause a problem because we can't write to source for anything other than the root package.

If we make it such that build_to: source forces auto_apply: root_package then we have a different problem - all those builders would switch from auto_apply: dependents to auto_apply: root_package and now a misconfiguration in a transitive dependency causes a builder that you didn't expect to run.

I'm open to the idea of adding help for this in the doctor command, but I'm not entirely sure how we'd want that to look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants