-
Notifications
You must be signed in to change notification settings - Fork 37
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
Add support for builders using mutable messages #217
Draft
garyp
wants to merge
139
commits into
master
Choose a base branch
from
garyp/generate-interfaces
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
garyp
force-pushed
the
garyp/generate-interfaces
branch
from
April 16, 2022 00:43
a96f9ca
to
5bf25ac
Compare
garyp
force-pushed
the
garyp/generate-interfaces
branch
from
April 16, 2022 01:26
5bf25ac
to
dca21ad
Compare
We'll need to revisit this to properly test all of the variations of allowed JSON enum value decoding behavior. But this at least aligns pbandk with protobuf-java behavior.
The full task name ends with `ToSonatypeRepository` rather than just `ToSonatype`. The previously-used task name is now ambiguous (after some of the changes made in the PR to update to Kotlin 1.9) and causes gradle to fail with this error: ``` Task 'publishIosArm64PublicationToSonatype' is ambiguous in root project 'pbandk' and its subprojects. Candidates are: 'publishIosArm64PublicationToSonatypeRepository', 'publishIosSimulatorArm64PublicationToSonatypeRepository'. ```
Also enable the new KLib API Dump in the latest version of the Binary Compatibility Validator gradle plugin. Previously we only tracked/verified the binary compatibility for the JVM and Android targets. With this new option we now also track the binary compatibility for non-JVM targets.
Also remove obsolete GitHub Actions workaround that was needed for building pbandk with older Kotlin versions.
Accessing the string containing the timestamp with an out-of-bounds index was throwing an unexpected error under wasmJs.
Also adopt kotlinx-io across all platforms for the conformance package.
The previous size of 200 worked fine when a message contained lots of fields without any custom options. But custom options on a field increase the size of the code that is generated for the field's field descriptor. Which means we can fit fewer field descriptor constructor calls in a single method before the method's bytecode becomes too big and generates a `MethodTooLargeException`. For now we'll just reduce the chunking size from 200 to 100 to give us some more breathing room. Sufficiently large and complex protobuf message definitions can still lead to an exception though. If that happens often then we can implement further improvements to the chunking. Fixes #252
- Mention Wasm and Windows support and limitations - Add project goals and where to find support - Update example generated code to the most-recent code output by pbandk - Proofread the rest of the content and update portions that have become out-of-date
Enum namings could end up being named as an invalid kotlin identifier, so updated the enum name generation to avoid invalid names.
This resolves a bunch of protobuf conformance test failures on Kotlin/JS that were being caused by protobuf.js. It also removes a dependency. There's a possibility that the protobuf.js implementation of low-level protobuf encoding/decoding is more optimized for JS runtimes than pbandk's Kotlin-based implementation. But I'd rather spend time on optimizing pbandk's implementation, which would also benefit other platforms in addition to Kotlin/JS, than on fighting bugs in protobuf.js that aren't going to be fixed any time soon. Also replaced the `utf8Len()` implementation that was based on code from Google's Protobuf Java library with a simpler implementation derived from Okio code.
- The new version requires `cmake` for building the `conformance_test_runner` instead of `configure`/`make`. - There are a bunch of newly-added tests, some of which pbandk doesn't pass. I've created follow-up issues to investigate the causes and added them to `failing_tests.txt` for now. - `plugin.proto` is no longer shipped as part of the well-known types jar from the protobuf project, so we now include it manually in pbandk's source tree.
The newest version of `test_messages_proto2.proto` now includes a group field and we need to be able to process it. The group support is piggy-backing on existing code for messages since they are logically the same except for their on-the-wire representation.
pbandk doesn't actually support Editions quite yet, so for now the runner just reports the backwards-compatible values that represent proto2 and proto3 support.
garyp
force-pushed
the
garyp/generate-interfaces
branch
from
September 5, 2024 10:54
2c0d1da
to
a21311e
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #31