jhump / protoreflect

Reflection (Rich Descriptors) for Go Protocol Buffers
Apache License 2.0
1.33k stars 170 forks source link

Regression upgrading from v1.14.1 to v1.15.4: extensions are resolved recursively instead of non-recursively #589

Closed stapelberg closed 7 months ago

stapelberg commented 8 months ago

I noticed a behavioral difference when upgrading from protoreflect v1.14.1 to protoreflect v1.15.4.

When using protoprint to generate a .proto file, a few import lines are missing compared to before. The affected imports are other .proto files that declare option extensions.

I tracked this down to desc/builder/resolver.go:resolveTypesInOptions thinking the extension is already known and therefore not adding a dependency.

This, in turn, is caused by the following difference:

In v1.14.1, extensions are added non-recursively (recursive == false):

But in v1.15.4, extensions are added recursively (publicImportsOnly == false, which is equivalent to recursive == true):

I hope this is enough information to work with. Let me know if you can’t make progress without a proper standalone reproducer, though extracting one will take me a couple of days I expect…

jhump commented 7 months ago

@stapelberg, thanks for this report! I don't need a full repro case. I see the issue. Thanks for debugging it so thoroughly and providing those details!

I'm opening a pull request with a fix now.