Remove v1.0 / v1.1 serialization format code. Require 3.6.0+ (which has newest serialization format) via increased Dart SDK constraint

[mkustermann]

This PR updates the Dart SDK constraint of package:native_assets_cli to be 3.6.0 pre-release or higher. That means users on older SDKs will use older versions of package:native_assets_cli. Users of 3.6.0+ SDK are guaranteed to have newer json protocol.

=> We can remove the code that deals with v1.0 / v1.1 serialization code.

[github-actions]

PR Health

Breaking changes ✔️

Package	Change	Current Version	New Version	Needed Version	Looking good?
native_assets_cli	Breaking	0.8.1-wip	0.9.0-wip	0.9.0-wip	✔️

Changelog Entry ✔️

Package	Changed Files

Changes to files need to be accounted for in their respective changelogs.

API leaks ✔️

The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.

Package	Leaked API symbols

License Headers ✔️

// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

Files
no missing headers

All source files should start with a license header.

Unrelated files missing license headers

Files
pkgs/ffigen/example/libclang-example/generated_bindings.dart
pkgs/ffigen/example/shared_bindings/generate.dart
pkgs/ffigen/example/shared_bindings/lib/generated/a_gen.dart
pkgs/ffigen/example/shared_bindings/lib/generated/a_shared_b_gen.dart
pkgs/ffigen/example/shared_bindings/lib/generated/base_gen.dart
pkgs/ffigen/example/simple/generated_bindings.dart
pkgs/ffigen/lib/src/header_parser/clang_bindings/clang_bindings.dart
pkgs/ffigen/test/collision_tests/expected_bindings/_expected_decl_decl_collision_bindings.dart
pkgs/ffigen/test/collision_tests/expected_bindings/_expected_decl_symbol_address_collision_bindings.dart
pkgs/ffigen/test/collision_tests/expected_bindings/_expected_decl_type_name_collision_bindings.dart
pkgs/ffigen/test/collision_tests/expected_bindings/_expected_reserved_keyword_collision_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_comment_markup_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_dart_handle_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_enum_int_mimic_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_forward_decl_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_functions_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_imported_types_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_native_func_typedef_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_opaque_dependencies_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_packed_structs_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_regress_384_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_struct_fptr_fields_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_typedef_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_unions_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_varargs_bindings.dart
pkgs/ffigen/test/large_integration_tests/_expected_cjson_bindings.dart
pkgs/ffigen/test/large_integration_tests/_expected_libclang_bindings.dart
pkgs/ffigen/test/large_integration_tests/_expected_sqlite_bindings.dart
pkgs/ffigen/test/native_test/_expected_native_test_bindings.dart
pkgs/jni/lib/src/third_party/generated_bindings.dart
pkgs/jni/lib/src/third_party/global_env_extensions.dart
pkgs/jni/lib/src/third_party/jni_bindings_generated.dart
pkgs/jnigen/android_test_runner/lib/main.dart
pkgs/jnigen/example/in_app_java/lib/android_utils.dart
pkgs/jnigen/example/kotlin_plugin/example/lib/main.dart
pkgs/jnigen/example/kotlin_plugin/lib/kotlin_bindings.dart
pkgs/jnigen/example/kotlin_plugin/lib/kotlin_plugin.dart
pkgs/jnigen/example/pdfbox_plugin/lib/pdfbox_plugin.dart
pkgs/jnigen/example/pdfbox_plugin/lib/src/third_party/org/apache/pdfbox/pdmodel/PDDocument.dart
pkgs/jnigen/example/pdfbox_plugin/lib/src/third_party/org/apache/pdfbox/pdmodel/PDDocumentInformation.dart
pkgs/jnigen/example/pdfbox_plugin/lib/src/third_party/org/apache/pdfbox/pdmodel/_package.dart
pkgs/jnigen/example/pdfbox_plugin/lib/src/third_party/org/apache/pdfbox/text/PDFTextStripper.dart
pkgs/jnigen/example/pdfbox_plugin/lib/src/third_party/org/apache/pdfbox/text/_package.dart
pkgs/jnigen/lib/src/bindings/descriptor.dart
pkgs/jnigen/lib/src/elements/elements.g.dart
pkgs/jnigen/test/jackson_core_test/third_party/bindings/com/fasterxml/jackson/core/_package.dart
pkgs/jnigen/tool/command_runner.dart
pkgs/native_assets_builder/test_data/native_dynamic_linking/bin/native_dynamic_linking.dart
pkgs/swift2objc/lib/src/config.dart
pkgs/swift2objc/lib/src/generate_wrapper.dart
pkgs/swift2objc/lib/src/generator/_core/utils.dart
pkgs/swift2objc/lib/src/generator/generator.dart
pkgs/swift2objc/lib/src/generator/generators/class_generator.dart
pkgs/swift2objc/lib/src/parser/parsers/declaration_parsers/parse_initializer_declaration.dart
pkgs/swift2objc/lib/src/parser/parsers/declaration_parsers/parse_property_declaration.dart
pkgs/swift2objc/lib/src/transformer/_core/unique_namer.dart
pkgs/swift2objc/lib/src/transformer/_core/utils.dart
pkgs/swift2objc/lib/src/transformer/transformers/transform_property.dart
pkgs/swift2objc/test/unit/parse_initializer_param_test.dart

Package publish validation ✔️

Package	Version	Status
package:ffi	2.1.3	already published at pub.dev
package:ffigen	15.0.0-wip	WIP (no publish necessary)
package:jni	0.12.0-wip	WIP (no publish necessary)
package:jnigen	0.12.0-wip	WIP (no publish necessary)
package:native_assets_cli	0.9.0-wip	WIP (no publish necessary)
package:objective_c	2.1.0-wip	WIP (no publish necessary)
package:swift2objc	0.0.1-wip	WIP (no publish necessary)
package:swiftgen	0.0.1-wip	WIP (no publish necessary)

Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation.

[mkustermann]

While the CLI protocol is unstable and being developed, we shouldn't be afraid of changing things. If the protocol changes in Dart SDK it has to change in these packages, which will require an increase in SDK constraint. That will make those packages's test fail on the stable Dart SDK on github CI.

=> I think it doesn't make much sense to even run the tests on stable Dart SDK.

@dcharkes Can we remove the CI config that runs on stable Dart SDK? I think it will just slow down development without any meaningful benefit.

[dcharkes]

While the CLI protocol is unstable and being developed, we shouldn't be afraid of changing things. If the protocol changes in Dart SDK it has to change in these packages, which will require an increase in SDK constraint. That will make those packages's test fail on the stable Dart SDK on github CI.

We already have users using this. This was added to avoid breaking them unnecessarily. Why can't we make backwards compatible changes?

Can we remove the CI config that runs on stable Dart SDK? I think it will just slow down development without any meaningful benefit.

The benefit is not unnecessarily breaking people experimenting with this.

We also have to deal with possible version skew between the Dart and Flutter SDK. So breaking the protocol in a non-backwards compatible way requires manual rolls into the Dart SDK and Flutter SDK. I'd really like to avoid that.

This PR updates the Dart SDK constraint of package:native_assets_cli to be 3.6.0 pre-release or higher. That means users on older SDKs will use older versions of package:native_assets_cli. Users of 3.6.0+ SDK are guaranteed to have newer json protocol.

👍 SGTM! (Maybe we can do 3.5.0? Or doesn't that remove meaningful old versions?)

[dcharkes]

Can we remove the dependency on package:yaml as well? (Or did the move to json happen too recently?)

[mkustermann]

Can we remove the dependency on package:yaml as well? (Or did the move to json happen too recently?)

I can make another PR for that - it's somewhat orthogonal to removing this json encoding/decoding fallback code.

👍 SGTM! (Maybe we can do 3.5.0? Or doesn't that remove meaningful old versions?)

Updated to 3.5.0.

We already have users using this. This was added to avoid breaking them unnecessarily.

Do we have a list of them (in case we need to reach out about breaking changes)?

Why can't we make backwards compatible changes?

It's possible to be backwards compatible, but incurs engineering churn (adding multi-version support, adding tests, ...) and we'll have to maintain that for at least 3 months (our cadance of stable versions). So if it slows the development of this down, I'd rather not commit to being compatible for too long (e.g. we may actually consider only allowing enabling the experiment on master/dev/beta channels and not stable -- this is what we did for wasm until it was stable)

[coveralls]

coverage: 90.809% (-0.08%) from 90.886%
when pulling 7ae2003 on remove-old-serialization-format
into 117cc75 on main.

[mkustermann]

I can make another PR for that - it's somewhat orthogonal to removing this json encoding/decoding fallback code.

Actually it's not much to remove, so I included it in this PR.

[mkustermann]

@dcharkes PTAL

[dcharkes]

So if it slows the development of this down, I'd rather not commit to being compatible for too long

Adding a new feature to the protocol is usually intensely discussed. Adding the backwards compatibility in the from/toJson and some tests is <5 minutes. So I think we're going to be wasting more time manually migrating users and answering questions on breakages than maintaining the backwards compatibility.

If there is a specific backwards compatibility that we want to break, which we currently can't due to the 3 month window, we can reevaluate. But I'd rather not remove the guardrails if we don't have a concrete case.

Do we have a list of them (in case we need to reach out about breaking changes)?

I don't think so. I'm aware of mediapipe, https://github.com/bdero/test_native_assets, intl, icu4x. I have heard from users using this and shipping apps on iOS, but I have not made a list of this.

[mkustermann]

I've moved the sdk dependency to 3.5.0, so nobody should get broken and tests pass on stable. @dcharkes do you have any other comments?

[dcharkes]

LGTM once the comment regarding the SDK version and also removing v1.2 is addressed.

[mkustermann]

Thanks, @dcharkes