Usage of protoc-doc-gen

[cdprete]

Hello.

It would be great if your plugin could support, out of the box, the integration with https://github.com/pseudomuto/protoc-gen-doc without the need to have somehow another execution (which would be nice to have, in the docs, an example for, at least) to run it.
Of course, if this integration should be enabled or not, should be decided by the user through a property (maybe defaulted to false).
Moreover, given that it's integrated, this would potentially avoid to download protoc twice and it would fit perfectly in the new "sanctioned path" feature.

PS: I didn't try to configure and use this myself yet.

[ascopes]

Hey.

So generally I've tried to avoid providing integrations for specific protoc plugin integrations, purely because of the fact that if I do this for each user's use case, I will end up with bespoke logic to handle dozens of addons.

In this case, they do provide executables you can download. The main issue with this is that they are tarballs rather than ZIPs. If they were ZIPs, you'd be able to just say this:

<binaryUrlPlugins>
  <binaryUrlPlugin>
    <url>zip:https://github.com/.../...zip!/protoc-doc-gen.exe</url>
  </binaryUrlPlugin>
</binaryUrlPlugins>

The issue here is right now, I don't support tarballs, which is what is used in this case. The reason for this is that I currently depend on Java NIO filesystem support to access JARs/ZIPs in-memory, and this integration is heavily embedded in how the core plugin works, since it also is used for how protobuf sources are discovered in archives, amongst many other things.

I have considered moving to Apache Commons VFS to avoid this limitation as they provide TAR support as well, but this is a fairly significant rewrite of the code to handle this, and last I looked, the Apache Commons Compress support for tarballs was not complete (things like sparse file entries broke it when I tried it recently). Another option is to use Commons Compress directly and wrap it all in a custom NIO file system directly but this can be equally awkward to do (and not a trivial amount of code).

This is something that has been high up on my list for a long time; I have a discussion open at #722 to consider doing this but it is mostly a case of finding time to do it and to ensure nothing breaks with regards to how current paths and URLs are handled by the plugin.

Another option would be to provide support for containerized plugins perhaps. That has some further complexities of how it needs to be handled though, and I am not certain how common/standard of a pattern this really is.

---

Regarding what to do in the meantime...

You could produce a small wrapper around this as a workaround in theory to facilitate this short term if this is an urgent requirement.

#!/usr/bin/env bash
set -eo pipefail
if [[ ! -f protoc-gen-doc.exe ]]; then
    # Download the tarball and extract it
    { curl --fail https://github.com/.../...tar.gz | tar x; chmod +x protoc-gen-doc.exe } 1>&2
fi
./protoc-gen-doc.exe "${@}"

If this was in ./protoc-gen-doc.sh, you'd be able to use this as a "binary url plugin" using file://protoc-gen-doc.sh. I am not sure how that would work on Windows though, whether you'd have to do this in a batch script as well? Not sure.

Another option is to use maven-exec-plugin or something like the Groovy GPlus Maven plugin to download the tarball from their releases prior to protobuf-maven-plugin being run, and extract it.

---

I'll try and find a way to get tarball support within the plugin but I cannot promise anything in the short term, as it is likely to be a significant amount of work to get it migrated properly, since the Apache VFS APIs are pretty much totally incompatible with the NIO file system APIs in the standard library from what I have seen, which means this will be a significant rewrite of most filesystem logic in the plugin unless a maintained and up-to-date VFS to NIO bridge API comes along.

[ascopes]

It does when enabled, but does not cover standard usage unfortunately. I came across this issue last week when enabling parallel test execution and this follows the advice from Apache Maven developers. What I might then be able to do is adjust the sanctioned path to avoid the copy as part of this work but that'll likely be a future change.

https://github.com/ascopes/protobuf-maven-plugin/actions/runs/17691376847/job/50285343286#step:6:1203 is an example of how this chokes on the current behaviour. It is only likely to occur in cases where highly concurrent builds run on the same system and try to resolve protoc at the same time, since it bypasses any locking.

I have the fix on that branch already, just haven't had time to fix one of the unit tests as it has been a hectic week this week.

[cdprete]

Mmm what about if you use https://docs.oracle.com/javase/8/docs/api/java/nio/channels/FileLock.html?

With an exclusive lock it should work imho since it's on OS level.

[ascopes]

We're already using that for ITs, the issue is we cannot set the executable bit within the context of those locks, so that always occurs in a racy way. As mentioned by Apache, we shouldn't be modifying artifacts that are in the M2 once resolved anyway though so making a copy is the best option.

apache/maven-resolver#1577 (comment)

[cdprete]

Yeah yeah, I was still mentioning it together with the sanctioned path defaulted to target, of course.

the issue is we cannot set the executable bit within the context of those locks

Weird 😅

[ascopes]

moving this thread to #722 for the VFS bits and pieces.

[ascopes]

#802 will attempt to cover this use case.