Merge #68398 #68405 #68422 #68424

68398: sql: fix comment reparse error r=rafiss a=e-mbrown Related to: #60722 Comment statement was dealing with reparse issues due to string syntax. This change alters the comment statements format function. Release note: None 68405: dev: munge PATH to remove ccache entries when calling into Bazel r=rickystewart a=rail Previously, if `ccache` is setup on a developer computer and the `ccache` directory was at the head of `PATH`, it lead to compile errors, because `ccache` tried to write to a directory which is outside of bazel's sandbox. This patch detects if a compiler is symlinked to `ccache` and removes the detected directory from `PATH`. Release note: None 68422: sql: fix create database re-parse issue r=rafiss a=e-mbrown Related to: #60722 The create database test were having re-parse issues due to a type error. This change replaces the underscore with an integer. Release note: None 68424: sql: fix show syntax re-parse issue r=rafiss a=e-mbrown Related to: #60722 Show test were having re-parse issues due to a type error. This change replaces the underscore with a string value. Release note: None Co-authored-by: e-mbrown <[email protected]> Co-authored-by: Rail Aliiev <[email protected]>
cockroachdb · Aug 4, 2021 · 0592999 · 0592999
5 parents 1443ced + 0a0cf5d + c747425 + 8da5cd4 + dc13c5b
commit 0592999
Show file tree

Hide file tree

