From afe8a618fef0fce112da2eb22311616bde7737f7 Mon Sep 17 00:00:00 2001 From: Pete Davison Date: Sat, 2 Sep 2023 15:24:01 -0500 Subject: [PATCH] feat: node refactor (#1316) * refactor: node reader interface * refactor: rewrite Taskfile() as anon recursive func * chore: NewNodeFromIncludedTaskfile * chore: changelog --- CHANGELOG.md | 8 +- errors/errors_taskfile.go | 10 +- setup.go | 10 +- taskfile/read/node.go | 35 ++++ taskfile/read/node_base.go | 18 ++ taskfile/read/node_file.go | 70 ++++++++ taskfile/read/taskfile.go | 329 ++++++++++++++++--------------------- 7 files changed, 286 insertions(+), 194 deletions(-) create mode 100644 taskfile/read/node.go create mode 100644 taskfile/read/node_base.go create mode 100644 taskfile/read/node_file.go diff --git a/CHANGELOG.md b/CHANGELOG.md index f2f7e424..e0f0e710 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## Unreleased + +- Prep work for remote Taskfiles (#1316 by @pd93). + ## v3.29.1 - 2023-08-26 - Update to Go 1.21 (bump minimum version to 1.20) (#1302 by @pd93) @@ -9,8 +13,8 @@ - Fix bug in usage of special variables like `{{.USER_WORKING_DIR}}` in combination with `includes` (#1046, #1205, #1250, #1293, #1312, #1274 by @andarto, #1309 by @andreynering). -- Fix bug on `--status` flag. Running this flag should not have side-effects: - it should not update the checksum on `.task`, only report its status (#1305, +- Fix bug on `--status` flag. Running this flag should not have side-effects: it + should not update the checksum on `.task`, only report its status (#1305, #1307 by @visciang, #1313 by @andreynering). ## v3.28.0 - 2023-07-24 diff --git a/errors/errors_taskfile.go b/errors/errors_taskfile.go index 18e66286..3c942977 100644 --- a/errors/errors_taskfile.go +++ b/errors/errors_taskfile.go @@ -7,7 +7,7 @@ import ( // TaskfileNotFoundError is returned when no appropriate Taskfile is found when // searching the filesystem. type TaskfileNotFoundError struct { - Dir string + URI string Walk bool } @@ -16,7 +16,7 @@ func (err TaskfileNotFoundError) Error() string { if err.Walk { walkText = " (or any of the parent directories)" } - return fmt.Sprintf(`task: No Taskfile found in "%s"%s. Use "task --init" to create a new one`, err.Dir, walkText) + return fmt.Sprintf(`task: No Taskfile found at "%s"%s`, err.URI, walkText) } func (err TaskfileNotFoundError) Code() int { @@ -38,12 +38,12 @@ func (err TaskfileAlreadyExistsError) Code() int { // TaskfileInvalidError is returned when the Taskfile contains syntax errors or // cannot be parsed for some reason. type TaskfileInvalidError struct { - FilePath string - Err error + URI string + Err error } func (err TaskfileInvalidError) Error() string { - return fmt.Sprintf("task: Failed to parse %s:\n%v", err.FilePath, err.Err) + return fmt.Sprintf("task: Failed to parse %s:\n%v", err.URI, err.Err) } func (err TaskfileInvalidError) Code() int { diff --git a/setup.go b/setup.go index 2d1b3e6e..23880395 100644 --- a/setup.go +++ b/setup.go @@ -76,13 +76,15 @@ func (e *Executor) setCurrentDir() error { func (e *Executor) readTaskfile() error { var err error - e.Taskfile, e.Dir, err = read.Taskfile(&read.ReaderNode{ + e.Taskfile, err = read.Taskfile(&read.FileNode{ Dir: e.Dir, Entrypoint: e.Entrypoint, - Parent: nil, - Optional: false, }) - return err + if err != nil { + return err + } + e.Dir = filepath.Dir(e.Taskfile.Location) + return nil } func (e *Executor) setupFuzzyModel() { diff --git a/taskfile/read/node.go b/taskfile/read/node.go new file mode 100644 index 00000000..c1b82851 --- /dev/null +++ b/taskfile/read/node.go @@ -0,0 +1,35 @@ +package read + +import ( + "strings" + + "github.com/go-task/task/v3/taskfile" +) + +type Node interface { + Read() (*taskfile.Taskfile, error) + Parent() Node + Optional() bool + Location() string +} + +func NewNodeFromIncludedTaskfile(parent Node, includedTaskfile taskfile.IncludedTaskfile) (Node, error) { + switch getScheme(includedTaskfile.Taskfile) { + // TODO: Add support for other schemes. + // If no other scheme matches, we assume it's a file. + // This also allows users to explicitly set a file:// scheme. + default: + path, err := includedTaskfile.FullTaskfilePath() + if err != nil { + return nil, err + } + return NewFileNode(parent, path, includedTaskfile.Optional) + } +} + +func getScheme(uri string) string { + if i := strings.Index(uri, "://"); i != -1 { + return uri[:i] + } + return "" +} diff --git a/taskfile/read/node_base.go b/taskfile/read/node_base.go new file mode 100644 index 00000000..5a1a5d64 --- /dev/null +++ b/taskfile/read/node_base.go @@ -0,0 +1,18 @@ +package read + +// BaseNode is a generic node that implements the Parent() and Optional() +// methods of the NodeReader interface. It does not implement the Read() method +// and it designed to be embedded in other node types so that this boilerplate +// code does not need to be repeated. +type BaseNode struct { + parent Node + optional bool +} + +func (node *BaseNode) Parent() Node { + return node.parent +} + +func (node *BaseNode) Optional() bool { + return node.optional +} diff --git a/taskfile/read/node_file.go b/taskfile/read/node_file.go new file mode 100644 index 00000000..f4246c57 --- /dev/null +++ b/taskfile/read/node_file.go @@ -0,0 +1,70 @@ +package read + +import ( + "os" + "path/filepath" + + "gopkg.in/yaml.v3" + + "github.com/go-task/task/v3/errors" + "github.com/go-task/task/v3/internal/filepathext" + "github.com/go-task/task/v3/taskfile" +) + +// A FileNode is a node that reads a taskfile from the local filesystem. +type FileNode struct { + BaseNode + Dir string + Entrypoint string +} + +func NewFileNode(parent Node, path string, optional bool) (*FileNode, error) { + path, err := exists(path) + if err != nil { + return nil, err + } + + return &FileNode{ + BaseNode: BaseNode{ + parent: parent, + optional: optional, + }, + Dir: filepath.Dir(path), + Entrypoint: filepath.Base(path), + }, nil +} + +func (node *FileNode) Location() string { + return filepathext.SmartJoin(node.Dir, node.Entrypoint) +} + +func (node *FileNode) Read() (*taskfile.Taskfile, error) { + if node.Dir == "" { + d, err := os.Getwd() + if err != nil { + return nil, err + } + node.Dir = d + } + + path, err := existsWalk(node.Location()) + if err != nil { + return nil, err + } + node.Dir = filepath.Dir(path) + node.Entrypoint = filepath.Base(path) + + f, err := os.Open(path) + if err != nil { + return nil, err + } + defer f.Close() + + var t taskfile.Taskfile + if err := yaml.NewDecoder(f).Decode(&t); err != nil { + return nil, &errors.TaskfileInvalidError{URI: filepathext.TryAbsToRel(path), Err: err} + } + + t.Location = path + return &t, nil +} diff --git a/taskfile/read/taskfile.go b/taskfile/read/taskfile.go index 4bc8a2fa..efd48fd2 100644 --- a/taskfile/read/taskfile.go +++ b/taskfile/read/taskfile.go @@ -6,8 +6,6 @@ import ( "path/filepath" "runtime" - "gopkg.in/yaml.v3" - "github.com/go-task/task/v3/errors" "github.com/go-task/task/v3/internal/filepathext" "github.com/go-task/task/v3/internal/sysinfo" @@ -31,193 +29,158 @@ var ( } ) -type ReaderNode struct { - Dir string - Entrypoint string - Optional bool - Parent *ReaderNode -} - // Taskfile reads a Taskfile for a given directory // Uses current dir when dir is left empty. Uses Taskfile.yml // or Taskfile.yaml when entrypoint is left empty -func Taskfile(readerNode *ReaderNode) (*taskfile.Taskfile, string, error) { - if readerNode.Dir == "" { - d, err := os.Getwd() +func Taskfile(node Node) (*taskfile.Taskfile, error) { + var _taskfile func(Node) (*taskfile.Taskfile, error) + _taskfile = func(node Node) (*taskfile.Taskfile, error) { + t, err := node.Read() if err != nil { - return nil, "", err - } - readerNode.Dir = d - } - - path, err := existsWalk(filepathext.SmartJoin(readerNode.Dir, readerNode.Entrypoint)) - if err != nil { - return nil, "", err - } - readerNode.Dir = filepath.Dir(path) - readerNode.Entrypoint = filepath.Base(path) - - t, err := readTaskfile(path) - if err != nil { - return nil, "", err - } - - // Annotate any included Taskfile reference with a base directory for resolving relative paths - _ = t.Includes.Range(func(key string, includedFile taskfile.IncludedTaskfile) error { - // Set the base directory for resolving relative paths, but only if not already set - if includedFile.BaseDir == "" { - includedFile.BaseDir = readerNode.Dir - t.Includes.Set(key, includedFile) - } - return nil - }) - - err = t.Includes.Range(func(namespace string, includedTask taskfile.IncludedTaskfile) error { - if t.Version.Compare(taskfile.V3) >= 0 { - tr := templater.Templater{Vars: t.Vars, RemoveNoValue: true} - includedTask = taskfile.IncludedTaskfile{ - Taskfile: tr.Replace(includedTask.Taskfile), - Dir: tr.Replace(includedTask.Dir), - Optional: includedTask.Optional, - Internal: includedTask.Internal, - Aliases: includedTask.Aliases, - AdvancedImport: includedTask.AdvancedImport, - Vars: includedTask.Vars, - BaseDir: includedTask.BaseDir, - } - if err := tr.Err(); err != nil { - return err - } + return nil, err } - path, err := includedTask.FullTaskfilePath() - if err != nil { - return err - } - - path, err = exists(path) - if err != nil { - if includedTask.Optional { - return nil - } - return err - } - - includeReaderNode := &ReaderNode{ - Dir: filepath.Dir(path), - Entrypoint: filepath.Base(path), - Parent: readerNode, - Optional: includedTask.Optional, - } - - if err := checkCircularIncludes(includeReaderNode); err != nil { - return err - } - - includedTaskfile, _, err := Taskfile(includeReaderNode) - if err != nil { - if includedTask.Optional { - return nil - } - return err - } - - if t.Version.Compare(taskfile.V3) >= 0 && len(includedTaskfile.Dotenv) > 0 { - return ErrIncludedTaskfilesCantHaveDotenvs - } - - if includedTask.AdvancedImport { - dir, err := includedTask.FullDirPath() - if err != nil { - return err - } - - // nolint: errcheck - includedTaskfile.Vars.Range(func(k string, v taskfile.Var) error { - o := v - o.Dir = dir - includedTaskfile.Vars.Set(k, o) - return nil - }) - // nolint: errcheck - includedTaskfile.Env.Range(func(k string, v taskfile.Var) error { - o := v - o.Dir = dir - includedTaskfile.Env.Set(k, o) - return nil - }) - - for _, task := range includedTaskfile.Tasks.Values() { - task.Dir = filepathext.SmartJoin(dir, task.Dir) - if task.IncludeVars == nil { - task.IncludeVars = &taskfile.Vars{} + // Annotate any included Taskfile reference with a base directory for resolving relative paths + if node, isFileNode := node.(*FileNode); isFileNode { + _ = t.Includes.Range(func(key string, includedFile taskfile.IncludedTaskfile) error { + // Set the base directory for resolving relative paths, but only if not already set + if includedFile.BaseDir == "" { + includedFile.BaseDir = node.Dir + t.Includes.Set(key, includedFile) + } + return nil + }) + } + + err = t.Includes.Range(func(namespace string, includedTask taskfile.IncludedTaskfile) error { + if t.Version.Compare(taskfile.V3) >= 0 { + tr := templater.Templater{Vars: t.Vars, RemoveNoValue: true} + includedTask = taskfile.IncludedTaskfile{ + Taskfile: tr.Replace(includedTask.Taskfile), + Dir: tr.Replace(includedTask.Dir), + Optional: includedTask.Optional, + Internal: includedTask.Internal, + Aliases: includedTask.Aliases, + AdvancedImport: includedTask.AdvancedImport, + Vars: includedTask.Vars, + BaseDir: includedTask.BaseDir, + } + if err := tr.Err(); err != nil { + return err } - task.IncludeVars.Merge(includedTask.Vars) - task.IncludedTaskfileVars = includedTaskfile.Vars - task.IncludedTaskfile = &includedTask } - } - if err = taskfile.Merge(t, includedTaskfile, &includedTask, namespace); err != nil { - return err - } - - if includedTaskfile.Tasks.Get("default") != nil && t.Tasks.Get(namespace) == nil { - defaultTaskName := fmt.Sprintf("%s:default", namespace) - task := t.Tasks.Get(defaultTaskName) - task.Aliases = append(task.Aliases, namespace) - task.Aliases = append(task.Aliases, includedTask.Aliases...) - t.Tasks.Set(defaultTaskName, task) - } - - return nil - }) - if err != nil { - return nil, "", err - } - - if t.Version.Compare(taskfile.V3) < 0 { - path = filepathext.SmartJoin(readerNode.Dir, fmt.Sprintf("Taskfile_%s.yml", runtime.GOOS)) - if _, err = os.Stat(path); err == nil { - osTaskfile, err := readTaskfile(path) + includeReaderNode, err := NewNodeFromIncludedTaskfile(node, includedTask) if err != nil { - return nil, "", err + if includedTask.Optional { + return nil + } + return err } - if err = taskfile.Merge(t, osTaskfile, nil); err != nil { - return nil, "", err + + if err := checkCircularIncludes(includeReaderNode); err != nil { + return err + } + + includedTaskfile, err := _taskfile(includeReaderNode) + if err != nil { + if includedTask.Optional { + return nil + } + return err + } + + if t.Version.Compare(taskfile.V3) >= 0 && len(includedTaskfile.Dotenv) > 0 { + return ErrIncludedTaskfilesCantHaveDotenvs + } + + if includedTask.AdvancedImport { + dir, err := includedTask.FullDirPath() + if err != nil { + return err + } + + // nolint: errcheck + includedTaskfile.Vars.Range(func(k string, v taskfile.Var) error { + o := v + o.Dir = dir + includedTaskfile.Vars.Set(k, o) + return nil + }) + // nolint: errcheck + includedTaskfile.Env.Range(func(k string, v taskfile.Var) error { + o := v + o.Dir = dir + includedTaskfile.Env.Set(k, o) + return nil + }) + + for _, task := range includedTaskfile.Tasks.Values() { + task.Dir = filepathext.SmartJoin(dir, task.Dir) + if task.IncludeVars == nil { + task.IncludeVars = &taskfile.Vars{} + } + task.IncludeVars.Merge(includedTask.Vars) + task.IncludedTaskfileVars = includedTaskfile.Vars + task.IncludedTaskfile = &includedTask + } + } + + if err = taskfile.Merge(t, includedTaskfile, &includedTask, namespace); err != nil { + return err + } + + if includedTaskfile.Tasks.Get("default") != nil && t.Tasks.Get(namespace) == nil { + defaultTaskName := fmt.Sprintf("%s:default", namespace) + task := t.Tasks.Get(defaultTaskName) + task.Aliases = append(task.Aliases, namespace) + task.Aliases = append(task.Aliases, includedTask.Aliases...) + t.Tasks.Set(defaultTaskName, task) + } + + return nil + }) + if err != nil { + return nil, err + } + + if t.Version.Compare(taskfile.V3) < 0 { + if node, isFileNode := node.(*FileNode); isFileNode { + path := filepathext.SmartJoin(node.Dir, fmt.Sprintf("Taskfile_%s.yml", runtime.GOOS)) + if _, err = os.Stat(path); err == nil { + osNode := &FileNode{ + BaseNode: BaseNode{ + parent: node, + optional: false, + }, + Entrypoint: path, + Dir: node.Dir, + } + osTaskfile, err := osNode.Read() + if err != nil { + return nil, err + } + if err = taskfile.Merge(t, osTaskfile, nil); err != nil { + return nil, err + } + } } } - } - // Set the location of the Taskfile - t.Location = path - - for _, task := range t.Tasks.Values() { - // If the task is not defined, create a new one - if task == nil { - task = &taskfile.Task{} + for _, task := range t.Tasks.Values() { + // If the task is not defined, create a new one + if task == nil { + task = &taskfile.Task{} + } + // Set the location of the taskfile for each task + if task.Location.Taskfile == "" { + task.Location.Taskfile = t.Location + } } - // Set the location of the taskfile for each task - if task.Location.Taskfile == "" { - task.Location.Taskfile = path - } - } - return t, readerNode.Dir, nil -} - -func readTaskfile(file string) (*taskfile.Taskfile, error) { - f, err := os.Open(file) - if err != nil { - return nil, err + return t, nil } - defer f.Close() - - var t taskfile.Taskfile - if err := yaml.NewDecoder(f).Decode(&t); err != nil { - return nil, &errors.TaskfileInvalidError{FilePath: filepathext.TryAbsToRel(file), Err: err} - } - return &t, nil + return _taskfile(node) } func exists(path string) (string, error) { @@ -236,7 +199,7 @@ func exists(path string) (string, error) { } } - return "", errors.TaskfileNotFoundError{Dir: path, Walk: false} + return "", errors.TaskfileNotFoundError{URI: path, Walk: false} } func existsWalk(path string) (string, error) { @@ -261,7 +224,7 @@ func existsWalk(path string) (string, error) { // Error if we reached the root directory and still haven't found a file // OR if the user id of the directory changes if path == parentPath || (parentOwner != owner) { - return "", errors.TaskfileNotFoundError{Dir: origPath, Walk: false} + return "", errors.TaskfileNotFoundError{URI: origPath, Walk: false} } owner = parentOwner @@ -269,22 +232,22 @@ func existsWalk(path string) (string, error) { } } -func checkCircularIncludes(node *ReaderNode) error { +func checkCircularIncludes(node Node) error { if node == nil { return errors.New("task: failed to check for include cycle: node was nil") } - if node.Parent == nil { + if node.Parent() == nil { return errors.New("task: failed to check for include cycle: node.Parent was nil") } curNode := node - basePath := filepathext.SmartJoin(node.Dir, node.Entrypoint) - for curNode.Parent != nil { - curNode = curNode.Parent - curPath := filepathext.SmartJoin(curNode.Dir, curNode.Entrypoint) - if curPath == basePath { + location := node.Location() + for curNode.Parent() != nil { + curNode = curNode.Parent() + curLocation := curNode.Location() + if curLocation == location { return fmt.Errorf("task: include cycle detected between %s <--> %s", - curPath, - filepathext.SmartJoin(node.Parent.Dir, node.Parent.Entrypoint), + curLocation, + node.Parent().Location(), ) } }