Skip to content

lib/go: Dedup include package - #16

Merged
tgallice merged 2 commits into
masterfrom
tg/go-plugin-dedup-includes
Oct 28, 2026
Merged

tgallice merged 2 commits into
masterfrom
tg/go-plugin-dedup-includes

Conversation

@tgallice

Copy link
Copy Markdown
Member

Dedup include package names in the Go plugin scope

plugin.BuildGoScope named each include after the last segment of its namespace, without checking for duplicates. When a program includes two packages with the same name, Go plugins
import both under that one name, and the generated code does not compile.

Example: influencer/apis/post/fetcher/v2/post_fetcher.thrift includes both base.post and influencer.types.post.v2.post. In influencer v1.0.72, the unimplemented.go and fake.go
files generated for it fail with post redeclared in this block. media_fetcher v2 and indexer v2 hit the same issue with media. The C++ generator already handles this case.

Changes

  • gocodegen.Scope.DedupIncludes(reserved...) gives each import path a distinct PkgName. An include keeps its own name when that name is free; otherwise it gets a numeric suffix
    (post0). Names reserved in one call stay reserved in later calls, so a plugin can add its own.
  • BuildGoScope reserves the thrift runtime package name and the Go predeclared identifiers (error, string...), so that an include cannot hide them.
  • Include.BaseName() returns the package's real name, before any renaming.
  • Include.ImportSpec() writes the import line, with an alias only when one is needed.
  • Scope.Imports() returns a single entry per import path.

GoType stays consistent: a type from a renamed include is generated as *post0.Post.

Compatibility

Includes are renamed only when there is a collision, and ImportSpec adds no alias in other cases. Code generated without collisions is unchanged. To benefit from the fix, a plugin
template must import with {{.ImportSpec}}; this is done for go-unimplemented-server and go-fake-server in a follow-up PR on upfluence-if.

  • Did you create an Apache Jira ticket? (Request account here, not required for trivial changes)
  • If a ticket exists: Does your pull request title follow the pattern "THRIFT-NNNN: describe my issue"?
  • Did you squash your changes to a single commit? (not required, but preferred)
  • Did you do your best to avoid breaking changes? If one was needed, did you label the Jira ticket with "Breaking-Change"?
  • If your change does not involve any code, include [skip ci] anywhere in the commit message to free up build resources.

@tgallice tgallice self-assigned this Oct 27, 2026
@tgallice
tgallice force-pushed the tg/go-plugin-dedup-includes branch from 95f4f23 to 94f5cf7 Compare October 28, 2026 08:51
@tgallice
tgallice merged commit 067f92d into master Oct 28, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants