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.

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

Rebuild protobuf parser using protoc 25.1 #95

Merged
merged 4 commits into from
Dec 13, 2023

Conversation

jameinel
Copy link
Contributor

@jameinel jameinel commented Dec 7, 2023

This rebuilds the Bakery making it compatible with Protobuf >= 3.20 (including 4.*)

The issue is that Builder objects were introduced in 3.20 and are mandatory for 4.* support. So if you regenerate the protocol definition to support 4.* then it won't import with 3.4 or 3.19 (as I tested).

This appears to work with both 4.0 and 3.20 at least. I don't know what versions of Protobuf we need to support.

Using:
```
$ protoc --version
libprotoc 3.12.4
$ protoc --python_out . id.proto
```

Note that this is probably still not 4.x compatible since the error message was
"cannot instantiate Descriptor directly"
Downloaded the binary directly and ran:
```
$ ~/dev/protobuf/bin/protoc  --version
libprotoc 25.1
$ ~/dev/protobuf/bin/protoc  --python_out . id.proto
```

I then tested that you can install python-protobuf 4.* and still import this
file.
The issue is that with 3.20 they introduced Builders, which are required in new
versions of protobuf, but only support in 3.20 and onwards.
@jameinel jameinel changed the title Protoc rebuild 25.1 Rebuild protobuf parser using protoc 25.1 Dec 7, 2023
@fabricematrat
Copy link
Contributor

@jameinel you should also modify the setup.py.

@jameinel
Copy link
Contributor Author

jameinel commented Dec 8, 2023

I updated setup.py as well.

@cjwatson cjwatson mentioned this pull request Dec 10, 2023
@cderici
Copy link

cderici commented Dec 12, 2023

@fabricematrat pinging for this anytime you can ☝️ thanks again for all the time you spent on this! ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants