From f3711d802289d2c6716dc2bae1eb1d142b892a44 Mon Sep 17 00:00:00 2001 From: Robert Grandl Date: Mon, 26 Aug 2024 13:40:30 -0700 Subject: [PATCH 1/4] Add build tags support `go build` allows the user to specify build tags. These tags enable the user to discard files based on various tags when they build the application binary. `weaver generate` which is a wrapper around `go build` doesn't allow the user to specify build tags. This PR adds built tags support for the `weaver generate` command. The syntax of passing build tags to the `weaver generate` command is similar to how build tags are specified when using `go build`. E.g., weaver generate -tags good,prod weaver generate --tags=good --- cmd/weaver/main.go | 16 +++- internal/tool/generate/generator.go | 31 +++++--- internal/tool/generate/generator_test.go | 77 +++++++++++++++++++- internal/tool/generate/testdata/tags/bad.go | 36 +++++++++ internal/tool/generate/testdata/tags/good.go | 37 ++++++++++ 5 files changed, 186 insertions(+), 11 deletions(-) create mode 100644 internal/tool/generate/testdata/tags/bad.go create mode 100644 internal/tool/generate/testdata/tags/good.go diff --git a/cmd/weaver/main.go b/cmd/weaver/main.go index 240faff70..9d7ef0917 100644 --- a/cmd/weaver/main.go +++ b/cmd/weaver/main.go @@ -74,11 +74,25 @@ func main() { switch flag.Arg(0) { case "generate": generateFlags := flag.NewFlagSet("generate", flag.ExitOnError) + tags := generateFlags.String("tags", "not specified", "Optional tags for the generate command") generateFlags.Usage = func() { fmt.Fprintln(os.Stderr, generate.Usage) } generateFlags.Parse(flag.Args()[1:]) - if err := generate.Generate(".", flag.Args()[1:], generate.Options{}); err != nil { + buildTags := "--tags=ignoreWeaverGen" + if *tags != "not specified" { // tags flag was specified + if *tags == "" || *tags == "." { + // User specified the tags flag but didn't provide any tags. + fmt.Fprintln(os.Stderr, "No tags provided.") + os.Exit(1) + } + // TODO(rgrandl): we assume that the user specify the tags properly. I.e., + // a single tag, or a list of tags separated by comma. We may want to do + // extra validation at some point. + buildTags = buildTags + "," + *tags + } + idx := len(flag.Args()) - len(generateFlags.Args()) + if err := generate.Generate(".", flag.Args()[idx:], generate.Options{BuildTags: buildTags}); err != nil { fmt.Fprintln(os.Stderr, err) os.Exit(1) } diff --git a/internal/tool/generate/generator.go b/internal/tool/generate/generator.go index 615bd02e7..75dc9ed29 100644 --- a/internal/tool/generate/generator.go +++ b/internal/tool/generate/generator.go @@ -54,7 +54,7 @@ const ( Usage = `Generate code for a Service Weaver application. Usage: - weaver generate [packages] + weaver generate [tags] [packages] Description: "weaver generate" generates code for the Service Weaver applications in the @@ -64,6 +64,9 @@ Description: file in the package's directory. For example, "weaver generate . ./foo" will create ./weaver_gen.go and ./foo/weaver_gen.go. + You specify build tags for "weaver generate" in the same way you specify build + tags for go build. See "go help build" for more information. + You specify packages for "weaver generate" in the same way you specify packages for go build, go test, go vet, etc. See "go help packages" for more information. @@ -86,13 +89,21 @@ Examples: weaver generate ./foo # Generate code for all packages in all subdirectories of current directory. - weaver generate ./...` + weaver generate ./... + + # Generate code for all files that have a "//go:build good" line at the top of + the file. + weaver generate -tags good + + # Generate code for all files that have a "//go:build good,prod" line at the + top of the file. + weaver generate -tags good,prod` ) // Options controls the operation of Generate. type Options struct { - // If non-nil, use the specified function to report warnings. - Warn func(error) + Warn func(error) // If non-nil, use the specified function to report warnings + BuildTags string } // Generate generates Service Weaver code for the specified packages. @@ -104,11 +115,13 @@ func Generate(dir string, pkgs []string, opt Options) error { } fset := token.NewFileSet() cfg := &packages.Config{ - Mode: packages.NeedName | packages.NeedSyntax | packages.NeedImports | packages.NeedTypes | packages.NeedTypesInfo, - Dir: dir, - Fset: fset, - ParseFile: parseNonWeaverGenFile, - BuildFlags: []string{"--tags=ignoreWeaverGen"}, + Mode: packages.NeedName | packages.NeedSyntax | packages.NeedImports | packages.NeedTypes | packages.NeedTypesInfo, + Dir: dir, + Fset: fset, + ParseFile: parseNonWeaverGenFile, + } + if len(opt.BuildTags) > 0 { + cfg.BuildFlags = []string{opt.BuildTags} } pkgList, err := packages.Load(cfg, pkgs...) if err != nil { diff --git a/internal/tool/generate/generator_test.go b/internal/tool/generate/generator_test.go index fbc24e36b..3c14cf791 100644 --- a/internal/tool/generate/generator_test.go +++ b/internal/tool/generate/generator_test.go @@ -102,7 +102,8 @@ func runGenerator(t *testing.T, directory, filename, contents string, subdirs [] // Run "weaver generate". opt := Options{ - Warn: func(err error) { t.Log(err) }, + Warn: func(err error) { t.Log(err) }, + BuildTags: "--tags=ignoreWeaverGen", } if err := Generate(tmp, []string{tmp}, opt); err != nil { return "", err @@ -237,6 +238,80 @@ func TestGenerator(t *testing.T) { } } +// TestGeneratorBuildWithTags runs "weaver generate" on all the files in +// testdata/tags and checks if the command succeeds. Each file should have some build tags. +func TestGeneratorBuildWithTags(t *testing.T) { + const dir = "testdata/tags" + files, err := os.ReadDir(dir) + if err != nil { + t.Fatalf("cannot list files in %q", dir) + } + + tmp := t.TempDir() + save := func(f, data string) { + if err := os.WriteFile(filepath.Join(tmp, f), []byte(data), 0644); err != nil { + t.Fatalf("error writing %s: %v", f, err) + } + } + + // Copy the files from dir to the temp directory. + for _, file := range files { + filename := file.Name() + if !strings.HasSuffix(filename, ".go") || strings.HasSuffix(filename, generatedCodeFile) { + continue + } + + // Read the test file. + bits, err := os.ReadFile(filepath.Join(dir, filename)) + if err != nil { + t.Fatalf("cannot read %q: %v", filename, err) + } + contents := string(bits) + save(filename, contents) + } + save("go.mod", goModFile) + + // Run "go mod tidy" + tidy := exec.Command("go", "mod", "tidy") + tidy.Dir = tmp + tidy.Stdout = os.Stdout + tidy.Stderr = os.Stderr + if err := tidy.Run(); err != nil { + t.Fatalf("go mod tidy: %v", err) + } + + // Run the "weaver generate" command with no build tags. Verify that the command + // doesn't succeed because bad.go does not compile. + err = Generate(tmp, []string{tmp}, Options{Warn: func(err error) { t.Log(err) }}) + if err == nil { + t.Fatal("expected generator to return an error; got nil error") + } + // Verify that no weaver_gen.go file was generated. + _, err = os.ReadFile(filepath.Join(tmp, generatedCodeFile)) + if err == nil { + t.Fatal("expected no generated file") + } + + // Run the "weaver generate" command with the build tag "good". Verify that the + // command succeeds because the bad.go file is ignored, and the weaver_gen.go + // contains only generated code for the good.go. + err = Generate(tmp, []string{tmp}, Options{Warn: func(err error) { t.Log(err) }, BuildTags: "--tags=good"}) + if err != nil { + t.Fatalf("unexpected generator error: %v", err) + } + // Verify that the weaver_gen.go file doesn't contain generated code for the bad service. + output, err := os.ReadFile(filepath.Join(tmp, generatedCodeFile)) + if err != nil { + t.Fatalf("unable to read the weaver_gen.go file: %v", err) + } + if strings.Contains(string(output), "bad") { + t.Fatalf("unexpected generated code for the bad service") + } + if !strings.Contains(string(output), "good") { + t.Fatalf("expected generated code for the good service") + } +} + // TestGeneratorErrors runs "weaver generate" on all of the files in // testdata/errors. // Every file in testdata/errors must begin with a single line header that looks diff --git a/internal/tool/generate/testdata/tags/bad.go b/internal/tool/generate/testdata/tags/bad.go new file mode 100644 index 000000000..c98d16baa --- /dev/null +++ b/internal/tool/generate/testdata/tags/bad.go @@ -0,0 +1,36 @@ +// Copyright 2024 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build !good + +package tags + +import ( + "context" + "fmt" + + "github.com/ServiceWeaver/weaver" +) + +type BadService interface { + DoSomething(context.Context) error +} + +type badServiceImpl struct { + weaver.Implements[BadService] +} + +func (b *badServiceImpl) DoSomething(context.Context) error { + Some code that does not compile +} diff --git a/internal/tool/generate/testdata/tags/good.go b/internal/tool/generate/testdata/tags/good.go new file mode 100644 index 000000000..ba2db8fae --- /dev/null +++ b/internal/tool/generate/testdata/tags/good.go @@ -0,0 +1,37 @@ +// Copyright 2024 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build good + +package tags + +import ( + "context" + "fmt" + + "github.com/ServiceWeaver/weaver" +) + +type GoodService interface { + DoSomething(context.Context) error +} + +type goodServiceImpl struct { + weaver.Implements[GoodService] +} + +func (g *goodServiceImpl) DoSomething(context.Context) error { + fmt.Println("Hello world!") + return nil +} From 12931f854e91f16c9159986a3b4f6fe01701e1ff Mon Sep 17 00:00:00 2001 From: Robert Grandl Date: Thu, 5 Sep 2024 17:16:20 -0700 Subject: [PATCH 2/4] Addressed sanjay's comments. --- cmd/weaver/main.go | 14 ++++---------- internal/tool/generate/generator.go | 4 ++-- internal/tool/generate/generator_test.go | 4 ++-- 3 files changed, 8 insertions(+), 14 deletions(-) diff --git a/cmd/weaver/main.go b/cmd/weaver/main.go index 9d7ef0917..74c09731f 100644 --- a/cmd/weaver/main.go +++ b/cmd/weaver/main.go @@ -74,25 +74,19 @@ func main() { switch flag.Arg(0) { case "generate": generateFlags := flag.NewFlagSet("generate", flag.ExitOnError) - tags := generateFlags.String("tags", "not specified", "Optional tags for the generate command") + tags := generateFlags.String("tags", "", "Optional tags for the generate command") generateFlags.Usage = func() { fmt.Fprintln(os.Stderr, generate.Usage) } generateFlags.Parse(flag.Args()[1:]) - buildTags := "--tags=ignoreWeaverGen" - if *tags != "not specified" { // tags flag was specified - if *tags == "" || *tags == "." { - // User specified the tags flag but didn't provide any tags. - fmt.Fprintln(os.Stderr, "No tags provided.") - os.Exit(1) - } + buildTags := "ignoreWeaverGen" + if *tags != "" { // tags flag was specified // TODO(rgrandl): we assume that the user specify the tags properly. I.e., // a single tag, or a list of tags separated by comma. We may want to do // extra validation at some point. buildTags = buildTags + "," + *tags } - idx := len(flag.Args()) - len(generateFlags.Args()) - if err := generate.Generate(".", flag.Args()[idx:], generate.Options{BuildTags: buildTags}); err != nil { + if err := generate.Generate(".", generateFlags.Args(), generate.Options{BuildTags: buildTags}); err != nil { fmt.Fprintln(os.Stderr, err) os.Exit(1) } diff --git a/internal/tool/generate/generator.go b/internal/tool/generate/generator.go index 75dc9ed29..ae1e7b6a4 100644 --- a/internal/tool/generate/generator.go +++ b/internal/tool/generate/generator.go @@ -54,7 +54,7 @@ const ( Usage = `Generate code for a Service Weaver application. Usage: - weaver generate [tags] [packages] + weaver generate [-tags taglist] [packages] Description: "weaver generate" generates code for the Service Weaver applications in the @@ -121,7 +121,7 @@ func Generate(dir string, pkgs []string, opt Options) error { ParseFile: parseNonWeaverGenFile, } if len(opt.BuildTags) > 0 { - cfg.BuildFlags = []string{opt.BuildTags} + cfg.BuildFlags = []string{"-tags", opt.BuildTags} } pkgList, err := packages.Load(cfg, pkgs...) if err != nil { diff --git a/internal/tool/generate/generator_test.go b/internal/tool/generate/generator_test.go index 3c14cf791..77ebf7d64 100644 --- a/internal/tool/generate/generator_test.go +++ b/internal/tool/generate/generator_test.go @@ -103,7 +103,7 @@ func runGenerator(t *testing.T, directory, filename, contents string, subdirs [] // Run "weaver generate". opt := Options{ Warn: func(err error) { t.Log(err) }, - BuildTags: "--tags=ignoreWeaverGen", + BuildTags: "ignoreWeaverGen", } if err := Generate(tmp, []string{tmp}, opt); err != nil { return "", err @@ -295,7 +295,7 @@ func TestGeneratorBuildWithTags(t *testing.T) { // Run the "weaver generate" command with the build tag "good". Verify that the // command succeeds because the bad.go file is ignored, and the weaver_gen.go // contains only generated code for the good.go. - err = Generate(tmp, []string{tmp}, Options{Warn: func(err error) { t.Log(err) }, BuildTags: "--tags=good"}) + err = Generate(tmp, []string{tmp}, Options{Warn: func(err error) { t.Log(err) }, BuildTags: "good"}) if err != nil { t.Fatalf("unexpected generator error: %v", err) } From ea8def11dd540a69b1b67d3933f78e79e6872b00 Mon Sep 17 00:00:00 2001 From: Robert Grandl Date: Fri, 6 Sep 2024 13:40:37 -0700 Subject: [PATCH 3/4] Rewrote TestGeneratorWithBuildTags to use runGenerator --- internal/tool/generate/generator_test.go | 93 +++++++++--------------- 1 file changed, 33 insertions(+), 60 deletions(-) diff --git a/internal/tool/generate/generator_test.go b/internal/tool/generate/generator_test.go index 77ebf7d64..5021d06d2 100644 --- a/internal/tool/generate/generator_test.go +++ b/internal/tool/generate/generator_test.go @@ -62,7 +62,8 @@ replace github.com/ServiceWeaver/weaver => %s // // If "weaver generate" succeeds, the produced weaver_gen.go file is written in // the provided directory with name ${filename}_weaver_gen.go. -func runGenerator(t *testing.T, directory, filename, contents string, subdirs []string) (string, error) { +func runGenerator(t *testing.T, directory, filename, contents string, subdirs []string, + buildTags []string) (string, error) { // runGenerator creates a temporary directory, copies the file and all // subdirs into it, writes a go.mod file, runs "go mod tidy", and finally // runs "weaver generate". @@ -103,7 +104,7 @@ func runGenerator(t *testing.T, directory, filename, contents string, subdirs [] // Run "weaver generate". opt := Options{ Warn: func(err error) { t.Log(err) }, - BuildTags: "ignoreWeaverGen", + BuildTags: "ignoreWeaverGen" + "," + strings.Join(buildTags, ","), } if err := Generate(tmp, []string{tmp}, opt); err != nil { return "", err @@ -135,7 +136,12 @@ func runGenerator(t *testing.T, directory, filename, contents string, subdirs [] if err := tidy.Run(); err != nil { t.Fatalf("go mod tidy: %v", err) } - gobuild := exec.Command("go", "build") + + var buildTagsArg string + if len(buildTags) > 0 { + buildTagsArg = "-tags=" + strings.Join(buildTags, ",") + } + gobuild := exec.Command("go", "build", buildTagsArg) gobuild.Dir = tmp gobuild.Stdout = os.Stdout gobuild.Stderr = os.Stderr @@ -219,7 +225,7 @@ func TestGenerator(t *testing.T) { } // Run "weaver generate". - output, err := runGenerator(t, dir, filename, contents, []string{"sub1", "sub2"}) + output, err := runGenerator(t, dir, filename, contents, []string{"sub1", "sub2"}, []string{}) if err != nil { t.Fatalf("error running generator: %v", err) } @@ -247,68 +253,35 @@ func TestGeneratorBuildWithTags(t *testing.T) { t.Fatalf("cannot list files in %q", dir) } - tmp := t.TempDir() - save := func(f, data string) { - if err := os.WriteFile(filepath.Join(tmp, f), []byte(data), 0644); err != nil { - t.Fatalf("error writing %s: %v", f, err) - } - } - - // Copy the files from dir to the temp directory. for _, file := range files { filename := file.Name() if !strings.HasSuffix(filename, ".go") || strings.HasSuffix(filename, generatedCodeFile) { continue } + t.Run(filename, func(t *testing.T) { + t.Parallel() - // Read the test file. - bits, err := os.ReadFile(filepath.Join(dir, filename)) - if err != nil { - t.Fatalf("cannot read %q: %v", filename, err) - } - contents := string(bits) - save(filename, contents) - } - save("go.mod", goModFile) - - // Run "go mod tidy" - tidy := exec.Command("go", "mod", "tidy") - tidy.Dir = tmp - tidy.Stdout = os.Stdout - tidy.Stderr = os.Stderr - if err := tidy.Run(); err != nil { - t.Fatalf("go mod tidy: %v", err) - } - - // Run the "weaver generate" command with no build tags. Verify that the command - // doesn't succeed because bad.go does not compile. - err = Generate(tmp, []string{tmp}, Options{Warn: func(err error) { t.Log(err) }}) - if err == nil { - t.Fatal("expected generator to return an error; got nil error") - } - // Verify that no weaver_gen.go file was generated. - _, err = os.ReadFile(filepath.Join(tmp, generatedCodeFile)) - if err == nil { - t.Fatal("expected no generated file") - } + // Read the test file. + bits, err := os.ReadFile(filepath.Join(dir, filename)) + if err != nil { + t.Fatalf("cannot read %q: %v", filename, err) + } + contents := string(bits) + // Run "weaver generate". + output, err := runGenerator(t, dir, filename, contents, []string{}, []string{"good"}) - // Run the "weaver generate" command with the build tag "good". Verify that the - // command succeeds because the bad.go file is ignored, and the weaver_gen.go - // contains only generated code for the good.go. - err = Generate(tmp, []string{tmp}, Options{Warn: func(err error) { t.Log(err) }, BuildTags: "good"}) - if err != nil { - t.Fatalf("unexpected generator error: %v", err) - } - // Verify that the weaver_gen.go file doesn't contain generated code for the bad service. - output, err := os.ReadFile(filepath.Join(tmp, generatedCodeFile)) - if err != nil { - t.Fatalf("unable to read the weaver_gen.go file: %v", err) - } - if strings.Contains(string(output), "bad") { - t.Fatalf("unexpected generated code for the bad service") - } - if !strings.Contains(string(output), "good") { - t.Fatalf("expected generated code for the good service") + if filename == "good.go" { + // Verify that the error is nil and the weaver_gen.go contains generated code for the good service. + if err != nil || !strings.Contains(output, "GoodService") { + t.Fatalf("expected generated code for the good service") + } + return + } + // For the bad.go verify that the error is not nil and there is no output. + if err == nil || len(output) > 0 { + t.Fatalf("expected no generated code for the good service") + } + }) } } @@ -361,7 +334,7 @@ func TestGeneratorErrors(t *testing.T) { } // Run "weaver generate". - output, err := runGenerator(t, dir, filename, contents, []string{}) + output, err := runGenerator(t, dir, filename, contents, []string{}, []string{}) errfile := strings.TrimSuffix(filename, ".go") + "_error.txt" if err == nil { os.Remove(filepath.Join(dir, errfile)) From baded6ee70cca7123fd4caefbbdcd351320d9b70 Mon Sep 17 00:00:00 2001 From: Robert Grandl Date: Tue, 10 Sep 2024 13:56:49 -0700 Subject: [PATCH 4/4] Fixed comments in generator_test --- internal/tool/generate/generator_test.go | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/internal/tool/generate/generator_test.go b/internal/tool/generate/generator_test.go index 5021d06d2..80d640df0 100644 --- a/internal/tool/generate/generator_test.go +++ b/internal/tool/generate/generator_test.go @@ -136,12 +136,7 @@ func runGenerator(t *testing.T, directory, filename, contents string, subdirs [] if err := tidy.Run(); err != nil { t.Fatalf("go mod tidy: %v", err) } - - var buildTagsArg string - if len(buildTags) > 0 { - buildTagsArg = "-tags=" + strings.Join(buildTags, ",") - } - gobuild := exec.Command("go", "build", buildTagsArg) + gobuild := exec.Command("go", "build", "-tags="+opt.BuildTags) gobuild.Dir = tmp gobuild.Stdout = os.Stdout gobuild.Stderr = os.Stderr @@ -225,7 +220,7 @@ func TestGenerator(t *testing.T) { } // Run "weaver generate". - output, err := runGenerator(t, dir, filename, contents, []string{"sub1", "sub2"}, []string{}) + output, err := runGenerator(t, dir, filename, contents, []string{"sub1", "sub2"}, nil) if err != nil { t.Fatalf("error running generator: %v", err) } @@ -268,7 +263,7 @@ func TestGeneratorBuildWithTags(t *testing.T) { } contents := string(bits) // Run "weaver generate". - output, err := runGenerator(t, dir, filename, contents, []string{}, []string{"good"}) + output, err := runGenerator(t, dir, filename, contents, nil, []string{"good"}) if filename == "good.go" { // Verify that the error is nil and the weaver_gen.go contains generated code for the good service. @@ -334,7 +329,7 @@ func TestGeneratorErrors(t *testing.T) { } // Run "weaver generate". - output, err := runGenerator(t, dir, filename, contents, []string{}, []string{}) + output, err := runGenerator(t, dir, filename, contents, nil, nil) errfile := strings.TrimSuffix(filename, ".go") + "_error.txt" if err == nil { os.Remove(filepath.Join(dir, errfile))