Showing 22 changed files with 532 additions and 28 deletions.
diff --git a/pkg/cmd/dev/builder.go b/pkg/cmd/dev/builder.go
@@ -61,7 +61,7 @@ func (d *dev) ensureDockerVolume(ctx context.Context, volume string) error {
 func (d *dev) getDockerRunArgs(
 	ctx context.Context, volume string, tty bool,
 ) (args []string, err error) {
-	err = ensureBinaryInPath("docker")
+	err = d.ensureBinaryInPath("docker")
 	if err != nil {
 		return
 	}

diff --git a/pkg/cmd/dev/datadriven_test.go b/pkg/cmd/dev/datadriven_test.go
@@ -127,6 +127,7 @@ func TestDatadriven(t *testing.T) {
 			dev := makeDevCmd()
 			dev.exec = devExec
 			dev.os = devOS
+			require.NoError(t, setupPath(dev))
 
 			if !verbose {
 				dev.cli.SetErr(ioutil.Discard)

diff --git a/pkg/cmd/dev/io/exec/exec.go b/pkg/cmd/dev/io/exec/exec.go
@@ -158,6 +158,33 @@ func (e *Exec) CommandContextNoRecord(ctx context.Context, name string, args ...
 	return err
 }
 
+// LookPath wraps around exec.LookPath, which searches for an executable named
+// file in the directories named by the PATH environment variable.
+func (e *Exec) LookPath(path string) (string, error) {
+	command := fmt.Sprintf("which %s", path)
+	e.logger.Print(command)
+
+	var fullPath string
+	if e.Recorder == nil || e.Recorder.Recording() {
+		// Do the real thing.
+		var err error
+		fullPath, err = exec.LookPath(path)
+		if err != nil {
+			return "", err
+		}
+	}
+
+	if e.Recorder == nil {
+		return fullPath, nil
+	}
+
+	if e.Recording() {
+		return fullPath, e.record(command, fullPath)
+	}
+	ret, err := e.replay(command)
+	return ret, err
+}
+
 func (e *Exec) commandContextImpl(
 	ctx context.Context, stdin io.Reader, silent bool, name string, args ...string,
 ) ([]byte, error) {

diff --git a/pkg/cmd/dev/io/os/os.go b/pkg/cmd/dev/io/os/os.go
@@ -153,6 +153,83 @@ func (o *OS) Symlink(to, from string) error {
 	return err
 }
 
+// Getenv wraps around os.Getenv, retrieving the value of the environment
+// variable named by the key.
+func (o OS) Getenv(key string) string {
+	command := fmt.Sprintf("getenv %s", key)
+	o.logger.Print(command)
+
+	var env string
+	if o.Recorder == nil || o.Recorder.Recording() {
+		env = os.Getenv(key)
+	}
+
+	if o.Recorder == nil {
+		return env
+	}
+
+	if o.Recording() {
+		err := o.record(command, env)
+		if err != nil {
+			return ""
+		}
+		return env
+	}
+	ret, _ := o.replay(command)
+	return ret
+}
+
+// Setenv wraps around os.Setenv, which sets the value of the environment
+// variable named by the key. It returns an error, if any.
+func (o *OS) Setenv(key, value string) error {
+	command := fmt.Sprintf("export %s=%s", key, value)
+	o.logger.Print(command)
+
+	if o.Recorder == nil || o.Recorder.Recording() {
+		// Do the real thing.
+		if err := os.Setenv(key, value); err != nil {
+			return err
+		}
+	}
+
+	if o.Recorder == nil {
+		return nil
+	}
+
+	if o.Recording() {
+		return o.record(command, "")
+	}
+	_, err := o.replay(command)
+	return err
+}
+
+// Readlink wraps around os.Readlink, which returns the destination of the named
+// symbolic link. If there is an error, it will be of type *PathError.
+func (o *OS) Readlink(filename string) (string, error) {
+	command := fmt.Sprintf("readlink %s", filename)
+	o.logger.Print(command)
+
+	var resolved string
+	if o.Recorder == nil || o.Recorder.Recording() {
+		// Do the real thing.
+		var err error
+		resolved, err = os.Readlink(filename)
+		if err != nil {
+			return "", err
+		}
+	}
+
+	if o.Recorder == nil {
+		return resolved, nil
+	}
+
+	if o.Recording() {
+		return resolved, o.record(command, resolved)
+	}
+	ret, err := o.replay(command)
+	return ret, err
+}
+
 // ReadFile wraps around ioutil.ReadFile, reading a file from disk and
 // returning the contents.
 func (o *OS) ReadFile(filename string) (string, error) {

diff --git a/pkg/cmd/dev/main.go b/pkg/cmd/dev/main.go
@@ -20,16 +20,17 @@ func main() {
 	log.SetFlags(0)
 	log.SetPrefix("")
 
-	// Disable ccache by setting CCACHE_DISABLE. We don't want it to write to files outside the bazel sandbox.
-	if err := os.Setenv("CCACHE_DISABLE", "1"); err != nil {
-		log.Fatal("Failed to set `CCACHE_DISABLE`")
-	}
 	if _, err := exec.LookPath("bazel"); err != nil {
 		log.Printf("ERROR: bazel not found in $PATH")
 		os.Exit(1)
 	}
 
 	dev := makeDevCmd()
+
+	if err := setupPath(dev); err != nil {
+		log.Fatalf("Failed to setup PATH: %v", err)
+	}
+
 	if err := dev.cli.Execute(); err != nil {
 		log.Printf("ERROR: %v", err)
 		os.Exit(1)

diff --git a/pkg/cmd/dev/testdata/bench.txt b/pkg/cmd/dev/testdata/bench.txt
@@ -1,9 +1,17 @@
 dev bench pkg/util/...
 ----
+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
 git grep -l ^func Benchmark -- pkg/util/*_test.go
 bazel run --color=yes --experimental_convenience_symlinks=ignore --config=test --config=dev //pkg/util:util_test -- -test.bench=.
 bazel run --color=yes --experimental_convenience_symlinks=ignore --config=test --config=dev //pkg/util/uuid:uuid_test -- -test.bench=.
 
 dev bench pkg/sql/parser --filter=BenchmarkParse
 ----
+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 run --color=yes --experimental_convenience_symlinks=ignore --config=test --config=dev //pkg/sql/parser:parser_test -- -test.bench=BenchmarkParse
diff --git a/pkg/cmd/dev/testdata/build.txt b/pkg/cmd/dev/testdata/build.txt
@@ -1,5 +1,9 @@
 dev build cockroach-short
 ----
+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 build --color=yes --experimental_convenience_symlinks=ignore //pkg/cmd/cockroach-short --config=dev
 bazel info workspace --color=no --config=dev
 mkdir go/src/github.com/cockroachdb/cockroach/bin
@@ -9,6 +13,10 @@ ln -s /private/var/tmp/_bazel/99e666e4e674209ecdb66b46371278df/execroot/cockroac
 
 dev build cockroach-short --cpus=12
 ----
+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 build --color=yes --experimental_convenience_symlinks=ignore --local_cpu_resources=12 //pkg/cmd/cockroach-short --config=dev
 bazel info workspace --color=no --config=dev
 mkdir go/src/github.com/cockroachdb/cockroach/bin
@@ -18,6 +26,10 @@ ln -s /private/var/tmp/_bazel/99e666e4e674209ecdb66b46371278df/execroot/cockroac
 
 dev build --debug cockroach-short
 ----
+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 build --color=yes --experimental_convenience_symlinks=ignore //pkg/cmd/cockroach-short --config=dev
 bazel info workspace --color=no --config=dev
 mkdir go/src/github.com/cockroachdb/cockroach/bin
@@ -27,6 +39,10 @@ ln -s /private/var/tmp/_bazel/99e666e4e674209ecdb66b46371278df/execroot/cockroac
 
 dev build cockroach-short --remote-cache 127.0.0.1:9090
 ----
+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 build --color=yes --experimental_convenience_symlinks=ignore --remote_local_fallback --remote_cache=grpc://127.0.0.1:9090 --experimental_remote_downloader=grpc://127.0.0.1:9090 //pkg/cmd/cockroach-short --config=dev
 bazel info workspace --color=no --config=dev
 mkdir go/src/github.com/cockroachdb/cockroach/bin
@@ -36,6 +52,10 @@ ln -s /private/var/tmp/_bazel/99e666e4e674209ecdb66b46371278df/execroot/cockroac
 
 dev build --skip-dev-config cockroach-short
 ----
+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 build --color=yes --experimental_convenience_symlinks=ignore //pkg/cmd/cockroach-short
 bazel info workspace --color=no
 mkdir go/src/github.com/cockroachdb/cockroach/bin

diff --git a/pkg/cmd/dev/testdata/builder.txt b/pkg/cmd/dev/testdata/builder.txt
@@ -1,5 +1,9 @@
 dev builder
 ----
+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
 docker volume inspect bzlcache
 bazel info workspace --color=no --config=dev
 mkdir go/src/github.com/cockroachdb/cockroach/artifacts

diff --git a/pkg/cmd/dev/testdata/generate.txt b/pkg/cmd/dev/testdata/generate.txt
@@ -1,4 +1,8 @@
 dev gen bazel
 ----
+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
 go/src/github.com/cockroachdb/cockroach/build/bazelutil/bazel-generate.sh
diff --git a/pkg/cmd/dev/testdata/recording/bench.txt b/pkg/cmd/dev/testdata/recording/bench.txt
@@ -1,3 +1,18 @@
+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
+----
+
 git grep -l ^func Benchmark -- pkg/util/*_test.go
 ----
 pkg/util/topk_test.go
@@ -11,5 +26,20 @@ bazel run --color=yes --experimental_convenience_symlinks=ignore --config=test -
 bazel run --color=yes --experimental_convenience_symlinks=ignore --config=test --config=dev //pkg/util/uuid:uuid_test -- -test.bench=.
 ----
 
+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 run --color=yes --experimental_convenience_symlinks=ignore --config=test --config=dev //pkg/sql/parser:parser_test -- -test.bench=BenchmarkParse
 ----
diff --git a/pkg/cmd/dev/testdata/recording/build.txt b/pkg/cmd/dev/testdata/recording/build.txt
@@ -1,3 +1,18 @@
+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 build --color=yes --experimental_convenience_symlinks=ignore //pkg/cmd/cockroach-short --config=dev
 ----
 
@@ -18,6 +33,21 @@ 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
 ----
 
+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 build --color=yes --experimental_convenience_symlinks=ignore --local_cpu_resources=12 //pkg/cmd/cockroach-short --config=dev
 ----
 
@@ -38,6 +68,21 @@ 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
 ----
 
+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 build --color=yes --experimental_convenience_symlinks=ignore //pkg/cmd/cockroach-short --config=dev
 ----
 
@@ -58,6 +103,21 @@ 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
 ----
 
+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 build --color=yes --experimental_convenience_symlinks=ignore --remote_local_fallback --remote_cache=grpc://127.0.0.1:9090 --experimental_remote_downloader=grpc://127.0.0.1:9090 //pkg/cmd/cockroach-short --config=dev
 ----
 
@@ -78,6 +138,21 @@ 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
 ----
 
+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 build --color=yes --experimental_convenience_symlinks=ignore //pkg/cmd/cockroach-short
 ----