fix(generate, publish clients): buf names client proto sources; generate proto --path over multi-input templates

cmd/generate/client.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"crypto/sha256"
 	"fmt"
-	"os"
 	"path/filepath"
 	"strings"
 	"time"
@@ -376,18 +375,17 @@ func buildLocalContractEndpoint(ctx context.Context, service *resources.Service,
 		if ok, _ := shared.FileExists(ctx, filepath.Join(protoDir, "buf.yaml")); !ok {
 			return nil, nil, nil, fmt.Errorf("service %s has no proto/buf.yaml; cannot build a descriptor set for endpoint %s", service.Name, endpoint.Name)
 		}
-		descriptorSet, err := buildDescriptorSet(ctx, protoDir)
+		// buf compiles the descriptor set and names its own source set in the
+		// same companion run, so Sources are exactly the files and import names
+		// the descriptor set was built from.
+		descriptorSet, protoSources, err := compileProtoDir(ctx, protoDir, true)
 		if err != nil {
 			return nil, nil, nil, fmt.Errorf("cannot build descriptor set: %w", err)
 		}
 		var set descriptorpb.FileDescriptorSet
 		if err = googleproto.Unmarshal(descriptorSet, &set); err != nil {
 			return nil, nil, nil, fmt.Errorf("cannot parse generated descriptor set: %w", err)
 		}
-		protoSources, err := collectProtoSources(protoDir)
-		if err != nil {
-			return nil, nil, nil, fmt.Errorf("cannot read proto sources: %w", err)
-		}
 		// The contract's package is the one the service's own protos declare
 		// their services in, as `generate contracts` settles it; an endpoint
 		// declaring none in service.codefly.yaml is the common case.
@@ -431,36 +429,6 @@ func buildLocalContractEndpoint(ctx context.Context, service *resources.Service,
 	return nil, nil, nil, fmt.Errorf("endpoint %s reports neither a grpc nor a rest contract", endpoint.Name)
 }
 
-// collectProtoSources reads every .proto file directly (and doubly) under
-// protoDir into proto.GenerateClient's Sources form, path relative to
-// protoDir — the same files buildDescriptorSet compiled, so generation from
-// Sources produces the same code the persisted descriptor set describes.
-func collectProtoSources(protoDir string) ([]coreproto.Source, error) {
-	var sources []coreproto.Source
-	err := filepath.WalkDir(protoDir, func(p string, d os.DirEntry, err error) error {
-		if err != nil || d.IsDir() || !strings.HasSuffix(p, ".proto") {
-			return err
-		}
-		rel, err := filepath.Rel(protoDir, p)
-		if err != nil {
-			return err
-		}
-		content, err := os.ReadFile(p) //nolint:gosec // p is walked from a service's own committed proto directory
-		if err != nil {
-			return err
-		}
-		sources = append(sources, coreproto.Source{Path: filepath.ToSlash(rel), Content: content})
-		return nil
-	})
-	if err != nil {
-		return nil, err
-	}
-	if len(sources) == 0 {
-		return nil, fmt.Errorf("no .proto files under %s", protoDir)
-	}
-	return sources, nil
-}
-
 // resolvePackageSource handles --from package:<id>@<version>: it finds the
 // composed module whose package manifest ID matches id, requires it to be
 // pinned at exactly version, and reads its committed API contract catalog.

cmd/generate/client_proto_sources_companion_test.go

@@ -0,0 +1,71 @@
+//go:build proto_companion_required
+
+package generate
+
+import (
+	"os"
+	"path/filepath"
+	"sort"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+// sourceNames is the source set buf names for a proto tree, through the
+// pinned companion.
+func sourceNames(t *testing.T, dir string) []string {
+	t.Helper()
+	_, sources, err := compileProtoDir(t.Context(), dir, true)
+	require.NoError(t, err)
+	var names []string
+	for _, source := range sources {
+		names = append(names, source.Path)
+	}
+	sort.Strings(names)
+	return names
+}
+
+func writeProtos(t *testing.T, root string, rels ...string) {
+	t.Helper()
+	for _, rel := range rels {
+		writeTestFile(t, filepath.Join(root, rel), "syntax = \"proto3\";\n")
+	}
+}
+
+// A v2 buf.yaml with a vendor module and an excluded tree: every source is
+// named relative to the module that holds it, and an excluded tree is not a
+// source.
+func TestBufNamesV2ModulesAndExcludesThroughPinnedCompanion(t *testing.T) {
+	dir := t.TempDir()
+	writeTestFile(t, filepath.Join(dir, "buf.yaml"), "version: v2\nmodules:\n  - path: .\n    excludes: [facade, vendor]\n  - path: vendor\n")
+	writeProtos(t, dir, "acme/v1/api.proto", "facade/api.proto", "vendor/upstream/base/v0/base.proto")
+	require.Equal(t, []string{"acme/v1/api.proto", "upstream/base/v0/base.proto"}, sourceNames(t, dir))
+}
+
+// A v1 buf.yaml's build.excludes keep a tree out of the source set.
+func TestBufNamesV1BuildExcludesThroughPinnedCompanion(t *testing.T) {
+	dir := t.TempDir()
+	writeTestFile(t, filepath.Join(dir, "buf.yaml"), "version: v1\nbuild:\n  excludes:\n    - facade\n")
+	writeProtos(t, dir, "acme/v1/api.proto", "facade/api.proto")
+	require.Equal(t, []string{"acme/v1/api.proto"}, sourceNames(t, dir))
+}
+
+// A v2 module's includes narrow it to the trees named.
+func TestBufNamesV2IncludesThroughPinnedCompanion(t *testing.T) {
+	dir := t.TempDir()
+	writeTestFile(t, filepath.Join(dir, "buf.yaml"), "version: v2\nmodules:\n  - path: .\n    includes: [acme]\n")
+	writeProtos(t, dir, "acme/v1/api.proto", "other/v1/other.proto")
+	require.Equal(t, []string{"acme/v1/api.proto"}, sourceNames(t, dir))
+}
+
+// A module folder that is a symlink contributes its files under their import
+// names: the link's target is copied in, since it is not mounted.
+func TestBufNamesASymlinkedModuleThroughPinnedCompanion(t *testing.T) {
+	root := t.TempDir()
+	dir, shared := filepath.Join(root, "proto"), filepath.Join(root, "shared")
+	writeProtos(t, shared, "upstream/base/v0/base.proto")
+	writeTestFile(t, filepath.Join(dir, "buf.yaml"), "version: v2\nmodules:\n  - path: .\n    excludes: [vendor]\n  - path: vendor\n")
+	writeProtos(t, dir, "acme/v1/api.proto")
+	require.NoError(t, os.Symlink(shared, filepath.Join(dir, "vendor")))
+	require.Equal(t, []string{"acme/v1/api.proto", "upstream/base/v0/base.proto"}, sourceNames(t, dir))
+}

cmd/generate/client_proto_sources_test.go

@@ -0,0 +1,90 @@
+package generate
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// buf names the source set: each file is read from the path buf found it at
+// and named by its import path, and a file buf lists only as an import is not
+// a source.
+func TestProtoSourcesFromListingNamesByImportPath(t *testing.T) {
+	root := t.TempDir()
+	for _, rel := range []string{"acme/v1/api.proto", "vendor/upstream/base/v0/base.proto"} {
+		writeTestFile(t, filepath.Join(root, rel), "syntax = \"proto3\";\n// "+rel+"\n")
+	}
+	listing := []byte(`{"path":"acme/v1/api.proto","import_path":"acme/v1/api.proto","is_import":false}
+{"path":"vendor/upstream/base/v0/base.proto","import_path":"upstream/base/v0/base.proto","is_import":false}
+{"path":"buf/validate/validate.proto","import_path":"buf/validate/validate.proto","is_import":true}
+`)
+	sources, err := protoSourcesFromListing(root, listing)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(sources) != 2 || sources[0].Path != "acme/v1/api.proto" || sources[1].Path != "upstream/base/v0/base.proto" {
+		t.Fatalf("sources = %+v", sources)
+	}
+	if string(sources[1].Content) != "syntax = \"proto3\";\n// vendor/upstream/base/v0/base.proto\n" {
+		t.Fatalf("content read from the wrong file: %q", sources[1].Content)
+	}
+}
+
+// A listed path that leaves the copied tree, or a listing with nothing to
+// compile, is refused rather than read.
+func TestProtoSourcesFromListingRefusesWhatItCannotRead(t *testing.T) {
+	root := t.TempDir()
+	if _, err := protoSourcesFromListing(root, []byte(`{"path":"../escape.proto","import_path":"escape.proto"}`)); err == nil {
+		t.Fatal("a path outside the tree was read")
+	}
+	if _, err := protoSourcesFromListing(root, nil); err == nil {
+		t.Fatal("an empty listing was accepted")
+	}
+	if _, err := os.Stat(filepath.Join(root, "escape.proto")); !os.IsNotExist(err) {
+		t.Fatal(err)
+	}
+}
+
+// A linked folder outside the tree is copied as what it holds; a link to a
+// folder that holds it, or to one already being copied, is refused by name
+// rather than recursed into.
+func TestCopyTreeFollowsLinkedFoldersAndRefusesLoops(t *testing.T) {
+	root := t.TempDir()
+	src, shared := filepath.Join(root, "proto"), filepath.Join(root, "shared")
+	writeTestFile(t, filepath.Join(src, "acme/v1/api.proto"), "a")
+	writeTestFile(t, filepath.Join(shared, "upstream/base.proto"), "b")
+	if err := os.Symlink(shared, filepath.Join(src, "vendor")); err != nil {
+		t.Fatal(err)
+	}
+	dst := filepath.Join(t.TempDir(), "copy")
+	if err := copyTree(src, dst); err != nil {
+		t.Fatal(err)
+	}
+	if data, err := os.ReadFile(filepath.Join(dst, "vendor/upstream/base.proto")); err != nil || string(data) != "b" {
+		t.Fatalf("linked folder not copied: %q, %v", data, err)
+	}
+
+	for name, link := range map[string]struct{ at, to string }{
+		"to its own folder":      {"acme/loop", "acme"},
+		"to the copied root":     {"acme/v1/up", "."},
+		"back through a link":    {"../shared/upstream/back", "."},
+		"to a folder above root": {"acme/out", ".."},
+	} {
+		t.Run(name, func(t *testing.T) {
+			at := filepath.Join(src, filepath.FromSlash(link.at))
+			if err := os.Symlink(filepath.Join(src, link.to), at); err != nil {
+				t.Fatal(err)
+			}
+			t.Cleanup(func() { _ = os.Remove(at) })
+			err := copyTree(src, filepath.Join(t.TempDir(), "copy"))
+			if err == nil || !strings.Contains(err.Error(), "looping link") {
+				t.Fatalf("copyTree = %v, want a looping link refused", err)
+			}
+			resolved, _ := filepath.EvalSymlinks(at)
+			if !strings.Contains(err.Error(), resolved) {
+				t.Fatalf("refusal %q does not name the link's target %s", err, resolved)
+			}
+		})
+	}
+}