From 9d74dc24013b5887823275b0e7a568edcfeb821f Mon Sep 17 00:00:00 2001 From: Vincent Vanackere Date: Fri, 27 Sep 2024 15:20:00 +0200 Subject: [PATCH] Implement checksum validation for generates (alternative design) Avoid writing any state in IsUpToDate, instead return the current source state and also check after the task is run that the sources checksum has not been modified --- help.go | 2 +- internal/fingerprint/checker.go | 4 +- internal/fingerprint/sources_checksum.go | 44 ++++++------- internal/fingerprint/sources_none.go | 6 +- internal/fingerprint/sources_timestamp.go | 22 +++---- internal/fingerprint/task.go | 22 ++++--- internal/mocks/sources_checkable.go | 78 +++++++++++++---------- status.go | 2 +- task.go | 34 +++++----- 9 files changed, 110 insertions(+), 104 deletions(-) diff --git a/help.go b/help.go index 622ef17be5..4dbb29c1b8 100644 --- a/help.go +++ b/help.go @@ -187,7 +187,7 @@ func (e *Executor) ToEditorOutput(tasks []*ast.Task, noStatus bool) (*editors.Ta if tasks[i].Method != "" { method = tasks[i].Method } - upToDate, err := fingerprint.IsTaskUpToDate(context.Background(), tasks[i], + upToDate, _, err := fingerprint.IsTaskUpToDate(context.Background(), tasks[i], fingerprint.WithMethod(method), fingerprint.WithTempDir(e.TempDir.Fingerprint), fingerprint.WithDry(e.Dry), diff --git a/internal/fingerprint/checker.go b/internal/fingerprint/checker.go index 6b81a23e69..59b29a91e1 100644 --- a/internal/fingerprint/checker.go +++ b/internal/fingerprint/checker.go @@ -13,8 +13,8 @@ type StatusCheckable interface { // SourcesCheckable defines any type that can check if the sources of a task are up-to-date. type SourcesCheckable interface { - SetUpToDate(t *ast.Task) error - IsUpToDate(t *ast.Task) (bool, error) + SetUpToDate(t *ast.Task, sourceState string) error + IsUpToDate(t *ast.Task) (upToDate bool, sourceState string, err error) Value(t *ast.Task) (any, error) OnError(t *ast.Task) error Kind() string diff --git a/internal/fingerprint/sources_checksum.go b/internal/fingerprint/sources_checksum.go index 9cccd1f795..957f3f40e6 100644 --- a/internal/fingerprint/sources_checksum.go +++ b/internal/fingerprint/sources_checksum.go @@ -28,9 +28,9 @@ func NewChecksumChecker(tempDir string, dry bool) *ChecksumChecker { } } -func (checker *ChecksumChecker) IsUpToDate(t *ast.Task) (bool, error) { +func (checker *ChecksumChecker) IsUpToDate(t *ast.Task) (bool, string, error) { if len(t.Sources) == 0 && len(t.Generates) == 0 { - return false, nil + return false, "", nil } checksumFile := checker.checksumFilePath(t) @@ -41,27 +41,18 @@ func (checker *ChecksumChecker) IsUpToDate(t *ast.Task) (bool, error) { newSourcesHash, err := checker.checksum(t, t.Sources) if err != nil { - return false, err + return false, "", err } newGeneratesHash, err := checker.checksum(t, t.Generates) if err != nil { - return false, err + return false, "", err } - if !checker.dry && oldSourcesHash != newSourcesHash { - // make sure current sources hash are saved to file before executing the task, - // the proper "generated" hash will be saved after the task is executed - _ = os.MkdirAll(filepathext.SmartJoin(checker.tempDir, "checksum"), 0o755) - if err = os.WriteFile(checksumFile, []byte(newSourcesHash+"\n"+"_"), 0o644); err != nil { - return false, err - } - } - - return oldSourcesHash == newSourcesHash && oldGeneratesdHash == newGeneratesHash, nil + return oldSourcesHash == newSourcesHash && oldGeneratesdHash == newGeneratesHash, newSourcesHash, nil } -func (checker *ChecksumChecker) SetUpToDate(t *ast.Task) error { +func (checker *ChecksumChecker) SetUpToDate(t *ast.Task, sourceHash string) error { if len(t.Sources) == 0 && len(t.Generates) == 0 { return nil } @@ -70,27 +61,28 @@ func (checker *ChecksumChecker) SetUpToDate(t *ast.Task) error { return nil } - checksumFile := checker.checksumFilePath(t) - - data, _ := os.ReadFile(checksumFile) - oldHashes := strings.TrimSpace(string(data)) - oldSourcesHash, oldGeneratesdHash, _ := strings.Cut(oldHashes, "\n") - newSourcesHash, err := checker.checksum(t, t.Sources) if err != nil { return err } + checksumFile := checker.checksumFilePath(t) + + if sourceHash != "" && newSourcesHash != sourceHash { + // sources have changed since the task was executed, remove the checksum file + // since the next execution will have a different checksum + os.Remove(checksumFile) + return nil + } + newGeneratesHash, err := checker.checksum(t, t.Generates) if err != nil { return err } - if oldSourcesHash != newSourcesHash || oldGeneratesdHash != newGeneratesHash { - _ = os.MkdirAll(filepathext.SmartJoin(checker.tempDir, "checksum"), 0o755) - if err = os.WriteFile(checksumFile, []byte(oldSourcesHash+"\n"+newGeneratesHash+"\n"), 0o644); err != nil { - return err - } + _ = os.MkdirAll(filepathext.SmartJoin(checker.tempDir, "checksum"), 0o755) + if err = os.WriteFile(checksumFile, []byte(newSourcesHash+"\n"+newGeneratesHash+"\n"), 0o644); err != nil { + return err } return nil diff --git a/internal/fingerprint/sources_none.go b/internal/fingerprint/sources_none.go index 5832320715..589980df75 100644 --- a/internal/fingerprint/sources_none.go +++ b/internal/fingerprint/sources_none.go @@ -6,11 +6,11 @@ import "github.com/go-task/task/v3/taskfile/ast" // It will always report that the task is not up-to-date. type NoneChecker struct{} -func (NoneChecker) IsUpToDate(t *ast.Task) (bool, error) { - return false, nil +func (NoneChecker) IsUpToDate(t *ast.Task) (bool, string, error) { + return false, "", nil } -func (NoneChecker) SetUpToDate(t *ast.Task) error { +func (NoneChecker) SetUpToDate(t *ast.Task, sourceState string) error { return nil } diff --git a/internal/fingerprint/sources_timestamp.go b/internal/fingerprint/sources_timestamp.go index 6ee0bb01e5..4486f91021 100644 --- a/internal/fingerprint/sources_timestamp.go +++ b/internal/fingerprint/sources_timestamp.go @@ -23,18 +23,18 @@ func NewTimestampChecker(tempDir string, dry bool) *TimestampChecker { } // IsUpToDate implements the Checker interface -func (checker *TimestampChecker) IsUpToDate(t *ast.Task) (bool, error) { +func (checker *TimestampChecker) IsUpToDate(t *ast.Task) (bool, string, error) { if len(t.Sources) == 0 { - return false, nil + return false, "", nil } sources, err := Globs(t.Dir, t.Sources) if err != nil { - return false, nil + return false, "", nil } generates, err := Globs(t.Dir, t.Generates) if err != nil { - return false, nil + return false, "", nil } timestampFile := checker.timestampFilePath(t) @@ -48,11 +48,11 @@ func (checker *TimestampChecker) IsUpToDate(t *ast.Task) (bool, error) { // Create the timestamp file for the next execution when the file does not exist. if !checker.dry { if err := os.MkdirAll(filepath.Dir(timestampFile), 0o755); err != nil { - return false, err + return false, "", err } f, err := os.Create(timestampFile) if err != nil { - return false, err + return false, "", err } f.Close() } @@ -65,26 +65,26 @@ func (checker *TimestampChecker) IsUpToDate(t *ast.Task) (bool, error) { // Get the max time of the generates. generateMaxTime, err := getMaxTime(generates...) if err != nil || generateMaxTime.IsZero() { - return false, nil + return false, "", nil } // Check if any of the source files is newer than the max time of the generates. shouldUpdate, err := anyFileNewerThan(sources, generateMaxTime) if err != nil { - return false, nil + return false, "", nil } // Modify the metadata of the file to the the current time. if !checker.dry { if err := os.Chtimes(timestampFile, taskTime, taskTime); err != nil { - return false, err + return false, "", err } } - return !shouldUpdate, nil + return !shouldUpdate, "", nil } -func (checker *TimestampChecker) SetUpToDate(t *ast.Task) error { +func (checker *TimestampChecker) SetUpToDate(t *ast.Task, sourceState string) error { return nil // TODO: implement } diff --git a/internal/fingerprint/task.go b/internal/fingerprint/task.go index cc15e2e5a0..acdfefcb63 100644 --- a/internal/fingerprint/task.go +++ b/internal/fingerprint/task.go @@ -59,7 +59,7 @@ func IsTaskUpToDate( ctx context.Context, t *ast.Task, opts ...CheckerOption, -) (bool, error) { +) (bool, string, error) { var statusUpToDate bool var sourcesUpToDate bool var err error @@ -88,7 +88,7 @@ func IsTaskUpToDate( if config.sourcesChecker == nil { config.sourcesChecker, err = NewSourcesChecker(config.method, config.tempDir, config.dry) if err != nil { - return false, err + return false, "", err } } @@ -99,41 +99,43 @@ func IsTaskUpToDate( if statusIsSet { statusUpToDate, err = config.statusChecker.IsUpToDate(ctx, t) if err != nil { - return false, err + return false, "", err } } + sourcesState := "" // If sources is set, check if they are up-to-date if sourcesIsSet { - sourcesUpToDate, err = config.sourcesChecker.IsUpToDate(t) + sourcesUpToDate, sourcesState, err = config.sourcesChecker.IsUpToDate(t) if err != nil { - return false, err + 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 + return statusUpToDate && sourcesUpToDate, sourcesState, nil } // If only status is set, the task is up-to-date if the status is up-to-date if statusIsSet { - return statusUpToDate, nil + return statusUpToDate, sourcesState, nil } // If only sources is set, the task is up-to-date if the sources are up-to-date if sourcesIsSet { - return sourcesUpToDate, nil + return sourcesUpToDate, sourcesState, 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 + return false, sourcesState, nil } func SetTaskUpToDate( ctx context.Context, t *ast.Task, + sourceState string, opts ...CheckerOption, ) error { var err error @@ -166,5 +168,5 @@ func SetTaskUpToDate( } } - return config.sourcesChecker.SetUpToDate(t) + return config.sourcesChecker.SetUpToDate(t, sourceState) } diff --git a/internal/mocks/sources_checkable.go b/internal/mocks/sources_checkable.go index b5048ab8b6..1f25889a1a 100644 --- a/internal/mocks/sources_checkable.go +++ b/internal/mocks/sources_checkable.go @@ -22,7 +22,7 @@ func (_m *SourcesCheckable) EXPECT() *SourcesCheckable_Expecter { } // IsUpToDate provides a mock function with given fields: t -func (_m *SourcesCheckable) IsUpToDate(t *ast.Task) (bool, error) { +func (_m *SourcesCheckable) IsUpToDate(t *ast.Task) (bool, string, error) { ret := _m.Called(t) if len(ret) == 0 { @@ -30,8 +30,9 @@ func (_m *SourcesCheckable) IsUpToDate(t *ast.Task) (bool, error) { } var r0 bool - var r1 error - if rf, ok := ret.Get(0).(func(*ast.Task) (bool, error)); ok { + var r1 string + var r2 error + if rf, ok := ret.Get(0).(func(*ast.Task) (bool, string, error)); ok { return rf(t) } if rf, ok := ret.Get(0).(func(*ast.Task) bool); ok { @@ -40,13 +41,19 @@ func (_m *SourcesCheckable) IsUpToDate(t *ast.Task) (bool, error) { r0 = ret.Get(0).(bool) } - if rf, ok := ret.Get(1).(func(*ast.Task) error); ok { + if rf, ok := ret.Get(1).(func(*ast.Task) string); ok { r1 = rf(t) } else { - r1 = ret.Error(1) + r1 = ret.Get(1).(string) } - return r0, r1 + if rf, ok := ret.Get(2).(func(*ast.Task) error); ok { + r2 = rf(t) + } else { + r2 = ret.Error(2) + } + + return r0, r1, r2 } // SourcesCheckable_IsUpToDate_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'IsUpToDate' @@ -67,12 +74,12 @@ func (_c *SourcesCheckable_IsUpToDate_Call) Run(run func(t *ast.Task)) *SourcesC return _c } -func (_c *SourcesCheckable_IsUpToDate_Call) Return(_a0 bool, _a1 error) *SourcesCheckable_IsUpToDate_Call { - _c.Call.Return(_a0, _a1) +func (_c *SourcesCheckable_IsUpToDate_Call) Return(upToDate bool, sourceState string, err error) *SourcesCheckable_IsUpToDate_Call { + _c.Call.Return(upToDate, sourceState, err) return _c } -func (_c *SourcesCheckable_IsUpToDate_Call) RunAndReturn(run func(*ast.Task) (bool, error)) *SourcesCheckable_IsUpToDate_Call { +func (_c *SourcesCheckable_IsUpToDate_Call) RunAndReturn(run func(*ast.Task) (bool, string, error)) *SourcesCheckable_IsUpToDate_Call { _c.Call.Return(run) return _c } @@ -122,12 +129,12 @@ func (_c *SourcesCheckable_Kind_Call) RunAndReturn(run func() string) *SourcesCh return _c } -// SetUpToDate provides a mock function with given fields: t -func (_m *SourcesCheckable) SetUpToDate(t *ast.Task) error { +// OnError provides a mock function with given fields: t +func (_m *SourcesCheckable) OnError(t *ast.Task) error { ret := _m.Called(t) if len(ret) == 0 { - panic("no return value specified for SetUpToDate") + panic("no return value specified for OnError") } var r0 error @@ -140,45 +147,45 @@ func (_m *SourcesCheckable) SetUpToDate(t *ast.Task) error { return r0 } -// SourcesCheckable_SetUpToDate_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'SetUpToDate' -type SourcesCheckable_SetUpToDate_Call struct { +// SourcesCheckable_OnError_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'OnError' +type SourcesCheckable_OnError_Call struct { *mock.Call } -// SetUpToDate is a helper method to define mock.On call +// OnError is a helper method to define mock.On call // - t *ast.Task -func (_e *SourcesCheckable_Expecter) SetUpToDate(t interface{}) *SourcesCheckable_SetUpToDate_Call { - return &SourcesCheckable_SetUpToDate_Call{Call: _e.mock.On("SetUpToDate", t)} +func (_e *SourcesCheckable_Expecter) OnError(t interface{}) *SourcesCheckable_OnError_Call { + return &SourcesCheckable_OnError_Call{Call: _e.mock.On("OnError", t)} } -func (_c *SourcesCheckable_SetUpToDate_Call) Run(run func(t *ast.Task)) *SourcesCheckable_SetUpToDate_Call { +func (_c *SourcesCheckable_OnError_Call) Run(run func(t *ast.Task)) *SourcesCheckable_OnError_Call { _c.Call.Run(func(args mock.Arguments) { run(args[0].(*ast.Task)) }) return _c } -func (_c *SourcesCheckable_SetUpToDate_Call) Return(_a0 error) *SourcesCheckable_SetUpToDate_Call { +func (_c *SourcesCheckable_OnError_Call) Return(_a0 error) *SourcesCheckable_OnError_Call { _c.Call.Return(_a0) return _c } -func (_c *SourcesCheckable_SetUpToDate_Call) RunAndReturn(run func(*ast.Task) error) *SourcesCheckable_SetUpToDate_Call { +func (_c *SourcesCheckable_OnError_Call) RunAndReturn(run func(*ast.Task) error) *SourcesCheckable_OnError_Call { _c.Call.Return(run) return _c } -// OnError provides a mock function with given fields: t -func (_m *SourcesCheckable) OnError(t *ast.Task) error { - ret := _m.Called(t) +// SetUpToDate provides a mock function with given fields: t, sourceState +func (_m *SourcesCheckable) SetUpToDate(t *ast.Task, sourceState string) error { + ret := _m.Called(t, sourceState) if len(ret) == 0 { - panic("no return value specified for OnError") + panic("no return value specified for SetUpToDate") } var r0 error - if rf, ok := ret.Get(0).(func(*ast.Task) error); ok { - r0 = rf(t) + if rf, ok := ret.Get(0).(func(*ast.Task, string) error); ok { + r0 = rf(t, sourceState) } else { r0 = ret.Error(0) } @@ -186,30 +193,31 @@ func (_m *SourcesCheckable) OnError(t *ast.Task) error { return r0 } -// SourcesCheckable_OnError_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'OnError' -type SourcesCheckable_OnError_Call struct { +// SourcesCheckable_SetUpToDate_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'SetUpToDate' +type SourcesCheckable_SetUpToDate_Call struct { *mock.Call } -// OnError is a helper method to define mock.On call +// SetUpToDate is a helper method to define mock.On call // - t *ast.Task -func (_e *SourcesCheckable_Expecter) OnError(t interface{}) *SourcesCheckable_OnError_Call { - return &SourcesCheckable_OnError_Call{Call: _e.mock.On("OnError", t)} +// - sourceState string +func (_e *SourcesCheckable_Expecter) SetUpToDate(t interface{}, sourceState interface{}) *SourcesCheckable_SetUpToDate_Call { + return &SourcesCheckable_SetUpToDate_Call{Call: _e.mock.On("SetUpToDate", t, sourceState)} } -func (_c *SourcesCheckable_OnError_Call) Run(run func(t *ast.Task)) *SourcesCheckable_OnError_Call { +func (_c *SourcesCheckable_SetUpToDate_Call) Run(run func(t *ast.Task, sourceState string)) *SourcesCheckable_SetUpToDate_Call { _c.Call.Run(func(args mock.Arguments) { - run(args[0].(*ast.Task)) + run(args[0].(*ast.Task), args[1].(string)) }) return _c } -func (_c *SourcesCheckable_OnError_Call) Return(_a0 error) *SourcesCheckable_OnError_Call { +func (_c *SourcesCheckable_SetUpToDate_Call) Return(_a0 error) *SourcesCheckable_SetUpToDate_Call { _c.Call.Return(_a0) return _c } -func (_c *SourcesCheckable_OnError_Call) RunAndReturn(run func(*ast.Task) error) *SourcesCheckable_OnError_Call { +func (_c *SourcesCheckable_SetUpToDate_Call) RunAndReturn(run func(*ast.Task, string) error) *SourcesCheckable_SetUpToDate_Call { _c.Call.Return(run) return _c } diff --git a/status.go b/status.go index f18a60986d..32f0df78b1 100644 --- a/status.go +++ b/status.go @@ -25,7 +25,7 @@ func (e *Executor) Status(ctx context.Context, calls ...*ast.Call) error { } // Check if the task is up-to-date - isUpToDate, err := fingerprint.IsTaskUpToDate(ctx, t, + isUpToDate, _, err := fingerprint.IsTaskUpToDate(ctx, t, fingerprint.WithMethod(method), fingerprint.WithTempDir(e.TempDir.Fingerprint), fingerprint.WithDry(e.Dry), diff --git a/task.go b/task.go index 7f034e8dc4..b720bcf010 100644 --- a/task.go +++ b/task.go @@ -196,6 +196,8 @@ func (e *Executor) RunTask(ctx context.Context, call *ast.Call) error { return err } + var taskSuccess bool + skipFingerprinting := e.ForceAll || (!call.Indirect && e.Force) if !skipFingerprinting { if err := ctx.Err(); err != nil { @@ -217,7 +219,7 @@ func (e *Executor) RunTask(ctx context.Context, call *ast.Call) error { method = t.Method } - upToDate, err := fingerprint.IsTaskUpToDate(ctx, t, + upToDate, sourcesState, err := fingerprint.IsTaskUpToDate(ctx, t, fingerprint.WithMethod(method), fingerprint.WithTempDir(e.TempDir.Fingerprint), fingerprint.WithDry(e.Dry), @@ -227,6 +229,21 @@ func (e *Executor) RunTask(ctx context.Context, call *ast.Call) error { return err } + if !e.Dry { + defer func() { + if !taskSuccess { + return + } + e.Logger.VerboseErrf(logger.Magenta, "task: %q setting task up to date\n", call.Task) + fingerprint.SetTaskUpToDate(ctx, t, sourcesState, + fingerprint.WithMethod(method), + fingerprint.WithTempDir(e.TempDir.Fingerprint), + fingerprint.WithDry(e.Dry), + fingerprint.WithLogger(e.Logger), + ) + }() + } + if upToDate && preCondMet { if e.Verbose || (!call.Silent && !t.Silent && !e.Taskfile.Silent && !e.Silent) { e.Logger.Errf(logger.Magenta, "task: Task %q is up to date\n", t.Name()) @@ -278,20 +295,7 @@ func (e *Executor) RunTask(ctx context.Context, call *ast.Call) error { return &errors.TaskRunError{TaskName: t.Task, Err: err} } } - if !skipFingerprinting { - // Get the fingerprinting method to use - method := e.Taskfile.Method - if t.Method != "" { - method = t.Method - } - e.Logger.VerboseErrf(logger.Magenta, "task: %q setting task up to date\n", call.Task) - fingerprint.SetTaskUpToDate(ctx, t, - fingerprint.WithMethod(method), - fingerprint.WithTempDir(e.TempDir.Fingerprint), - fingerprint.WithDry(e.Dry), - fingerprint.WithLogger(e.Logger), - ) - } + taskSuccess = true e.Logger.VerboseErrf(logger.Magenta, "task: %q finished\n", call.Task) return nil })