Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[jnigen] Fix summarizer encoding #1127

Closed
wants to merge 3 commits into from
Closed

[jnigen] Fix summarizer encoding #1127

wants to merge 3 commits into from

Conversation

HosseinYousefi
Copy link
Member

Closes #1118

@glanium Please check if this works for you.

Use this dev dependency to test:

jnigen:
  git:
    url: https://github.com/dart-lang/native
    ref: fix-encoding
    path: pkgs/jnigen/

@HosseinYousefi HosseinYousefi requested a review from dcharkes May 7, 2024 11:36
@github-actions GitHub Actions
Copy link

github-actions bot commented May 7, 2024 •
edited
Loading

PR Health

Changelog Entry ✔️

Details
Package Changed Files

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

Coverage ✔️

Details
File Coverage

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

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/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/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/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_cli/test/model/checksum_test.dart

Package publish validation ❗

Details
Package Version Status
package:ffi 2.1.2 already published at pub.dev
package:ffigen 12.0.0 already published at pub.dev
package:native_assets_cli 0.6.0-wip WIP (no publish necessary)
package:objective_c 1.0.0 already published at pub.dev
package:swiftgen 0.0.1-wip WIP (no publish necessary)
package:jni 0.9.1 (error) pub publish dry-run failed; add the publish-ignore-warnings label to ignore
package:jnigen 0.9.1 (error) pub publish dry-run failed; add the publish-ignore-warnings label to ignore

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

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

@coveralls
Copy link

coveralls commented May 7, 2024 •
edited
Loading

Coverage Status

coverage: 90.815%. remained the same
when pulling 2445bac on fix-encoding
into 1807dee on main.

@glanium
Copy link

glanium commented May 7, 2024 •
edited
Loading

Ok
I've tried but same FormatException error!
Sorry, My guess is wrong.

I don't know what wrong with my machine >.<

this is output using -o option.

test.txt

@glanium
Copy link

glanium commented May 7, 2024 •
edited
Loading

#877 (comment)

Oh, this screenshot says Unhandled exception.
So the problem lies elsewhere?

https://github.com/dart-lang/native/blob/95d226d290d4d5c583c4e45b04048442f244c656/pkgs/jnigen/lib/src/summary/summary.dart#L169C1-L169C53

This collectOutputStream method might throw FormatException?

ApiSummarizer.jar's stderr's encoding is not in UTF-8?

@HosseinYousefi
Copy link
Member Author

HosseinYousefi commented May 7, 2024 •
edited
Loading

ApiSummarizer.jar's stderr's encoding is not in UTF-8?

I changed that now as well. Probably easier to revert these changes and instead use a jvm flag when compiling using mvn. -Dfile.encoding=UTF-8 to apply universally.

Java 18 and onwards will not have this problem btw as it is going to be UTF-8 by default! https://openjdk.org/jeps/400 – well, maybe not for console I/O but at least for other things!

@HosseinYousefi
Copy link
Member Author

Closing this in favor of #1141

@HosseinYousefi HosseinYousefi deleted the fix-encoding branch May 13, 2024 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dcharkes dcharkes dcharkes approved these changes

Assignees
No one assigned
Labels
package:jni package:jnigen
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[jnigen] Jnigen fails with FormatException again
4 participants
@HosseinYousefi @coveralls @glanium @dcharkes