Skip to content

[native_assets_cli] Use JSON instead of YAML #1019

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

Merged
merged 3 commits into from
Mar 19, 2024
Merged

[native_assets_cli] Use JSON instead of YAML #1019

merged 3 commits into from
Mar 19, 2024

Conversation

dcharkes
Copy link
Collaborator

@dcharkes dcharkes changed the base branch from hook-build-dot-dart to main March 15, 2024 17:16
Copy link

github-actions bot commented Mar 15, 2024

PR Health

Breaking changes ✔️

Details
Package Change Current Version New Version Needed Version Looking good?
native_assets_cli Breaking 0.5.0-wip 0.5.0-wip 0.5.0-wip ✔️

Changelog Entry ✔️

Details
Package Changed Files

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

Coverage ⚠️

Details
File Coverage
pkgs/native_assets_cli/lib/src/api/asset.dart 💔 Not covered
pkgs/native_assets_cli/lib/src/api/build_config.dart 💚 100 %
pkgs/native_assets_cli/lib/src/api/build_output.dart 💚 100 %
pkgs/native_assets_cli/lib/src/model/asset.dart 💚 100 %
pkgs/native_assets_cli/lib/src/model/build_config.dart 💚 100 %
pkgs/native_assets_cli/lib/src/model/build_output.dart 💚 100 %
pkgs/native_assets_cli/lib/src/model/c_compiler_config.dart 💚 100 %
pkgs/native_assets_cli/lib/src/model/data_asset.dart 💚 100 %
pkgs/native_assets_cli/lib/src/model/dependencies.dart 💚 100 %
pkgs/native_assets_cli/lib/src/model/metadata.dart 💚 100 %
pkgs/native_assets_cli/lib/src/model/native_code_asset.dart 💚 99 %
pkgs/native_assets_cli/lib/src/utils/json.dart 💚 100 %

This check for test coverage is informational (issues shown here will not fail the PR).

This check can be disabled by tagging the PR with skip-coverage-check

License Headers ✔️

Details
// 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/ffi/example/main.dart
pkgs/ffigen/example/libclang-example/generated_bindings.dart
pkgs/ffigen/example/objective_c/avf_audio_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/example/swift/swift_api_bindings.dart
pkgs/ffigen/lib/src/config_provider/config_spec.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_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/lang/jcharacter.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/_init.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/c_based/dart_bindings/com/fasterxml/jackson/core/_package.dart
pkgs/jnigen/test/jackson_core_test/third_party/dart_only/dart_bindings/com/fasterxml/jackson/core/_package.dart
pkgs/jnigen/tool/command_runner.dart

Package publish validation ✔️

Details
Package Version Status
package:ffi 2.1.2 already published at pub.dev
package:ffigen 12.0.0-wip WIP (no publish necessary)
package:jni 0.8.0-wip WIP (no publish necessary)
package:jnigen 0.8.0-wip WIP (no publish necessary)
package:native_assets_cli 0.5.0-wip WIP (no publish necessary)

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

@dcharkes dcharkes requested a review from mosuem March 18, 2024 11:49
@coveralls
Copy link

coveralls commented Mar 18, 2024

Coverage Status

coverage: 93.677% (+0.03%) from 93.643%
when pulling cd12725 on use-json
into ba431cb on main.

@mosuem
Copy link
Member

mosuem commented Mar 19, 2024

Should this rather be based of the move-to-hook/ PR?

@dcharkes
Copy link
Collaborator Author

Should this rather be based of the move-to-hook/ PR?

Yeah, I tried to do that. but then the CI didn't trigger, so I based it on main again... 🤷

@@ -6,7 +6,7 @@
/// kernel file.
///
/// The `native_assets.yaml` embedded in a kernel file has a different format
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

native_assets.yaml?

Copy link
Collaborator Author

@dcharkes dcharkes Mar 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, that's not part of the protocol it's internal to the native_assets_builder and the CFE.

Maybe we should consider moving that to JSON as well. If so, in a separate PR. (It will require different changes in dartdev/flutter_tools.)

@auto-submit auto-submit bot merged commit 71ada4b into main Mar 19, 2024
@auto-submit auto-submit bot deleted the use-json branch March 19, 2024 12:12
copybara-service bot pushed a commit to dart-lang/sdk that referenced this pull request Mar 19, 2024
dart-lang/native#1018
dart-lang/native#1019

Change-Id: I0371ebbbba872bf90514f0f2e255957f6a89c8c5
Cq-Include-Trybots: luci.dart.try:pkg-linux-debug-try,pkg-linux-release-arm64-try,pkg-linux-release-try,pkg-mac-release-arm64-try,pkg-mac-release-try,pkg-win-release-arm64-try,pkg-win-release-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/357605
Reviewed-by: Moritz Sümmermann <[email protected]>
Commit-Queue: Daco Harkes <[email protected]>
HosseinYousefi pushed a commit that referenced this pull request Mar 21, 2024

Verified

This commit was signed with the committer’s verified signature.
HosseinYousefi Hossein Yousefi
* #991
dcharkes added a commit to flutter/flutter that referenced this pull request Mar 25, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Roll of a bunch of breaking changes from the native_assets_builder and
native_assets_cli upstream. Most notably:

* dart-lang/native#946
* dart-lang/native#1018
* dart-lang/native#1019

This PR also updates the template in `flutter create
--template=package_ffi` to use the rewritten API.

This PR does not change any functionality in Flutter.

For reference, the same roll in the Dart SDK:

* https://dart-review.googlesource.com/c/sdk/+/357605
* https://dart-review.googlesource.com/c/sdk/+/357623

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] All existing and new tests are passing.

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[test-exempt]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants