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

Allow passing command line args to set builder options #801

Closed
jakemac53 opened this Issue Jan 3, 2018 · 9 comments

Comments

Projects
None yet
3 participants
@jakemac53
Copy link
Contributor

jakemac53 commented Jan 3, 2018

Today the only way to pass config through the BuilderOptions object is via a custom build.yaml, but it would also be useful to be able to do this in a one-off fashion based on command line args.

The driving use case for this today would be to switch your web compiler between dart2js and dartdevc on the fly (without having to edit the build.yaml).

My proposed syntax for this would be --define=<builder_key>=<option>=<value>. All values would first attempt to be parsed as JSON, and if that fails then they would be treated as a raw String. You would not be able to override the top level settings like generate_for and enabled, only the builder options.

Pros:

  • Supports overriding all options regardless of expected type.

Cons:

  • It is quite verbose, enabling dart2js would look like this --define=build_web_compilers|entrypoint=compiler=dart2js.
  • Options that require map/list values would be clunky and look a bit different than the typical yaml, for example --define "some_package|some_builder=custom_option={\"foo\": true}".

@jakemac53 jakemac53 added this to the M1: Future priority milestone Jan 3, 2018

@jakemac53

This comment has been minimized.

Copy link
Contributor Author

jakemac53 commented Jan 3, 2018

We would also have to decide if this should override that setting for all builders globally or just the root package, and if that would need to be configurable.

@natebosch

This comment has been minimized.

Copy link
Member

natebosch commented Jan 3, 2018

We would also have to decide if this should override that setting for all builders globally or just the root package, and if that would need to be configurable.

In bazel an argument on the command line overrides the option for all uses of that builder across all targets.

It is quite verbose

We could consider adding shorthand for some common ones. Or we could look at adding something like 'option groups'

targets:
  my_package:
    builders:
      build_web_compilers|entrypoint:
        option_groups:
          dev:
            compiler: ddc
          prod:
            compiler: dart2js
pub run build_runner build --options=dev
@jakemac53

This comment has been minimized.

Copy link
Contributor Author

jakemac53 commented Jan 3, 2018

In bazel an argument on the command line overrides the option for all uses of that builder across all targets.

Ya, I think that makes the most sense for build_runner as well.

We could consider adding shorthand for some common ones. Or we could look at adding something like 'option groups'

Ya, I was thinking that we could punt on this specific thing and instead just add a debug/release mode which are built in as well. That covers most things, but it would end up invalidating all builders instead of just the ones that actually do something with the mode.

@jakemac53

This comment has been minimized.

Copy link
Contributor Author

jakemac53 commented Jan 8, 2018

Another option here could be allowing creators of builders to define explicit aliases for certain options.

For instance the ddc builder could look like:

builders:
  entrypoint:
    target: "build_web_compilers"
    import: "package:build_web_compilers/builders.dart"
    builder_factories: ["webEntrypointBuilder"]
    build_extensions:
      .dart:
        - .dart.bootstrap.js
        - .dart.js
        - .dart.js.map
    required_inputs:  [".dart", ".ddc.js", ".module"]
    build_to: cache
    auto_apply: root_package
    defaults:
      generate_for: ["web/**", "test/**.browser_test.dart"]
    option_aliases:
      - web_compiler:
        option: compiler
        description: "Sets the web compiler for your dart project"

That would add the --web_compiler flag, which would be an alias for --define="build_web_compilers|entrypoint=compiler=<compiler>".

We would have to disallow duplicate aliases though.

@matanlurey

This comment has been minimized.

Copy link
Contributor

matanlurey commented Jan 8, 2018

FWIW, I'd prefer this to work the most like Bazel, if possible.

@jakemac53

This comment has been minimized.

Copy link
Contributor Author

jakemac53 commented Jan 8, 2018

FWIW, I'd prefer this to work the most like Bazel, if possible.

I think we have pretty broad agreement that the command line flags should be global at this point. Is there anything specific you see here that is different? Do you think we should try and make the args look exactly identical?

@jakemac53 jakemac53 added the P1 high label Jan 10, 2018

@jakemac53

This comment has been minimized.

Copy link
Contributor Author

jakemac53 commented Jan 18, 2018

@natebosch what do you think about my option_aliases proposal?

@natebosch

This comment has been minimized.

Copy link
Member

natebosch commented Jan 18, 2018

I think theoption_aliases idea would make sense if we expected build_runner to be the final UX for most users - but wrapping CLIs throw a bit of a wrench in to things:

Is it worth the complexity of building this if web_compiler is our motivating use case, given that webdev does want to bake in knowledge of build_web_compilers and could do the aliasing?

If we decide it's worth the complexity within build_runner does that also push more complexity up to wrapping CLIs? Today we rely on the --help being basically static to build_runner and exposing it to the wrapper. With this proposal --help would essentially be dynamic and only the generated entrypoint would know the full set of options.

@jakemac53

This comment has been minimized.

Copy link
Contributor Author

jakemac53 commented Jan 18, 2018

Is it worth the complexity of building this if web_compiler is our motivating use case, given that webdev does want to bake in knowledge of build_web_compilers and could do the aliasing?

True, we could do the verbose option and just allow wrapper scripts to add shorthand options, I would be fine with that.

If we decide it's worth the complexity within build_runner does that also push more complexity up to wrapping CLIs? Today we rely on the --help being basically static to build_runner and exposing it to the wrapper. With this proposal --help would essentially be dynamic and only the generated entrypoint would know the full set of options.

Depends how we implemented it I guess, I think we could make something that would have limited impact on wrapper scripts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.