Skip to content

Commit cd54850

Browse files
committed
fix(protogen): map Python dependency imports
Python files generated for one module import dependency schemas through their Go module paths. The dependency wheels install those schemas under module-relative Python packages instead, so a consumer wheel can build successfully and then fail during import. Read module roots from vendor/modules.txt and replace the longest matching prefix with the dependency package path. Require a dot boundary so similarly named and undeclared modules remain unchanged. Include the selected StarPC Python plugin and its frozen uv.lock in the cache fingerprint. Record the new fingerprint only after generation succeeds, and ignore Python lock changes when the Python plugin is not selected. Install uv in CI before the Go suite runs its generated Python import check. Generated cross-module imports now resolve without aliases, and plugin changes cannot reuse stale service code. Signed-off-by: Christian Stewart <christian@aperture.us>
1 parent 882e385 commit cd54850

6 files changed

Lines changed: 235 additions & 16 deletions

File tree

‎.github/workflows/tests.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ jobs:
1919
- uses: actions/setup-go@v7
2020
with:
2121
go-version: '1.26'
22+
- uses: astral-sh/setup-uv@v7
2223
- run: bun install
2324
- name: Cache tools
2425
uses: actions/cache@v6

‎protogen/cache.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,11 +97,15 @@ func (c *Cache) SetToolVersions(versions string) {
9797
// - The file content hash has changed
9898
// - The protoc flags have changed
9999
// - Force is true
100-
func (c *Cache) NeedsRegeneration(packageKey string, protoFiles []string, projectDir string, flagsHash string, force bool) (bool, error) {
100+
func (c *Cache) NeedsRegeneration(packageKey string, protoFiles []string, projectDir string, flagsHash string, toolVersions string, force bool) (bool, error) {
101101
if force {
102102
return true, nil
103103
}
104104

105+
if c.ToolVersions != toolVersions {
106+
return true, nil
107+
}
108+
105109
// Check if flags changed
106110
if c.ProtocFlagsHash != flagsHash {
107111
return true, nil

‎protogen/cache_test.go‎

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,14 @@
11
package protogen
22

33
import (
4+
"context"
5+
"crypto/sha256"
6+
"encoding/hex"
7+
"io"
48
"os"
59
"path/filepath"
610
"slices"
11+
"strings"
712
"testing"
813
)
914

@@ -194,3 +199,115 @@ func TestHashProtoFilesPortableAndContentSensitive(t *testing.T) {
194199
t.Fatalf("content hash must change when a proto source changes")
195200
}
196201
}
202+
203+
func TestToolVersionChangeInvalidatesCachedPackage(t *testing.T) {
204+
dir, files := writeProtoTree(t, map[string]string{
205+
"foo.proto": "syntax = \"proto3\";\npackage foo;\n",
206+
})
207+
cache := NewCache()
208+
cache.ProtocFlagsHash = "flags"
209+
cache.ToolVersions = "starpc-python=old"
210+
if err := cache.UpdatePackage("example/foo", files, nil, dir); err != nil {
211+
t.Fatal(err)
212+
}
213+
214+
stale, err := cache.NeedsRegeneration(
215+
"example/foo", files, dir, "flags", "starpc-python=old", false,
216+
)
217+
if err != nil {
218+
t.Fatal(err)
219+
}
220+
if stale {
221+
t.Fatal("matching tool state invalidated cached output")
222+
}
223+
stale, err = cache.NeedsRegeneration(
224+
"example/foo", files, dir, "flags", "starpc-python=new", false,
225+
)
226+
if err != nil {
227+
t.Fatal(err)
228+
}
229+
if !stale {
230+
t.Fatal("changed tool state reused cached output")
231+
}
232+
}
233+
234+
func TestGetToolVersionsIncludesUVLock(t *testing.T) {
235+
dir := t.TempDir()
236+
lock := []byte("version = 1\nrevision = 3\n")
237+
if err := os.WriteFile(filepath.Join(dir, "uv.lock"), lock, 0o644); err != nil {
238+
t.Fatal(err)
239+
}
240+
generator := &Generator{
241+
ProjectDir: dir,
242+
Config: NewConfig(),
243+
Plugins: &Plugins{StarpcPython: &Plugin{}},
244+
}
245+
versions := generator.getToolVersions()
246+
want := sha256.Sum256(lock)
247+
if !strings.Contains(versions, "uv.lock="+hex.EncodeToString(want[:])) {
248+
t.Fatalf("tool versions omit uv.lock digest: %q", versions)
249+
}
250+
}
251+
252+
func TestGetToolVersionsIgnoresUVLockWithoutStarpcPython(t *testing.T) {
253+
dir := t.TempDir()
254+
if err := os.WriteFile(filepath.Join(dir, "uv.lock"), []byte("changed"), 0o644); err != nil {
255+
t.Fatal(err)
256+
}
257+
generator := &Generator{
258+
ProjectDir: dir,
259+
Config: NewConfig(),
260+
Plugins: &Plugins{},
261+
}
262+
if versions := generator.getToolVersions(); strings.Contains(versions, "uv.lock=") {
263+
t.Fatalf("unselected Python plugin invalidated tool state: %q", versions)
264+
}
265+
}
266+
267+
func TestGenerateFailureDoesNotPersistToolVersions(t *testing.T) {
268+
dir := t.TempDir()
269+
vendorDir := filepath.Join(dir, "vendor")
270+
if err := os.MkdirAll(vendorDir, 0o755); err != nil {
271+
t.Fatal(err)
272+
}
273+
if err := os.WriteFile(filepath.Join(dir, "bad.proto"), []byte("not protobuf"), 0o644); err != nil {
274+
t.Fatal(err)
275+
}
276+
if err := os.WriteFile(filepath.Join(dir, "uv.lock"), []byte("changed plugin"), 0o644); err != nil {
277+
t.Fatal(err)
278+
}
279+
config := NewConfig()
280+
config.ProjectDir = dir
281+
config.Targets = []string{"bad.proto"}
282+
cache := NewCache()
283+
cache.ToolVersions = "accepted"
284+
plugins := &Plugins{
285+
Languages: Languages{LanguagePython: {}},
286+
RPCLibraries: RPCLibraries{RPCLibraryStarpcPython: {}},
287+
StarpcPython: &Plugin{
288+
Name: "starpc-python",
289+
BinaryName: "protoc-gen-starpc-python",
290+
Path: "/bin/false",
291+
Type: PluginTypePython,
292+
OutFlag: "starpc-python_out",
293+
},
294+
}
295+
generator := &Generator{
296+
Config: config,
297+
Plugins: plugins,
298+
Cache: cache,
299+
ProjectDir: dir,
300+
ModuleDir: dir,
301+
ModulePath: "example.com/project",
302+
VendorDir: vendorDir,
303+
OutDir: vendorDir,
304+
Stdout: io.Discard,
305+
Stderr: io.Discard,
306+
}
307+
if err := generator.Generate(context.Background()); err == nil {
308+
t.Fatal("invalid source generation unexpectedly succeeded")
309+
}
310+
if cache.ToolVersions != "accepted" {
311+
t.Fatalf("failed generation persisted tool state %q", cache.ToolVersions)
312+
}
313+
}

‎protogen/generator.go‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import (
44
"bufio"
55
"bytes"
66
"context"
7+
"crypto/sha256"
8+
"encoding/hex"
79
"fmt"
810
"io"
911
"os"
@@ -125,9 +127,8 @@ func (g *Generator) Generate(ctx context.Context) error {
125127
fmt.Fprintf(g.Stdout, "Found %d proto files\n", len(protoFiles))
126128
}
127129

128-
// Get tool versions for cache invalidation
130+
// Get tool versions for cache invalidation.
129131
toolVersions := g.getToolVersions()
130-
g.Cache.SetToolVersions(toolVersions)
131132

132133
// Build protoc arguments
133134
protocArgs := g.buildProtocArgs()
@@ -157,7 +158,7 @@ func (g *Generator) Generate(ctx context.Context) error {
157158
currentPackages[packageKey] = struct{}{}
158159

159160
// Check if regeneration is needed
160-
needsRegen, err := g.Cache.NeedsRegeneration(packageKey, files, g.ProjectDir, flagsHash, g.Config.Force)
161+
needsRegen, err := g.Cache.NeedsRegeneration(packageKey, files, g.ProjectDir, flagsHash, toolVersions, g.Config.Force)
161162
if err != nil {
162163
return fmt.Errorf("failed to check cache for %s: %w", dir, err)
163164
}
@@ -247,6 +248,7 @@ func (g *Generator) Generate(ctx context.Context) error {
247248
g.Cache.CleanOrphanedPackages(currentPackages)
248249

249250
g.Cache.SetProtocFlags(protocArgs, g.ModuleDir)
251+
g.Cache.SetToolVersions(toolVersions)
250252
// Save cache
251253
cacheFile, _ := g.Config.GetCacheFilePath()
252254
if err := g.Cache.Save(cacheFile); err != nil {
@@ -453,6 +455,13 @@ func (g *Generator) getToolVersions() string {
453455
}
454456
}
455457

458+
if g.Plugins != nil && g.Plugins.StarpcPython != nil {
459+
if data, err := os.ReadFile(filepath.Join(g.ProjectDir, "uv.lock")); err == nil {
460+
digest := sha256.Sum256(data)
461+
versions = append(versions, "uv.lock="+hex.EncodeToString(digest[:]))
462+
}
463+
}
464+
456465
return strings.Join(versions, ",")
457466
}
458467

‎protogen/postprocess.go‎

Lines changed: 50 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"os"
77
"path/filepath"
88
"regexp"
9+
"slices"
910
"strings"
1011
)
1112

@@ -221,13 +222,14 @@ func (p *PostProcessor) ProcessGoFile(filePath string) error {
221222
return nil
222223
}
223224

224-
// ProcessPythonFile rewrites local canonical module imports to adjacent imports.
225+
// ProcessPythonFile rewrites canonical Go module imports to the module-relative
226+
// packages installed by the current project and its vendored dependencies.
225227
func (p *PostProcessor) ProcessPythonFile(filePath string) error {
226228
data, err := os.ReadFile(filePath)
227229
if err != nil {
228230
return err
229231
}
230-
prefix := strings.ReplaceAll(strings.ReplaceAll(p.ModulePath, "/", "."), "-", "_") + "."
232+
prefixes := p.pythonImportPrefixes()
231233
lines := strings.Split(string(data), "\n")
232234
modified := false
233235
for i, line := range lines {
@@ -236,24 +238,60 @@ func (p *PostProcessor) ProcessPythonFile(filePath string) error {
236238
continue
237239
}
238240
from := strings.TrimPrefix(trimmed, "from ")
239-
if !strings.HasPrefix(from, prefix) {
240-
continue
241-
}
242-
rest := strings.TrimPrefix(from, prefix)
243-
space := strings.IndexByte(rest, ' ')
244-
if space <= 0 {
245-
continue
241+
for _, prefix := range prefixes {
242+
if !strings.HasPrefix(from, prefix) {
243+
continue
244+
}
245+
rest := strings.TrimPrefix(from, prefix)
246+
if strings.IndexByte(rest, ' ') <= 0 {
247+
continue
248+
}
249+
indent := line[:len(line)-len(trimmed)]
250+
lines[i] = indent + "from " + rest
251+
modified = true
252+
break
246253
}
247-
indent := line[:len(line)-len(trimmed)]
248-
lines[i] = indent + "from " + rest
249-
modified = true
250254
}
251255
if !modified {
252256
return nil
253257
}
254258
return os.WriteFile(filePath, []byte(strings.Join(lines, "\n")), 0o644) //nolint:gosec
255259
}
256260

261+
func (p *PostProcessor) pythonImportPrefixes() []string {
262+
modules := map[string]struct{}{p.ModulePath: {}}
263+
modulesFile := filepath.Join(p.VendorDir, "modules.txt")
264+
if data, err := os.ReadFile(modulesFile); err == nil {
265+
scanner := bufio.NewScanner(bytes.NewReader(data))
266+
for scanner.Scan() {
267+
line := scanner.Text()
268+
if !strings.HasPrefix(line, "# ") || strings.HasPrefix(line, "## ") {
269+
continue
270+
}
271+
fields := strings.Fields(strings.TrimPrefix(line, "# "))
272+
if len(fields) < 2 || (fields[1] != "=>" && !strings.HasPrefix(fields[1], "v")) {
273+
continue
274+
}
275+
modules[fields[0]] = struct{}{}
276+
}
277+
}
278+
prefixes := make([]string, 0, len(modules))
279+
for module := range modules {
280+
if module == "" {
281+
continue
282+
}
283+
prefix := strings.ReplaceAll(strings.ReplaceAll(module, "/", "."), "-", "_")
284+
prefixes = append(prefixes, prefix+".")
285+
}
286+
slices.SortFunc(prefixes, func(a, b string) int {
287+
if len(a) != len(b) {
288+
return len(b) - len(a)
289+
}
290+
return strings.Compare(a, b)
291+
})
292+
return prefixes
293+
}
294+
257295
// ProcessTsFile processes a TypeScript file.
258296
// Rewrites relative import paths to @go/ format for vendor dependencies.
259297
//

‎protogen/postprocess_test.go‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,3 +122,53 @@ func TestProcessPythonFileRewritesPyi(t *testing.T) {
122122
t.Fatalf("got %q", got)
123123
}
124124
}
125+
126+
func TestProcessPythonFileRewritesVendoredModuleImports(t *testing.T) {
127+
projectDir := t.TempDir()
128+
vendorDir := filepath.Join(projectDir, "vendor")
129+
if err := os.MkdirAll(vendorDir, 0o755); err != nil {
130+
t.Fatal(err)
131+
}
132+
modules := `# github.com/aperturerobotics/starpc v0.52.0
133+
## explicit; go 1.25.0
134+
# github.com/aperturerobotics/starpc/extensions v0.1.0
135+
## explicit; go 1.25.0
136+
# github.com/foo/bar v1.0.0
137+
## explicit; go 1.25.0
138+
github.com/foo/bar/pkg
139+
`
140+
if err := os.WriteFile(filepath.Join(vendorDir, "modules.txt"), []byte(modules), 0o644); err != nil {
141+
t.Fatal(err)
142+
}
143+
file := filepath.Join(projectDir, "resource_pb2.py")
144+
content := `from github.com.s4wave.spacewave.local import local_pb2
145+
from github.com.aperturerobotics.starpc.rpcstream import rpcstream_pb2
146+
from github.com.aperturerobotics.starpc.extensions.echo import echo_pb2
147+
from github.com.foo.bar.pkg import package_pb2
148+
from github.com.foo.barista.foo import barista_pb2
149+
from google.protobuf import timestamp_pb2
150+
from undeclared.example import example_pb2
151+
`
152+
if err := os.WriteFile(file, []byte(content), 0o644); err != nil {
153+
t.Fatal(err)
154+
}
155+
pp := NewPostProcessor(projectDir, vendorDir, "github.com/s4wave/spacewave", nil, false)
156+
if err := pp.ProcessPythonFile(file); err != nil {
157+
t.Fatal(err)
158+
}
159+
got, err := os.ReadFile(file)
160+
if err != nil {
161+
t.Fatal(err)
162+
}
163+
want := `from local import local_pb2
164+
from rpcstream import rpcstream_pb2
165+
from echo import echo_pb2
166+
from pkg import package_pb2
167+
from github.com.foo.barista.foo import barista_pb2
168+
from google.protobuf import timestamp_pb2
169+
from undeclared.example import example_pb2
170+
`
171+
if string(got) != want {
172+
t.Fatalf("unexpected dependency import rewrite:\n%s", got)
173+
}
174+
}

0 commit comments

Comments
 (0)