desc/builder: feature request: auto-de-duplicate builders and already-built descriptors in transitive graph about protoreflect HOT 4 CLOSED

I am pretty sure the problem is that you are mixing builders with already-built descriptors. When you do builder.FromMessage(jmd), you get a message builder that references already-built file descriptors for all of its dependencies. And then later, you separately add those same dependencies. The typical issue with this would be a case where you have multiple files with the same name -- the copy that comes from your builder plus the reference to the already-built file. But when a message field refers to other messages in the same file, it can also cause import issues where your file builder incorrectly is trying to import the already-built copy of the file with the same path.

I'm afraid, for what you're doing, you'd need to re-create each message descriptor so that the field's refer to other message builders, not to already-built descriptors.

I'll leave this issue open as a feature request: we could possibly add a new field to BuilderOptions that allows lenience. The safest case would be, if there are both builders and already-built descriptors referenced for the same element (identified by fully-qualified name), then always use the builder. But I think this could be a bit tricky to implement, and there are other edge cases that wouldn't be clearly handled (such as cases where the transitive graph includes more than one already-built descriptor for the same element but no builders, or cases where the graph includes multiple builders for the same element).

from protoreflect.

Thanks, jhump, yes, that confirms my suspicion.

Anyway, I've moved on and instead of building I simply stripped down un-necessary symbols from the original descriptor messages (not the descriptors themselves, as there they are interfaces and offer no Remove API), and it served my purposes.

Besides your proposed improvement (I agree it can be a bit tricky, but still useful to some extent), do you have better ideas on how to retrieve the minimal transitive enclosure (for a given symbol)? I would expect the usage is not so much limited.

from protoreflect.

I implemented something like that in the buf CLI code:
https://github.com/bufbuild/buf/blob/v1.32.0/private/bufpkg/bufimage/bufimageutil/bufimageutil.go#L177
In particular, your sample code doesn't handle custom options, Any messages, or source code info.

That link above basically just uses descriptor protos, not full-blown descriptors. It creates its own minimal index of elements, just enough to do the trimming (which is hopefully cheaper than wrapping everything in full-blown descriptors, but admittedly not yet benchmarked...).

from protoreflect.

Note that link is for informational purposes only. You might fork that code and adapt it to your needs. It is not usable as is -- if you write a program that tries to import that package, it will panic during initialization. (Everything under github.com/bufbuild/buf/private is that way -- it's meant to be an "internal" package, but it's not named internal so that we can use it from some of our own stuff outside this repo.)

from protoreflect.