Merge pull request #78 from warptools/refactor-run-cmd-file-search

Refactor the run command's search for files.

app/enter/ferk_cli.go

@@ -192,7 +192,7 @@ func cmdFerk_selectPlot(c *cli.Context) (*wfapi.Plot, *string, error) {
  if err != nil {
  return nil, nil, err
  }
- m, p, f, _, _, err := dab.FindActionableFromFS(os.DirFS("/"), pth, "", false, dab.ActionableSearch_Any)
+ m, p, f, _, _, err := dab.SearchFSAndLoadActionable(os.DirFS("/"), pth, "", false, dab.ActionableSearch_Any)
  _, _ = m, f // TODO support these
  if err != nil {
  return nil, nil, err

app/run/run_cli.go

@@ -2,13 +2,16 @@ package runcli
 
 import (
  "fmt"
- "io/ioutil"
+ "io/fs"
  "os"
  "path/filepath"
 
  "github.com/ipld/go-ipld-prime"
  "github.com/ipld/go-ipld-prime/codec/json"
+ "github.com/serum-errors/go-serum"
  "github.com/urfave/cli/v2"
+ "github.com/warpfork/go-fsx"
+ "github.com/warpfork/go-fsx/osfs"
 
  appbase "github.com/warptools/warpforge/app/base"
  "github.com/warptools/warpforge/app/base/util"
@@ -46,6 +49,106 @@ var runCmdDef = &cli.Command{
  },
 }
 
+type runTargets struct {
+ fs fsx.FS
+ list []*runTarget
+}
+type runTarget struct {
+ originalRequest string // The user-given argument that resulted in this target. A filesystem path fragment, generally. Might be "." or "someformula.wf" or "./foo/...". Can be the same for multiple run targets.
+ mainFilename string // The actual filename to consult. May contain a module, or a plot, or a formula -- we don't know yet -- but we've at least checked that it exists.
+ isModule bool // Set to true if we picked this file in such a way that it really has to contain a module. (Not any kind of security boundary, but is often true, and elides some guessing at later stages.)
+}
+
+func (rts *runTargets) append(rt runTarget) {
+ rts.list = append(rts.list, &rt)
+}
+
+// findRunTargets turns CLI args into a set of paths for each thing that the args described.
+// This might be quite a few things: the args can include a list, but also a "..." can imply a whole directory walk.
+//
+// For all the requests that are specific (i.e. not using "..."), we check that the file exists -- you probably want to hear about any typos before we start launching into heavy duty work.
+// For any requests that are using "...", we do the directory walk up-front. This lets us estimate how much work is about to happen.
+// We don't actually load or parse any files yet -- just check existence.
+//
+// Nonexistent specific requests result in errors.
+// A "..." that has no matches produces no comment.
+// The first error encountered causes return; we do not accumulate multiple errors.
+//
+// TODO: this probably should be extracted to `pkg/dab`.
+func findRunTargets(args cli.Args, fs fsx.FS) (results runTargets, err error) {
+ results = runTargets{
+ fs: fs,
+ }
+
+ // If there were no positional args at all: we'll take that as meaning "try to do the cwd, as a module".
+ // FUTURE:TODO: this should probably use `SearchFSAndLoadActionable` -- so that it "DTRT" if used in a subdir of a module.
+ if !args.Present() {
+ filename := filepath.Join(".", dab.MagicFilename_Module)
+ results.append(runTarget{
+ originalRequest: ".",
+ mainFilename: filename,
+ isModule: true,
+ })
+ if isFile, _ := fsx.IsPathFile(results.fs, filename); !isFile {
+ err = serum.Errorf(wfapi.ECodeArgument, "cannot run nothing; no module file exists in current directory. (Hint: Module files should have the name %q.)", dab.MagicFilename_Module)
+ }
+ return
+ }
+
+ // Loop over all the args. They're cumulative.
+ for _, arg := range args.Slice() {
+ // If we have a "...": do a walk. Gather any files with the name expected for modules.
+ if filepath.Base(arg) == "..." {
+ e2 := fsx.WalkDir(fs, filepath.Dir(arg),
+ func(path string, _ fsx.DirEntry, err error) error {
+ if err != nil {
+ return err
+ }
+ if filepath.Base(path) == dab.MagicFilename_Module {
+ results.append(runTarget{
+ originalRequest: arg,
+ mainFilename: path,
+ isModule: true,
+ })
+ }
+ return nil
+ },
+ )
+ if e2 != nil {
+ err = serum.Errorf(wfapi.ECodeArgument, "error while walking for modules matching %q: %w", arg, e2)
+ return
+ }
+ continue
+ }
+
+ // This one's a path to some single file or directory, then.
+ fi, e2 := os.Stat(arg)
+ if e2 != nil {
+ err = serum.Errorf(wfapi.ECodeArgument, "error looking for runnable content at %q: %w", arg, e2)
+ return
+ }
+ if fi.IsDir() { // If it's a dir, we'll look for module files.
+ filename := filepath.Join(arg, dab.MagicFilename_Module)
+ if isFile, _ := fsx.IsPathFile(results.fs, filename); !isFile {
+ err = serum.Errorf(wfapi.ECodeArgument, "cannot run anything at %q: since it's a directory, expected a module file. (Hint: Module files should have the name %q.)", arg, dab.MagicFilename_Module)
+ return
+ }
+ results.append(runTarget{
+ originalRequest: arg,
+ mainFilename: filename,
+ isModule: true,
+ })
+ } else { // We'll presume plain file, then.
+ // This could contain a formula, or a module. We don't inspect that at this stage.
+ results.append(runTarget{
+ originalRequest: arg,
+ mainFilename: arg,
+ })
+ }
+ }
+ return
+}
+
 func cmdRun(c *cli.Context) error {
  ctx := c.Context
  logger := logging.Ctx(ctx)
@@ -61,62 +164,30 @@ func cmdRun(c *cli.Context) error {
  return err
  }
  logger.Debug("", "pwd: %s", cwd)
- if !c.Args().Present() {
- filename := filepath.Join(cwd, dab.MagicFilename_Module) // execute the module in the current directory
- logger.Debug("", "working directory module: %s", filename)
- _, err = util.ExecModule(ctx, nil, pltCfg, filename)
- if err != nil {
- return err
- }
- return nil
- }
 
- if filepath.Base(c.Args().First()) == "..." {
- // recursively execute module.json files
- return filepath.Walk(filepath.Dir(c.Args().First()),
- func(path string, info os.FileInfo, err error) error {
- if err != nil {
- return err
- }
- if filepath.Base(path) == dab.MagicFilename_Module {
- path = filepath.Join(cwd, path) // need to provide absolute path
- if c.Bool("verbose") {
- logger.Debug("", "executing %q", path)
- }
- _, err = util.ExecModule(ctx, nil, pltCfg, path)
- if err != nil {
- return err
- }
- }
- return nil
- })
+ rts, err := findRunTargets(c.Args(), osfs.DirFS("."))
+ if err != nil {
+ return err
  }
- // a list of individual files or directories has been provided
- for _, fileName := range c.Args().Slice() {
- info, err := os.Stat(fileName)
- if err != nil {
- return err
- }
- fileName, err := filepath.Abs(fileName)
- if err != nil {
- return err
- }
- if info.IsDir() {
- _, err := util.ExecModule(ctx, nil, pltCfg, filepath.Join(fileName, dab.MagicFilename_Module))
+ for _, target := range rts.list {
+ fullPath := filepath.Join(cwd, target.mainFilename)
+ if target.isModule {
+ logger.Debug("", "executing module from file %q, as requested by the argument %q", fullPath, target.originalRequest)
+ _, err := util.ExecModule(ctx, nil, pltCfg, fullPath)
  if err != nil {
  return err
  }
  } else {
- // formula or module file provided
- t, err := dab.GetFileType(fileName)
+ t, err := dab.GetFileType(target.mainFilename) // FIXME this is based on filename; `dab.GuessDocumentType` would probably do more useful things.
  if err != nil {
  return err
  }
 
  switch t {
  case dab.FileType_Formula:
+ logger.Debug("", "executing formula from file %q, as requested by the argument %q", fullPath, target.originalRequest)
  // unmarshal FormulaAndContext from file data
- f, err := ioutil.ReadFile(fileName)
+ f, err := fs.ReadFile(rts.fs, target.mainFilename)
  if err != nil {
  return err
  }
@@ -132,7 +203,7 @@ func cmdRun(c *cli.Context) error {
  if err != nil {
  return err
  }
- formulaDir := filepath.Dir(fileName)
+ formulaDir := filepath.Dir(filepath.Join(cwd, target.mainFilename))
  frmExecCfg, err := config.FormulaExecConfig(&formulaDir)
  if err != nil {
  return err
@@ -141,13 +212,13 @@ func cmdRun(c *cli.Context) error {
  return err
  }
  case dab.FileType_Module:
- logger.Debug("", "executing module")
- _, err := util.ExecModule(ctx, nil, pltCfg, fileName)
+ logger.Debug("", "executing module from file %q, as requested by the argument %q", fullPath, target.originalRequest)
+ _, err := util.ExecModule(ctx, nil, pltCfg, fullPath)
  if err != nil {
  return err
  }
  default:
- return fmt.Errorf("unsupported file %s", fileName)
+ return fmt.Errorf("unsupported file %s", fullPath)
  }
  }
  }

go.mod

@@ -19,7 +19,8 @@ require (
  github.com/polydawn/refmt v0.89.0
  github.com/serum-errors/go-serum v0.8.1-0.20230120233340-7c9bffa81fc6
  github.com/urfave/cli/v2 v2.25.1
- github.com/warpfork/go-testmark v0.11.1-0.20221127032233-5cd7a73883c2
+ github.com/warpfork/go-fsx v0.4.0
+ github.com/warpfork/go-testmark v0.12.1
  go.opentelemetry.io/otel v1.14.0
  go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.14.0
  go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.14.0

go.sum

@@ -324,8 +324,11 @@ github.com/stretchr/testify v1.8.2 h1:+h33VjcLVPDHtOdpUCuF+7gSuG3yGIftsP1YvFihtJ
 github.com/urfave/cli v1.22.10/go.mod h1:Gos4lmkARVdJ6EkW0WaNv/tZAAMe9V7XWyB60NtXRu0=
 github.com/urfave/cli/v2 v2.25.1 h1:zw8dSP7ghX0Gmm8vugrs6q9Ku0wzweqPyshy+syu9Gw=
 github.com/urfave/cli/v2 v2.25.1/go.mod h1:GHupkWPMM0M/sj1a2b4wUrWBPzazNrIjouW6fmdJLxc=
-github.com/warpfork/go-testmark v0.11.1-0.20221127032233-5cd7a73883c2 h1:bXKTlluQYzhX6TWXRFSdYNcWjy5reSiAs4kOBmwavTU=
-github.com/warpfork/go-testmark v0.11.1-0.20221127032233-5cd7a73883c2/go.mod h1:jhEf8FVxd+F17juRubpmut64NEG6I2rgkUhlcqqXwE0=
+github.com/warpfork/go-fsx v0.3.0/go.mod h1:oTACCMj+Zle+vgVa5SAhGAh7WksYpLgGUCKEAVc+xPg=
+github.com/warpfork/go-fsx v0.4.0 h1:mlSH89UOECT5+NdRo8gPaE92Pm1xvt6cbzGkFa4QcsA=
+github.com/warpfork/go-fsx v0.4.0/go.mod h1:oTACCMj+Zle+vgVa5SAhGAh7WksYpLgGUCKEAVc+xPg=
+github.com/warpfork/go-testmark v0.12.1 h1:rMgCpJfwy1sJ50x0M0NgyphxYYPMOODIJHhsXyEHU0s=
+github.com/warpfork/go-testmark v0.12.1/go.mod h1:kHwy7wfvGSPh1rQJYKayD4AbtNaeyZdcGi9tNJTaa5Y=
 github.com/warpfork/go-wish v0.0.0-20220906213052-39a1cc7a02d0 h1:GDDkbFiaK8jsSDJfjId/PEGEShv6ugrt4kYsC5UIDaQ=
 github.com/warpfork/go-wish v0.0.0-20220906213052-39a1cc7a02d0/go.mod h1:x6AKhvSSexNrVSrViXSHUEbICjmGXhtgABaHIySUSGw=
 github.com/xanzy/ssh-agent v0.3.3 h1:+/15pJfg/RsTxqYcX6fHqOXZwwMP+2VyYWJeWM2qQFM=

pkg/dab/filetype.go

@@ -35,6 +35,9 @@ var types = map[FileType]struct{}{
 // Errors:
 //
 // - warpforge-error-invalid -- if the file name is not recognized
+//
+// DEPRECATED: there's almost no situation where `dab.GuessDocumentType`
+// and looking at the actual content wouldn't be preferable.
 func GetFileType(name string) (FileType, error) {
  base := filepath.Base(name)
  if ft, ok := magicFileTypes[base]; ok {

pkg/dab/find.go

@@ -46,11 +46,11 @@ func open(fsys fs.FS, path string) (fs.File, error) {
  return f, nil
 }
 
-// FindActionableFromFS loads either module (and plot) from the fileystem,
+// SearchFSAndLoadActionable loads either module (and plot) from the fileystem,
 // or instead a Formula,
 // while also accepting directories as input and applying reasonable heuristics.
 //
-// FindActionableFromFS is suitable for finding *one* module/plot/formula;
+// SearchFSAndLoadActionable is suitable for finding *one* module/plot/formula;
 // finding groupings of modules (i.e., handling args of "./..." forms) is a different feature.
 //
 // The 'fsys' parameter is typically `os.DirFS("/")` except in test environments.
@@ -91,7 +91,7 @@ func open(fsys fs.FS, path string) (fs.File, error) {
 // - warpforge-error-module-invalid -- when a read module contains invalid data
 // - warpforge-error-searching-filesystem -- when the search of the filesystem produces an invalid result
 // - warpforge-error-serialization -- when IPLD deserialization fails
-func FindActionableFromFS(
+func SearchFSAndLoadActionable(
  fsys fs.FS,
  basisPath string, searchPath string, searchUp bool,
  accept ActionableSearch,

pkg/dab/find_test.go

@@ -74,11 +74,11 @@ func (tt *testcaseFindActionableFromFS) run(t *testing.T) {
  }
  if len(tt.outputs.panicPattern) > 0 {
  qt.Assert(t, func() {
- FindActionableFromFS(fsys, in.basis, in.search, in.searchUp, in.mode)
+ SearchFSAndLoadActionable(fsys, in.basis, in.search, in.searchUp, in.mode)
  }, qt.PanicMatches, tt.outputs.panicPattern)
  t.Skipf("expected panic caught")
  }
- m, p, f, path, rem, err := FindActionableFromFS(fsys, in.basis, in.search, in.searchUp, in.mode)
+ m, p, f, path, rem, err := SearchFSAndLoadActionable(fsys, in.basis, in.search, in.searchUp, in.mode)
  isNotNil := func(b bool) qt.Checker {
  if b {
  return qt.IsNotNil