diff --git a/CHANGELOG.md b/CHANGELOG.md index 75a230703c..abb1322765 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,13 @@ - Further improved fingerprinting performance on large repositories: hashing source files now reuses a single buffer, reducing memory allocations by ~98% and wall-clock time by ~7% (#2925 by @vmaerten). +- Fixed the fingerprint variable (`{{.CHECKSUM}}`/`{{.TIMESTAMP}}`) ignoring a + `method:` set at the Taskfile level: the variable now follows the same method + resolution as the up-to-date check. Only the variable matching the effective + method is injected, so a task inheriting a Taskfile-level `method: timestamp` + gets `{{.TIMESTAMP}}` and no longer a `{{.CHECKSUM}}` (which now renders as an + empty string), and neither variable is injected when the effective method is + `none` (#2924 by @vmaerten). ## v3.52.0 - 2026-07-02 diff --git a/executor.go b/executor.go index 783f18ed0d..14b422f3e1 100644 --- a/executor.go +++ b/executor.go @@ -10,6 +10,7 @@ import ( "github.com/puzpuzpuz/xsync/v4" "github.com/sajari/fuzzy" + "github.com/go-task/task/v3/internal/fingerprint" "github.com/go-task/task/v3/internal/logger" "github.com/go-task/task/v3/internal/output" "github.com/go-task/task/v3/internal/sort" @@ -122,6 +123,17 @@ func (e *Executor) Options(opts ...ExecutorOption) { } } +// fingerprinter is built on the fly rather than once in Setup because fields +// like Dry may be mutated between runs. +func (e *Executor) fingerprinter() *fingerprint.Fingerprinter { + return fingerprint.NewFingerprinter( + e.Taskfile.Method, + e.TempDir.Fingerprint, + e.Dry, + e.Logger, + ) +} + // WithDir sets the working directory of the [Executor]. By default, the // directory is set to the user's current working directory. func WithDir(dir string) ExecutorOption { diff --git a/help.go b/help.go index 9998bd38ad..c6399363eb 100644 --- a/help.go +++ b/help.go @@ -12,7 +12,6 @@ import ( "golang.org/x/sync/errgroup" "github.com/go-task/task/v3/internal/editors" - "github.com/go-task/task/v3/internal/fingerprint" "github.com/go-task/task/v3/internal/logger" "github.com/go-task/task/v3/internal/sort" "github.com/go-task/task/v3/taskfile/ast" @@ -151,17 +150,7 @@ func (e *Executor) ToEditorOutput(tasks []*ast.Task, noStatus bool, nested bool) return nil } - // Get the fingerprinting method to use - method := e.Taskfile.Method - if tasks[i].Method != "" { - method = tasks[i].Method - } - upToDate, err := fingerprint.IsTaskUpToDate(context.Background(), tasks[i], - fingerprint.WithMethod(method), - fingerprint.WithTempDir(e.TempDir.Fingerprint), - fingerprint.WithDry(e.Dry), - fingerprint.WithLogger(e.Logger), - ) + upToDate, err := e.fingerprinter().UpToDate(context.Background(), tasks[i]) if err != nil { return err } diff --git a/internal/fingerprint/fingerprinter.go b/internal/fingerprint/fingerprinter.go new file mode 100644 index 0000000000..280abb96e9 --- /dev/null +++ b/internal/fingerprint/fingerprinter.go @@ -0,0 +1,148 @@ +package fingerprint + +import ( + "context" + + "github.com/go-task/task/v3/internal/logger" + "github.com/go-task/task/v3/taskfile/ast" +) + +type ( + FingerprinterOption func(*Fingerprinter) + + // A Fingerprinter answers whether a task is up-to-date. It owns the + // resolution of the fingerprinting method and the checkers behind it. + Fingerprinter struct { + defaultMethod string + tempDir string + dry bool + logger *logger.Logger + statusChecker StatusCheckable + sourcesChecker SourcesCheckable + } +) + +func WithStatusChecker(checker StatusCheckable) FingerprinterOption { + return func(f *Fingerprinter) { + f.statusChecker = checker + } +} + +func WithSourcesChecker(checker SourcesCheckable) FingerprinterOption { + return func(f *Fingerprinter) { + f.sourcesChecker = checker + } +} + +// NewFingerprinter uses defaultMethod for tasks that don't declare one. +func NewFingerprinter( + defaultMethod string, + tempDir string, + dry bool, + logger *logger.Logger, + opts ...FingerprinterOption, +) *Fingerprinter { + f := &Fingerprinter{ + defaultMethod: defaultMethod, + tempDir: tempDir, + dry: dry, + logger: logger, + } + for _, opt := range opts { + opt(f) + } + return f +} + +func (f *Fingerprinter) resolveMethod(t *ast.Task) string { + if t.Method != "" { + return t.Method + } + return f.defaultMethod +} + +// Kind names the fingerprint variable ("checksum", "timestamp" or "none") the +// resolved method injects. An invalid method is reported as "checksum" here and +// rejected by the entry points that build a checker. +func (f *Fingerprinter) Kind(t *ast.Task) string { + if f.sourcesChecker != nil { + return f.sourcesChecker.Kind() + } + switch method := f.resolveMethod(t); method { + case "timestamp", "none": + return method + default: + return "checksum" + } +} + +// SourceValue returns the value of the fingerprint variable for the given task. +// It is potentially expensive, so only call it when the task references it. +func (f *Fingerprinter) SourceValue(t *ast.Task) (any, error) { + sourcesChecker, err := f.resolveSourcesChecker(t) + if err != nil { + return nil, err + } + return sourcesChecker.Value(t) +} + +// UpToDate considers both the status commands and the sources of a task; one +// that declares neither never is. +func (f *Fingerprinter) UpToDate(ctx context.Context, t *ast.Task) (bool, error) { + var statusUpToDate bool + var sourcesUpToDate bool + + statusChecker := f.statusChecker + if statusChecker == nil { + statusChecker = NewStatusChecker(f.logger) + } + sourcesChecker, err := f.resolveSourcesChecker(t) + if err != nil { + return false, err + } + + statusIsSet := len(t.Status) != 0 + sourcesIsSet := len(t.Sources) != 0 + + if statusIsSet { + statusUpToDate, err = statusChecker.IsUpToDate(ctx, t) + if err != nil { + return false, err + } + } + + if sourcesIsSet { + sourcesUpToDate, err = sourcesChecker.IsUpToDate(t) + if err != nil { + return false, err + } + } + + if statusIsSet && sourcesIsSet { + return statusUpToDate && sourcesUpToDate, nil + } + if statusIsSet { + return statusUpToDate, nil + } + if sourcesIsSet { + return sourcesUpToDate, nil + } + return false, nil +} + +// OnError lets the resolved sources checker clean up after a failed run. +func (f *Fingerprinter) OnError(t *ast.Task) error { + sourcesChecker, err := f.resolveSourcesChecker(t) + if err != nil { + return err + } + return sourcesChecker.OnError(t) +} + +// resolveSourcesChecker is the single place where a task is mapped to a checker. +func (f *Fingerprinter) resolveSourcesChecker(t *ast.Task) (SourcesCheckable, error) { + if f.sourcesChecker != nil { + return f.sourcesChecker, nil + } + return NewSourcesChecker(f.resolveMethod(t), f.tempDir, f.dry) +} diff --git a/internal/fingerprint/task_test.go b/internal/fingerprint/fingerprinter_test.go similarity index 68% rename from internal/fingerprint/task_test.go rename to internal/fingerprint/fingerprinter_test.go index 3452b19c91..f8451aa4e6 100644 --- a/internal/fingerprint/task_test.go +++ b/internal/fingerprint/fingerprinter_test.go @@ -1,7 +1,10 @@ package fingerprint import ( + "os" + "path/filepath" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -23,7 +26,7 @@ import ( // | false | not set | false | // | false | true | false | // | false | false | false | -func TestIsTaskUpToDate(t *testing.T) { +func TestFingerprinterUpToDate(t *testing.T) { t.Parallel() tests := []struct { @@ -162,14 +165,94 @@ func TestIsTaskUpToDate(t *testing.T) { tt.setupMockSourcesChecker(mockSourcesChecker) } - result, err := IsTaskUpToDate( - t.Context(), - tt.task, + f := NewFingerprinter("checksum", "", false, nil, WithStatusChecker(mockStatusChecker), WithSourcesChecker(mockSourcesChecker), ) + result, err := f.UpToDate(t.Context(), tt.task) require.NoError(t, err) assert.Equal(t, tt.expected, result) }) } } + +// The task's own method wins over the Taskfile default, for the injected +// variable as much as for the up-to-date check. +func TestFingerprinterMethodResolution(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + defaultMethod string + method string + expectedKind string + expectedValue any + }{ + { + name: "task method wins over the default", + defaultMethod: "checksum", + method: "timestamp", + expectedKind: "timestamp", + expectedValue: time.Time{}, + }, + { + name: "default method is inherited when the task declares none", + defaultMethod: "timestamp", + expectedKind: "timestamp", + expectedValue: time.Time{}, + }, + { + name: "checksum is inherited too", + defaultMethod: "checksum", + expectedKind: "checksum", + expectedValue: "", + }, + { + name: "none is inherited too", + defaultMethod: "none", + expectedKind: "none", + expectedValue: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "source.txt"), []byte("content"), 0o644)) + task := &ast.Task{ + Dir: dir, + Method: tt.method, + Sources: []*ast.Glob{{Glob: "source.txt"}}, + } + + f := NewFingerprinter(tt.defaultMethod, t.TempDir(), true, nil) + + assert.Equal(t, tt.expectedKind, f.Kind(task)) + + // A timestamp checker yields a time, the other two a string. + value, err := f.SourceValue(task) + require.NoError(t, err) + assert.IsType(t, tt.expectedValue, value) + }) + } +} + +// Only the entry points that need a checker reject an invalid method; Kind +// tolerates it, so that --force runs still compile. +func TestFingerprinterInvalidMethod(t *testing.T) { + t.Parallel() + + const wantErr = `task: invalid method "Checksum"` + task := &ast.Task{Sources: []*ast.Glob{{Glob: "source.txt"}}} + f := NewFingerprinter("Checksum", t.TempDir(), true, nil) + + assert.Equal(t, "checksum", f.Kind(task)) + + _, err := f.SourceValue(task) + require.ErrorIs(t, err, ErrInvalidMethod) + require.EqualError(t, err, wantErr) + _, err = f.UpToDate(t.Context(), task) + require.EqualError(t, err, wantErr) + require.EqualError(t, f.OnError(task), wantErr) +} diff --git a/internal/fingerprint/sources.go b/internal/fingerprint/sources.go index 34d3a04bee..fbc6af502c 100644 --- a/internal/fingerprint/sources.go +++ b/internal/fingerprint/sources.go @@ -1,6 +1,14 @@ package fingerprint -import "fmt" +import ( + "fmt" + + "github.com/go-task/task/v3/errors" +) + +// ErrInvalidMethod lets callers that only need a fingerprint value tell a bad +// method name apart from a checker failing on the sources themselves. +var ErrInvalidMethod = errors.New("invalid method") func NewSourcesChecker(method, tempDir string, dry bool) (SourcesCheckable, error) { switch method { @@ -11,6 +19,6 @@ func NewSourcesChecker(method, tempDir string, dry bool) (SourcesCheckable, erro case "none": return NoneChecker{}, nil default: - return nil, fmt.Errorf(`task: invalid method "%s"`, method) + return nil, fmt.Errorf(`task: %w "%s"`, ErrInvalidMethod, method) } } diff --git a/internal/fingerprint/task.go b/internal/fingerprint/task.go deleted file mode 100644 index 2b48e114c9..0000000000 --- a/internal/fingerprint/task.go +++ /dev/null @@ -1,132 +0,0 @@ -package fingerprint - -import ( - "context" - - "github.com/go-task/task/v3/internal/logger" - "github.com/go-task/task/v3/taskfile/ast" -) - -type ( - CheckerOption func(*CheckerConfig) - CheckerConfig struct { - method string - dry bool - tempDir string - logger *logger.Logger - statusChecker StatusCheckable - sourcesChecker SourcesCheckable - } -) - -func WithMethod(method string) CheckerOption { - return func(config *CheckerConfig) { - config.method = method - } -} - -func WithDry(dry bool) CheckerOption { - return func(config *CheckerConfig) { - config.dry = dry - } -} - -func WithTempDir(tempDir string) CheckerOption { - return func(config *CheckerConfig) { - config.tempDir = tempDir - } -} - -func WithLogger(logger *logger.Logger) CheckerOption { - return func(config *CheckerConfig) { - config.logger = logger - } -} - -func WithStatusChecker(checker StatusCheckable) CheckerOption { - return func(config *CheckerConfig) { - config.statusChecker = checker - } -} - -func WithSourcesChecker(checker SourcesCheckable) CheckerOption { - return func(config *CheckerConfig) { - config.sourcesChecker = checker - } -} - -func IsTaskUpToDate( - ctx context.Context, - t *ast.Task, - opts ...CheckerOption, -) (bool, error) { - var statusUpToDate bool - var sourcesUpToDate bool - var err error - - // Default config - config := &CheckerConfig{ - method: "none", - tempDir: "", - dry: false, - logger: nil, - statusChecker: nil, - sourcesChecker: nil, - } - - // Apply functional options - for _, opt := range opts { - opt(config) - } - - // If no status checker was given, set up the default one - if config.statusChecker == nil { - config.statusChecker = NewStatusChecker(config.logger) - } - - // If no sources checker was given, set up the default one - if config.sourcesChecker == nil { - config.sourcesChecker, err = NewSourcesChecker(config.method, config.tempDir, config.dry) - if err != nil { - return false, err - } - } - - statusIsSet := len(t.Status) != 0 - sourcesIsSet := len(t.Sources) != 0 - - // If status is set, check if it is up-to-date - if statusIsSet { - statusUpToDate, err = config.statusChecker.IsUpToDate(ctx, t) - if err != nil { - return false, err - } - } - - // If sources is set, check if they are up-to-date - if sourcesIsSet { - sourcesUpToDate, err = config.sourcesChecker.IsUpToDate(t) - if err != nil { - return false, err - } - } - - // If both status and sources are set, the task is up-to-date if both are up-to-date - if statusIsSet && sourcesIsSet { - return statusUpToDate && sourcesUpToDate, nil - } - - // If only status is set, the task is up-to-date if the status is up-to-date - if statusIsSet { - return statusUpToDate, nil - } - - // If only sources is set, the task is up-to-date if the sources are up-to-date - if sourcesIsSet { - return sourcesUpToDate, nil - } - - // If no status or sources are set, the task should always run - // i.e. it is never considered "up-to-date" - return false, nil -} diff --git a/status.go b/status.go index ae40f5ba5f..21fe861bb7 100644 --- a/status.go +++ b/status.go @@ -4,33 +4,18 @@ import ( "context" "fmt" - "github.com/go-task/task/v3/internal/fingerprint" "github.com/go-task/task/v3/taskfile/ast" ) // Status returns an error if any the of given tasks is not up-to-date func (e *Executor) Status(ctx context.Context, calls ...*Call) error { for _, call := range calls { - - // Compile the task t, err := e.CompiledTask(call) if err != nil { return err } - // Get the fingerprinting method to use - method := e.Taskfile.Method - if t.Method != "" { - method = t.Method - } - - // Check if the task is up-to-date - isUpToDate, err := fingerprint.IsTaskUpToDate(ctx, t, - fingerprint.WithMethod(method), - fingerprint.WithTempDir(e.TempDir.Fingerprint), - fingerprint.WithDry(e.Dry), - fingerprint.WithLogger(e.Logger), - ) + isUpToDate, err := e.fingerprinter().UpToDate(ctx, t) if err != nil { return err } @@ -42,13 +27,5 @@ func (e *Executor) Status(ctx context.Context, calls ...*Call) error { } func (e *Executor) statusOnError(t *ast.Task) error { - method := t.Method - if method == "" { - method = e.Taskfile.Method - } - checker, err := fingerprint.NewSourcesChecker(method, e.TempDir.Fingerprint, e.Dry) - if err != nil { - return err - } - return checker.OnError(t) + return e.fingerprinter().OnError(t) } diff --git a/task.go b/task.go index 98d340c976..482dc5cef7 100644 --- a/task.go +++ b/task.go @@ -15,7 +15,6 @@ import ( "github.com/go-task/task/v3/errors" "github.com/go-task/task/v3/internal/env" "github.com/go-task/task/v3/internal/execext" - "github.com/go-task/task/v3/internal/fingerprint" "github.com/go-task/task/v3/internal/logger" "github.com/go-task/task/v3/internal/output" "github.com/go-task/task/v3/internal/slicesext" @@ -221,17 +220,7 @@ func (e *Executor) RunTask(ctx context.Context, call *Call) error { return err } - // Get the fingerprinting method to use - method := e.Taskfile.Method - if t.Method != "" { - method = t.Method - } - upToDate, err := fingerprint.IsTaskUpToDate(ctx, t, - fingerprint.WithMethod(method), - fingerprint.WithTempDir(e.TempDir.Fingerprint), - fingerprint.WithDry(e.Dry), - fingerprint.WithLogger(e.Logger), - ) + upToDate, err := e.fingerprinter().UpToDate(ctx, t) if err != nil { return err } diff --git a/task_test.go b/task_test.go index b56930e77e..b651a14fd9 100644 --- a/task_test.go +++ b/task_test.go @@ -653,6 +653,81 @@ func TestStatusChecksumMissingGenerated(t *testing.T) { // nolint:paralleltest / require.NoError(t, err, "generated.txt should be recreated after third run") } +// The injected fingerprint variable follows the method the up-to-date check +// uses, including when that method comes from the Taskfile level. +func TestFingerprintVarMethod(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + dir string + executorOpts []task.ExecutorOption + wantErr string + assertOutput func(t *testing.T, output string) + }{ + { + name: "TIMESTAMP is injected when the method is inherited from the Taskfile", + dir: "testdata/method_taskfile_timestamp", + assertOutput: func(t *testing.T, output string) { + t.Helper() + // An unresolved variable renders as an empty string, so this + // has to match an actual timestamp, not just the prefix. + assert.Regexp(t, `ts=\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}`, output) + }, + }, + { + name: "no variable is injected when the effective method is none", + dir: "testdata/method_taskfile_none", + assertOutput: func(t *testing.T, output string) { + t.Helper() + assert.Contains(t, output, "cs=\n") + }, + }, + { + name: "an invalid method doesn't fail a run that skips fingerprinting", + dir: "testdata/method_invalid", + executorOpts: []task.ExecutorOption{task.WithForce(true)}, + assertOutput: func(t *testing.T, output string) { + t.Helper() + assert.Contains(t, output, "cs=[]\n") + }, + }, + { + name: "an invalid method is still reported by the up-to-date check", + dir: "testdata/method_invalid", + wantErr: `task: invalid method "checksums"`, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + _ = os.RemoveAll(filepathext.SmartJoin(tt.dir, ".task")) + + var buff bytes.Buffer + opts := append([]task.ExecutorOption{ + task.WithDir(tt.dir), + task.WithStdout(&buff), + task.WithStderr(&buff), + task.WithTempDir(task.TempDir{ + Remote: filepathext.SmartJoin(tt.dir, ".task"), + Fingerprint: filepathext.SmartJoin(tt.dir, ".task"), + }), + }, tt.executorOpts...) + e := task.NewExecutor(opts...) + require.NoError(t, e.Setup()) + + err := e.Run(t.Context(), &task.Call{Task: "build"}) + if tt.wantErr != "" { + require.ErrorContains(t, err, tt.wantErr) + return + } + require.NoError(t, err) + tt.assertOutput(t, buff.String()) + }) + } +} + func writeFile(t *testing.T, dir, name, content string) { t.Helper() require.NoError(t, os.WriteFile(filepathext.SmartJoin(dir, name), []byte(content), 0o644)) diff --git a/testdata/method_invalid/Taskfile.yml b/testdata/method_invalid/Taskfile.yml new file mode 100644 index 0000000000..443290b36e --- /dev/null +++ b/testdata/method_invalid/Taskfile.yml @@ -0,0 +1,9 @@ +version: '3' + +tasks: + build: + method: checksums # typo: not a valid method + cmds: + - echo "cs=[{{.CHECKSUM}}]" + sources: + - ./source.txt diff --git a/testdata/method_invalid/source.txt b/testdata/method_invalid/source.txt new file mode 100644 index 0000000000..587be6b4c3 --- /dev/null +++ b/testdata/method_invalid/source.txt @@ -0,0 +1 @@ +x diff --git a/testdata/method_taskfile_none/Taskfile.yml b/testdata/method_taskfile_none/Taskfile.yml new file mode 100644 index 0000000000..e212720cfa --- /dev/null +++ b/testdata/method_taskfile_none/Taskfile.yml @@ -0,0 +1,10 @@ +version: '3' + +method: none + +tasks: + build: + cmds: + - echo "cs={{.CHECKSUM}}" + sources: + - ./source.txt diff --git a/testdata/method_taskfile_none/source.txt b/testdata/method_taskfile_none/source.txt new file mode 100644 index 0000000000..5a18cd2fbf --- /dev/null +++ b/testdata/method_taskfile_none/source.txt @@ -0,0 +1 @@ +source diff --git a/testdata/method_taskfile_timestamp/Taskfile.yml b/testdata/method_taskfile_timestamp/Taskfile.yml new file mode 100644 index 0000000000..dc0f139e5d --- /dev/null +++ b/testdata/method_taskfile_timestamp/Taskfile.yml @@ -0,0 +1,10 @@ +version: '3' + +method: timestamp + +tasks: + build: + cmds: + - echo "ts={{.TIMESTAMP}}" + sources: + - ./source.txt diff --git a/testdata/method_taskfile_timestamp/source.txt b/testdata/method_taskfile_timestamp/source.txt new file mode 100644 index 0000000000..5a18cd2fbf --- /dev/null +++ b/testdata/method_taskfile_timestamp/source.txt @@ -0,0 +1 @@ +source diff --git a/variables.go b/variables.go index 900f87c0d9..1a30652201 100644 --- a/variables.go +++ b/variables.go @@ -208,25 +208,24 @@ func (e *Executor) compiledTask(call *Call, evaluateShVars bool) (*ast.Task, err } } - if len(origTask.Sources) > 0 && origTask.Method != "none" { - var checker fingerprint.SourcesCheckable - - if origTask.Method == "timestamp" { - checker = fingerprint.NewTimestampChecker(e.TempDir.Fingerprint, e.Dry) - } else { - checker = fingerprint.NewChecksumChecker(e.TempDir.Fingerprint, e.Dry) - } - - if origTask.ReferencesFingerprintVar(checker.Kind()) { - value, err := checker.Value(&new) - if err != nil { + if len(origTask.Sources) > 0 { + fingerprinter := e.fingerprinter() + kind := fingerprinter.Kind(&new) + if kind != "none" && origTask.ReferencesFingerprintVar(kind) { + // An invalid method must not fail compilation: --force skips + // fingerprinting altogether, and the up-to-date check reports it + // on every other path. + value, err := fingerprinter.SourceValue(&new) + if err != nil && !errors.Is(err, fingerprint.ErrInvalidMethod) { return nil, err } - vars.Set(strings.ToUpper(checker.Kind()), ast.Var{Live: value}) + if err == nil { + vars.Set(strings.ToUpper(kind), ast.Var{Live: value}) - // Adding new variables, requires us to refresh the templaters - // cache of the the values manually - cache.ResetCache() + // Adding new variables, requires us to refresh the templaters + // cache of the the values manually + cache.ResetCache() + } } }