Skip to content

Commit

Permalink
testdriver: fix data race introduced in #1101
Browse files Browse the repository at this point in the history
With #1101 a data race was accidentally introduced where the global
testsetup.VimCommand was overwritten.
We now make a local copy that is modified instead.

Fixing this revealed that #1101 wasn't enough. Testscript conditions
that matches a vim version also used the testscript subcommand instead
of real vim. It now uses the real vim.

Closes #1104
  • Loading branch information
leitzler committed Sep 13, 2021
1 parent 0bfdc4c commit 9265905
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 7 deletions.
23 changes: 19 additions & 4 deletions testdriver/testdriver.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,8 @@ func NewTestDriver(c *Config) (*TestDriver, error) {
"GOVIMTESTDRIVER_SOCKET="+res.driverListener.Addr().String(),
)

vimCmd := cmd
vimCmd := make([]string, len(cmd))
copy(vimCmd, cmd)
if e := os.Getenv("VIM_COMMAND"); e != "" {
vimCmd = strings.Fields(e)
}
Expand Down Expand Up @@ -249,6 +250,7 @@ func NewTestDriver(c *Config) (*TestDriver, error) {
func (d *TestDriver) Logf(format string, a ...interface{}) {
fmt.Fprintf(d.log, format+"\n", a...)
}

func (d *TestDriver) LogStripANSI(r io.Reader) {
scanner := bufio.NewScanner(r)
for {
Expand Down Expand Up @@ -588,7 +590,6 @@ func (d *TestDriver) listenDriver() error {
}
return nil
})

if err != nil {
return fmt.Errorf("%v", err)
}
Expand Down Expand Up @@ -933,8 +934,22 @@ func Condition(cond string) (bool, error) {
}

func getVimFlavourVersion(c testsetup.Command) (string, error) {
var allArgs []string
allArgs = append(allArgs, c...)
allArgs := make([]string, len(c))
// testscript prepends PATH with the directory that contain
// subcommands. Since we are going to execute vim here, and also
// also have a subcommand named "vim", we must specify the entire path if
// "vim" is a part of c.
for i := range c {
if c[i] != "vim" {
allArgs[i] = c[i]
continue
}
abs, err := testsetup.RealVimPath()
if err != nil {
return "", fmt.Errorf("failed to find vim binary: %v", err)
}
allArgs[i] = abs
}
allArgs = append(allArgs, "-v", "--cmd", "echo v:versionlong | qall", "--not-a-term")
cmd := exec.Command(allArgs[0], allArgs[1:]...)
out, err := cmd.CombinedOutput()
Expand Down
12 changes: 9 additions & 3 deletions testsetup/testsetup.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,10 @@ func EnvLookupFlavorCommand() (flav govim.Flavor, cmd Command, err error) {
// executable found. Reason being that testscript prepends
// the PATH with the subcommand dir causing a conflict where
// the first vim found is the subcommand.
// If there is only one vim executable it will be returned
// as fallback.
func RealVimPath() (path string, err error) {
var firstFound bool
var firstFound string
for _, dir := range filepath.SplitList(os.Getenv("PATH")) {
path := filepath.Join(dir, "vim")
d, err := os.Stat(path)
Expand All @@ -172,10 +174,14 @@ func RealVimPath() (path string, err error) {
if m := d.Mode(); m.IsDir() || m&0111 == 0 {
continue
}
if firstFound {
if firstFound != "" {
return path, nil
}
firstFound = true
firstFound = path
}
if firstFound != "" {
// fallback
return firstFound, nil
}
return "", fmt.Errorf("not found")
}

0 comments on commit 9265905

Please sign in to comment.