Skip to content

Commit

Permalink
dev: have doctor advise to set a particular tmpdir
Browse files Browse the repository at this point in the history
Bazel's default behavior of rooting the `tmpdir` to an "in-sandbox"
directory has been a point of confusion for CRL developers. The sandbox
directory does not exist after the test is run (unless `--sandbox_debug`
is provided), which is sometimes confusing for folks who expect their
test's temp files to be present where the logs suggest they should be
(see cockroachdb#82413). Furthermore, the long `tmpdir` used in these cases breaks
tests that create Unix sockets on OS's where Unix sockets have a maximum
path length.

Avoid these problems by having `doctor` just tell you to manually set a
`test_tmpdir`. We add `/tmp` to `gitignore` in case people want to root
it at the `tmp` directory in their checkout.

Closes cockroachdb#72918.
Closes cockroachdb#82413.

Release note: None
  • Loading branch information
rickystewart committed Jun 6, 2022
1 parent 2fc45fc commit d324f2b
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 2 deletions.
1 change: 1 addition & 0 deletions .gitignore
Expand Up @@ -14,6 +14,7 @@ artifacts
/bin.*
/lib
/lib.*
/tmp
.buildinfo
# cockroach-data, cockroach{,.race}-{darwin,linux,windows}-*
/cockroach*
Expand Down
2 changes: 1 addition & 1 deletion dev
Expand Up @@ -3,7 +3,7 @@
set -euo pipefail

# Bump this counter to force rebuilding `dev` on all machines.
DEV_VERSION=34
DEV_VERSION=35

THIS_DIR=$(cd "$(dirname "$0")" && pwd)
BINARY_DIR=$THIS_DIR/bin/dev-versions
Expand Down
27 changes: 26 additions & 1 deletion pkg/cmd/dev/doctor.go
Expand Up @@ -32,7 +32,7 @@ const (
// doctorStatusVersion is the current "version" of the status checks performed
// by `dev doctor``. Increasing it will force doctor to be re-run before other
// dev commands can be run.
doctorStatusVersion = 4
doctorStatusVersion = 5

noCacheFlag = "no-cache"
)
Expand Down Expand Up @@ -287,6 +287,15 @@ slightly slower and introduce a noticeable delay in first-time build setup.`
log.Println(failedNogoTestMsg)
}

// Check whether the user has configured a custom tmpdir.
present := d.checkLinePresenceInBazelRcUser(workspace, "test --test_tmpdir=")
if !present {
failures = append(failures, "You haven't configured a tmpdir for your tests.\n"+
"Please add a `test --test_tmpdir=/PATH/TO/TMPDIR` line to your .bazelrc.user:\n"+
fmt.Sprintf(" echo \"test --test_tmpdir=%s\" >> .bazelrc.user\n", filepath.Join(workspace, "tmp"))+
"(You can choose any directory as a tmpdir.)")
}

// We want to make sure there are no other failures before trying to
// set up the cache.
if !noCache && len(failures) == 0 {
Expand Down Expand Up @@ -359,3 +368,19 @@ func (d *dev) checkPresenceInBazelRc(expectedBazelRcLine string) (string, error)
}
return errString, nil
}

// checkSubstringPresenceInBazelRcUser checks whether the .bazelrc.user file
// contains a line starting with the given prefix. Returns true iff a matching
// line is in the file. Failures to find the file are ignored.
func (d *dev) checkLinePresenceInBazelRcUser(workspace, expectedSubstr string) bool {
contents, err := d.os.ReadFile(filepath.Join(workspace, ".bazelrc.user"))
if err != nil {
return false
}
for _, line := range strings.Split(contents, "\n") {
if strings.HasPrefix(line, expectedSubstr) {
return true
}
}
return false
}

0 comments on commit d324f2b

Please sign in to comment.