Question on forked dependencies

[hypnoglow]

There are 2 forked dependencies in use: https://github.com/TheThingsIndustries/protoc-gen-validate and https://github.com/TheThingsIndustries/protoc-gen-star

This causes troubles becase all consumers of protoc-gen-fieldmask module also have to add replace directives to their go.mod files, e.g.

replace github.com/lyft/protoc-gen-star => github.com/TheThingsIndustries/protoc-gen-star v0.4.14-gogo.4

replace github.com/envoyproxy/protoc-gen-validate => github.com/TheThingsIndustries/protoc-gen-validate v0.3.0-java-fieldmask.2

Is there any strong reason to keep using these forks? Are this forks long-running i.e. the updates will never land into upstream? If so, maybe we should rework them so only the required functionality is kept, and other is discarded, to break upstream dependency.

As another option, would it make sense to move required functionality from https://github.com/TheThingsIndustries/protoc-gen-validate directly to this repository and again remove the dependency? E.g. as protoc-gen-fieldmask supports only Go we don't need upstream features for Java and Python. Probably we could just copy required code here so it's all in one place.

WDYT?

Sorry if my questions make no sense, as I may have incorrect assumptions about these forks.

[rvolosatovs]

The forks are required, because:

• protoc-gen-star: We need these merged lang/go: Add Context.FieldType{PackageName,ImportPath} lyft/protoc-gen-star#67 Gogo extension support lyft/protoc-gen-star#51, once they are merged the fork can be removed
• protoc-gen-validate: Upstream generates validators per-message, while we want to have them per-field, modifying the expected Validate() error signature to allow fields to be specified is breaking and adding an additional assertion to check for ValidateFields(...string) error, like we expect would add additional overhead, which I don't think they would be happy about. In our case we also do not want to have such overhead. That said, I suppose a possibility would be to make it configurable and generate per-message validators by default and generate per-field validators(or both) if configured to do so. At this point I don't think we can justify spending time and resources on doing this, given that we do not gain anything from it - the package is meant to be used as a binary anyway and compiles perfectly fine with no magic https://github.com/TheThingsIndustries/docker-protobuf/blob/7220a24abbcdcc54dd965e5f83aea16f676468d7/Dockerfile#L66-L67 If you're willing to contribute to this - please, by all means, feel free to do so and we will be happy to accept it. FTR, here's the diff we're using currently bufbuild/protoc-gen-validate@v0.3.0-java...TheThingsIndustries:v0.3.0-java-fieldmask.3

[codyaray]

@rvolosatovs Do you think these protoc-gen-star will actually ever be merged? One was closed but I haven't seen if there was a replacement PR merged or anything

[rvolosatovs]

@rvolosatovs Do you think these protoc-gen-star will actually ever be merged? One was closed but I haven't seen if there was a replacement PR merged or anything

Not sure, but so far it does not look too promising.
We will stop using gogo though, so hopefully we will be able to drop some forks.
Refs TheThingsNetwork/lorawan-stack#2798