bazel,dev: add robust support/validation for checked-in generated code

While testing cockroachdb#68663, I noticed that there were a few build targets that
didn't look as I expected since they referenced checked-in generated
code in order to generate that same code in the sandbox -- e.g., the
checked-in `batch_generated.go` was used to generate an (identical) file
`batch_generated-gen.go` within the sandbox. I filed the issue cockroachdb#68654 to
investigate this, and gave a college try to stamping out the recursive
structure by refactoring the affected code. I wasn't successful :(

While looking at this I encountered comments by Irfan in
`pkg/{roachpb/gen,sql/opt/optgen/cmd/langgen}/BUILD.bazel` explaining
that this generated code needs to remain checked-in in order to
bootstrap new versions of the code, and that our tooling needs to have
support for extracting this code back into the workspace so devs can
have a sane workflow. Fair enough, so this PR implements all of that.

* Add `dev generate go` to generate the checked-in generated code;
* fix up `dev generate` so that it generates for `bazel`, `docs`, and
  `go` altogether (so `dev generate` is a one-step thing you can do to
  generate everything you'll need to pass CI);
* build the generated Go code via the sandbox in CI and test to make
  sure it matches what's checked in.

Closes cockroachdb#68654.

Release note: None

build/bazelutil/checked_in_genfiles.txt

@@ -0,0 +1,8 @@
+# These files are generated by the build system, but also must be
+# checked into tree. The syntax here is:
+#   $TARGET|$GENERATED_FILENAME|$FILENAME_TO_RENAME_TO
+# This list is consumed by `dev` as well as in CI for validation.
+# Lines beginning with # are ignored.
+//pkg/roachpb:gen-batch-generated|batch_generated-gen.go|batch_generated.go
+//pkg/sql/opt/optgen/lang:gen-expr|expr-gen.og.go|expr.og.go
+//pkg/sql/opt/optgen/lang:gen-operator|operator-gen.og.go|operator.og.go

build/teamcity/cockroach/ci/builds/build_impl.sh

@@ -10,14 +10,16 @@ fi
 
 CONFIG="$1"
 
-# Only build docs on Linux.
-DOC_TARGETS=
+# Only build generated files on Linux x86_64.
+GENFILES_TARGETS=
 if [ "$CONFIG" == "crosslinux" ]
 then
-   DOC_TARGETS=$(grep '^//' docs/generated/bazel_targets.txt)
+    DOC_TARGETS=$(grep '^//' docs/generated/bazel_targets.txt)
+    GO_TARGETS=$(grep -v '^#' build/bazelutil/checked_in_genfiles.txt | cut -d'|' -f1)
+    GENFILES_TARGETS="$DOC_TARGETS $GO_TARGETS"
 fi
 
 bazel build //pkg/cmd/bazci --config=ci
 $(bazel info bazel-bin)/pkg/cmd/bazci/bazci_/bazci --compilation_mode opt \
 		       --config "$CONFIG" \
-		       build //pkg/cmd/cockroach-short //c-deps:libgeos $DOC_TARGETS
+		       build //pkg/cmd/cockroach-short //c-deps:libgeos $GENFILES_TARGETS

build/teamcity/cockroach/ci/builds/build_linux_x86_64.sh

@@ -47,6 +47,22 @@ do
         FAILED=1
     fi
 done
+# Ensure checked-in generated files are byte-for-byte identical with what's in
+# the checkout.
+for LINE in $(grep -v '^#' $root/build/bazelutil/checked_in_genfiles.txt)
+do
+    target=$(echo $LINE | cut -d'|' -f1)
+    dir=$(echo $target | sed 's|//||g' | cut -d: -f1)
+    old_basename=$(echo $LINE | cut -d'|' -f2)
+    new_basename=$(echo $LINE | cut -d'|' -f3)
+    RESULT=$(diff $root/artifacts/bazel-bin/$dir/$old_basename $root/$dir/$new_basename)
+    if [[ ! $? -eq 0 ]]
+    then
+        echo "Generated file $dir/$new_basename does not match with checked-in version. Got diff:"
+        echo "$RESULT"
+        FAILED=1
+    fi
+done
 
 if [[ ! -z "$FAILED" ]]
 then

pkg/cmd/dev/build.go

@@ -168,7 +168,40 @@ func (d *dev) stageArtifacts(ctx context.Context, targets []string, hoistGenerat
 		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 {
+			// We definitely don't want any code that was put in the sandbox for gomock.
+			if strings.Contains(file, "_gomock_gopath") {
+				continue
+			}
+			// First case: generated Go code that's checked into tree.
+			relPath := strings.TrimPrefix(file, bazelBin+"/")
+			dst, ok := renameMap[relPath]
+			if ok {
+				err := d.os.CopyFile(file, filepath.Join(workspace, dst))
+				if err != nil {
+					return err
+				}
+				continue
+			}
+			// Second case: the pathname contains github.com/cockroachdb/cockroach.
 			const cockroachURL = "github.com/cockroachdb/cockroach/"
 			ind := strings.LastIndex(file, cockroachURL)
 			if ind > 0 {
@@ -182,6 +215,8 @@ func (d *dev) stageArtifacts(ctx context.Context, targets []string, hoistGenerat
 				}
 				continue
 			}
+			// Third case: apply a heuristic to see whether this file makes sense to be
+			// staged.
 			pathComponents := strings.Split(file, string(os.PathSeparator))
 			var skip bool
 			for _, component := range pathComponents[:len(pathComponents)-1] {
@@ -198,7 +233,7 @@ func (d *dev) stageArtifacts(ctx context.Context, targets []string, hoistGenerat
 			}
 			if !skip {
 				// Failures here don't mean much. Just ignore them.
-				_ = d.os.CopyFile(file, filepath.Join(workspace, strings.TrimPrefix(file, bazelBin+"/")))
+				_ = d.os.CopyFile(file, filepath.Join(workspace, relPath))
 			}
 		}
 	}

pkg/cmd/dev/generate.go

@@ -31,6 +31,7 @@ func makeGenerateCmd(runE func(cmd *cobra.Command, args []string) error) *cobra.
         dev generate
         dev generate bazel
         dev generate docs
+        dev generate go
 `,
 		Args: cobra.MinimumNArgs(0),
 		// TODO(irfansharif): Errors but default just eaten up. Let's wrap these
@@ -45,15 +46,18 @@ func (d *dev) generate(cmd *cobra.Command, targets []string) error {
 		"bazel":   d.generateBazel,
 		"docs":    d.generateDocs,
 		"execgen": d.generateUnimplemented,
+		"go":      d.generateGo,
 		"optgen":  d.generateUnimplemented,
 		"proto":   d.generateUnimplemented,
 	}
 
 	if len(targets) == 0 {
-		// Collect all the targets.
-		for target := range generatorTargetMapping {
-			targets = append(targets, target)
-		}
+		// Default: generate everything.
+		// TODO(ricky): This could be implemented more efficiently --
+		// `generate docs` and `generate go` re-do some of the same
+		// work and call into Bazel more often than necessary. Fix that
+		// when people start to complain.
+		targets = append(targets, "bazel", "docs", "go")
 	}
 
 	for _, target := range targets {
@@ -132,6 +136,61 @@ func (d *dev) generateDocs(cmd *cobra.Command) error {
 	return nil
 }
 
+func (d *dev) generateGo(cmd *cobra.Command) error {
+	ctx := cmd.Context()
+	workspace, err := d.getWorkspace(ctx)
+	if err != nil {
+		return err
+	}
+	// List targets we need to build.
+	contents, err := d.os.ReadFile(filepath.Join(workspace, "build/bazelutil/checked_in_genfiles.txt"))
+	if err != nil {
+		return err
+	}
+	var lines []string
+	for _, line := range strings.Split(contents, "\n") {
+		line = strings.TrimSpace(line)
+		if len(line) == 0 || strings.HasPrefix(line, "#") {
+			continue
+		}
+		lines = append(lines, line)
+	}
+	var targets []string
+	for _, line := range lines {
+		targets = append(targets, strings.Split(line, "|")[0])
+	}
+	// Build targets.
+	var args []string
+	args = append(args, "build", "--color=yes", "--experimental_convenience_symlinks=ignore")
+	args = append(args, mustGetRemoteCacheArgs(remoteCacheAddr)...)
+	args = append(args, getConfigFlags()...)
+	args = append(args, targets...)
+	err = d.exec.CommandContextInheritingStdStreams(ctx, "bazel", args...)
+	if err != nil {
+		return err
+	}
+	// Copy from bazel-bin to workspace.
+	bazelBin, err := d.getBazelBin(ctx)
+	if err != nil {
+		return err
+	}
+	for _, line := range lines {
+		components := strings.Split(line, "|")
+		target := components[0]
+		dir := strings.Split(strings.TrimPrefix(target, "//"), ":")[0]
+		oldBasename := components[1]
+		newBasename := components[2]
+		err = d.os.CopyFile(filepath.Join(bazelBin, dir, oldBasename),
+			filepath.Join(workspace, dir, newBasename))
+		if err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 func (*dev) generateUnimplemented(*cobra.Command) error {
-	return errors.New("To generate Go code, run `dev build` with the flag `--hoist-generated-code`")
+	return errors.New("To hoist all generated code into the workspace, run " +
+		"`dev build` with the flag `--hoist-generated-code`; to build the generated Go " +
+		"code needed to pass CI, run `dev generate go`")
 }

pkg/cmd/dev/testdata/build.txt

@@ -76,4 +76,8 @@ bazel info bazel-bin --color=no --config=dev
 rm go/src/github.com/cockroachdb/cockroach/cockroach-short
 ln -s /private/var/tmp/_bazel/99e666e4e674209ecdb66b46371278df/execroot/cockroach/bazel-out/darwin-fastbuild/bin/pkg/cmd/cockroach-short/cockroach-short_/cockroach-short go/src/github.com/cockroachdb/cockroach/cockroach-short
 find /private/var/tmp/_bazel/99e666e4e674209ecdb66b46371278df/execroot/cockroach/bazel-out/darwin-fastbuild/bin/pkg -name *.go
+cat go/src/github.com/cockroachdb/cockroach/build/bazelutil/checked_in_genfiles.txt
 cp /private/var/tmp/_bazel/99e666e4e674209ecdb66b46371278df/execroot/cockroach/bazel-out/darwin-fastbuild/bin/pkg/kv/kvserver/kvserver_go_proto_/github.com/cockroachdb/cockroach/pkg/kv/kvserver/storage_services.pb.go go/src/github.com/cockroachdb/cockroach/pkg/kv/kvserver/storage_services.pb.go
+cp /private/var/tmp/_bazel/99e666e4e674209ecdb66b46371278df/execroot/cockroach/bazel-out/darwin-fastbuild/bin/pkg/roachpb/batch_generated-gen.go go/src/github.com/cockroachdb/cockroach/pkg/roachpb/batch_generated.go
+cp /private/var/tmp/_bazel/99e666e4e674209ecdb66b46371278df/execroot/cockroach/bazel-out/darwin-fastbuild/bin/pkg/sql/opt/optgen/lang/expr-gen.og.go go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/optgen/lang/expr.og.go
+cp /private/var/tmp/_bazel/99e666e4e674209ecdb66b46371278df/execroot/cockroach/bazel-out/darwin-fastbuild/bin/pkg/sql/opt/optgen/lang/operator-gen.og.go go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/optgen/lang/operator.og.go

pkg/cmd/dev/testdata/generate.txt

@@ -24,3 +24,17 @@ cp /private/var/tmp/_bazel/99e666e4e674209ecdb66b46371278df/execroot/cockroach/b
 cp /private/var/tmp/_bazel/99e666e4e674209ecdb66b46371278df/execroot/cockroach/bazel-out/darwin-fastbuild/bin/docs/generated/sql/functions.md go/src/github.com/cockroachdb/cockroach/docs/generated/sql/functions.md
 cp /private/var/tmp/_bazel/99e666e4e674209ecdb66b46371278df/execroot/cockroach/bazel-out/darwin-fastbuild/bin/docs/generated/sql/operators.md go/src/github.com/cockroachdb/cockroach/docs/generated/sql/operators.md
 cp /private/var/tmp/_bazel/99e666e4e674209ecdb66b46371278df/execroot/cockroach/bazel-out/darwin-fastbuild/bin/docs/generated/sql/window_functions.md go/src/github.com/cockroachdb/cockroach/docs/generated/sql/window_functions.md
+
+dev gen go
+----
+getenv PATH
+which cc
+readlink /usr/local/opt/ccache/libexec/cc
+export PATH=/usr/local/opt/make/libexec/gnubin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/go/bin:/Library/Apple/usr/bin
+bazel info workspace --color=no --config=dev
+cat go/src/github.com/cockroachdb/cockroach/build/bazelutil/checked_in_genfiles.txt
+bazel build --color=yes --experimental_convenience_symlinks=ignore --config=dev //pkg/roachpb:gen-batch-generated //pkg/sql/opt/optgen/lang:gen-expr //pkg/sql/opt/optgen/lang:gen-operator
+bazel info bazel-bin --color=no --config=dev
+cp /private/var/tmp/_bazel/99e666e4e674209ecdb66b46371278df/execroot/cockroach/bazel-out/darwin-fastbuild/bin/pkg/roachpb/batch_generated-gen.go go/src/github.com/cockroachdb/cockroach/pkg/roachpb/batch_generated.go
+cp /private/var/tmp/_bazel/99e666e4e674209ecdb66b46371278df/execroot/cockroach/bazel-out/darwin-fastbuild/bin/pkg/sql/opt/optgen/lang/expr-gen.og.go go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/optgen/lang/expr.og.go
+cp /private/var/tmp/_bazel/99e666e4e674209ecdb66b46371278df/execroot/cockroach/bazel-out/darwin-fastbuild/bin/pkg/sql/opt/optgen/lang/operator-gen.og.go go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/optgen/lang/operator.og.go

pkg/cmd/dev/testdata/recording/build.txt

@@ -213,9 +213,32 @@ find /private/var/tmp/_bazel/99e666e4e674209ecdb66b46371278df/execroot/cockroach
 ----
 /private/var/tmp/_bazel/99e666e4e674209ecdb66b46371278df/execroot/cockroach/bazel-out/darwin-fastbuild/bin/_bazel/bin/pkg/cmd/dev/dev_test_/testmain.go
 /private/var/tmp/_bazel/99e666e4e674209ecdb66b46371278df/execroot/cockroach/bazel-out/darwin-fastbuild/bin/pkg/kv/kvclient/rangefeed/mock_rangefeed_gomock_gopath/src/google.golang.org/grpc/preloader.go
+/private/var/tmp/_bazel/99e666e4e674209ecdb66b46371278df/execroot/cockroach/bazel-out/darwin-fastbuild/bin/pkg/kv/kvclient/rangefeed/mock_rangefeed_gomock_gopath/src/github.com/cockroachdb/cockroach/pkg/util/uuid/uuid_wrapper.go
 /private/var/tmp/_bazel/99e666e4e674209ecdb66b46371278df/execroot/cockroach/bazel-out/darwin-fastbuild/bin/pkg/kv/kvserver/kvserver_go_proto_/github.com/cockroachdb/cockroach/pkg/kv/kvserver/storage_services.pb.go
+/private/var/tmp/_bazel/99e666e4e674209ecdb66b46371278df/execroot/cockroach/bazel-out/darwin-fastbuild/bin/pkg/roachpb/batch_generated-gen.go
+/private/var/tmp/_bazel/99e666e4e674209ecdb66b46371278df/execroot/cockroach/bazel-out/darwin-fastbuild/bin/pkg/sql/opt/optgen/lang/expr-gen.og.go
+/private/var/tmp/_bazel/99e666e4e674209ecdb66b46371278df/execroot/cockroach/bazel-out/darwin-fastbuild/bin/pkg/sql/opt/optgen/lang/operator-gen.og.go
+----
+----
+
+cat go/src/github.com/cockroachdb/cockroach/build/bazelutil/checked_in_genfiles.txt
+----
+----
+# Comment
+//pkg/roachpb:gen-batch-generated|batch_generated-gen.go|batch_generated.go
+//pkg/sql/opt/optgen/lang:gen-expr|expr-gen.og.go|expr.og.go
+//pkg/sql/opt/optgen/lang:gen-operator|operator-gen.og.go|operator.og.go
 ----
 ----
 
 cp /private/var/tmp/_bazel/99e666e4e674209ecdb66b46371278df/execroot/cockroach/bazel-out/darwin-fastbuild/bin/pkg/kv/kvserver/kvserver_go_proto_/github.com/cockroachdb/cockroach/pkg/kv/kvserver/storage_services.pb.go go/src/github.com/cockroachdb/cockroach/pkg/kv/kvserver/storage_services.pb.go
 ----
+
+cp /private/var/tmp/_bazel/99e666e4e674209ecdb66b46371278df/execroot/cockroach/bazel-out/darwin-fastbuild/bin/pkg/roachpb/batch_generated-gen.go go/src/github.com/cockroachdb/cockroach/pkg/roachpb/batch_generated.go
+----
+
+cp /private/var/tmp/_bazel/99e666e4e674209ecdb66b46371278df/execroot/cockroach/bazel-out/darwin-fastbuild/bin/pkg/sql/opt/optgen/lang/expr-gen.og.go go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/optgen/lang/expr.og.go
+----
+
+cp /private/var/tmp/_bazel/99e666e4e674209ecdb66b46371278df/execroot/cockroach/bazel-out/darwin-fastbuild/bin/pkg/sql/opt/optgen/lang/operator-gen.og.go go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/optgen/lang/operator.og.go
+----

pkg/cmd/dev/testdata/recording/generate.txt

@@ -124,3 +124,48 @@ cp /private/var/tmp/_bazel/99e666e4e674209ecdb66b46371278df/execroot/cockroach/b
 
 cp /private/var/tmp/_bazel/99e666e4e674209ecdb66b46371278df/execroot/cockroach/bazel-out/darwin-fastbuild/bin/docs/generated/sql/window_functions.md go/src/github.com/cockroachdb/cockroach/docs/generated/sql/window_functions.md
 ----
+
+getenv PATH
+----
+/usr/local/opt/ccache/libexec:/usr/local/opt/make/libexec/gnubin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/go/bin:/Library/Apple/usr/bin
+
+which cc
+----
+/usr/local/opt/ccache/libexec/cc
+
+readlink /usr/local/opt/ccache/libexec/cc
+----
+../bin/ccache
+
+export PATH=/usr/local/opt/make/libexec/gnubin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/go/bin:/Library/Apple/usr/bin
+----
+
+bazel info workspace --color=no --config=dev
+----
+go/src/github.com/cockroachdb/cockroach
+
+cat go/src/github.com/cockroachdb/cockroach/build/bazelutil/checked_in_genfiles.txt
+----
+----
+# Comment
+//pkg/roachpb:gen-batch-generated|batch_generated-gen.go|batch_generated.go
+//pkg/sql/opt/optgen/lang:gen-expr|expr-gen.og.go|expr.og.go
+//pkg/sql/opt/optgen/lang:gen-operator|operator-gen.og.go|operator.og.go
+----
+----
+
+bazel build --color=yes --experimental_convenience_symlinks=ignore --config=dev //pkg/roachpb:gen-batch-generated //pkg/sql/opt/optgen/lang:gen-expr //pkg/sql/opt/optgen/lang:gen-operator
+----
+
+bazel info bazel-bin --color=no --config=dev
+----
+/private/var/tmp/_bazel/99e666e4e674209ecdb66b46371278df/execroot/cockroach/bazel-out/darwin-fastbuild/bin
+
+cp /private/var/tmp/_bazel/99e666e4e674209ecdb66b46371278df/execroot/cockroach/bazel-out/darwin-fastbuild/bin/pkg/roachpb/batch_generated-gen.go go/src/github.com/cockroachdb/cockroach/pkg/roachpb/batch_generated.go
+----
+
+cp /private/var/tmp/_bazel/99e666e4e674209ecdb66b46371278df/execroot/cockroach/bazel-out/darwin-fastbuild/bin/pkg/sql/opt/optgen/lang/expr-gen.og.go go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/optgen/lang/expr.og.go
+----
+
+cp /private/var/tmp/_bazel/99e666e4e674209ecdb66b46371278df/execroot/cockroach/bazel-out/darwin-fastbuild/bin/pkg/sql/opt/optgen/lang/operator-gen.og.go go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/optgen/lang/operator.og.go
+----

pkg/roachpb/gen/BUILD.bazel

@@ -7,19 +7,11 @@ load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library")
 # order to generate newer revisions within the sandbox. For another example of
 # this pattern, see what we do in langgen[4].
 #
-# All dependants of //pkg/roachpb will need to depend on the file generated
+# All dependents of //pkg/roachpb will need to depend on the file generated
 # within the Bazel sandbox, which is achieved by [5]. Conversely, to instruct
 # gazelle/Bazel to resolve gen's import of //pkg/roachpb appropriately, we add
 # the resolve directive below.
 #
-# TODO(irfansharif): We'll eventually want to be able to implant the sandbox
-# generated files back into the source tree. This will be necessary for IDEs,
-# but also for a sane workflow for the engineers working on roachpb. If they
-# were only using Bazel today, roachpb dependent builds would misbehave as we
-# wouldn't be updating the checked-in batch_generated.go file used to build
-# roachpb itself (assuming any diffs there would result in different
-# code/compilation behavior). See #58018.
-#
 # [1]: //pkg/roachpb/gen-batch-generated
 # [2]: //pkg/roachpb:bootstrap
 # [3]: //pkg/roachpb:batch_generated.go

pkg/sql/opt/optgen/cmd/langgen/BUILD.bazel

@@ -6,19 +6,11 @@ load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library")
 # target[2], that sources pre-generated og.go files that are already checked
 # into the source tree. These files are used to compile langgen, which is then
 # used to generate newer revisions of those same generated files. All
-# dependants of [3] will need to depend on the og.go files generated in the
+# dependents of [3] will need to depend on the og.go files generated in the
 # Bazel sandbox, which is achieved by [4]. Conversely, to instruct
 # gazelle/Bazel to resolve langgen's import of [3] appropriately, we add the
 # resolve directive below.
 #
-# TODO(irfansharif): We'll eventually want to be able to implant the sandbox
-# generated files back into the source tree. This will be necessary for IDEs,
-# but also for a sane workflow for the engineers working on langgen. If they
-# were only using Bazel today, langgen dependent builds would misbehave as we
-# wouldn't be updating the checked-in og.go files used to build langgen itself
-# (assuming any diffs there would result in different code/compilation
-# behavior). See https://github.com/cockroachdb/cockroach/issues/58018.
-#
 # [1]: https://en.wikipedia.org/wiki/Bootstrapping_(compilers)
 # [2]: //pkg/sql/opt/optgen/lang:bootstrap
 # [3]: github.com/cockroachdb/cockroach/pkg/sql/opt/optgen/lang