RegisterSchema once per golang package #490

Fixes #490
by only generating one `RegisterSchema()` function per `$Go.package()`.

For example, the unit tests persistent-simple.capnp and
persistent-samepkg.capnp now both have the annotation:

`$Go.package("persistent_simple");`

Their generated go code thus starts with:

`package persistent_simple`

but only one of them will include a `RegisterSchema()`.

Note: if `capnp` invokes `capnpc-go` to generate go code but only includes
one .capnp file at a time on the command line, this will not detect any
*other* .capnp files that might have the same $Go.package. In that situation
ask the user to take responsibility for setting `--schemas true` and
`--schemas false` for individual invocations of `capnp`.

e.g. This will work:

`capnpc -o go persistent-simple.capnp persistent-samepkg.capnp`

This will not work:

`capnpc -o go persistent-simple.capnp; capnpc -o go persistent-samepkg.capnp`

capnpc-go/capnpc-go.go

@@ -43,9 +43,10 @@ const (
 // genoptions are parameters that control code generation.
 // Usually passed on the command line.
 type genoptions struct {
-	promises      bool
-	schemas       bool
-	structStrings bool
+	promises           bool
+	schemas            bool
+	structStrings      bool
+	forceSchemasAlways bool
 }
 
 type renderer interface {
@@ -73,6 +74,8 @@ func (tr *templateRenderer) Bytes() []byte {
 	return tr.buf.Bytes()
 }
 
+type pkgMap map[string]bool
+
 // generator builds up the generated code for a single file.
 type generator struct {
 	r       renderer
@@ -81,15 +84,17 @@ type generator struct {
 	imports imports
 	data    staticData
 	opts    genoptions
+	pkgDone pkgMap
 }
 
-func newGenerator(fileID uint64, nodes nodeMap, opts genoptions) *generator {
+func newGenerator(fileID uint64, nodes nodeMap, pkgDone pkgMap, opts genoptions) *generator {
 	g := &generator{
 		r:       &templateRenderer{t: templates},
 		fileID:  fileID,
 		nodes:   nodes,
 		imports: newImports(),
 		opts:    opts,
+		pkgDone: pkgDone,
 	}
 	g.data.init(fileID)
 	return g
@@ -1163,21 +1168,22 @@ func (g *generator) defineFile() error {
 			return err
 		}
 	}
-	if g.opts.schemas {
+	if g.opts.schemas && (!g.pkgDone[f.pkg] || g.opts.forceSchemasAlways) {
 		if err := g.defineSchemaVar(); err != nil {
 			return err
 		}
+		g.pkgDone[f.pkg] = true
 	}
 	return nil
 }
 
-func generateFile(reqf schema.CodeGeneratorRequest_RequestedFile, nodes nodeMap, opts genoptions) error {
+func generateFile(reqf schema.CodeGeneratorRequest_RequestedFile, nodes nodeMap, pkgDone pkgMap, opts genoptions) error {
 	if opts.structStrings && !opts.schemas {
 		return errors.New("cannot generate struct String() methods without embedding schemas")
 	}
 	id := reqf.Id()
 	fname, _ := reqf.Filename()
-	g := newGenerator(id, nodes, opts)
+	g := newGenerator(id, nodes, pkgDone, opts)
 	if err := g.defineFile(); err != nil {
 		return err
 	}
@@ -1218,6 +1224,7 @@ func main() {
 	flag.BoolVar(&opts.promises, "promises", true, "generate code for promises")
 	flag.BoolVar(&opts.schemas, "schemas", true, "embed schema information in generated code")
 	flag.BoolVar(&opts.structStrings, "structstrings", true, "generate String() methods for structs (-schemas must be true)")
+	flag.BoolVar(&opts.forceSchemasAlways, "forceschemasalways", false, "(temporary, will be removed) force RegisterSchema() code in every generated .go file even if it is in the same package as another go file. Perhaps useful if the code generation erroneously omits a RegisterSchemas()")
 	flag.Parse()
 
 	msg, err := capnp.NewDecoder(os.Stdin).Decode()
@@ -1230,6 +1237,7 @@ func main() {
 		fmt.Fprintln(os.Stderr, "capnpc-go: reading input:", err)
 		os.Exit(1)
 	}
+	pkgDone := make(pkgMap)
 	nodes, err := buildNodeMap(req)
 	if err != nil {
 		fmt.Fprintln(os.Stderr, "capnpc-go:", err)
@@ -1239,9 +1247,14 @@ func main() {
 	reqFiles, _ := req.RequestedFiles()
 	for i := 0; i < reqFiles.Len(); i++ {
 		reqf := reqFiles.At(i)
-		err := generateFile(reqf, nodes, opts)
+		fname, err := reqf.Filename()
+		if err != nil {
+			fmt.Fprintf(os.Stderr, "capnpc-go: failed to get filename %d/%d: %v\n", i + 1, reqFiles.Len(), err)
+			success = false
+			break
+		}
+		err = generateFile(reqf, nodes, pkgDone, opts)
 		if err != nil {
-			fname, _ := reqf.Filename()
 			fmt.Fprintf(os.Stderr, "capnpc-go: generating %s: %v\n", fname, err)
 			success = false
 		}

capnpc-go/capnpc-go_test.go

@@ -187,7 +187,7 @@ func TestRemoteScope(t *testing.T) {
 		t.Fatal("buildNodeMap:", err)
 	}
 	collect := func(test scopeTest) (g *generator, typ schema.Type, from *node, ok bool) {
-		g = newGenerator(0xd68755941d99d05e, nodes, genoptions{})
+		g = newGenerator(0xd68755941d99d05e, nodes, make(pkgMap), genoptions{})
 		v := nodes[test.constID]
 		if v == nil {
 			t.Errorf("Can't find const @%#x for %s test", test.constID, test.name)
@@ -277,7 +277,7 @@ func TestDefineConstNodes(t *testing.T) {
 	if err != nil {
 		t.Fatal("buildNodeMap:", err)
 	}
-	g := newGenerator(0xc260cb50ae622e10, nodes, genoptions{})
+	g := newGenerator(0xc260cb50ae622e10, nodes, make(pkgMap), genoptions{})
 	getCalls := traceGenerator(g)
 	err = g.defineConstNodes(nodes[0xc260cb50ae622e10].nodes)
 	if err != nil {
@@ -374,7 +374,7 @@ func TestDefineFile(t *testing.T) {
 			t.Errorf("buildNodeMap %s: %v", test.fname, err)
 			continue
 		}
-		g := newGenerator(reqFiles.At(0).Id(), nodes, test.opts)
+		g := newGenerator(reqFiles.At(0).Id(), nodes, make(pkgMap), test.opts)
 		if err := g.defineFile(); err != nil {
 			t.Errorf("defineFile %s %+v: %v", test.fname, test.opts, err)
 			continue
@@ -387,7 +387,7 @@ func TestDefineFile(t *testing.T) {
 
 		// Generation should be deterministic between runs.
 		for i := 0; i < iterations-1; i++ {
-			g := newGenerator(reqFiles.At(0).Id(), nodes, test.opts)
+			g := newGenerator(reqFiles.At(0).Id(), nodes, make(pkgMap), test.opts)
 			if err := g.defineFile(); err != nil {
 				t.Errorf("defineFile %s %+v [iteration %d]: %v", test.fname, test.opts, i+2, err)
 				continue
@@ -531,6 +531,12 @@ func TestPersistent(t *testing.T) {
 	//        capnpc-go -promises=0 -schemas=0 -structstrings=0
 	// ```
 	t.Parallel()
+	dir, err := setupTempDir()
+	if err != nil {
+		panic(err)
+	}
+	defer os.RemoveAll(dir)
+	pkgDone := make(pkgMap)
 
 	defaultOptions := genoptions{
 		promises:      true,
@@ -542,14 +548,10 @@ func TestPersistent(t *testing.T) {
 		opts  genoptions
 	}{
 		{"persistent-simple.capnp.out", defaultOptions},
+		{"persistent-samepkg.capnp.out", defaultOptions},
 	}
+	var tobuild []string
 	for _, test := range tests {
-		dir, err := setupTempDir()
-		if err != nil {
-			t.Fatalf("%s: %v", test.fname, err)
-			break;
-		}
-		defer os.RemoveAll(dir)
 		genfname := test.fname+".go"
 
 		data, err := readTestFile(test.fname)
@@ -581,7 +583,7 @@ func TestPersistent(t *testing.T) {
 			t.Errorf("buildNodeMap %q: %v", test.fname, err)
 			continue
 		}
-		g := newGenerator(reqFiles.At(0).Id(), nodes, test.opts)
+		g := newGenerator(reqFiles.At(0).Id(), nodes, pkgDone, test.opts)
 		err = g.defineFile()
 		if err != nil {
 			reqFname, _ := reqFiles.At(0).Filename()
@@ -595,13 +597,15 @@ func TestPersistent(t *testing.T) {
 			t.Fatalf("Writing generated code %q: %v", genfpath, err)
 			break
 		}
+		tobuild = append(tobuild, genfname)
 
+		if len(tobuild) < len(tests) {
+			continue
+		}
 		// Relies on persistent-simple.capnp with $Go.package("persistent_simple")
 		// not being ("main"). Thus `go build` skips writing an executable.
-		args := []string{
-			"build", genfname,
-		}
-		cmd := exec.Command("go", args...)
+		tobuild = append([]string{"build"}, tobuild...)
+		cmd := exec.Command("go", tobuild...)
 		cmd.Dir = dir
 		cmd.Stdin = strings.NewReader("")
 		var sout strings.Builder
@@ -611,13 +615,13 @@ func TestPersistent(t *testing.T) {
 		if err = cmd.Run(); err != nil {
 			if gotcode, ok := err.(*exec.ExitError); ok {
 				exitcode := gotcode.ExitCode()
-				t.Errorf("go %+v exitcode:%d", args, exitcode)
+				t.Errorf("go %+v exitcode:%d", tobuild, exitcode)
 				t.Errorf("sout:\n%s", sout.String())
 				t.Errorf("serr:\n%s", serr.String())
 				t.Errorf("\n%s:\n%s", genfname, src)
 				continue
 			} else {
-				t.Errorf("go %+v: %v", args, err)
+				t.Errorf("go %+v: %v", tobuild, err)
 				continue
 			}
 		}

capnpc-go/testdata/persistent-samepkg.capnp

@@ -0,0 +1,34 @@
+# Copyright (c) 2023 Sandstorm Development Group, Inc. and contributors
+# Licensed under the MIT License:
+#
+# Permission is hereby granted, free of charge, to any person obtaining a copy
+# of this software and associated documentation files (the "Software"), to deal
+# in the Software without restriction, including without limitation the rights
+# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+# copies of the Software, and to permit persons to whom the Software is
+# furnished to do so, subject to the following conditions:
+#
+# The above copyright notice and this permission notice shall be included in
+# all copies or substantial portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+# THE SOFTWARE.
+
+using Go = import "/go.capnp";
+
+@0x941fad1fe66eeb50;
+
+$Go.package("persistent_simple");
+$Go.import("capnproto.org/go/capnp/v3/capnpc-go/testdata/persistent-simple");
+
+interface SomeOtherFile {
+}
+
+struct ThisIsInTheSamePackageAsTheOtherfile {
+  string @0 :Text;
+}

capnpc-go/testdata/persistent-simple.capnp

@@ -19,7 +19,7 @@
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 # THE SOFTWARE.
 
-using Go = import "go.capnp";
+using Go = import "/go.capnp";
 
 @0xbcfe42e3392b05a8;