Skip to content

Commit

Permalink
RegisterSchema once per golang package #490 (#544)
Browse files Browse the repository at this point in the history 
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` is invoked with only one .capnp file at a time on its command
line, `capnpc-go` will not detect any *other* .capnp files that might have the
same $Go.package. For that use case, the user might be able to cope by running
`capnpc -o - persistent-simple.capnp | capnpc-go --forceschemasalways` and
editing the generated code to rename all but one `RegisterSchema()` and
add calls to the renamed code inside the one remainig `RegisterSchema()`...
though that is not a supported solution.

e.g. Use this:

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

Because the following will fail to compile due to duplicate `RegisterSchema()`
definitions:

```
capnpc -o go persistent-simple.capnp
capnpc -o go persistent-samepkg.capnp
```
  • Loading branch information
@davidhubbard
davidhubbard committed Sep 11, 2023
1 parent 01cac8d commit 2a85fea
Show file tree
Hide file tree
Showing 7 changed files with 186 additions and 69 deletions.
59 changes: 43 additions & 16 deletions capnpc-go/capnpc-go.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -78,16 +79,18 @@ type generator struct {
r renderer
fileID uint64
nodes nodeMap
pkgs pkgMap
imports imports
data staticData
opts genoptions
}

func newGenerator(fileID uint64, nodes nodeMap, opts genoptions) *generator {
func newGenerator(fileID uint64, trees nodeTrees, opts genoptions) *generator {
g := &generator{
r: &templateRenderer{t: templates},
fileID: fileID,
nodes: nodes,
nodes: trees.nodes,
pkgs: trees.pkgs,
imports: newImports(),
opts: opts,
}
Expand Down Expand Up @@ -141,14 +144,32 @@ func writeByteLiteral(out *bytes.Buffer, name string, data []byte) {
}

func (g *generator) defineSchemaVar() error {
msg, seg, _ := capnp.NewMessage(capnp.SingleSegment(nil))
req, _ := schema.NewRootCodeGeneratorRequest(seg)
fnodes := g.nodes[g.fileID].nodes
ids := make([]uint64, len(fnodes))
for i, n := range fnodes {
ids[i] = n.Id()
var ids []uint64
// Unless g.opts.forceSchemasAlways overrides it, only call g.r.Render() if
// .done is false, then set .done = true
if !g.opts.forceSchemasAlways {
pkg, ok := g.pkgs[g.nodes[g.fileID].pkg]
if !ok {
return fmt.Errorf("BUG: file %v, $Go.package %q: not found", g.fileID, g.nodes[g.fileID].pkg)
}
if pkg.done {
return nil // RegisterSchema was written in another file
}
ids = make([]uint64, len(pkg.nodeId))
copy(ids, pkg.nodeId)
pkg.done = true
} else {
// Restore the old behavior: every file has a RegisterSchema with only its own nodes
fnodes := g.nodes[g.fileID].nodes
ids = make([]uint64, len(fnodes))
for i, n := range fnodes {
ids[i] = n.Id()
}
}
sort.Sort(uint64Slice(ids))

msg, seg, _ := capnp.NewMessage(capnp.SingleSegment(nil))
req, _ := schema.NewRootCodeGeneratorRequest(seg)
// TODO(light): find largest object size and use that to allocate list
nodes, _ := req.NewNodes(int32(len(g.nodes)))
i := 0
Expand Down Expand Up @@ -1171,13 +1192,13 @@ func (g *generator) defineFile() error {
return nil
}

func generateFile(reqf schema.CodeGeneratorRequest_RequestedFile, nodes nodeMap, opts genoptions) error {
func generateFile(reqf schema.CodeGeneratorRequest_RequestedFile, trees nodeTrees, 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, trees, opts)
if err := g.defineFile(); err != nil {
return err
}
Expand Down Expand Up @@ -1218,6 +1239,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()
Expand All @@ -1230,7 +1252,7 @@ func main() {
fmt.Fprintln(os.Stderr, "capnpc-go: reading input:", err)
os.Exit(1)
}
nodes, err := buildNodeMap(req)
trees, err := makeNodeTrees(req)
if err != nil {
fmt.Fprintln(os.Stderr, "capnpc-go:", err)
os.Exit(1)
Expand All @@ -1239,9 +1261,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, trees, opts)
if err != nil {
fname, _ := reqf.Filename()
fmt.Fprintf(os.Stderr, "capnpc-go: generating %s: %v\n", fname, err)
success = false
}
Expand Down
106 changes: 63 additions & 43 deletions capnpc-go/capnpc-go_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,11 @@ func TestBuildNodeMap(t *testing.T) {
t.Errorf("Reading code generator request %s: %v", test.name, err)
continue
}
nodes, err := buildNodeMap(req)
trees, err := makeNodeTrees(req)
if err != nil {
t.Errorf("%s: buildNodeMap: %v", test.name, err)
}
f := nodes[test.fileID]
f := trees.nodes[test.fileID]
if f == nil {
t.Errorf("%s: node map is missing file node @%#x", test.name, test.fileID)
continue
Expand All @@ -120,7 +120,7 @@ func TestBuildNodeMap(t *testing.T) {
}
// Test map lookup
for _, k := range test.fileNodes {
n := nodes[k]
n := trees.nodes[k]
if n == nil {
t.Errorf("%s: missing @%#x from node map", test.name, k)
} else if n.Id() != k {
Expand Down Expand Up @@ -182,13 +182,13 @@ func TestRemoteScope(t *testing.T) {
},
}
req := mustReadGeneratorRequest(t, "scopes.capnp.out")
nodes, err := buildNodeMap(req)
trees, err := makeNodeTrees(req)
if err != nil {
t.Fatal("buildNodeMap:", err)
}
collect := func(test scopeTest) (g *generator, typ schema.Type, from *node, ok bool) {
g = newGenerator(0xd68755941d99d05e, nodes, genoptions{})
v := nodes[test.constID]
g = newGenerator(0xd68755941d99d05e, trees, genoptions{})
v := trees.nodes[test.constID]
if v == nil {
t.Errorf("Can't find const @%#x for %s test", test.constID, test.name)
return nil, schema.Type{}, nil, false
Expand Down Expand Up @@ -273,13 +273,13 @@ func formatImportSpecs(specs []importSpec) string {

func TestDefineConstNodes(t *testing.T) {
req := mustReadGeneratorRequest(t, "const.capnp.out")
nodes, err := buildNodeMap(req)
trees, err := makeNodeTrees(req)
if err != nil {
t.Fatal("buildNodeMap:", err)
}
g := newGenerator(0xc260cb50ae622e10, nodes, genoptions{})
g := newGenerator(0xc260cb50ae622e10, trees, genoptions{})
getCalls := traceGenerator(g)
err = g.defineConstNodes(nodes[0xc260cb50ae622e10].nodes)
err = g.defineConstNodes(trees.nodes[0xc260cb50ae622e10].nodes)
if err != nil {
t.Fatal("defineConstNodes:", err)
}
Expand Down Expand Up @@ -369,12 +369,12 @@ func TestDefineFile(t *testing.T) {
t.Errorf("Reading code generator request %q: %d RequestedFiles", test.fname, reqFiles.Len())
continue
}
nodes, err := buildNodeMap(req)
trees, err := makeNodeTrees(req)
if err != nil {
t.Errorf("buildNodeMap %s: %v", test.fname, err)
continue
}
g := newGenerator(reqFiles.At(0).Id(), nodes, test.opts)
g := newGenerator(reqFiles.At(0).Id(), trees, test.opts)
if err := g.defineFile(); err != nil {
t.Errorf("defineFile %s %+v: %v", test.fname, test.opts, err)
continue
Expand All @@ -387,7 +387,10 @@ 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)
for _, v := range trees.pkgs {
v.done = false
}
g := newGenerator(reqFiles.At(0).Id(), trees, test.opts)
if err := g.defineFile(); err != nil {
t.Errorf("defineFile %s %+v [iteration %d]: %v", test.fname, test.opts, i+2, err)
continue
Expand Down Expand Up @@ -531,6 +534,11 @@ 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)

defaultOptions := genoptions{
promises: true,
Expand All @@ -541,17 +549,13 @@ func TestPersistent(t *testing.T) {
fname string
opts genoptions
}{
{"persistent-simple.capnp.out", defaultOptions},
// The capnp.out is generated with:
// capnp compile --no-standard-import -I../../std -o- persistent-simple.capnp \
// persistent-samepkg.capnp > persistent-simple-and-samepkg.capnp.out
{"persistent-simple-and-samepkg.capnp.out", defaultOptions},
}
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"

var tobuild []string
for testIndex, test := range tests {
data, err := readTestFile(test.fname)
if err != nil {
t.Errorf("reading %s: %v", test.fname, err)
Expand All @@ -576,32 +580,49 @@ func TestPersistent(t *testing.T) {
t.Errorf("Reading code generator request %q: %d RequestedFiles", test.fname, reqFiles.Len())
continue
}
nodes, err := buildNodeMap(req)
trees, err := makeNodeTrees(req)
if err != nil {
t.Errorf("buildNodeMap %q: %v", test.fname, err)
continue
}
g := newGenerator(reqFiles.At(0).Id(), nodes, test.opts)
err = g.defineFile()
if err != nil {
reqFname, _ := reqFiles.At(0).Filename()
t.Errorf("defineFile %q %+v: file %q: %v", test.fname, test.opts, reqFname, err)
continue
var srcDump string
ok := false
for i := 0; i < reqFiles.Len(); i++ {
reqf := reqFiles.At(i)
reqFname, err := reqf.Filename()
if err != nil {
t.Errorf("Reading code generator request %q: reqFiles[%d] failed: %v", test.fname, i, err)
break
}
genfname := reqFname+".go"
g := newGenerator(reqf.Id(), trees, test.opts)
err = g.defineFile()
if err != nil {
t.Errorf("defineFile %q %+v: file[%d] %q: %v", test.fname, test.opts, i, reqFname, err)
break
}
src := g.generate()
genfpath := filepath.Join(dir, genfname)
err = os.WriteFile(genfpath, []byte(src), 0660)
if err != nil {
t.Fatalf("Writing generated code %q: %v", genfpath, err)
return
}
tobuild = append(tobuild, genfname)
ok = true
srcDump += fmt.Sprintf("\n%s:\n%s", genfname, src)
}
src := g.generate()
genfpath := filepath.Join(dir, genfname)
err = os.WriteFile(genfpath, []byte(src), 0660)
if err != nil {
t.Fatalf("Writing generated code %q: %v", genfpath, err)
break
if !ok {
continue
}

if testIndex + 1 < 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
Expand All @@ -611,13 +632,12 @@ 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)
t.Errorf("serr:\n%s%s", serr.String(), srcDump)
continue
} else {
t.Errorf("go %+v: %v", args, err)
t.Errorf("go %+v: %v", tobuild, err)
continue
}
}
Expand Down

0 comments on commit 2a85fea

Please sign in to comment.