From 4d15a8be8f89e5da34dfa29b7a15cd9837b7d8c5 Mon Sep 17 00:00:00 2001 From: Pete Davison Date: Sat, 22 Feb 2025 16:00:37 +0000 Subject: [PATCH] feat: remove logger from taskfile package (#2082) * refactor: remove logger from the taskfile node interface * refactor: functional options on taskfile.Reader * feat: use pass in debug/prompt functions to Reader rather than task Logger * chore: reader docstrings * fix: typo --- setup.go | 21 ++++-- taskfile/node.go | 9 +-- taskfile/node_file.go | 11 ++- taskfile/node_http.go | 6 +- taskfile/reader.go | 157 ++++++++++++++++++++++++++++++++---------- taskfile/taskfile.go | 11 ++- 6 files changed, 149 insertions(+), 66 deletions(-) diff --git a/setup.go b/setup.go index 21275189..e038640c 100644 --- a/setup.go +++ b/setup.go @@ -56,7 +56,7 @@ func (e *Executor) Setup() error { } func (e *Executor) getRootNode() (taskfile.Node, error) { - node, err := taskfile.NewRootNode(e.Logger, e.Entrypoint, e.Dir, e.Insecure, e.Timeout) + node, err := taskfile.NewRootNode(e.Entrypoint, e.Dir, e.Insecure, e.Timeout) if err != nil { return nil, err } @@ -65,14 +65,21 @@ func (e *Executor) getRootNode() (taskfile.Node, error) { } func (e *Executor) readTaskfile(node taskfile.Node) error { + debugFunc := func(s string) { + e.Logger.VerboseOutf(logger.Magenta, s) + } + promptFunc := func(s string) error { + return e.Logger.Prompt(logger.Yellow, s, "n", "y", "yes") + } reader := taskfile.NewReader( node, - e.Insecure, - e.Download, - e.Offline, - e.Timeout, - e.TempDir.Remote, - e.Logger, + taskfile.WithInsecure(e.Insecure), + taskfile.WithDownload(e.Download), + taskfile.WithOffline(e.Offline), + taskfile.WithTimeout(e.Timeout), + taskfile.WithTempDir(e.TempDir.Remote), + taskfile.WithDebugFunc(debugFunc), + taskfile.WithPromptFunc(promptFunc), ) graph, err := reader.Read() if err != nil { diff --git a/taskfile/node.go b/taskfile/node.go index e9209700..486a0a16 100644 --- a/taskfile/node.go +++ b/taskfile/node.go @@ -11,7 +11,6 @@ import ( "github.com/go-task/task/v3/errors" "github.com/go-task/task/v3/internal/experiments" - "github.com/go-task/task/v3/internal/logger" ) type Node interface { @@ -26,7 +25,6 @@ type Node interface { } func NewRootNode( - l *logger.Logger, entrypoint string, dir string, insecure bool, @@ -37,11 +35,10 @@ func NewRootNode( if entrypoint == "-" { return NewStdinNode(dir) } - return NewNode(l, entrypoint, dir, insecure, timeout) + return NewNode(entrypoint, dir, insecure, timeout) } func NewNode( - l *logger.Logger, entrypoint string, dir string, insecure bool, @@ -58,9 +55,9 @@ func NewNode( case "git": node, err = NewGitNode(entrypoint, dir, insecure, opts...) case "http", "https": - node, err = NewHTTPNode(l, entrypoint, dir, insecure, timeout, opts...) + node, err = NewHTTPNode(entrypoint, dir, insecure, timeout, opts...) default: - node, err = NewFileNode(l, entrypoint, dir, opts...) + node, err = NewFileNode(entrypoint, dir, opts...) } diff --git a/taskfile/node_file.go b/taskfile/node_file.go index 4ea9001c..99e2ec9c 100644 --- a/taskfile/node_file.go +++ b/taskfile/node_file.go @@ -9,7 +9,6 @@ import ( "github.com/go-task/task/v3/internal/execext" "github.com/go-task/task/v3/internal/filepathext" - "github.com/go-task/task/v3/internal/logger" ) // A FileNode is a node that reads a taskfile from the local filesystem. @@ -18,10 +17,10 @@ type FileNode struct { Entrypoint string } -func NewFileNode(l *logger.Logger, entrypoint, dir string, opts ...NodeOption) (*FileNode, error) { +func NewFileNode(entrypoint, dir string, opts ...NodeOption) (*FileNode, error) { var err error base := NewBaseNode(dir, opts...) - entrypoint, base.dir, err = resolveFileNodeEntrypointAndDir(l, entrypoint, base.dir) + entrypoint, base.dir, err = resolveFileNodeEntrypointAndDir(entrypoint, base.dir) if err != nil { return nil, err } @@ -50,10 +49,10 @@ func (node *FileNode) Read(ctx context.Context) ([]byte, error) { // resolveFileNodeEntrypointAndDir resolves checks the values of entrypoint and dir and // populates them with default values if necessary. -func resolveFileNodeEntrypointAndDir(l *logger.Logger, entrypoint, dir string) (string, string, error) { +func resolveFileNodeEntrypointAndDir(entrypoint, dir string) (string, string, error) { var err error if entrypoint != "" { - entrypoint, err = Exists(l, entrypoint) + entrypoint, err = Exists(entrypoint) if err != nil { return "", "", err } @@ -68,7 +67,7 @@ func resolveFileNodeEntrypointAndDir(l *logger.Logger, entrypoint, dir string) ( return "", "", err } } - entrypoint, err = ExistsWalk(l, dir) + entrypoint, err = ExistsWalk(dir) if err != nil { return "", "", err } diff --git a/taskfile/node_http.go b/taskfile/node_http.go index 55ae08bd..6e152972 100644 --- a/taskfile/node_http.go +++ b/taskfile/node_http.go @@ -11,7 +11,6 @@ import ( "github.com/go-task/task/v3/errors" "github.com/go-task/task/v3/internal/execext" "github.com/go-task/task/v3/internal/filepathext" - "github.com/go-task/task/v3/internal/logger" ) // An HTTPNode is a node that reads a Taskfile from a remote location via HTTP. @@ -19,12 +18,10 @@ type HTTPNode struct { *BaseNode URL *url.URL // stores url pointing actual remote file. (e.g. with Taskfile.yml) entrypoint string // stores entrypoint url. used for building graph vertices. - logger *logger.Logger timeout time.Duration } func NewHTTPNode( - l *logger.Logger, entrypoint string, dir string, insecure bool, @@ -45,7 +42,6 @@ func NewHTTPNode( URL: url, entrypoint: entrypoint, timeout: timeout, - logger: l, }, nil } @@ -58,7 +54,7 @@ func (node *HTTPNode) Remote() bool { } func (node *HTTPNode) Read(ctx context.Context) ([]byte, error) { - url, err := RemoteExists(ctx, node.logger, node.URL, node.timeout) + url, err := RemoteExists(ctx, node.URL, node.timeout) if err != nil { return nil, err } diff --git a/taskfile/reader.go b/taskfile/reader.go index 241770c7..f2f5394c 100644 --- a/taskfile/reader.go +++ b/taskfile/reader.go @@ -14,7 +14,6 @@ import ( "github.com/go-task/task/v3/errors" "github.com/go-task/task/v3/internal/compiler" "github.com/go-task/task/v3/internal/filepathext" - "github.com/go-task/task/v3/internal/logger" "github.com/go-task/task/v3/internal/templater" "github.com/go-task/task/v3/taskfile/ast" ) @@ -28,40 +27,115 @@ Continue?` Continue?` ) -// A Reader will recursively read Taskfiles from a given source using a directed -// acyclic graph (DAG). -type Reader struct { - graph *ast.TaskfileGraph - node Node - insecure bool - download bool - offline bool - timeout time.Duration - tempDir string - logger *logger.Logger - promptMutex sync.Mutex -} +type ( + // ReaderDebugFunc is a function that is called when the reader wants to + // log debug messages + ReaderDebugFunc func(string) + // ReaderPromptFunc is a function that is called when the reader wants to + // prompt the user in some way + ReaderPromptFunc func(string) error + // ReaderOption is a function that configures a Reader. + ReaderOption func(*Reader) + // A Reader will recursively read Taskfiles from a given source using a directed + // acyclic graph (DAG). + Reader struct { + graph *ast.TaskfileGraph + node Node + insecure bool + download bool + offline bool + timeout time.Duration + tempDir string + debugFunc ReaderDebugFunc + promptFunc ReaderPromptFunc + promptMutex sync.Mutex + } +) +// NewReader constructs a new Taskfile Reader using the given Node and options. func NewReader( node Node, - insecure bool, - download bool, - offline bool, - timeout time.Duration, - tempDir string, - logger *logger.Logger, + opts ...ReaderOption, ) *Reader { - return &Reader{ + reader := &Reader{ graph: ast.NewTaskfileGraph(), node: node, - insecure: insecure, - download: download, - offline: offline, - timeout: timeout, - tempDir: tempDir, - logger: logger, + insecure: false, + download: false, + offline: false, + timeout: time.Second * 10, + tempDir: os.TempDir(), + debugFunc: nil, + promptFunc: nil, promptMutex: sync.Mutex{}, } + for _, opt := range opts { + opt(reader) + } + return reader +} + +// WithInsecure enables insecure connections when reading remote taskfiles. By +// default, insecure connections are rejected. +func WithInsecure(insecure bool) ReaderOption { + return func(r *Reader) { + r.insecure = insecure + } +} + +// WithDownload forces the reader to download a fresh copy of the taskfile from +// the remote source. +func WithDownload(download bool) ReaderOption { + return func(r *Reader) { + r.download = download + } +} + +// WithOffline stops the reader from being able to make network connections. +// It will still be able to read local files and cached copies of remote files. +func WithOffline(offline bool) ReaderOption { + return func(r *Reader) { + r.offline = offline + } +} + +// WithTimeout sets the timeout for reading remote taskfiles. By default, the +// timeout is set to 10 seconds. +func WithTimeout(timeout time.Duration) ReaderOption { + return func(r *Reader) { + r.timeout = timeout + } +} + +// WithTempDir sets the temporary directory to be used by the reader. By +// default, the reader uses `os.TempDir()`. +func WithTempDir(tempDir string) ReaderOption { + return func(r *Reader) { + r.tempDir = tempDir + } +} + +// WithDebugFunc sets the debug function to be used by the reader. If set, this +// function will be called with debug messages. This can be useful if the caller +// wants to log debug messages from the reader. By default, no debug function is +// set and the logs are not written. +func WithDebugFunc(debugFunc ReaderDebugFunc) ReaderOption { + return func(r *Reader) { + r.debugFunc = debugFunc + } +} + +// WithPromptFunc sets the prompt function to be used by the reader. If set, +// this function will be called with prompt messages. The function should +// optionally log the message to the user and return nil if the prompt is +// accepted and the execution should continue. Otherwise, it should return an +// error which describes why the the prompt was rejected. This can then be +// caught and used later when calling the Read method. By default, no prompt +// function is set and all prompts are automatically accepted. +func WithPromptFunc(promptFunc ReaderPromptFunc) ReaderOption { + return func(r *Reader) { + r.promptFunc = promptFunc + } } func (r *Reader) Read() (*ast.TaskfileGraph, error) { @@ -73,6 +147,19 @@ func (r *Reader) Read() (*ast.TaskfileGraph, error) { return r.graph, nil } +func (r *Reader) debugf(format string, a ...any) { + if r.debugFunc != nil { + r.debugFunc(fmt.Sprintf(format, a...)) + } +} + +func (r *Reader) promptf(format string, a ...any) error { + if r.promptFunc != nil { + return r.promptFunc(fmt.Sprintf(format, a...)) + } + return nil +} + func (r *Reader) include(node Node) error { // Create a new vertex for the Taskfile vertex := &ast.TaskfileVertex{ @@ -132,7 +219,7 @@ func (r *Reader) include(node Node) error { return err } - includeNode, err := NewNode(r.logger, entrypoint, include.Dir, r.insecure, r.timeout, + includeNode, err := NewNode(entrypoint, include.Dir, r.insecure, r.timeout, WithParent(node), ) if err != nil { @@ -246,7 +333,7 @@ func (r *Reader) loadNodeContent(node Node) ([]byte, error) { } else if err != nil { return nil, err } - r.logger.VerboseOutf(logger.Magenta, "task: [%s] Fetched cached copy\n", node.Location()) + r.debugf("task: [%s] Fetched cached copy\n", node.Location()) return cached, nil } @@ -270,14 +357,14 @@ func (r *Reader) loadNodeContent(node Node) ([]byte, error) { } else if err != nil { return nil, err } - r.logger.VerboseOutf(logger.Magenta, "task: [%s] Network timeout. Fetched cached copy\n", node.Location()) + r.debugf("task: [%s] Network timeout. Fetched cached copy\n", node.Location()) return cached, nil } else if err != nil { return nil, err } - r.logger.VerboseOutf(logger.Magenta, "task: [%s] Fetched remote copy\n", node.Location()) + r.debugf("task: [%s] Fetched remote copy\n", node.Location()) // Get the checksums checksum := checksum(b) @@ -286,17 +373,17 @@ func (r *Reader) loadNodeContent(node Node) ([]byte, error) { var prompt string if cachedChecksum == "" { // If the checksum doesn't exist, prompt the user to continue - prompt = fmt.Sprintf(taskfileUntrustedPrompt, node.Location()) + prompt = taskfileUntrustedPrompt } else if checksum != cachedChecksum { // If there is a cached hash, but it doesn't match the expected hash, prompt the user to continue - prompt = fmt.Sprintf(taskfileChangedPrompt, node.Location()) + prompt = taskfileChangedPrompt } if prompt != "" { if err := func() error { r.promptMutex.Lock() defer r.promptMutex.Unlock() - return r.logger.Prompt(logger.Yellow, prompt, "n", "y", "yes") + return r.promptf(prompt, node.Location()) }(); err != nil { return nil, &errors.TaskfileNotTrustedError{URI: node.Location()} } @@ -307,7 +394,7 @@ func (r *Reader) loadNodeContent(node Node) ([]byte, error) { } // Cache the file - r.logger.VerboseOutf(logger.Magenta, "task: [%s] Caching downloaded file\n", node.Location()) + r.debugf("task: [%s] Caching downloaded file\n", node.Location()) if err = cache.write(node, b); err != nil { return nil, err } diff --git a/taskfile/taskfile.go b/taskfile/taskfile.go index d14b7603..3502638a 100644 --- a/taskfile/taskfile.go +++ b/taskfile/taskfile.go @@ -12,7 +12,6 @@ import ( "github.com/go-task/task/v3/errors" "github.com/go-task/task/v3/internal/filepathext" - "github.com/go-task/task/v3/internal/logger" "github.com/go-task/task/v3/internal/sysinfo" ) @@ -41,7 +40,7 @@ var ( // at the given URL with any of the default Taskfile files names. If any of // these match a file, the first matching path will be returned. If no files are // found, an error will be returned. -func RemoteExists(ctx context.Context, l *logger.Logger, u *url.URL, timeout time.Duration) (*url.URL, error) { +func RemoteExists(ctx context.Context, u *url.URL, timeout time.Duration) (*url.URL, error) { // Create a new HEAD request for the given URL to check if the resource exists req, err := http.NewRequestWithContext(ctx, "HEAD", u.String(), nil) if err != nil { @@ -89,7 +88,6 @@ func RemoteExists(ctx context.Context, l *logger.Logger, u *url.URL, timeout tim // If the request was successful, return the URL if resp.StatusCode == http.StatusOK { - l.VerboseOutf(logger.Magenta, "task: [%s] Not found - Using alternative (%s)\n", alt.String(), taskfile) return alt, nil } } @@ -102,7 +100,7 @@ func RemoteExists(ctx context.Context, l *logger.Logger, u *url.URL, timeout tim // given path with any of the default Taskfile files names. If any of these // match a file, the first matching path will be returned. If no files are // found, an error will be returned. -func Exists(l *logger.Logger, path string) (string, error) { +func Exists(path string) (string, error) { fi, err := os.Stat(path) if err != nil { return "", err @@ -117,7 +115,6 @@ func Exists(l *logger.Logger, path string) (string, error) { for _, taskfile := range defaultTaskfiles { alt := filepathext.SmartJoin(path, taskfile) if _, err := os.Stat(alt); err == nil { - l.VerboseOutf(logger.Magenta, "task: [%s] Not found - Using alternative (%s)\n", path, taskfile) return filepath.Abs(alt) } } @@ -130,14 +127,14 @@ func Exists(l *logger.Logger, path string) (string, error) { // calling the exists function until it finds a file or reaches the root // directory. On supported operating systems, it will also check if the user ID // of the directory changes and abort if it does. -func ExistsWalk(l *logger.Logger, path string) (string, error) { +func ExistsWalk(path string) (string, error) { origPath := path owner, err := sysinfo.Owner(path) if err != nil { return "", err } for { - fpath, err := Exists(l, path) + fpath, err := Exists(path) if err == nil { return fpath, nil }