Skip to content

Commit 6e959cd

Browse files
fix(generate client): generate every language before writing the library (#1163) (#1169)
A failed run deleted the library's generated tree before the companion ran: GenerateClientLibrary rewrote contract/ first, then each language wiped its gen/ (_gen/ and the facades for Python) and only afterwards asked the companion to generate. When the companion failed, the library was left without bindings. Builds older than #1128 also wiped every module at the Python package root, including hand-written ones. Every language now runs the companion into scratch directories first (stageLanguage). contract/ and each language directory are written only once all of them have succeeded (stagedLanguage.apply), with the existing ownership rules unchanged: the wipe still spares hand-written files and scaffolds. This is how `generate proto` already stages. OpenAPI clients stage the same way, generating against a copy of the library's go.mod so the import paths match. The buf/validate half of #1163 is already fixed at its owner: core 15188366 gives Python's buf.yaml the dependencies every other language has, and main carries it since #1128 moved core to 88f4434c. The CLI that reported the issue predated that pin. A new companion-gated test holds the Python sources path to it: a proto importing buf/validate generates and its bindings import. Also fixes two pre-existing test problems: assertPythonImports ran "proto:<tag>" without the registry, and TestGenerateClientPythonMapsUpstreamProtoModules still expected option-only imports that companion 0.0.17's Python plugin drops. Co-authored-by: antoinetoussaint-byte <312536060+antoinetoussaint-byte@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent c0d39d1 commit 6e959cd

5 files changed

Lines changed: 470 additions & 115 deletions

File tree

‎cmd/generate/client_test.go‎

Lines changed: 170 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,10 @@ import (
1616
"testing"
1717

1818
"github.com/codefly-dev/cli/pkg/conformance/conformancetest"
19+
"github.com/codefly-dev/cli/pkg/generators"
1920
coreproto "github.com/codefly-dev/core/companions/proto"
2021
"github.com/codefly-dev/core/composition"
22+
"github.com/codefly-dev/core/languages"
2123
"github.com/codefly-dev/core/resources"
2224
googleproto "google.golang.org/protobuf/proto"
2325
"google.golang.org/protobuf/types/descriptorpb"
@@ -270,13 +272,166 @@ func TestGenerateClientPythonMapsUpstreamProtoModules(t *testing.T) {
270272
}
271273
}
272274

275+
// A shared module a message's fields use is imported from its
276+
// distribution, which the library declares. One used only for an option
277+
// (buf/validate, google/api) is dropped by the companion's Python plugin
278+
// (companion 0.0.17), so the bindings neither import nor depend on it.
273279
bindings := filepath.Join(genDir, "accounts", "v1", "accounts_pb2.py")
274-
assertFileContains(t, bindings, "from buf.validate import validate_pb2")
275-
assertFileContains(t, bindings, "from google.api import annotations_pb2")
280+
assertFileContains(t, bindings, "from google.rpc import status_pb2")
281+
assertFileContains(t, bindings, "from google.type import money_pb2")
282+
assertFileLacks(t, bindings, "buf.validate")
283+
assertFileLacks(t, bindings, "from google.api import")
276284

277285
pyproject := filepath.Join(clientOutput, "python", "pyproject.toml")
278-
assertFileContains(t, pyproject, `"protovalidate"`)
279286
assertFileContains(t, pyproject, `"googleapis-common-protos"`)
287+
assertFileLacks(t, pyproject, `"protovalidate"`)
288+
}
289+
290+
// TestGenerateClientPythonFromSourcesResolvesProtovalidate: a local service
291+
// whose proto validates its fields with buf/validate generates a Python
292+
// client from its own sources (#1163). Python's buf configuration used to
293+
// resolve googleapis alone, so the import did not exist for Python while Go
294+
// and TypeScript generated the same contract; the companion's configuration
295+
// now gives every language the same dependencies, and this holds the CLI's
296+
// Python path to it.
297+
func TestGenerateClientPythonFromSourcesResolvesProtovalidate(t *testing.T) {
298+
requireQualify(t)
299+
output := filepath.Join(t.TempDir(), "lib")
300+
entry := localProtoEntry(t, validatedAccountsProto)
301+
if len(entry.ProtoSources) == 0 {
302+
t.Fatal("the local entry carries no proto sources, so this would not exercise the sources path")
303+
}
304+
305+
if err := generatePythonLibrary(output, entry); err != nil {
306+
t.Fatalf("generate python client: %v", err)
307+
}
308+
309+
// The import is used only for a field option, which the companion's
310+
// Python plugin drops, so the bindings stand alone and import.
311+
pythonDir := filepath.Join(output, "python")
312+
bindings := filepath.Join(pythonDir, "accounts_client", "_gen", "accounts", "v1", "accounts_pb2.py")
313+
assertFileContains(t, bindings, "source: accounts/v1/accounts.proto")
314+
if _, err := os.Stat(filepath.Join(pythonDir, "accounts_client", "_gen", "buf")); !os.IsNotExist(err) {
315+
t.Errorf("_gen/buf was generated: protovalidate is the upstream distribution's (stat error: %v)", err)
316+
}
317+
assertPythonImports(t, pythonDir, "accounts_client._gen.accounts.v1.accounts_pb2")
318+
}
319+
320+
// TestGenerateClientFailureLeavesLibraryUntouched: a generation the companion
321+
// fails leaves the library byte for byte as it was (#1163) — its generated
322+
// bindings and facade, the contract it records, and the developer's own
323+
// modules and tests beside them. It used to wipe the generated tree, and
324+
// before that every module at the package root, before the companion ran.
325+
func TestGenerateClientFailureLeavesLibraryUntouched(t *testing.T) {
326+
requireQualify(t)
327+
output := filepath.Join(t.TempDir(), "lib")
328+
if err := generatePythonLibrary(output, localProtoEntry(t, validatedAccountsProto)); err != nil {
329+
t.Fatalf("first generation: %v", err)
330+
}
331+
pythonDir := filepath.Join(output, "python")
332+
for path, content := range map[string]string{
333+
filepath.Join(pythonDir, "accounts_client", "work_context.py"): "def stamp(ctx):\n return ctx\n",
334+
filepath.Join(pythonDir, "tests", "test_work_context.py"): "def test_stamp():\n pass\n",
335+
} {
336+
if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
337+
t.Fatal(err)
338+
}
339+
if err := os.WriteFile(path, []byte(content), 0o644); err != nil {
340+
t.Fatal(err)
341+
}
342+
}
343+
before := hashTree(t, output)
344+
345+
// The descriptor compiles, and the sources the companion is handed import
346+
// a file nothing provides: generation fails inside the companion, after
347+
// the contract is known, which is where a wipe-first run lost the tree.
348+
broken := localProtoEntry(t, validatedAccountsProto)
349+
source := &broken.ProtoSources[0]
350+
source.Content = []byte(strings.Replace(string(source.Content),
351+
`import "buf/validate/validate.proto";`,
352+
`import "buf/validate/validate.proto";`+"\nimport \"absent/nowhere.proto\";", 1))
353+
if err := generatePythonLibrary(output, broken); err == nil {
354+
t.Fatal("generating from sources importing a missing file succeeded")
355+
}
356+
if after := hashTree(t, output); after != before {
357+
t.Fatal("a failed generation changed the library")
358+
}
359+
assertFileContains(t, filepath.Join(pythonDir, "accounts_client", "work_context.py"), "def stamp")
360+
}
361+
362+
// validatedAccountsProto validates a field with buf/validate, as the
363+
// authority interface's protos do.
364+
const validatedAccountsProto = `syntax = "proto3";
365+
366+
package accounts.v1;
367+
368+
import "buf/validate/validate.proto";
369+
370+
message Account {
371+
string id = 1 [(buf.validate.field).string.min_len = 1];
372+
}
373+
374+
service Accounts {
375+
rpc Get(Account) returns (Account);
376+
}
377+
`
378+
379+
// localProtoEntry builds the contract entry --from module/service builds for
380+
// a live service, through the same buildLocalContractEndpoint, for a service
381+
// whose proto/ holds accounts/v1/accounts.proto with content and a buf.yaml
382+
// declaring the shared modules it imports.
383+
func localProtoEntry(t *testing.T, content string) generators.ContractEntry {
384+
t.Helper()
385+
ctx := context.Background()
386+
serviceDir := filepath.Join(t.TempDir(), "api")
387+
protoDir := filepath.Join(serviceDir, "proto")
388+
if err := os.MkdirAll(filepath.Join(protoDir, "accounts", "v1"), 0o755); err != nil {
389+
t.Fatal(err)
390+
}
391+
bufYAML := "version: v1\ndeps:\n - buf.build/googleapis/googleapis\n - buf.build/bufbuild/protovalidate\n"
392+
if err := os.WriteFile(filepath.Join(protoDir, "buf.yaml"), []byte(bufYAML), 0o644); err != nil {
393+
t.Fatal(err)
394+
}
395+
if err := os.WriteFile(filepath.Join(protoDir, "accounts", "v1", "accounts.proto"), []byte(content), 0o644); err != nil {
396+
t.Fatal(err)
397+
}
398+
if err := (&resources.Service{Name: "api", Version: "0.0.1"}).SaveAtDir(ctx, serviceDir); err != nil {
399+
t.Fatal(err)
400+
}
401+
service, err := resources.LoadServiceFromDir(ctx, serviceDir)
402+
if err != nil {
403+
t.Fatal(err)
404+
}
405+
endpoint := &basev0.Endpoint{
406+
Name: "grpc", Service: "api", Api: standards.GRPC,
407+
ApiDetails: &basev0.API{Value: &basev0.API_Grpc{Grpc: &basev0.GrpcAPI{}}},
408+
}
409+
descriptorSet, contract, sources, err := buildLocalContractEndpoint(ctx, service, endpoint)
410+
if err != nil {
411+
t.Fatalf("buildLocalContractEndpoint: %v", err)
412+
}
413+
return generators.ContractEntry{
414+
Endpoint: *contract,
415+
ContractBytes: descriptorSet,
416+
PackageID: "local",
417+
PackageVersion: "0.0.1",
418+
ModuleName: "accounts",
419+
FacadeModuleDefault: generators.DefaultFacadeModuleName(contract),
420+
ProtoSources: sources,
421+
}
422+
}
423+
424+
// generatePythonLibrary generates entry's Python bindings, no facade, into
425+
// output as the accounts-client library.
426+
func generatePythonLibrary(output string, entry generators.ContractEntry) error {
427+
return generators.GenerateClientLibrary(context.Background(), &generators.ClientLibraryRequest{
428+
Name: "accounts-client",
429+
Output: output,
430+
Languages: []languages.Language{languages.PYTHON},
431+
Entries: []generators.ContractEntry{entry},
432+
NoFacade: true,
433+
Force: true,
434+
})
280435
}
281436

