Support descriptor_set_in

[Maragues]

Support specifying descriptor_set_in files for a GenerateProtoTask

Usage (in gradle.kts)

protobuf {
  protoc { .. }

  generateProtoTasks {
    ofSourceSet("main").forEach {
      it.addDescriptorSetFiles(project.layout.projectDirectory.files("sample/demo-descriptor-set.desc"))

      (..)
    }
  }
}

It's a very naive implementation, but it does the job for me

I also considered other options

• Read all .desc files from proto sourceset
• Create a new sourceSet for descriptor sets

Let me know if you think any of these options is better for the project.

I'm not an expert at all on protobuf and you might find this PR not appropriate for the project, considering protocolbuffers/protobuf#3973

Related: #251

Testing

If you think the PR is appropriate for merging, I would like to add tests for this feature. But I'll need some guidance on where to add them and how to run tests locally.

Use Case

I want to generate code from the descriptor set by passing the proto file names contained in the binary file. This requires some extra tweaks on the plugin that I don't think are worth sharing upstream, but this is the protoc command I'm interested in obtaining

protoc -ISomePaths --java_out=... --descriptor_set_in=/path/demo-descriptor-set.desc demo.proto

because demo.proto does not exist in SomePaths, protoc falls back to looking for demo.proto in the descriptor_set_in and generates class Demo extends GeneratedMessageV3

[Maragues]

If someone is interested in the extra code to pass file names to generate code for, check this branch

But it feels hacky and I don't think the official plugin wants something like that.

[Maragues]

@ejona86 Sorry to ping you directly, but since now one replied after 1 week, I don't know if any maintainer noticed this PR

[ejona86]

BTW: I did look at this a few weeks ago and have been thinking about it. We're reworking/cleaning/modernizing things right now such that I want to delay a configuration addition like this. Below is just a few of the thoughts I have gathered.

This PR

I agree it doesn't make sense to look for a desc in the normal source set, because this desc is only for use as a dependency. That means having it as a configuration dependency would align more with the current architecture. We could maybe search the configuration for .desc top-level files, and we may not want to search within jar files as desc probably isn't a unique enough file extension.

I don't know how often .desc files will be used just for dependencies. I have been thinking for a while it would probably be better (smaller, less surprising since comments are dropped) to include .desc files instead of .proto files into jars. But until we start consuming such things, there's not much point in using desc files as dependencies; most people would probably just have the .protos. And if we change this we'll also want to update the Maven plugin.

Your actual use case

Generating code based on .desc makes sense to me without further infrastructure, but there's the annoying problem of needing to define the file names to generate. We could easily search in the source set (or a new one) for .desc files but we won't know what files need to be generated from them. Those could be configured explicitly, but it is weird to not have that defined in the source set. We could read a file in the source set that has the list of files to generate. Overall, awkward and I can't think of prior art that's had to do something similar.

We could potentially generate code for all files in the .desc. That would make it fit better with the current source set which is for sources to generate. Then we could do something else for desc containing dependencies.

[Maragues]

Thanks for your detailed answer & thoughts

I have been thinking for a while it would probably be better [...] to include .desc files instead of .proto files into jars

That sounds like a good move to me. This would be the case when you declare the dependency as protobuf group:project:version, right?

There's the annoying problem of needing to define the file names to generate

I cover that in a separate branch. The plugin allows clients to specify which files to generate, but a proper PR should check for collisions and print warning messages.

In any case, I understand that this PR won't be merged, but I hope it's useful to spark the conversation. I don't know if you want to close it or if you prefer to keep it open for now.

[ejona86]

Let's leave the PR open for now, at least as a reminder.