Open dcharkes opened 11 months ago
In short, I would suggest that you version the protocol based on the default languageVersion
from package_config.json
.
So specify input/output like:
out_dir
(since Dart 3.1
): Path where files referenced in assets
must be placed. package_root
(since Dart 3.1
): Path to root package. This is the user package/project the native assets are built for.preferred_link_mode
(since Dart 3.2
): Either static
or dynamic
...assets
(since Dart 3.1
): List of objects on the form:
name
(since Dart 3.1
): Name of the native asset when referenced from @Native
attribute, must start with package:<my_package>/path/to/file.dart
link_mode
(since Dart 3.1
): Either static
or dynamic
...supporting_asset
(since Dart 3.3
): Path to file that is included as a blob...If my_package
has has native assets and I declare:
name: my_package
environment:
sdk: ^3.1.0 # This will make the default languageVersion in package_config `3.1`
Then my build script would :
preferred_link_mode
field, and,supporting_asset
field.Thus, if my_package
wants access to the preferred_link_mode
field, I have to bump the SDK constraint to at-least ^3.2.0
.
There is no risk that my build script assumes preferred_link_mode
is always there, and then when built with an older Dart SDK it crashes.
Similarly, if my_package
wants to output the supporting_asset
field, then I must bump the SDK constraint to at-least ^3.3.0
.
There is no risk my_package
outputs the supporting_asset
field and relies on the Dart SDK copying this, but when used with an older Dart SDK my package can't find the "supporting asset" and fails in weird manners.
It's a bit of work, but a table with inputs and output, field name, description and minimum language version would go far.
After more discussions with @jonasfj, relying on the language versions of a package doesn't work.
A package with language version 3.4 could use a helper package to parse BuildConfig
and that helper package could have language version 3.3.
If we would like to support version skew, we should do protocol negotiation. build.dart
(and link.dart
) should support a mode in which they only return a protocol version. (And the native assets builder should be able to speak every version of the protocol indefinitely.)
If we would like to completely avoid version skew, we could consider not exposing a JSON protocol, but only BuildConfig
and BuildOutput
in a dart:
library. Importing a dart:
library means by construction a package works with the version of the Dart / Flutter that is running. (We could break the protocol in arbitrary ways, but we could never break the public API.)
A poor mans version of a dart:
library would be to use a helper package, but pin the SDK constraint on a single dart version sdk: '>=3.3.0 <3.4.0'
and release a new version of that package with every Dart release. This seems rather brittle.
Update on the approach:
BuildConfig
ever. So newer Dart SDKs don't break older build hooks.BuildConfig
have default values in Dart if they are missing in the JSON. So newer build hooks are not broken on older SDKs.BuildOutput
will be serialized based on the version number from BuildConfig
, so older SDKs are not broken by newer build hooks.BuildOutput
can be parsed based on older JSON formats, so older build hooks are not broken with newer SDKs.This means maintaining an implementation for older versions of BuildConfig
and BuildOutput
, which we do and cover with tests.
A changelog for both can be found in:
If we ever want to support breaking "1.", we should implement a handshake. E.g. hook/build.dart --version
should return only back the version and then the SDK should talk with that version of BuildConfig
.
- We don't do any breaking changes in the
BuildConfig
ever. So newer Dart SDKs don't break older build hooks.
We run into a similar issue with versioning resources.json
. And it would be nice if we could use the same solution in multiple place.
Ways to handle this more gracefully:
parse
API of a package starts taking a Map<Version, Uri>
instead of Uri
. If the file path comes from a command-line invocation (e.g. --config=path/to/config.json
) then that needs to be a multi argument.{ "v1" : ..., "v2" : ... }
.
Downside: huge file.
Upside: version skew between an older reader reading a newer file is completely hidden in the API of the package. The package itself deals with version skew both ways. (Newer readers reading older formats already works due to the parsers always being able to parse older versions.)For all three options, we still want to aim to not do major version bumps, so these are all in case we really need to in the future. For all three options, if we ever have to do a major version bump, this enables the eco system to migrate. We would then at some point do a breaking change announcement to remove an older major version. For all three options, we will break the current build and link hook implementations.
From an encapsulation POV, I believe option 3 to be the cleanest. The package itself deals with version skew internally, it doesn't leak out into the API, and the callers don't have to know about it.
@mosuem Did I miss any pros/cons for the different options? You were advocating for option 2, some pros I missed there?
A package with language version 3.4 could use a helper package to parse
BuildConfig
and that helper package could have language version 3.3.
Actually, this could probably be made to work. But it might not be easy.
Suppose we have a world with:
myapp
, an application that uses foo
.foo
, a package with native assets that uses the helper bar
package to create a build script.bar
, a helper package that provides utilities for creating a build script.When the build system has to build myapp
, then the build system will:
foo
has native assets that must be builtfoo
in package_config.json
.
Suppose this is 3.4
.foo
using native build protocol version 3.4
.For this to work, then bar
(the helper package), will (once invoked) need to, either:
3.4
as a parameter indicating the protocol version.package_config.json
to find the default language version of foo
,
which indicates the protocol version to be used.
(This is probably the easiest thing to do)Obviously not all versions of bar
can support all versions of the native build protocol. Which notably could cause issues like:
foo
bumps their default language version from 3.4
to 3.5
, and if 3.5
protocol is not supported by bar
then this will break the build.
foo
is published.bar
to be released.bar
is released that supports newer protocol versions, and foo
bumps default language version, but forgets to bump the constraint on bar
, then a consumer of foo
might get an old version of bar
which can't build the latest version of foo
.
bar: ^1.2.3
.Maybe, I'm missing something, and maybe it's too complicated.
But it kind of feel like writing the native build protocol version in the pubspec.yaml
is a natural solution. Even if it means that a helper package like bar
will have to support said version of the native build protocol.
And the best way to write the native protocol version in the pubspec.yaml
is to use the default language version (which is derived from the SDK constraint, environment.sdk
).
Or maybe the native build protocol version should be a completely different field in pubspec.yaml
, and use a different version number than the SDK / language version.
Talking to Daco, I can certainly see how (3) is just much simpler all around :smile:
Some notes from discussion with @jonasfj:
{ "v1" : ... , "v2" : ... }
is at some point stop adding v1
if we see all packages in the package graph have a language version of which the SDK always at least speaks v2
.
native_assets_cli
has a language version but the resolution of that package is likely going to be one of the newest ones (assuming we didn't do any breaking API changes), so we can't use that for the hooks.package:native_assets_cli
and package:native_toolchain_c
. And if both of those packages support some new feature in the protocol, your build/link hook should automatically update to use that feature after a pub get.v1
a year or two after we've introduced v2
instead of all the magic.Ways to handle this more gracefully:
_v2
when wanting to make a breaking change. This would prevent duplication inside the JSON for all other parts of JSON that are not going v2
. Old parsers would just ignore the _v2
s, just like they would have ignored a toplevel v2
in option 3. (Thanks @mkustermann for the input!)For both option 3 and 4, the version:
in the format would basically never be bumped to 2.x
. Technically speaking, when removing at some point a xxx
when a xxx_v2
has been there for a while, would be the actual breaking change. But, we don't want version comparing to fail. Instead the logic should be the same as dart:
libs, a breaking change announcement, and then at some point just the removal. Not every breaking change to dart:
libs triggers a major version bump to Dart. (Side note major version bumps to Dart are not actually indicative of breaking changes either, one can have two Dart packages that target a different major version e.g. 2.12 and 3.4 still work together!)
Then the question is should these formats have a version at all? Shouldn't they simply be versioned with the SDK? I believe it's always safe to have a version. The question is more what kind of logic is tied to that version. Should users ever have to worry about it? Does it show up in error messages etc?
I think our goal could be for Dart & Flutter stable releases that these versions are never really visible for users.
On dart dev releases and Flutter master channel they might show up while things are rolling. (Version skew between dart
and flutter
in a Flutter SDK for the build and link config and output.)
(From an implementation point of view, writing the parser and serializer from a certain version is easier than just assuming everything is nullable. It simplifies writing unit tests for checking that version skew works. Of course this is not really a concern for users.)
With https://github.com/dart-lang/native/pull/26 we added a version number to to the
BuildConfig
andBuildOutput
. The way this works is that the SDK passes the version of the protocol that has rolled in to the Dart (and Flutter) SDK. If a user'sbuild.dart
cannot deal with that, the build is supposed to fail. Similarly, thebuild.dart
passes its value of the protocol in the build output. The Dart (and Flutter) SDK check the version and error if the output cannot be used.@jonasfj suggested that we could use the Dart SDK version instead of the a version number in the protocol and use the SDK lower bound on the package being build to determine what version of the
BuildConfig
to provide to a package. The main benefit of this approach would be that if we need to do a breaking change to the build configuration, we don't end up in a situation where all dependencies with native assets need to be migrated before developers can update their Dart / Flutter SDK.If the resolved
package:native_assets_cli
dependency in the package uniquely determines the protocol version (e.g. all protocol version bump also are a package version bump) we can also support the same behavior by looking at version of that package a package requires.(With both approaches, the implementation in
package:native_asset_cli
needs to have a version number in the build config construction so it can keep constructing older versions.)