overhaul how we load module info to format Go files

gofmt.go

@@ -23,6 +23,7 @@ import (
 	"runtime"
 	"runtime/pprof"
 	"strings"
+	"sync"
 
 	"golang.org/x/sync/semaphore"
 	exec "golang.org/x/sys/execabs"
@@ -287,21 +288,18 @@ func processFile(filename string, info fs.FileInfo, in io.Reader, r *reporter, e
 	// Apply gofumpt's changes before we print the code in gofumpt's format.
 
 	// If either -lang or -modpath aren't set, fetch them from go.mod.
-	if *langVersion == "" || *modulePath == "" {
-		out, err := exec.Command("go", "mod", "edit", "-json").Output()
-		if err == nil && len(out) > 0 {
-			var mod struct {
-				Go     string
-				Module struct {
-					Path string
-				}
-			}
-			_ = json.Unmarshal(out, &mod)
-			if *langVersion == "" {
-				*langVersion = mod.Go
+	lang := *langVersion
+	modpath := *modulePath
+	if lang == "" || modpath == "" {
+		dir := filepath.Dir(filename)
+		mod, ok := moduleCacheByDir.Load(dir)
+		if ok && mod != nil {
+			mod := mod.(*module)
+			if lang == "" {
+				lang = mod.Go
 			}
-			if *modulePath == "" {
-				*modulePath = mod.Module.Path
+			if modpath == "" {
+				modpath = mod.Module.Path
 			}
 		}
 	}
@@ -311,8 +309,8 @@ func processFile(filename string, info fs.FileInfo, in io.Reader, r *reporter, e
 	// We also skip walking vendor directories entirely, but that happens elsewhere.
 	if explicit || !isGenerated(file) {
 		gformat.File(fileSet, file, gformat.Options{
-			LangVersion: *langVersion,
-			ModulePath:  *modulePath,
+			LangVersion: lang,
+			ModulePath:  modpath,
 			ExtraRules:  *extraRules,
 		})
 	}
@@ -532,7 +530,47 @@ func gofmtMain(s *sequencer) {
 	}
 }
 
+type module struct {
+	Go     string
+	Module struct {
+		Path string
+	}
+}
+
+func loadModuleInfo(dir string) interface{} {
+	cmd := exec.Command("go", "mod", "edit", "-json")
+	cmd.Dir = dir
+
+	// Spawning "go mod edit" will open files by design,
+	// such as the named pipe to obtain stdout.
+	// TODO(mvdan): if we run into "too many open files" errors again in the
+	// future, we probably need to turn fdSem into a weighted semaphore so this
+	// operation can acquire a weight larger than 1.
+	fdSem <- true
+	out, err := cmd.Output()
+	defer func() { <-fdSem }()
+
+	if err != nil || len(out) == 0 {
+		return nil
+	}
+	mod := new(module)
+	if err := json.Unmarshal(out, mod); err != nil {
+		return nil
+	}
+	return mod
+}
+
+// Written to by fileWeight, read from fileWeight and processFile.
+// A present but nil value means that loading the module info failed.
+// Note that we don't require the keys to be absolute directories,
+// so duplicates are possible. The same can happen with symlinks.
+var moduleCacheByDir sync.Map // map[dirString]*module
+
 func fileWeight(path string, info fs.FileInfo) int64 {
+	dir := filepath.Dir(path)
+	if _, ok := moduleCacheByDir.Load(dir); !ok {
+		moduleCacheByDir.Store(dir, loadModuleInfo(dir))
+	}
 	if info == nil {
 		return exclusive
 	}

testdata/scripts/octal-literals.txt

@@ -1,31 +1,33 @@
-cd module
-cp foo.go foo.go.orig
-
 # Initially, the Go language version is too low.
-gofumpt foo.go
-cmp stdout foo.go.orig
+gofumpt -l .
+! stdout .
 
 # We can give an explicitly newer version.
-gofumpt -lang=1.13 foo.go
-cmp stdout foo.go.golden
+gofumpt -lang=1.13 -l .
+stdout -count=1 'foo\.go'
+stdout -count=1 'nested[/\\]nested\.go'
 
 # If we bump the version in go.mod, it should be picked up.
 exec go mod edit -go=1.13
+gofumpt -l .
+stdout -count=1 'foo\.go'
+! stdout 'nested'
+
+# Ensure we produce the output we expect, and that it's stable.
 gofumpt foo.go
 cmp stdout foo.go.golden
-
 gofumpt -d foo.go.golden
 ! stdout .
 
-# We can give an explicitly older version.
-gofumpt -lang=v1 foo.go
-cmp stdout foo.go.orig
+# We can give an explicitly older version, too
+gofumpt -lang=v1 -l .
+! stdout .
 
--- module/go.mod --
+-- go.mod --
 module test
 
 go 1.12
--- module/foo.go --
+-- foo.go --
 package p
 
 const (
@@ -34,7 +36,7 @@ const (
 	k = 0o_7_5_5
 	l = 1022
 )
--- module/foo.go.golden --
+-- foo.go.golden --
 package p
 
 const (
@@ -43,3 +45,16 @@ const (
 	k = 0o_7_5_5
 	l = 1022
 )
+-- nested/go.mod --
+module nested
+
+go 1.11
+-- nested/nested.go --
+package p
+
+const (
+	i = 0
+	j = 022
+	k = 0o_7_5_5
+	l = 1022
+)

testdata/scripts/workspaces.txt

@@ -1,22 +1,16 @@
 [!go1.18] skip
 
-# Octal literal prefixes were introduced in Go 1.13. If we are outside of the
-# module, language version should not be set.
-gofumpt a/go112
-cmp stdout a/go112
+# Whether we run gofumpt from inside or outside a module,
+# we should always use the information from its go.mod.
+# We also test that we don't get confused by the presence of go.work.
 
-cd a
-gofumpt go112
-cmp stdout go113
-
--- a/go112 --
-package main
+gofumpt a/go112.go
+cmp stdout a/go113.go
 
-const x = 0777
--- a/go113 --
-package main
+cd a
+gofumpt go112.go
+cmp stdout go113.go
 
-const x = 0o777
 -- go.work --
 go 1.18
 use ./a
@@ -26,6 +20,14 @@ module a
 go 1.18
 -- a/a.go --
 package a
+-- a/go112.go --
+package main
+
+const x = 0777
+-- a/go113.go --
+package main
+
+const x = 0o777
 -- b/go.mod --
 module b
 go 1.18

ulimit_unix_test.go

@@ -0,0 +1,94 @@
+// Copyright (c) 2019, Daniel Martí <[email protected]>
+// See LICENSE for licensing information
+
+// TODO: replace with the unix build tag once we require Go 1.19 or later
+//go:build linux
+
+package main
+
+import (
+	"bytes"
+	"fmt"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"strconv"
+	"testing"
+
+	qt "github.com/frankban/quicktest"
+	"golang.org/x/sys/unix"
+)
+
+func init() {
+	// Here rather than in TestMain, to reuse the unix build tag.
+	if limit := os.Getenv("TEST_WITH_FILE_LIMIT"); limit != "" {
+		n, err := strconv.ParseUint(limit, 10, 64)
+		if err != nil {
+			panic(err)
+		}
+		rlimit := unix.Rlimit{Cur: n, Max: n}
+		if err := unix.Setrlimit(unix.RLIMIT_NOFILE, &rlimit); err != nil {
+			panic(err)
+		}
+		os.Exit(main1())
+	}
+}
+
+func TestWithLowOpenFileLimit(t *testing.T) {
+	// Safe to run in parallel, as we only change the limit for child processes.
+	t.Parallel()
+
+	tempDir := t.TempDir()
+	testBinary, err := os.Executable()
+	qt.Assert(t, err, qt.IsNil)
+
+	const (
+		// Enough directories to run into the ulimit.
+		// Enough number of files in total to run into the ulimit.
+		numberDirs        = 500
+		numberFilesPerDir = 20
+		numberFilesTotal  = numberDirs * numberFilesPerDir
+	)
+	t.Logf("writing %d tiny Go files", numberFilesTotal)
+	var allGoFiles []string
+	for i := 0; i < numberDirs; i++ {
+		// Prefix "p", so the package name is a valid identifier.
+		// Add one go.mod file per directory as well,
+		// which will help catch data races when loading module info.
+		dirName := fmt.Sprintf("p%03d", i)
+		dirPath := filepath.Join(tempDir, dirName)
+		err := os.MkdirAll(dirPath, 0o777)
+		qt.Assert(t, err, qt.IsNil)
+
+		err = os.WriteFile(filepath.Join(dirPath, "go.mod"),
+			[]byte(fmt.Sprintf("module %s\n\ngo 1.16", dirName)), 0o666)
+		qt.Assert(t, err, qt.IsNil)
+
+		for j := 0; j < numberFilesPerDir; j++ {
+			filePath := filepath.Join(dirPath, fmt.Sprintf("%03d.go", j))
+			err := os.WriteFile(filePath,
+				// Extra newlines so that "-l" prints all paths.
+				[]byte(fmt.Sprintf("package %s\n\n\n", dirName)), 0o666)
+			qt.Assert(t, err, qt.IsNil)
+			allGoFiles = append(allGoFiles, filePath)
+		}
+	}
+	if len(allGoFiles) != numberFilesTotal {
+		panic("allGoFiles doesn't have the expected number of files?")
+	}
+	runGofmt := func(paths ...string) {
+		t.Logf("running with %d paths", len(paths))
+		cmd := exec.Command(testBinary, append([]string{"-l"}, paths...)...)
+		// 256 is a relatively common low limit, e.g. on Mac.
+		cmd.Env = append(os.Environ(), "TEST_WITH_FILE_LIMIT=256")
+		out, err := cmd.Output()
+		var stderr []byte
+		if err, _ := err.(*exec.ExitError); err != nil {
+			stderr = err.Stderr
+		}
+		qt.Assert(t, err, qt.IsNil, qt.Commentf("stderr:\n%s", stderr))
+		qt.Assert(t, bytes.Count(out, []byte("\n")), qt.Equals, len(allGoFiles))
+	}
+	runGofmt(tempDir)
+	runGofmt(allGoFiles...)
+}