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

Support and document buf #1425

Open
thesamet opened this issue Oct 31, 2022 · 13 comments · May be fixed by #1552
Open

Support and document buf #1425

thesamet opened this issue Oct 31, 2022 · 13 comments · May be fixed by #1552

Comments

@thesamet
Copy link
Contributor

In order to support build by bufs, we need the following:

  • Look into whether the JVM-based executables can be used instead of the native build of scalapb since this also supports windows (https://repo1.maven.org/maven2/com/thesamet/scalapb/protoc-gen-scala/0.11.12/)

  • Automatically upload releases of scalapb, scalapb-validate and zio-grpc to buf, through the respective projects .github/workflow/release.yml - would it be possible to use the universal (Java based) executable.

  • Add buf as an installation method in docs/main/markdown/installation.md. Add note that this support is experimental, add an FAQ in faq.md linking to this section.

@Luka-J9
Copy link

Luka-J9 commented Oct 31, 2022

I haven't explored grpc compilers (outside the netty one that comes by default). I know some are in your purview (ZIO I think), should publishing those be included in the scope of work as well?

It might also be nice to include some instructions on publishing your own? (Maybe include a nice GHA)

Also I think we don't need to use the JVM based build since under the covers the plugins are wrapped up in docker. I think the only requirement is that it's compiled in x86, which the native plugin is already. For reference my tests last week used the native plugin on an M1. Unless there is some other functional reason to use the JVM implementation.

@Luka-J9
Copy link

Luka-J9 commented Oct 31, 2022

Another thought. Given the fact that buf will need to be run before normal code compilation in this model should we consider writing an sbt wrapper around buf so it hooks in cleanly?

@thesamet
Copy link
Contributor Author

Should publishing those be included in the scope of work as well?

Yes, I can take on doing this for zio-grpc and scalapb-validate native plugins to buf after this is done to ScalaPB (so I can follow the pattern established here), and check that it all works together.

Also I think we don't need to use the JVM based build since under the covers the plugins are wrapped up in docker.

Great, let's use the x86 native plugin then! I didn't know that.

Given the fact that buf will need to be run before normal code compilation in this model should we consider writing an sbt wrapper around buf so it hooks in cleanly?

Yes, I think this should be considered, and if so - should it be a mode of operation for sbt-protoc, or a separate plugin with possibly some code re-use. Few important features of sbt-protoc to be considered: unpacking jars with proto dependencies, ensuring scalapb-options.proto file get read, checks whether source changed before running.

@thesamet
Copy link
Contributor Author

thesamet commented Oct 31, 2022

What's the advantage for an sbt user that already chose to use buf outside sbt, to have their sbt build use buf as well and not sbt-protoc with stock protoc? Trying to understand the added value of building the plugin/wrapper mentioned in your message.

@Luka-J9
Copy link

Luka-J9 commented Oct 31, 2022

unpacking jars with proto dependencies, ensuring scalapb-options.proto file get read, checks whether source changed before running.

So interestingly with buf, using the plugin, all code, including 3rd party dependencies can be compiled client side, so no need to unpack jars, or even vend jars. Because there are no jars to manage this way you don't actually need to publish scalapb-options as well, since now there can be no mismatch between jar published and the generation options in the proto-file. This solves problems where there is different generation preferences amongst different teams of people, or the use/non use of different plugins. For example this issue where overrides were needed due to validate. I've run into other issues where the who was overriding what was less clear, and with buf this can be defined once on the client to suite their needs/preferences.

If you choose not to also compile imports/"build the world" you would only need the jars that represent the generated code, the protofiles would not be necessary to unpack. However given the fact that you'd need to make sure your buf deps are in line with the jars I'd imagine that this would be viewed as not ideal. This problem (making sure your buf deps and jar deps line up) is also problematic currently when using buf in tandem with sbt-protoc.

What's the advantage for an sbt user that already chose to use buf outside sbt, to have their sbt build use buf as well and not sbt-protoc with stock protoc?

I think buf should be viewed as a replacement for protoc, as it is built as an implementation of protoc with dependency management/backwards compatibility checks in mind. Hence the suggestion of an sbt-buf, as you'd be linking the lifecycle of buf to sbt builds much like how sbt-protoc is linking the lifecycle of protoc with sbt builds.

@noirbizarre
Copy link

noirbizarre commented Nov 16, 2022

What's the advantage for an sbt user that already chose to use buf outside sbt, to have their sbt build use buf as well and not sbt-protoc with stock protoc? Trying to understand the added value of building the plugin/wrapper mentioned in your message.

My 2 cents: I think (because this is my use case) the main benefit is for the case where you need to publish synchronized versions for multiple languages.

In our case, having each project consuming the proto files an generating the proper langage files have been a nightmare lately. Having a single repository able to publish the same proto files version and langage specific generated files for Python, Scala, Go... simplified everything: no proto files to copy, no git submodules, no synchronized toolchain to maintain... Just consume the right versionned package

There is 2 aspects on buf support:

  • publishing the protoc-gen-scala on buf.build allowing easy scala packages builds with buf
  • having buf support into scalapb as an sbt plugin to benefit from buf features (linting, declarative dependencies...)

@onmomo
Copy link

onmomo commented Jul 26, 2023

any chance to bump the scalapb buf module to the latest scalapb version?

@thesamet
Copy link
Contributor Author

@Luka-J9 is this something that can be automated?

@Luka-J9
Copy link

Luka-J9 commented Jul 26, 2023

@Luka-J9 is this something that can be automated?

Yep! Will need a new GitHub action I believe - I'm on vacation this week but I can look into it when I get back

@Luka-J9
Copy link

Luka-J9 commented Jul 26, 2023

Something to note: Buf is fairly prescriptive about how protobuf should be structured. I'll assume that we just want to publish - so I'll turn those off those warnings for the initial publish.

The main two ones are the file structure being scalapb.v1 rather than scalapb and enums being prefixed by the enums name (e.g. EXACT -> MATCH_TYPE_EXACT)

@thesamet let me know if the above assumption is valid or if you want me to address the lint issues as well

@thesamet
Copy link
Contributor Author

@Luka-J9 - what is scalapb.v1? Let's not rename enums for making the linter happy at this time.

@Luka-J9
Copy link

Luka-J9 commented Jul 26, 2023

@Luka-J9 - what is scalapb.v1? Let's not rename enums for making the linter happy at this time.

It prescribes packaging and writing protos in a versioned directory/package. That way you can make breaking changes in a different package (e.g. scalapb.v2) while maintaining the previous version

@Luka-J9 Luka-J9 linked a pull request Jul 28, 2023 that will close this issue
@Luka-J9
Copy link

Luka-J9 commented Jul 28, 2023

@thesamet - Took a quick look and it seemed easy enough to add instead of waiting until Monday I added a PR that I believe will automatically publish on push to master (draft release) and when a tagged release is cut (official release).

I'm not sure how to test that it works as intended but I linked the reference documentation. The relevant linting rules have been disabled per the previous conversation.

Once this works a follow up PR can be added to ensure no breaking changes can be committed and probably a lint/breaking test on PR rather than just on merge.

I can follow up on any feedback in the PR but my availability is spotty until Monday. Thanks!

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 a pull request may close this issue.

4 participants