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

Clean up Starlark options parser #22365

Open
katre opened this issue May 14, 2024 · 1 comment
Open

Clean up Starlark options parser #22365

katre opened this issue May 14, 2024 · 1 comment
Assignees
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Configurability Issues for Configurability team type: feature request

Comments

@katre
Copy link
Member

katre commented May 14, 2024

StarlarkOptionsParser is part of overall options parsing but still separate: it uses an entirely different API from OptionsParser and many code paths that handle options only handle native options as a result.

Ideally, native flags (based on OptionsBase and @Option) would use the same API as Starlark build settings.

An ideal end state API might look something like this:

  1. A top-level OptionsParser that is responsible for splitting flags into keys and values
    1. Handling both --flag value and --flag=value syntax
    2. Handling the no boolean-negation prefix
    3. Possibly handling string->value object conversion
  2. A number of plugins for the parser to actually identify specific options (and parse values?)
    1. A FieldOptionPlugin that handles "native" flags on OptionsBase subclasses, annotated with @Option
    2. A StarlarkOptionPlugin that handles Starlark flags (including loading the actual target and rule to determine options data).
  3. A unified system for then reporting flags values, handling diffs and merges, etc.

All of this is ambitious and won't happen soon, but I do plan to start some degree of convergence in order to durably expose Starlark option metadata via the existing OptionDefinition type.

@katre katre added type: feature request P2 We'll consider working on this in future. (Assignee optional) team-Configurability Issues for Configurability team labels May 14, 2024
@katre katre self-assigned this May 14, 2024
@katre
Copy link
Member Author

katre commented May 14, 2024

This is motivated by code cleanups necessary for platform based flags.

copybara-service bot pushed a commit that referenced this issue May 14, 2024
Work towards starlark options parser cleanup: #22365.

PiperOrigin-RevId: 633649715
Change-Id: Ie9820981fdcbbcea49ebd0c8042aeb621e378492
copybara-service bot pushed a commit that referenced this issue May 14, 2024
Work towards starlark options parser cleanup: #22365.

PiperOrigin-RevId: 633649782
Change-Id: I73180180142b5b23e91f1ee8c845ea606cfdc85e
copybara-service bot pushed a commit that referenced this issue May 14, 2024
Work towards starlark options parser cleanup: #22365.

PiperOrigin-RevId: 633670413
Change-Id: If56ed3a805d7540f8100ae344170991c961de53a
copybara-service bot pushed a commit that referenced this issue May 14, 2024
Work towards starlark options parser cleanup: #22365.

PiperOrigin-RevId: 633687955
Change-Id: I1e31b90c2ae701c29f5a36117f9cf0079e1e8d1b
copybara-service bot pushed a commit that referenced this issue May 20, 2024
…o a subclass.

Work towards starlark options parser cleanup: #22365.

PiperOrigin-RevId: 635437320
Change-Id: Ib7baa5b880a091d747e0d296761eb50753b29e91
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Configurability Issues for Configurability team type: feature request
Projects
None yet
Development

No branches or pull requests

1 participant