Support other extensions for published image

[sato9818]

This PR adds new option that allows users to specify an image filename published by bufBuild.
Users can specify other formats than json.

[CLAassistant]

All committers have signed the CLA.

[andrewparmet]

Two initial thoughts:

First, I see a list of file names that Buf recognizes in the docs:

• $ buf build -o image.binpb
• $ buf build -o image.binpb.gz
• $ buf build -o image.binpb.zst
• $ buf build -o image.json
• $ buf build -o image.json.gz
• $ buf build -o image.json.zst
• $ buf build -o image.txtpb
• $ buf build -o image.txtpb.gz
• $ buf build -o image.txtpb.zst

Do you know if the current implementation of buf check breaking works on every format? I think we should add an integration test to cover each case.

Second, I'm thinking about the plugin API. Previously the contract was:

• The plugin builds a file and exposes a constant that lets you get that file by name, but doesn't expose the absolute path so you have to construct that yourself.
• You have no control over the format of the image file, nor its name.

This change introduces a new contract:

• The plugin builds a file and allows you to specify the whole file name, including the extension, which may not be valid for Buf. This has advantages and disadvantages:
  • bad: It's possible for a user to specify invalid input
  • bad: We have to direct users to documentation for what inputs are legal rather than allowing that to be naturally discoverable
  • good: We're decoupled from the Buf tool version, so users can opt into new formats without this plugin needing an update
  • neutral: Allowing users to specify the file name rather than just the extension backs off of a previous opinion of the plugin, which is that its generated image should be called image. This is slightly more impact than the title of this PR (and the previously filed issue) would imply is necessary.

On the whole both contracts are limiting if users want to use that file for purposes that aren't built-in to the plugin since we offer no way to grab the built image file without building a file path yourself. Given that the plugin exposes the file via the constant today, the README should be updated to call out the bufBuild task and how to get the built image file.

Stepping back I think it's a good feature to be able to produce an image in any supported format. This leaves a few action items before we can move forward:

• This change should include a new (parameterized) integration test, or modify the existing test, that builds a Buf image in every supported format and verifies that buf check breaking can be run against it successfully. The test should probably verify a failure to make sure the check works end-to-end.
• We should agree on the correct plugin API. Specifically:
  • Should the image file name be configurable or just the extension?
  • If just the extension, then what should the configuration API look like? Looks like a cross product of format x compression, so that would suggest two enums.
  • The plugin currently has a two-part API for image configuration: publishSchema, which controls (a) if an image is generated and (b) if it's going to be published, and imageArtifactDetails, which controls the Maven coordinates of the published image. This change introduces a third dimension so we should work to unify these options in a single API if it makes sense to do so.

Lots to think about. Thanks for taking on this new feature!

[sato9818]

@andrewparmet
Thank you for the kind and detailed review!

Firstly, I'm going to implement a integration test to check if buf check breaking works correctly.

Secondly, regarding the new plugin API, I think it's fine just to be able to configure the image extension. I agree with your opinion that we should let users choose their desired format and compression pair. I think we should control users to choose the pair from this list.

Lastly, I don't think we should unify three options: publishSchema, imageArtifactDetails and bufBuildPublicationFileName. In my usecase, I require the capability to generate an image file with a specified extension without publishing it.

[andrewparmet]

For this PR I think it's ok to leave the configuration APIs as they are, and I'll tweak them for some coherence afterwards.

Yes, that list is more thorough than what I saw.

May I ask what your use case is? I'm wondering if this plugin can more directly support it.

[sato9818]

My team is using Armeria as a grpc framework, which takes a descriptor file for its doc service.
But in Armeria, the json format descriptor is not supported. So, we would like other format than json like .bin.

[sato9818]

@andrewparmet

I added some tests to check if buf breaking works correctly with other extensions than json.

It seems buf build supports only these extensions as outputs:

• bin
• bin.gz
• bin.zst
• binpb
• binpb.gz
• binpb.zst
• json
• json.gz
• json.zst
• txtpb
• txtpb.gz
• txtpb.zst

Could you please review the code again?

[sato9818]

Hi @andrewparmet.
I was wondering if there might be some time to take a look at this soon.

If there's anything I can do to help move it along, just let me know. I'm all ears for any feedback you might have.

[andrewparmet]

Sorry for the delay. A few light questions.

[sato9818]

@andrewparmet
Thank you for your code change suggestion!

I changed the code to separate the extension option into a format and compression method option.

[andrewparmet]

Released in 0.9.0