Skip to content
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

Question: how to configure build.yaml to "disable" provided/inherited builder? #3595

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

BenVercammen
Copy link
Contributor

I'm using this PR as a way to ask a question, but it might result in some extra addition to the golden test to show this functionality (if it actually exists).

So, my question is this: given the extra not_enabled_builder, is there a way to have it not show up in the resulting build.dart file?

The test that's failing now, is generated_script_integration_test.dart.

I've been trying various things to keep this builder from ending in the build.dart file, but to no avail so far.

One thing that helps a little, is the addition of provides_builder.build.yaml with the following contents:

---
# disable all inherited builders...
targets:
  $default:
    auto_apply_builders: false

The only problem is that it will omit all builders from the provides_builder package.

Is there a way to disable a builder in such a granulary way?

…ve the "not_enabled_builder" show up in the generated `build.dart` file?
@BenVercammen BenVercammen changed the title Question: how to configure build.yaml to "disable" provided builder? Question: how to configure build.yaml to "disable" provided/inherited builder? Oct 11, 2023
@jakemac53
Copy link
Contributor

There isn't a way to enable/disable a builder for packages other than the current package, except by overriding individual packages build files as you have done in your example.

@BenVercammen
Copy link
Contributor Author

Thanks for the quick response! Unfortunately, I'm not completely sure on what I should put inside the provides_builder.build.yaml file to still keep some of the builders. Or is it truly "all or nothing"?

I've been trying out some things, but for now, no matter what I put inside it (even nothing at all), it seems that just adding the provides_builder.build.yaml file will result in none of the provides_builder builders to be included in the build.dart file at all.

I still feel it would be a good addition to the package to provide a test case where the (expected) result of a "individual package override file" is showcased.

So any pointers on what to put inside the file are more than welcome.

@jakemac53
Copy link
Contributor

Or is it truly "all or nothing"?

Yes it is all or nothing when overriding a build.yaml file - you would have to copy everything they have and then alter the parts you want to alter.

If you just want to disable a given builder for your package, or that package, you can do that like this:

targets:
  $default:
    builders:
      some_package:some_builder:
        enabled: false

@BenVercammen BenVercammen force-pushed the disable-provided-builder branch from b879dfb to 9a52350 Compare October 18, 2023 09:09
@BenVercammen
Copy link
Contributor Author

BenVercammen commented Oct 18, 2023

I've been looking into this, but I somehow cannot get the enabled: false to have any effect...

Could you please check out my branch and show me what I'm doing wrong?

I've made the following adjustments to the setup:

  • Added extra (to be disabled) builder in provides_builder
  • Added extra other_builder package, defining another builder (to be disabled)
  • Adjusted build.yaml in _test to "disable" those 2 builders (to no avail)
  • Added other_builder.build.yaml file (empty/only comments) which will actually remove the other_builder from builder.dart

So I'm still getting something wrong, but can't seem to figure it out by myself what I'm missing here...

Edit: I'm running Generates a build script matching the golden in generated_script_integration_test.dart

@jakemac53
Copy link
Contributor

The enabled: false configuration in a given package does not remove the builder from the build.dart file entirely - it just disables it for the current package.

Any builder which is defined will end up in the build.dart file today. Adding the empty other_builder.build.yaml file effectively just undefines the builder, which is why that works.

What is your use case for wanting the import to be removed entirely?

@BenVercammen
Copy link
Contributor Author

I seem to be struggling with a couple of things at the same time. A little background on my setup:

  • I have my own package called codegen that defines some custom code generators
  • It also contains a couple of @JsonSerializable classes, used within the code generators, so these need to be generated first
  • It also has dependencies on another package that contains several builders, some of which I don't need

Now, I ran into the following issues:

  1. I had the same builder being imported twice for some reason, resulting in DuplicateAssetNodeException. I currently can't reproduce this anymore, but that was one of the main reasons to start looking into the build.dart file, trying to remove unnecessary builders
  2. My current issue is that the json_serializable only works when I don't have any other builders defined in codegen/build.dart, or when the generated .g.dart part files are present. I already figured out that having an import to my own package inside the generated build.dart file seems to be the main cause. However, I haven't been able to reproduce it yet within my branch of the build project...

I'll try to get to a minimal example by next week.

In the mean time, any pointers on how to get a better look at the asset graph or other potential debugging tips are always welcome.

@jakemac53
Copy link
Contributor

  • It also contains a couple of @JsonSerializable classes, used within the code generators, so these need to be generated first

This is why you are running into difficulties for sure. It isn't a supported use case to use code generation in the same package as your builders (or at least not within libraries imported by your builders), since we put them all into a single program, so all code for builders needs to already be generated.

The easiest solution to this is to create a separate package for the code in your package wich requires codegen, and then depend on that package. That package will have a separate build script so it will work fine, and your package can just depend on that package.

2. when the generated .g.dart part files are present

Yeah, this is another sort of workaround but you will run into issues whenever those files are invalidated. You can make it work (manually restoring the files when they get deleted then running a build again), but it is painful. We do this in some of our packages though.

@jakemac53
Copy link
Contributor

jakemac53 commented Oct 25, 2023

If your package does not actually use its own builders though, then an even easier workaround would be to just simply not include your own builders in the script, as you were trying to do. That sounds to me like a valid use case and probably a common enough pattern that it would be worth supporting.

@BenVercammen
Copy link
Contributor Author

If your package does not actually use its own builders though, then an even easier workaround would be to just simply not include your own builders in the script, as you were trying to do. That sounds to me like a valid use case and probably a common enough pattern that it would be worth supporting.

Okay, so would it be useful if I try to come up with a PR to exclude disabled builders from the build.dart file?

In the meantime I'll also try to split up the packages as you suggested.

@jakemac53
Copy link
Contributor

Okay, so would it be useful if I try to come up with a PR to exclude disabled builders from the build.dart file?

Sure, I would definitely review a contribution like that, as long as it doesn't make generating the build script significantly slower for some reason (but I don't think it should).

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

Successfully merging this pull request may close these issues.

2 participants