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

Skylark support for custom build flags #5577

Closed
gregestren opened this issue Jul 11, 2018 · 10 comments
Closed

Skylark support for custom build flags #5577

gregestren opened this issue Jul 11, 2018 · 10 comments
Assignees
Labels
P1 I'll work on this now. (Assignee required) team-Configurability platforms, toolchains, cquery, select(), config transitions

Comments

@gregestren
Copy link
Contributor

Part of the Skylark build configuration project.

This is a tracking issue for the ability of Skylark writers to define custom command line flags to configure build properties.

For example:

$ bazel build //my:binary --some_feature_specific_to_my_app=fancy_mode

This complements the Bazel Configurability Roadmap. Write questions / comments / status updates here. See there for the bigger picture.

@juliexxia
Copy link
Contributor

Planning on implementing a new skylark module 'config' to help rules define themselves as build setting rules. Waiting on #5637 to make that change so we don't break users who have global vars named 'config'.

bazel-io pushed a commit that referenced this issue Oct 31, 2018
For skylark rules that are declared build settings, we auto-add a nonconfigurable build_setting_default attribute which must be set on a per target basis. We also add access to the build setting value via the rule context object. All of this functionality is hidden behind the --experimental_build_setting_api flag which by default is flipped off.

This includes:
- Add a build_setting parameter to starlark rule construction.
- If a starlark rule sets the build_setting parameter, auto-add a mandatory 'build_setting_default' attribute of the same type.
- Allow the value of a build setting skylark rules to be accessed via ctx.build_setting_value.
- Get rid of BuildSettingDescriptor since we don't need both it and BuildSetting.
- Add the --experimental_build_setting_api flag to SkylarkSemantics flags.

Work towards issue #5577

PiperOrigin-RevId: 219537101
bazel-io pushed a commit that referenced this issue Nov 27, 2018
@gregestren gregestren added P1 I'll work on this now. (Assignee required) team-Configurability platforms, toolchains, cquery, select(), config transitions and removed category: extensibility > configurability labels Nov 28, 2018
bazel-io pushed a commit that referenced this issue Nov 29, 2018
…s/skylark/starlark)

#5577

PiperOrigin-RevId: 223357995
bazel-io pushed a commit that referenced this issue Dec 1, 2018
This includes the the following features available to native options parsing:
* --(no)boolean_flag syntax
* --flag=value syntax
* --flag value syntax
* single-dash syntax
* for multiple values of same option, last option wins
* internal-only options are treated like they don't exist

There is potential here to combine some of the code in StarlarkOptionsParser.java with OptionsParserImpl.java since they support the same formats of flags. This is complicated by the fact that for native flags, you have a lot of information (type, internal vs external) before parsing whereas for starlark flags, you need to parse out the name first and load it to get this information. So I haven't done the combining in this CL.

This brings up an interesting point that now all commands that use starlark flags will need to do loading. For now we limit starlark parsing to commands that already do building (which is possibly all the commands we care about).

This behavior is still guarded behind --experimental_build_setting_api

Work towards #5574 and #5577

PiperOrigin-RevId: 223600443
@gregestren
Copy link
Contributor Author

gregestren commented Jan 28, 2019

2018 EOY update:

This feature is partially done.

The flags have been created, but they haven't been fully piped through all pieces of Bazel's code yet. This will increase in priority over the first half of 2019.

bazel-io pushed a commit that referenced this issue Apr 9, 2019
Calculate and supply defaults for all read starlark options before transitions are applied. Future work includes:
- combining skyfunction calls for inputs and outputs of starlark transitions
- enforcing that BuildOptions.starlarkOptions will never hold default values (opposite of native options which always hold default values) in order to make BuildConfigurationWithDefault == BuildConfigurationWithNoValue
- Making sure BuildConfiguration properly reflects StarlarkOptions

Work towards #5574, #5577, #5578

RELNOTES: None.
PiperOrigin-RevId: 242681946
@gregestren
Copy link
Contributor Author

gregestren commented Apr 26, 2019

April '19 update:

Major strides: this works! Lots of small pieces still being worked on, like complex types or select() support on user-defined flags. This work is being actively prioritized by @juliexxia over this quarter - more precise details available on request.

@yzhao1012
Copy link

Sorry to spam:

  • Is there a e2e example showcasing this feature?
    I am having a hard time grasp how the whole thing works.

Thanks in advance!

PS: I am guessing this can be used to allow bazel to apply specified command line flags directed by the build rule itself. Is that the case?

@ghost
Copy link

ghost commented May 9, 2019

April '19 update:

Major strides: this works! Lots of small pieces still being worked on, like complex types or select() support on user-defined flags. This work is being actively prioritized by @juliexxia over this quarter - more precise details available on request.

@gregestren @juliexxia Any chance that user-defined flags will be available/visible to repository rules during the loading phase?

@juliexxia
Copy link
Contributor

