-
Notifications
You must be signed in to change notification settings - Fork 57
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
[native_assets_cli] User defines #39
Comments
My original thought was that the configuration system is based on hierarchical
The final build configuration for a particular package's Those user-defined global or per-package settings can come from command line arguments to the build (e.g. The actual build/bundling tool (e.g. The Dart code that is operating on such json can use wrappers on the json that view into things they care about (e.g. via inline classes): class BuildConfig {
final Map<String, Object> config;
}
extension on BuildConfig {
CxxConfig get cxxConfig => CxxConfig(config['build.cxx']);
}
inline class CxxConfig {
final Map<String, Object> config;
int get optimizationLevel => ...;
} Then we have flexible configuration mechanism:
|
Having configuration that can be used by multiple targets such as optimization level is a good idea indeed. Then if some package wants some specific configuration, they can just use their own package-name as the key. (That at least rules out using
I was thinking that instead of modification it would be in non-conflicting. The final config: out_dir: ...
target: ...
dependency_metadata:
my_package: // some meta data set by the build.dart of my_package, used in later native asset builds
key: value
c_compiler:
ar: ...
cc: ...
ld: ...
user_defines:
cxx: // generic which can be used by any package
optlevel: 3
my_package: // likely only used during the build.dart of my_package.
key: value The But maybe nesting them in If we nest in |
For what it's worth, I think it's fine to add a top-level key in build_config:
<package>:
<config-key>: <config-value>
my_package_with_native_assets:
my_config_option: true
optlevel: 3
... It's sometimes nice not to have a lot of top-level config files. Nesting it under Do consider that a package published on pub.dev might have a |
My current thinking based on the above. End user specificationPubspecEnd users can specify config in the pubspec native_assets:
defines:
<package>:
<config-key>: <config-value>
my_package_with_native_assets: # Only passed to my_package_with_native_assets's build.dart
enable_some_feature: true
_all: # Passed to every build.dart (`package:_all` should never exist.)
optlevel: 3 Skip for now: conditional user-definesIf we would like to support branching on build_mode / architecture / other already predefined keys, we could consider adding something like filters: native_assets:
defines:
my_package_with_native_assets: # Only passed to my_package_with_native_assets's build.dart
_where:
_build_mode: release
optimize: true However, the downside is that this kind of logic doesn't scale. What if we add more predefined things to the config passed in to We kind of want to opposite of the metadata here, metadata is output by I think for now we should skip conditional user-defines in the pubspec, and do these via command-line args. @GregoryConrad What is your use case for conditional user-defines? Command-line argsThe config in the pubspec.yaml can be overridden in invocations of The defines for dart code are with
(And for flutter Since these arguments are already taken, we could use optional: command-line args from fileWe could follow Flutters Not: Environment variablesWe should stop passing through environment variables to prevent builds accidentally depending on them and breaking build caching that way. Implementationnative_assets_cliThe out_dir: ...
target: ...
dependency_metadata:
my_package: // some meta data set by the build.dart of my_package, used in later native asset builds
key: value
c_compiler:
ar: ...
cc: ...
ld: ...
user_defines:
optlevel: 3
enable_some_feature: true These are defines just for the build for the native assets of this package. native_assets_builderNeeds an extra argument for
And it gets the right information per package to put in the (Note that if we ever add conditional defines in pubspec or a script, they should not be able to depend on the fields not passed to dry-run such as architecture and API levels.) Launchers
Design questionsnative-assets defines vs build definesWhat naming should we use. Should we settle on We settled on The question is whether these other types of "builds" would benefit from defines as well. If they do, we could settle on the "build" name here: build_defines:
<package>:
<config-key>: <config-value>
|
cc @simolus3 |
The yaml-based end-user specification covers my use-cases, but given that there are more complex ideas mentioned here (like conditional user defines), I want to bring up the idea of allowing users to configure their dependencies in Dart? E.g. if I was able to put say a void main(List<String> args) async {
final global = await BuildConfig.fromArgs(args);
global.configure(allPackages, (config) {
config.addToList('c_compiler.flags', '-Wall');
// There could also be type-safe extensions on config to describe common options
config.linker.preferredMode = Linker.staticLibrary;
});
// Or configure a specific package
global.configure(package('sqlite3'), (config) async {
config['source'] = 'url';
config['url'] = 'https://sqlite.org/2023/sqlite-amalgamation-3430100.zip';
// Since this is code, I could make some options conditional
if (global.buildMode == BuildMode.release) {
// Read additional options we want from a file
final optionsFile = global.trackInput('assets/release_options.txt');
final content = await optionsFile.readAsString();
config.cCompiler.addDefine('foo': content);
}
});
} With a nice DSL and typed extensions to configure common things like the C toolchain, this may be as convenient as a declarative configuration in yaml. A benefit is that it can scale to complex configuration options much better. |
I can't remember what my exact use case was, but a big one is enabling Rust cargo features based on a user's preferences/needs. In C/C++ land, another might be to supply a value to |
RE: One other use case would be
The three message files could be defined in the User defines are basically the (Having two files Related:
cc @mosuem |
Then any change to build rules easily becomes a breaking change for the entire ecosystem. I'm generally very concerned if using native-assets requires using an API that isn't specified in a Of course
@dcharkes, I like your example, but when you say: "user-defines" do you mean:
Is it only the root package that is allowed to have "user-defines", and are they "user-defines" specified in dependencies and transitive dependencies ignored? @sigurdm this is a bit similar to what is proposed in dart-lang/pub#3917 Should we seriously consider making a generic configuration mechanism? @dcharkes, I would suggest avoiding magic values like _all: # Passed to every build.dart (`package:_all` should never exist.)
optlevel: 3 If you must just do: native_assets:
defines:
<package>:
<config-key>: <config-value>
my_package_with_native_assets: # Only passed to my_package_with_native_assets's build.dart
enable_some_feature: true
define_for_all: # Passed to every build.dart
optlevel: 3 But it might also be reasonable to ship the initial MVP without a In fact, it might be worth down scoping and not shipping with "user-defines" initially, is there any reason it can't be added at a later stage? |
The latter. The former would not support A third option would be: Inverse meta-data from any package to its transitive dependencies. Some conceptual use case that would benefit from that is a bunch of packages standardizing on some config keys, and then some package similar to flutter_lints would provide a bunch of default "inverse meta data" (we should probably just call it config). That being said, it's a bit of a contrived example, so only showing it to direct dependencies would probably be better.
If the config is a
The native assets (dynamic library bundling), can be shipped without indeed. |
From options:
I thought you wanted (A). I see little point in doing (C). Because if you know that I suppose (C) makes some sense if (C) is an optional dependency, but we don't really have that concept. And if you supply config to package |
Now that we have introduced pub workspaces and share the native asset builds across all packages in the workspace, the user-defines should be workspace wide. Otherwise the cache will be invalidated. If we would have user defines per root package, you'd end up in the following scenario. Suppose your workspace is:
And now you do:
This leads me to believe that we should have user-defines in the workspace This will lead to:
(Side note: If we enable passing in user-defines on command-line, that will necessarily also lead to cache invalidations. I don't see an obvious way around that.) |
@dcharkes could you key the cache of the build output by using a hash of the user defines? That way you can have multiple different user defines (and CLI wouldn’t be a problem either for cache invalidation). |
@GregoryConrad That has two significant downsides:
The design approach with the recent refactoring with Do you have any use cases in which user-defines (a) per pub workspace or (b) per invocation is not sufficient? |
No use cases at the moment; just figured I’d float the idea. Main concern would be a mono repo where multiple dependents reuse the same native asset differently, but I can’t say I have that need at the moment. |
From #1962 (comment): We want to minimize the impact of user-defines on cache invalidation. So we want to make the user-defines scoped per package, and not give access to the user-defines meant for other packages. (If some packages want to collaborate on a user-define. They could pick one package that consumes it, and makes it available as metadata for the other packages.) |
We'd like users to be able to configure the build of their native dependencies.
We'd probably want to support multiple ways of providing this user-defined config, similar to
package:cli_config
. However, in this case we also need to reason about the command-line API of the launcher scripts (dartdev and flutter_tools).One downside of the
package:cli_config
standard is that it requires passing--config=
to pass the config file. Instead (or in addition) we might want to consider a default place to pass configuration. This could be thepubspec.yaml
to have a single location for configuration.Two options would be (1) a top level
Originally posted by @GregoryConrad in dart-lang/sdk#50565 (comment) (modified by me)
@jonasfj probably has something to say about listing arbitrary key-values under the packages independencies
.Alternatively, we could make a toplevel
build_config.yaml
(which corresponds in naming tobuild.dart
).(Priority: not part of MVP, but will add this soon after.)
The text was updated successfully, but these errors were encountered: