dart-lang / dart-lang/native

Unify configuration logic with package:jnigen through package:config

Open
#470 4 comments 1 reaction 0 assignees View on GitHub
package:ffigen
Dominant language
Dart
Stars
275
Forks
144
Avg merge
2d 7h
Merged PRs (30d)
55

Description

Our current configuration provider is config-file only (with a single command-line override for compiler options).

To unify the experience between different packages, we would like to unify the config providing logic.

We could do this by introducing a shared `package:config` that is opinionated about (1) the layout of config files (JSON/YAML, snake_cased or dash-cased identifiers), (2) overrides through cli `-Dsome.key=value` and environment variables `SOME__KEY=value`.

I've created a prototype for this on https://github.com/dcharkes/config

Applying this logic in our https://github.com/dart-lang/ffigen/tree/master/lib/src/config_provider, requires some massaging of logic.

### Not loadable as dynamic

Currently the validation of values assumes they can be loaded as `dynamic` and be cast later.

This will not work for the following reasons:

1. For environment variables / CLI arguments we might need to pass a separator token for splitting if we want `List` or `String`.
2. We need to define for every lookup whether we join the defines from all three sources (for example include paths would be joined, while some boolean flag from the command line would override the boolean flag in the config file.

We will need to rewrite the `extractor`/`validator` in the spec to provide the right information. For example `config.getStringList(key, cliSeparator: ';', combineAll: true)`.

### Validation

Currently, all types of validation happen in the config provider. This contains two types of validation:

1. Whether something is of the right type, for example whether it's a boolean.
2. FFIgen specific validation, for example `dartClassNameValidator`.

The first type of validation logic can be done by `package:config`, while the second type needs to be in `package:ffigen`.

I haven't made a plan yet on what is the right type of abstraction to this validation.

One thing we could think about is we want the error reporting to be done by `package:config` in a consistent way, that we would provide some kind of callbacks API. That way we can make sure we always use the same way to represent the paths, the same sentence structure etc.

### Extraction

Currently, `package:config` only supports overriding config from CLI/environment for String/List/bool/int. The rest must be provided by a config file. However, even when it's provided by the config file, it would still be transformed by `package:ffigen` to a data structure (which could potentially fail).

I'm thinking that validate and extract should be combined into a single function that we then provide as a callback. Then it could be also type-safe.
For these to be combined we need to either use exceptions to communicate validation failures (which would then be caught and outputted to the logger in `package:ffigen`) or we always return nullable values add an `void Function(String) errorReporter` function to the config.

### Related

* JNIgen with `package:config`
* https://github.com/dart-lang/jnigen/pull/197
* JNIgen open issue to have better validation
* https://github.com/dart-lang/native/issues/688

Contributor guide

Open the contributing guide

Research direction

Start by reading the existing logic under lib/src/config_provider and compare it with the prototype in dcharkes/config, then review the related JNIgen package:config work and native issue 688. The issue needs an agreed design for source precedence, extraction, validation, and error reporting before implementation can be considered done.

Written by the indexing model from the issue text.

Assessment

Tech stack
dart
Domain
tooling
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.