@yzhao1012 no apologies needed - documentation is coming very soon but unfortuantely I can't think of an e2e example that exists right now (partially because all functionality is still experimental). Stay tuned!

@beasleyr-vmw How would you envision using flags for repository rules? Since user-defined flags are a part of configuration they almost certainly won't carry any meaningful value during the loading phase when repository rule implementations fxns are executed. But what's your use case?

@ghost
Copy link

ghost commented May 9, 2019

@beasleyr-vmw How would you envision using flags for repository rules? Since user-defined flags are a part of configuration they almost certainly won't carry any meaningful value during the loading phase when repository rule implementations fxns are executed. But what's your use case?

Vision: Just as environment variables are exposed to repository rules, I'm imagining user arguments visible to repository rules, even if as rudimentary unparsed/unvalidated key-value pairs.

$ bazel build ... --//:product=my_product
def _impl(repository_ctx):
    product = repository_ctx.options.get("//:product")

Use case: I use repository rules to teach Bazel how to integrate with a proprietary artifact manager. To configure/influence the repo rule, my users have to specify their options using environment variables. I'd like to use command-line flags instead.

Motivation: Other parts of our build with product-specific logic rely on --define= options. This means our users have to specify their intended product twice: env PRODUCT=my_product bazel build ... --define=product=my_product. While the new user config options covered by this GitHub issue allow us to move away from --define, we still have the duplicate definition problem. A developer who only works on one product can cheat by updating their .bashrc and .bazelrc, but anyone switching between products can run into trouble if they manually override one without the other.

Thanks for the consideration. Please LMK if I can clarify anything, should move over to bazel-discuss, etc.

bazel-io pushed a commit that referenced this issue Jul 1, 2019
*** Reason for rollback ***

Rolling forward flag flip (--experimental_build_setting_api=True) now that b/134580627 is fixed

*** Original change description ***

Automated rollback of commit 536a166.

*** Reason for rollback ***

Found an incremental correctness concern with build settings. Rolling back while investigating and to rollback for the bazel release

*** Original change description ***

Turn on --experimental_build_setting_api

RELNOTES: Turn on --experimental_build_setting_api by default for starlark build settings (see https://docs.bazel.build/versions/master/skylark/config.html#user-defined-build-settings for more info)

SKIP_CI=bl...

***

Related to #5577

ROLLBACK_OF=252715211
PiperOrigin-RevId: 255967767
siberex pushed a commit to siberex/bazel that referenced this issue Jul 4, 2019
*** Reason for rollback ***

Rolling forward flag flip (--experimental_build_setting_api=True) now that b/134580627 is fixed

*** Original change description ***

Automated rollback of commit 536a166.

*** Reason for rollback ***

Found an incremental correctness concern with build settings. Rolling back while investigating and to rollback for the bazel release

*** Original change description ***

Turn on --experimental_build_setting_api

RELNOTES: Turn on --experimental_build_setting_api by default for starlark build settings (see https://docs.bazel.build/versions/master/skylark/config.html#user-defined-build-settings for more info)

SKIP_CI=bl...

***

Related to bazelbuild#5577

ROLLBACK_OF=252715211
PiperOrigin-RevId: 255967767
irengrig pushed a commit to irengrig/bazel that referenced this issue Jul 15, 2019
*** Reason for rollback ***

Rolling forward flag flip (--experimental_build_setting_api=True) now that b/134580627 is fixed

*** Original change description ***

Automated rollback of commit 536a166.

*** Reason for rollback ***

Found an incremental correctness concern with build settings. Rolling back while investigating and to rollback for the bazel release

*** Original change description ***

Turn on --experimental_build_setting_api

RELNOTES: Turn on --experimental_build_setting_api by default for starlark build settings (see https://docs.bazel.build/versions/master/skylark/config.html#user-defined-build-settings for more info)

SKIP_CI=bl...

***

Related to bazelbuild#5577

ROLLBACK_OF=252715211
PiperOrigin-RevId: 255967767
@moroten
Copy link
Contributor

moroten commented Oct 11, 2019

When trying it all out, --//:myflag=foo works, but --@rules_my_language//:myflag=foo does not. I've tried to make an alias and then go for the first alternative, but that fails as well with ERROR: Unrecognized option: //:myflag.

Is it a design choice to not allow command line flags from external dependencies? Is a bug so I should file a bug report? Is it a feature request to allow it in v2, possibly via the WORKSPACE file?

@gregestren
Copy link
Contributor Author

Re: documentation and examples:

@gregestren
Copy link
Contributor Author

gregestren commented Oct 11, 2019

@moroten Good question. I'd imagine this is just a TODO. Can you file a distinct bug for that?

Since the core feature is available, I'm going to close this issue in preference for specific bugs / feature enhancement requestss.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 I'll work on this now. (Assignee required) team-Configurability platforms, toolchains, cquery, select(), config transitions
Projects
None yet
Development

No branches or pull requests

5 participants