Make fs.Globber take in an iofs.FS (#2953)

* Make fs.Globber take in an iofs.FS

* Lint

src/build/BUILD

@@ -48,6 +48,7 @@ go_test(
         "///third_party/go/github.com_stretchr_testify//require",
         "//src/cli/logging",
         "//src/core",
+        "//src/fs",
         "//src/output",
         "//src/plz",
     ],

src/build/build_step.go

@@ -318,7 +318,7 @@ func buildTarget(state *core.BuildState, target *core.BuildTarget, runRemotely b
 
 		// Add optional outputs to target metadata
 		metadata.OptionalOutputs = make([]string, 0)
-		for _, output := range fs.Glob(state.Config.Parse.BuildFileName, target.TmpDir(), target.OptionalOutputs, nil, true) {
+		for _, output := range fs.Glob(fs.HostFS, state.Config.Parse.BuildFileName, target.TmpDir(), target.OptionalOutputs, nil, true) {
 			log.Debug("Add discovered optional output to metadata %s", output)
 			metadata.OptionalOutputs = append(metadata.OptionalOutputs, output)
 		}
@@ -709,7 +709,7 @@ func moveOutputs(state *core.BuildState, target *core.BuildTarget) ([]string, bo
 	}
 	// Optional outputs get moved but don't contribute to the hash or for incrementality.
 	// Glob patterns are supported on these.
-	for _, output := range fs.Glob(state.Config.Parse.BuildFileName, tmpDir, target.OptionalOutputs, nil, true) {
+	for _, output := range fs.Glob(fs.HostFS, state.Config.Parse.BuildFileName, tmpDir, target.OptionalOutputs, nil, true) {
 		log.Debug("Discovered optional output %s", output)
 		tmpOutput := filepath.Join(tmpDir, output)
 		realOutput := filepath.Join(outDir, output)

src/build/build_step_stress_test.go

@@ -12,6 +12,7 @@ import (
 
 	"github.com/thought-machine/please/src/cli/logging"
 	"github.com/thought-machine/please/src/core"
+	"github.com/thought-machine/please/src/fs"
 	"github.com/thought-machine/please/src/plz"
 )
 
@@ -22,7 +23,7 @@ const size = 1000
 var state *core.BuildState
 
 func TestBuildLotsOfTargets(t *testing.T) {
-	config, _ := core.ReadConfigFiles(core.HostFS(), nil, nil)
+	config, _ := core.ReadConfigFiles(fs.HostFS, nil, nil)
 	config.Please.NumThreads = 10
 	state = core.NewBuildState(config)
 	state.Parser = &fakeParser{

src/build/build_step_test.go

@@ -519,7 +519,7 @@ func TestSha1SingleHash(t *testing.T) {
 }
 
 func newStateWithHashCheckers(label, hashFunction string, hashCheckers ...string) (*core.BuildState, *core.BuildTarget) {
-	config, _ := core.ReadConfigFiles(core.HostFS(), nil, nil)
+	config, _ := core.ReadConfigFiles(fs.HostFS, nil, nil)
 	if hashFunction != "" {
 		config.Build.HashFunction = hashFunction
 	}
@@ -538,7 +538,7 @@ func newStateWithHashCheckers(label, hashFunction string, hashCheckers ...string
 }
 
 func newStateWithHashFunc(label, hashFunc string) (*core.BuildState, *core.BuildTarget) {
-	config, _ := core.ReadConfigFiles(core.HostFS(), nil, nil)
+	config, _ := core.ReadConfigFiles(fs.HostFS, nil, nil)
 	config.Build.HashFunction = hashFunc
 	state := core.NewBuildState(config)
 	state.Config.Parse.BuildFileName = []string{"BUILD_FILE"}
@@ -552,7 +552,7 @@ func newStateWithHashFunc(label, hashFunc string) (*core.BuildState, *core.Build
 }
 
 func newState(label string) (*core.BuildState, *core.BuildTarget) {
-	config, _ := core.ReadConfigFiles(core.HostFS(), nil, nil)
+	config, _ := core.ReadConfigFiles(fs.HostFS, nil, nil)
 	state := core.NewBuildState(config)
 	state.Config.Parse.BuildFileName = []string{"BUILD_FILE"}
 	target := core.NewBuildTarget(core.ParseBuildLabel(label, ""))

src/core/config_test.go

@@ -13,11 +13,12 @@ import (
 	"github.com/thought-machine/go-flags"
 
 	"github.com/thought-machine/please/src/cli"
+	"github.com/thought-machine/please/src/fs"
 )
 
 func TestPlzConfigWorking(t *testing.T) {
 	RepoRoot = "/repo/root"
-	config, err := ReadConfigFiles(HostFS(), []string{"src/core/test_data/working.plzconfig"}, nil)
+	config, err := ReadConfigFiles(fs.HostFS, []string{"src/core/test_data/working.plzconfig"}, nil)
 
 	assert.NoError(t, err)
 	assert.Equal(t, "pexmabob", config.Python.PexTool)
@@ -31,12 +32,12 @@ func TestPlzConfigWorking(t *testing.T) {
 }
 
 func TestPlzConfigFailing(t *testing.T) {
-	_, err := ReadConfigFiles(HostFS(), []string{"src/core/test_data/failing.plzconfig"}, nil)
+	_, err := ReadConfigFiles(fs.HostFS, []string{"src/core/test_data/failing.plzconfig"}, nil)
 	assert.Error(t, err)
 }
 
 func TestPlzConfigProfile(t *testing.T) {
-	config, err := ReadConfigFiles(HostFS(), []string{"src/core/test_data/working.plzconfig"}, []string{"dev"})
+	config, err := ReadConfigFiles(fs.HostFS, []string{"src/core/test_data/working.plzconfig"}, []string{"dev"})
 	assert.NoError(t, err)
 	assert.Equal(t, "pexmabob", config.Python.PexTool)
 	assert.Equal(t, "/opt/java/bin/javac", config.Java.JavacTool)
@@ -46,7 +47,7 @@ func TestPlzConfigProfile(t *testing.T) {
 }
 
 func TestMultiplePlzConfigFiles(t *testing.T) {
-	config, err := ReadConfigFiles(HostFS(), []string{
+	config, err := ReadConfigFiles(fs.HostFS, []string{
 		"src/core/test_data/working.plzconfig",
 		"src/core/test_data/failing.plzconfig",
 	}, nil)
@@ -56,7 +57,7 @@ func TestMultiplePlzConfigFiles(t *testing.T) {
 }
 
 func TestConfigSlicesOverwrite(t *testing.T) {
-	config, err := ReadConfigFiles(HostFS(), []string{"src/core/test_data/slices.plzconfig"}, nil)
+	config, err := ReadConfigFiles(fs.HostFS, []string{"src/core/test_data/slices.plzconfig"}, nil)
 	assert.NoError(t, err)
 	// This should be completely overwritten by the config file
 	assert.Equal(t, []string{"/sbin"}, config.Build.Path)
@@ -175,29 +176,29 @@ func TestPleaseTildeLocationOverride(t *testing.T) {
 }
 
 func TestReadSemver(t *testing.T) {
-	config, err := ReadConfigFiles(HostFS(), []string{"src/core/test_data/version_good.plzconfig"}, nil)
+	config, err := ReadConfigFiles(fs.HostFS, []string{"src/core/test_data/version_good.plzconfig"}, nil)
 	assert.NoError(t, err)
 	assert.EqualValues(t, 2, config.Please.Version.Major)
 	assert.EqualValues(t, 3, config.Please.Version.Minor)
 	assert.EqualValues(t, 4, config.Please.Version.Patch)
-	_, err = ReadConfigFiles(HostFS(), []string{"src/core/test_data/version_bad.plzconfig"}, nil)
+	_, err = ReadConfigFiles(fs.HostFS, []string{"src/core/test_data/version_bad.plzconfig"}, nil)
 	assert.Error(t, err)
 }
 
 func TestReadDurations(t *testing.T) {
-	config, err := ReadConfigFiles(HostFS(), []string{"src/core/test_data/duration_good.plzconfig"}, nil)
+	config, err := ReadConfigFiles(fs.HostFS, []string{"src/core/test_data/duration_good.plzconfig"}, nil)
 	assert.NoError(t, err)
 	assert.EqualValues(t, 500*time.Millisecond, config.Build.Timeout)
 	assert.EqualValues(t, 5*time.Second, config.Test.Timeout)
-	_, err = ReadConfigFiles(HostFS(), []string{"src/core/test_data/duration_bad.plzconfig"}, nil)
+	_, err = ReadConfigFiles(fs.HostFS, []string{"src/core/test_data/duration_bad.plzconfig"}, nil)
 	assert.Error(t, err)
 }
 
 func TestReadByteSizes(t *testing.T) {
-	config, err := ReadConfigFiles(HostFS(), []string{"src/core/test_data/bytesize_good.plzconfig"}, nil)
+	config, err := ReadConfigFiles(fs.HostFS, []string{"src/core/test_data/bytesize_good.plzconfig"}, nil)
 	assert.NoError(t, err)
 	assert.EqualValues(t, 500*1000*1000, config.Cache.DirCacheHighWaterMark)
-	_, err = ReadConfigFiles(HostFS(), []string{"src/core/test_data/bytesize_bad.plzconfig"}, nil)
+	_, err = ReadConfigFiles(fs.HostFS, []string{"src/core/test_data/bytesize_bad.plzconfig"}, nil)
 	assert.Error(t, err)
 }
 
@@ -210,29 +211,29 @@ func TestCompletions(t *testing.T) {
 }
 
 func TestConfigVerifiesOptions(t *testing.T) {
-	config, err := ReadConfigFiles(HostFS(), []string{"src/core/test_data/testrunner_good.plzconfig"}, nil)
+	config, err := ReadConfigFiles(fs.HostFS, []string{"src/core/test_data/testrunner_good.plzconfig"}, nil)
 	assert.NoError(t, err)
 	assert.Equal(t, "pytest", config.Python.TestRunner)
-	_, err = ReadConfigFiles(HostFS(), []string{"src/core/test_data/testrunner_bad.plzconfig"}, nil)
+	_, err = ReadConfigFiles(fs.HostFS, []string{"src/core/test_data/testrunner_bad.plzconfig"}, nil)
 	assert.Error(t, err)
 }
 
 func TestDefaultHashCheckers(t *testing.T) {
-	config, err := ReadConfigFiles(HostFS(), nil, nil)
+	config, err := ReadConfigFiles(fs.HostFS, nil, nil)
 
 	assert.NoError(t, err)
 	assert.ElementsMatch(t, []string{"sha1", "sha256", "blake3"}, config.Build.HashCheckers)
 }
 
 func TestHashCheckersConfig(t *testing.T) {
-	config, err := ReadConfigFiles(HostFS(), []string{"src/core/test_data/hashcheckers.plzconfig"}, nil)
+	config, err := ReadConfigFiles(fs.HostFS, []string{"src/core/test_data/hashcheckers.plzconfig"}, nil)
 
 	assert.NoError(t, err)
 	assert.ElementsMatch(t, []string{"blake3"}, config.Build.HashCheckers)
 }
 
 func TestOverrideHashCheckersConfig(t *testing.T) {
-	config, err := ReadConfigFiles(HostFS(), []string{"src/core/test_data/hashcheckers.plzconfig"}, nil)
+	config, err := ReadConfigFiles(fs.HostFS, []string{"src/core/test_data/hashcheckers.plzconfig"}, nil)
 	assert.NoError(t, err)
 
 	err = config.ApplyOverrides(map[string]string{"build.hashcheckers": "sha256"})
@@ -241,7 +242,7 @@ func TestOverrideHashCheckersConfig(t *testing.T) {
 }
 
 func TestOverrideHashCheckersNoConfig(t *testing.T) {
-	config, err := ReadConfigFiles(HostFS(), nil, nil)
+	config, err := ReadConfigFiles(fs.HostFS, nil, nil)
 	assert.NoError(t, err)
 
 	err = config.ApplyOverrides(map[string]string{"build.hashcheckers": "sha1,blake3"})
@@ -250,15 +251,15 @@ func TestOverrideHashCheckersNoConfig(t *testing.T) {
 }
 
 func TestUnknownHashChecker(t *testing.T) {
-	config, err := ReadConfigFiles(HostFS(), nil, nil)
+	config, err := ReadConfigFiles(fs.HostFS, nil, nil)
 	assert.NoError(t, err)
 
 	err = config.ApplyOverrides(map[string]string{"build.hashcheckers": "fake-algo"})
 	assert.Error(t, err)
 }
 
 func TestBuildEnvSection(t *testing.T) {
-	config, err := ReadConfigFiles(HostFS(), []string{"src/core/test_data/buildenv.plzconfig"}, nil)
+	config, err := ReadConfigFiles(fs.HostFS, []string{"src/core/test_data/buildenv.plzconfig"}, nil)
 	assert.NoError(t, err)
 	expected := []string{
 		"BAR_BAR=first",
@@ -271,7 +272,7 @@ func TestBuildEnvSection(t *testing.T) {
 func TestPassEnv(t *testing.T) {
 	t.Setenv("FOO", "first")
 	t.Setenv("BAR", "second")
-	config, err := ReadConfigFiles(HostFS(), []string{"src/core/test_data/passenv.plzconfig"}, nil)
+	config, err := ReadConfigFiles(fs.HostFS, []string{"src/core/test_data/passenv.plzconfig"}, nil)
 	assert.NoError(t, err)
 	expected := []string{
 		"BAR=second",
@@ -284,7 +285,7 @@ func TestPassEnv(t *testing.T) {
 func TestPassUnsafeEnv(t *testing.T) {
 	t.Setenv("FOO", "first")
 	t.Setenv("BAR", "second")
-	config, err := ReadConfigFiles(HostFS(), []string{"src/core/test_data/passunsafeenv.plzconfig"}, nil)
+	config, err := ReadConfigFiles(fs.HostFS, []string{"src/core/test_data/passunsafeenv.plzconfig"}, nil)
 	assert.NoError(t, err)
 	expected := []string{
 		"BAR=second",
@@ -301,7 +302,7 @@ func TestPassUnsafeEnvExcludedFromHash(t *testing.T) {
 	err = os.Unsetenv("BAR")
 	require.NoError(t, err)
 
-	config, err := ReadConfigFiles(HostFS(), []string{"src/core/test_data/passunsafeenv.plzconfig"}, nil)
+	config, err := ReadConfigFiles(fs.HostFS, []string{"src/core/test_data/passunsafeenv.plzconfig"}, nil)
 	require.NoError(t, err)
 
 	expected := config.Hash()
@@ -313,7 +314,7 @@ func TestPassUnsafeEnvExcludedFromHash(t *testing.T) {
 }
 
 func TestBuildPathWithPathEnv(t *testing.T) {
-	config, err := ReadConfigFiles(HostFS(), []string{"src/core/test_data/passenv.plzconfig"}, nil)
+	config, err := ReadConfigFiles(fs.HostFS, []string{"src/core/test_data/passenv.plzconfig"}, nil)
 	assert.NoError(t, err)
 	assert.Equal(t, config.Build.Path, strings.Split(os.Getenv("PATH"), ":"))
 }
@@ -348,7 +349,7 @@ func TestUpdateArgsWithQuotedAliases(t *testing.T) {
 }
 
 func TestParseNewFormatAliases(t *testing.T) {
-	c, err := ReadConfigFiles(HostFS(), []string{"src/core/test_data/alias.plzconfig"}, nil)
+	c, err := ReadConfigFiles(fs.HostFS, []string{"src/core/test_data/alias.plzconfig"}, nil)
 	assert.NoError(t, err)
 	assert.Equal(t, 2, len(c.Alias))
 	a := c.Alias["auth"]
@@ -358,7 +359,7 @@ func TestParseNewFormatAliases(t *testing.T) {
 }
 
 func TestAttachAliasFlags(t *testing.T) {
-	c, err := ReadConfigFiles(HostFS(), []string{"src/core/test_data/alias.plzconfig"}, nil)
+	c, err := ReadConfigFiles(fs.HostFS, []string{"src/core/test_data/alias.plzconfig"}, nil)
 	assert.NoError(t, err)
 	t.Setenv("GO_FLAGS_COMPLETION", "1")
 	p := flags.NewParser(&struct{}{}, 0)
@@ -394,7 +395,7 @@ func TestAttachAliasFlags(t *testing.T) {
 }
 
 func TestPrintAliases(t *testing.T) {
-	c, err := ReadConfigFiles(HostFS(), []string{"src/core/test_data/alias.plzconfig"}, nil)
+	c, err := ReadConfigFiles(fs.HostFS, []string{"src/core/test_data/alias.plzconfig"}, nil)
 	assert.NoError(t, err)
 	var buf bytes.Buffer
 	c.PrintAliases(&buf)
@@ -436,7 +437,7 @@ func TestEnsurePleaseLocation(t *testing.T) {
 }
 
 func TestPluginConfig(t *testing.T) {
-	config, err := ReadConfigFiles(HostFS(), []string{"src/core/test_data/plugin.plzconfig"}, nil)
+	config, err := ReadConfigFiles(fs.HostFS, []string{"src/core/test_data/plugin.plzconfig"}, nil)
 	assert.NoError(t, err)
 	assert.Equal(t, []string{"fooc"}, config.Plugin["foo"].ExtraValues["fooctool"])
 }

src/core/state.go

@@ -1192,12 +1192,12 @@ func (state *BuildState) ForArch(arch cli.Arch) *BuildState {
 	configPath := ".plzconfig_" + arch.String()
 
 	config := state.Config.copyConfig()
-	if err := readConfigFile(HostFS(), config, configPath, false); err != nil {
+	if err := readConfigFile(fs.HostFS, config, configPath, false); err != nil {
 		log.Fatalf("%v", err)
 	}
 
 	repoConfig := state.Config.copyConfig()
-	if err := readConfigFile(HostFS(), repoConfig, configPath, false); err != nil {
+	if err := readConfigFile(fs.HostFS, repoConfig, configPath, false); err != nil {
 		log.Fatalf("%v", err)
 	}
 

src/core/subrepo.go

@@ -8,6 +8,7 @@ import (
 	"strings"
 
 	"github.com/thought-machine/please/src/cli"
+	"github.com/thought-machine/please/src/fs"
 )
 
 // A Subrepo stores information about a registered subrepository, typically one
@@ -82,7 +83,7 @@ func (s *Subrepo) Dir(dir string) string {
 
 func readConfigFilesInto(repoConfig *Configuration, files []string) error {
 	for _, file := range files {
-		err := readConfigFile(HostFS(), repoConfig, file, true)
+		err := readConfigFile(fs.HostFS, repoConfig, file, true)
 		if err != nil {
 			return err
 		}

src/core/utils.go

@@ -4,7 +4,6 @@ import (
 	"bytes"
 	"crypto/sha1"
 	"fmt"
-	iofs "io/fs"
 	"os"
 	"path/filepath"
 	"strings"
@@ -37,17 +36,6 @@ func FindRepoRoot() bool {
 	return RepoRoot != ""
 }
 
-type osFS func(name string) (*os.File, error)
-
-func (r osFS) Open(name string) (iofs.File, error) {
-	return r(name)
-}
-
-// HostFS returns an io/fs.FS that behaves the same as the host OS i.e. the same way os.Open works.
-func HostFS() iofs.FS {
-	return osFS(os.Open)
-}
-
 // MustFindRepoRoot returns the root directory of the current repo and sets the initial working dir.
 // It dies on failure, although will fall back to looking for a Bazel WORKSPACE file first.
 func MustFindRepoRoot() string {
@@ -65,7 +53,7 @@ func MustFindRepoRoot() string {
 	}
 	// Check the config for a default repo location. Of course, we have to load system-level config
 	// in order to do that...
-	config, err := ReadConfigFiles(HostFS(), defaultGlobalConfigFiles(), nil)
+	config, err := ReadConfigFiles(fs.HostFS, defaultGlobalConfigFiles(), nil)
 	if err != nil {
 		log.Fatalf("Error reading config file: %s", err)
 	}

src/fs/BUILD

@@ -10,6 +10,7 @@ go_library(
         "home.go",
         "sort.go",
         "walk.go",
+        "iofs.go",
     ],
     pgo_file = "//:pgo",
     visibility = ["PUBLIC"],