282437
// writeUpstreamModulesContractsFixture compiles a proto importing the
@@ -765,6 +920,17 @@ func assertFileContains(t *testing.T, path, substr string) {
765920
}
766921
}
767922

923+
func assertFileLacks(t *testing.T, path, substr string) {
924+
t.Helper()
925+
data, err := os.ReadFile(path)
926+
if err != nil {
927+
t.Fatalf("read %s: %v", path, err)
928+
}
929+
if strings.Contains(string(data), substr) {
930+
t.Errorf("%s contains %q", path, substr)
931+
}
932+
}
933+
768934
func goVetLibrary(t *testing.T, goDir string) {
769935
t.Helper()
770936
cmd := exec.Command("go", "vet", "./...")
@@ -789,7 +955,7 @@ func assertPythonImports(t *testing.T, pythonDir, dottedModule string) {
789955
t.Fatalf("CompanionImage: %v", err)
790956
}
791957
cmd := exec.CommandContext(ctx, "docker", "run", "--rm",
792-
"-v", pythonDir+":/out", "-w", "/out", image.Name+":"+image.Tag,
958+
"-v", pythonDir+":/out", "-w", "/out", image.FullName(),
793959
"/venv/bin/python", "-c", "import "+dottedModule)
794960
if out, err := cmd.CombinedOutput(); err != nil {
795961
t.Fatalf("import %s failed: %v\n%s", dottedModule, err, out)

‎docs/commands.md‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3425,6 +3425,17 @@ from. `library.codefly.yaml` always carries a `sources:` list (one element for
34253425
`generate client`, one per aggregated contract for `sync solution-sdk` below) so
34263426
both commands share one schema.
34273427

3428+
Every requested language is generated into scratch directories before anything
3429+
under the library is written. A run that fails in the proto companion (a
3430+
contract that does not compile, a buf dependency that does not resolve) leaves
3431+
the library exactly as it was: its contract, its generated tree and the files
3432+
written by hand beside them. Once every language has generated, the command
3433+
replaces only what it owns: `gen/`, `src/` and `_gen/`, and each facade (a Python
3434+
facade is recognised by its generated-code notice). Scaffold files are written
3435+
only when missing, except that a Python `pyproject.toml` gains any dependency its
3436+
bindings now need. A Go library is compiled after it is written, so a library
3437+
that generates but does not build is left in place with the build error reported.
3438+
34283439
`--from module/service[/endpoint]` loads the service's Builder to build a
34293440
one-entry contract in memory (the same way `generate contracts` does) and
34303441
generates from the service's own proto sources. `--from package:`/`contracts:`

0 commit comments

Comments
 (0)