Inline the binary protos #25
Merged
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
| @@ -101,6 +101,7 @@ public function Name(): string { | |||
| } | |||
|
|
|||
| public function FileDescriptorProtoBytes(): string { | |||
| return (string)\gzuncompress(\file_get_contents(\realpath(\dirname(__FILE__)) . '/any_file_descriptor.pb.bin.gz')); | |||
There was a problem hiding this comment.
Anything in the generated/ directory is autogenerated through:
bazel run //:gen
The diffs are:
- All the files ending in
.pb.bin.gzwere removed. - We inlined their content in the associated
proto.php.
Comment on lines
-235
to
-238
| // The protoc compiler will complain that this contains invalid UTF-8 data, | ||
| // but we will ignore that for now. |
There was a problem hiding this comment.
This is the reason we need this PR: the complain is turning into an error in newer version of the protoc.
| binaryProtoStr := "" | ||
| for i := 0; i < len(binaryProto); i++ { | ||
| c := binaryProto[i] | ||
| binaryProtoStr += fmt.Sprintf("\\x%x", c) |
There was a problem hiding this comment.
%x prints a single byte in hexadecimal (perfect for the \x?? format in Hacklang)
protoc-gen-hack/plugin.go was generating the gzipped binary ProtoDescriptorProto and returning it to protoc so we could save it to a file. protoc was warning us about having those file's content being invalid UTF-8. Unfortunately newer protoc binary reject files with such a ayload. We do want to keep the binary FileDescriptorProto logic for servers that can send the binary descriptor to their clients but we can inline it in each generated file. While we don't need to gzip the content anymore, I kept this logic as we were generating large binary strings in Hacklang.
jchaffraix-slack
force-pushed
the
julien_inline_binary_proto
branch
from
9c57b27
to
f50a83b
Compare
jchaffraix-slack
added a commit
that referenced
this pull request
Apr 26, 2024
Inlining the protobufs in #25 unblocked migrating to newer version of Protobuf as the proto compiler (protoc) error'd.
Merged
jchaffraix-slack
added a commit
that referenced
this pull request
Apr 26, 2024
Inlining the protobufs in #25 unblocked migrating to newer version of Protobuf as the proto compiler (protoc) error'd.
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.
protoc-gen-hack/plugin.go was generating the gzipped binary ProtoDescriptorProto and returning it to protoc so we could save it to a file. protoc was warning us about having those file's content being invalid UTF-8.
Unfortunately newer protoc binary reject files with such a ayload. We do want to keep the binary FileDescriptorProto logic for servers that can send the binary descriptor to their clients but we can inline it in each generated file.
While we don't need to gzip the content anymore, I kept this logic as we were generating large binary strings in Hacklang.