Work around vtprotobuf's well-known type locality handling. - #85
Draft
thaJeztah wants to merge 2 commits into
Draft
Work around vtprotobuf's well-known type locality handling.#85thaJeztah wants to merge 2 commits into
thaJeztah wants to merge 2 commits into
Conversation
Work around vtprotobuf's well-known type locality handling.
go-plugin replaces the standard protobuf well-known types with its own
implementations under github.com/knqyf263/go-plugin/types/known. These
types retain their original protobuf package names (google.protobuf),
but live in separate Go packages such as anypb, structpb, typepb, etc.
vtprotobuf determines whether a message is local using its protobuf
package name:
message.Desc.ParentFile().Package()
and records generated packages in LocalPackages using the same key.
That assumption breaks for go-plugin's replacement well-known types.
For example, google.protobuf.Type and google.protobuf.Any are considered
local to each other even though their Go types live in separate typepb
and anypb packages. This can cause vtprotobuf to emit unqualified types
such as:
(*Any)(m.Value)
instead of:
(*anypb.Any)(m.Value)
It also affects vtprotobuf's special handling of well-known types, where
a type considered local has its Go import path cleared. With go-plugin's
layout this can result in invalid imports such as:
__ ""
or incorrect references to vtprotobuf's own well-known type packages.
The proper fix belongs in vtprotobuf: locality should be determined by
the generated Go package/import path rather than only by the protobuf
package name. Testing this locally by comparing message.GoIdent.GoImportPath
with the import path of the file being generated fixes the failures and
allows the full go-plugin test suite to pass.
Until that can be fixed upstream, keep the workaround local to
go-plugin. Reapply go-plugin's well-known type import remapping to the
temporary protogen.Plugin used for vtprotobuf generation, preserve local
well-known type generation, rewrite vtprotobuf's hard-coded well-known
type package paths, qualify cross-package well-known type references,
and normalize duplicate imports introduced by those rewrites.
This is intentionally a stop-gap to allow go-plugin to build with
vtprotobuf v0.6.x without carrying a fork of vtprotobuf.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Author
|
Not expecting this to be merged; large parts were AI-assisted to find what's causing the incompatibility with vtprotobuf; the correct fix would be to have a fix in upstream (see the summary above), but wanted to see if a stop-gap solution was possible. |
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Work around vtprotobuf's well-known type locality handling.
go-plugin replaces the standard protobuf well-known types with its own
implementations under github.com/knqyf263/go-plugin/types/known. These
types retain their original protobuf package names (google.protobuf),
but live in separate Go packages such as anypb, structpb, typepb, etc.
vtprotobuf determines whether a message is local using its protobuf
package name:
and records generated packages in LocalPackages using the same key.
That assumption breaks for go-plugin's replacement well-known types.
For example, google.protobuf.Type and google.protobuf.Any are considered
local to each other even though their Go types live in separate typepb
and anypb packages. This can cause vtprotobuf to emit unqualified types
such as:
instead of:
It also affects vtprotobuf's special handling of well-known types, where
a type considered local has its Go import path cleared. With go-plugin's
layout this can result in invalid imports such as:
or incorrect references to vtprotobuf's own well-known type packages.
The proper fix belongs in vtprotobuf: locality should be determined by
the generated Go package/import path rather than only by the protobuf
package name. Testing this locally by comparing message.GoIdent.GoImportPath
with the import path of the file being generated fixes the failures and
allows the full go-plugin test suite to pass.
Until that can be fixed upstream, keep the workaround local to
go-plugin. Reapply go-plugin's well-known type import remapping to the
temporary protogen.Plugin used for vtprotobuf generation, preserve local
well-known type generation, rewrite vtprotobuf's hard-coded well-known
type package paths, qualify cross-package well-known type references,
and normalize duplicate imports introduced by those rewrites.
This is intentionally a stop-gap to allow go-plugin to build with
vtprotobuf v0.6.x without carrying a fork of vtprotobuf.
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.