Merge #76355

76355: bazel: various improvements to generation r=ajwerner a=ajwerner This adds targets for `:stringer`, `:gomock`, `:execgen`, `:optgen`, `:misc`, and `:docs` to `//pkg/gen` and then some aliases `:code` and `:gen`. This PR then leverages these targets to do all code generation. Co-authored-by: Andrew Werner <[email protected]>
cockroachdb · Feb 12, 2022 · 7a65c42 · 7a65c42
2 parents 2bc16c3 + 560d766
commit 7a65c42
Show file tree

Hide file tree

Showing 56 changed files with 1,597 additions and 348 deletions.
diff --git a/build/STRINGER.bzl b/build/STRINGER.bzl
@@ -9,13 +9,17 @@ def stringer(src, typ, name, additional_args=[]):
         # golang.org/x/sys/execabs which requires all PATH lookups to
         # result in absolute paths. To account for this, we resolve the
         # relative path returned by location to an absolute path.
-        cmd = """
-         GO_REL_PATH=`dirname $(location @go_sdk//:bin/go)`
-         GO_ABS_PATH=`cd $$GO_REL_PATH && pwd`
-         # Set GOPATH to something to workaround https://github.com/golang/go/issues/43938
-         env PATH=$$GO_ABS_PATH HOME=$(GENDIR) GOPATH=/nonexist-gopath \
-         $(location @com_github_cockroachdb_tools//cmd/stringer:stringer) -output=$@ -type={} {} $<
-      """.format(typ, " ".join(additional_args)),
+        cmd = """\
+GO_REL_PATH=`dirname $(location @go_sdk//:bin/go)`
+GO_ABS_PATH=`cd $$GO_REL_PATH && pwd`
+# Set GOPATH to something to workaround https://github.com/golang/go/issues/43938
+env PATH=$$GO_ABS_PATH HOME=$(GENDIR) GOPATH=/nonexist-gopath \
+$(location @com_github_cockroachdb_tools//cmd/stringer:stringer) -output=$@ -type={typ} {args} $<
+""".format(
+         typ = typ,
+         args = " ".join(additional_args),
+        ),
+        visibility = [":__pkg__", "//pkg/gen:__pkg__"],
         tools = [
             "@go_sdk//:bin/go",
             "@com_github_cockroachdb_tools//cmd/stringer",

diff --git a/docs/generated/BUILD.bazel b/docs/generated/BUILD.bazel
@@ -17,6 +17,10 @@ genrule(
     exec_tools = [
         "//pkg/util/log/gen",
     ],
+    visibility = [
+        ":__pkg__",
+        "//pkg/gen:__pkg__",
+    ],
 )
 
 genrule(
@@ -31,6 +35,10 @@ genrule(
     exec_tools = [
         "//pkg/util/log/logconfig:gen",
     ],
+    visibility = [
+        ":__pkg__",
+        "//pkg/gen:__pkg__",
+    ],
 )
 
 genrule(
@@ -45,11 +53,19 @@ genrule(
     exec_tools = [
         "//pkg/util/log/eventpb:gen",
     ],
+    visibility = [
+        ":__pkg__",
+        "//pkg/gen:__pkg__",
+    ],
 )
 
 genrule(
     name = "gen-logformats-md",
     outs = ["logformats.md"],
     cmd = "$(location //pkg/cmd/docgen) logformats $(location logformats.md)",
     exec_tools = ["//pkg/cmd/docgen"],
+    visibility = [
+        ":__pkg__",
+        "//pkg/gen:__pkg__",
+    ],
 )
diff --git a/docs/generated/http/BUILD.bazel b/docs/generated/http/BUILD.bazel
@@ -71,4 +71,8 @@ genrule(
         "@com_github_pseudomuto_protoc_gen_doc//cmd/protoc-gen-doc",
         "//pkg/cmd/docgen",
     ],
+    visibility = [
+        ":__pkg__",
+        "//pkg/gen:__pkg__",
+    ],
 )
diff --git a/docs/generated/settings/BUILD.bazel b/docs/generated/settings/BUILD.bazel
@@ -3,11 +3,19 @@ genrule(
     outs = ["settings.html"],
     cmd = "$(location //pkg/cmd/cockroach-short) gen settings-list --format=rawhtml > $@",
     exec_tools = ["//pkg/cmd/cockroach-short"],
+    visibility = [
+        ":__pkg__",
+        "//pkg/gen:__pkg__",
+    ],
 )
 
 genrule(
     name = "settings_for_tenants",
     outs = ["settings-for-tenants.txt"],
     cmd = "$(location //pkg/cmd/cockroach-short) gen settings-list --without-system-only > $@",
     exec_tools = ["//pkg/cmd/cockroach-short"],
+    visibility = [
+        ":__pkg__",
+        "//pkg/gen:__pkg__",
+    ],
 )
diff --git a/docs/generated/sql/BUILD.bazel b/docs/generated/sql/BUILD.bazel
@@ -10,4 +10,8 @@ genrule(
 $(location //pkg/cmd/docgen) functions $(RULEDIR) --quiet
 """,
     exec_tools = ["//pkg/cmd/docgen"],
+    visibility = [
+        ":__pkg__",
+        "//pkg/gen:__pkg__",
+    ],
 )
diff --git a/docs/generated/sql/bnf/BUILD.bazel b/docs/generated/sql/bnf/BUILD.bazel
@@ -242,6 +242,10 @@ genrule(
     $(location //pkg/cmd/docgen) grammar bnf $(RULEDIR) --quiet --addr $(location //pkg/sql/parser:sql.y)
     """,
     exec_tools = ["//pkg/cmd/docgen"],
+    visibility = [
+        ":__pkg__",
+        "//pkg/gen:__pkg__",
+    ],
 )
 
 genrule(
@@ -257,4 +261,8 @@ genrule(
     COCKROACH_REQUIRE_RAILROAD=1 $(location //pkg/cmd/docgen) grammar svg $$bnf_locs $(RULEDIR) --addr $(location //pkg/sql/parser:sql.y) --railroad $(location @railroadjar//:rr.war)
     """,
     exec_tools = ["//pkg/cmd/docgen"],
+    visibility = [
+        ":__pkg__",
+        "//pkg/gen:__pkg__",
+    ],
 )
diff --git a/pkg/cmd/dev/acceptance.go b/pkg/cmd/dev/acceptance.go
@@ -41,7 +41,7 @@ func (d *dev) acceptance(cmd *cobra.Command, _ []string) error {
 	)
 
 	// First we have to build cockroach.
-	crossArgs, targets, err := d.getBasicBuildArgs(ctx, []string{"//pkg/cmd/cockroach-short:cockroach-short"}, true)
+	crossArgs, targets, err := d.getBasicBuildArgs(ctx, []string{"//pkg/cmd/cockroach-short:cockroach-short"})
 	if err != nil {
 		return err
 	}

diff --git a/pkg/cmd/dev/build.go b/pkg/cmd/dev/build.go
@@ -27,8 +27,7 @@ import (
 )
 
 const (
-	crossFlag        = "cross"
-	skipGenerateFlag = "skip-generate"
+	crossFlag = "cross"
 )
 
 type buildTarget struct {
@@ -56,7 +55,6 @@ func makeBuildCmd(runE func(cmd *cobra.Command, args []string) error) *cobra.Com
         You can optionally set a config, as in --cross=windows.
         Defaults to linux if not specified. The config should be the name of a
         build configuration specified in .bazelrc, minus the "cross" prefix.`)
-	buildCmd.Flags().Bool(skipGenerateFlag, false, "skip staging generated files into the workspace")
 	buildCmd.Flags().Lookup(crossFlag).NoOptDefVal = "linux"
 	addCommonBuildFlags(buildCmd)
 	return buildCmd
@@ -99,12 +97,8 @@ func (d *dev) build(cmd *cobra.Command, commandLine []string) error {
 	targets, additionalBazelArgs := splitArgsAtDash(cmd, commandLine)
 	ctx := cmd.Context()
 	cross := mustGetFlagString(cmd, crossFlag)
-	skipGenerate := mustGetFlagBool(cmd, skipGenerateFlag)
-	if cross != "" {
-		skipGenerate = true
-	}
 
-	args, buildTargets, err := d.getBasicBuildArgs(ctx, targets, skipGenerate)
+	args, buildTargets, err := d.getBasicBuildArgs(ctx, targets)
 	if err != nil {
 		return err
 	}
@@ -247,17 +241,6 @@ func (d *dev) stageArtifacts(ctx context.Context, targets []buildTarget) error {
 		}
 	}
 
-	shouldHoist := false
-	for _, target := range targets {
-		if target.fullName == "//:go_path" {
-			shouldHoist = true
-		}
-	}
-	if shouldHoist {
-		if err := d.hoistGeneratedCode(ctx, workspace, bazelBin); err != nil {
-			return err
-		}
-	}
 	return nil
 }
 
@@ -278,7 +261,7 @@ func targetToBinBasename(target string) string {
 // `CommandContext`), and the second is the full list of targets to be built
 // (e.g. after translation, so short -> "//pkg/cmd/cockroach-short").
 func (d *dev) getBasicBuildArgs(
-	ctx context.Context, targets []string, skipGenerate bool,
+	ctx context.Context, targets []string,
 ) (args []string, buildTargets []buildTarget, _ error) {
 	if len(targets) == 0 {
 		// Default to building the cockroach binary.
@@ -332,98 +315,12 @@ func (d *dev) getBasicBuildArgs(
 			break
 		}
 	}
-	shouldSkipGenerate := true
-	for _, target := range buildTargets {
-		if strings.Contains(target.fullName, "//pkg/cmd/cockroach") {
-			shouldSkipGenerate = false
-			break
-		}
-	}
-	if shouldSkipGenerate {
-		skipGenerate = true
-	}
-	// If we're hoisting generated code, we also want to build //:go_path.
-	if !skipGenerate {
-		args = append(args, "//:go_path")
-		buildTargets = append(buildTargets, buildTarget{fullName: "//:go_path"})
-	}
 	if shouldBuildWithTestConfig {
 		args = append(args, "--config=test")
 	}
-
 	return args, buildTargets, nil
 }
 
-// Hoist generated code out of the sandbox and into the workspace.
-// Note that you must build //:go_path before building this.
-func (d *dev) hoistGeneratedCode(ctx context.Context, workspace string, bazelBin string) error {
-	// Clean up ignored .go files. Do this by listing all the
-	// ignored files and filtering out irrelevant ones.
-	// We do this to get rid of stale generated files that might
-	// confuse IDE's, especially if you switch between branches that
-	// have different generated code.
-	lines, err := d.exec.CommandContextSilent(ctx, "git", "status", "--ignored", "--short", filepath.Join(workspace, "pkg"))
-	if err != nil {
-		return err
-	}
-	for _, line := range strings.Split(string(lines), "\n") {
-		line = strings.TrimSpace(line)
-		if line == "" || !strings.HasPrefix(line, "!! ") {
-			continue
-		}
-		filename := strings.TrimPrefix(line, "!! ")
-		if !strings.HasSuffix(filename, ".go") || strings.Contains(filename, "zcgo_flags") {
-			continue
-		}
-		if err := d.os.Remove(filename); err != nil {
-			return err
-		}
-	}
-	// Enumerate generated .go files in the sandbox so we can hoist
-	// them out.
-	cockroachDir := filepath.Join(bazelBin, "go_path", "src", "github.com", "cockroachdb", "cockroach")
-	goFiles, err := d.os.ListFilesWithSuffix(cockroachDir, ".go")
-	if err != nil {
-		return err
-	}
-	fileContents, err := d.os.ReadFile(filepath.Join(workspace, "build/bazelutil/checked_in_genfiles.txt"))
-	if err != nil {
-		return err
-	}
-	renameMap := make(map[string]string)
-	for _, line := range strings.Split(fileContents, "\n") {
-		line = strings.TrimSpace(line)
-		if len(line) == 0 || strings.HasPrefix(line, "#") {
-			continue
-		}
-		components := strings.Split(line, "|")
-		target := components[0]
-		dir := strings.Split(strings.TrimPrefix(target, "//"), ":")[0]
-		oldBasename := components[1]
-		newBasename := components[2]
-		renameMap[filepath.Join(dir, oldBasename)] = filepath.Join(dir, newBasename)
-	}
-
-	for _, file := range goFiles {
-		// First case: generated Go code that's checked into tree.
-		relPath := strings.TrimPrefix(file, cockroachDir+"/")
-		dst, ok := renameMap[relPath]
-		if ok {
-			err := d.os.CopyFile(file, filepath.Join(workspace, dst))
-			if err != nil {
-				return err
-			}
-			continue
-		}
-		// Otherwise, just copy the file to the same place in the workspace.
-		err := d.os.CopyFile(file, filepath.Join(workspace, relPath))
-		if err != nil {
-			return err
-		}
-	}
-	return nil
-}
-
 // Given a list of Bazel arguments, find the ones starting with --config= and
 // return them.
 func getConfigArgs(args []string) (ret []string) {