Hotreloading (#294)

* add inspect buildinfo cmd; add content file hash to buildinfo file
* add file hash to artifact metadata
* verify of hash of file contents upon target verification
* check file by file when extracting the artifact or invalidating targets
* ignore node_modules/.cache
* remove file hash content on artifact meta
* if child task has changed check the target of the task
* fix symlink when it has changed file pointer after build
* assure input hash comparison is similar to input hash during build
* fix path for invalid file cleanup
* prevent missuse of cleanup
* Fix symlink path bug
* add input hash versioning

---------

Co-authored-by: equanox <[email protected]>
Co-authored-by: Tasos Papalyras <[email protected]>

.github/workflows/main.yml

@@ -27,7 +27,7 @@ jobs:
       - name: Install bob
         run: make install-prod
 
-      - name: bob login 
+      - name: bob login
         run: bob auth init --token ${{ secrets.BOB_TOKEN }}
 
       - name: Install nix derivations

bob/build.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/benchkram/errz"
 
+	"github.com/benchkram/bob/bob/bobfile"
 	"github.com/benchkram/bob/bob/playbook"
 )
 
@@ -45,3 +46,18 @@ func (b *B) Build(ctx context.Context, taskName string) (err error) {
 
 	return nil
 }
+
+// AggregateWithNixDeps does aggregation together with evaluating nix dependecies.
+// Nic dependencies are altering a tasks input hash.
+// Use this function for building `bob inspect` cmds.
+func (b *B) AggregateWithNixDeps(taskName string) (aggregate *bobfile.Bobfile, err error) {
+	defer errz.Recover(&err)
+
+	ag, err := b.Aggregate()
+	errz.Fatal(err)
+
+	err = b.nix.BuildNixDependenciesInPipeline(ag, taskName)
+	errz.Fatal(err)
+
+	return ag, nil
+}

bob/playbook/build_internal.go

@@ -49,15 +49,15 @@ func (p *Playbook) build(ctx context.Context, task *bobtask.Task) (pt *processed
 		}
 	}()
 
-	rebuildRequired, rebuildCause, err := p.TaskNeedsRebuild(task.TaskID, pt)
+	rebuild, err := p.TaskNeedsRebuild(task.TaskID)
 	errz.Fatal(err)
-	boblog.Log.V(2).Info(fmt.Sprintf("TaskNeedsRebuild [rebuildRequired: %t] [cause:%s]", rebuildRequired, rebuildCause))
+	boblog.Log.V(2).Info(fmt.Sprintf("TaskNeedsRebuild [rebuildRequired: %t] [cause:%s]", rebuild.IsRequired, rebuild.Cause))
 
 	// task might need a rebuild due to an input change.
 	// Could still be possible to load the targets from the artifact store.
 	// If a task needs a rebuild due to a dependency change => rebuild.
-	if rebuildRequired {
-		switch rebuildCause {
+	if rebuild.IsRequired {
+		switch rebuild.Cause {
 		case InputNotFoundInBuildInfo:
 			hashIn, err := task.HashIn()
 			errz.Fatal(err)
@@ -66,19 +66,19 @@ func (p *Playbook) build(ctx context.Context, task *bobtask.Task) (pt *processed
 			err = p.pullArtifact(ctx, hashIn, task, false)
 			errz.Fatal(err)
 
-			success, err := task.ArtifactExtract(hashIn)
+			success, err := task.ArtifactExtract(hashIn, rebuild.VerifyResult.InvalidFiles)
 			if err != nil {
 				// if local artifact is corrupted due to incomplete previous download, try a fresh download
 				if errors.Is(err, io.ErrUnexpectedEOF) {
 					err = p.pullArtifact(ctx, hashIn, task, true)
 					errz.Fatal(err)
-					success, err = task.ArtifactExtract(hashIn)
+					success, err = task.ArtifactExtract(hashIn, rebuild.VerifyResult.InvalidFiles)
 				}
 			}
 
 			errz.Fatal(err)
 			if success {
-				rebuildRequired = false
+				rebuild.IsRequired = false
 
 				// In case an artifact was synced from the remote store no buildinfo exists...
 				// To avoid subsequent artifact extraction the Buildinfo is created after
@@ -89,13 +89,13 @@ func (p *Playbook) build(ctx context.Context, task *bobtask.Task) (pt *processed
 				errz.Fatal(err)
 			}
 		case TargetInvalid:
-			boblog.Log.V(2).Info(fmt.Sprintf("%-*s\t%s, extracting artifact", p.namePad, coloredName, rebuildCause))
+			boblog.Log.V(2).Info(fmt.Sprintf("%-*s\t%s, extracting artifact", p.namePad, coloredName, rebuild.Cause))
 			hashIn, err := task.HashIn()
 			errz.Fatal(err)
-			success, err := task.ArtifactExtract(hashIn)
+			success, err := task.ArtifactExtract(hashIn, rebuild.VerifyResult.InvalidFiles)
 			errz.Fatal(err)
 			if success {
-				rebuildRequired = false
+				rebuild.IsRequired = false
 			}
 		case TargetNotInLocalStore:
 		case TaskForcedRebuild:
@@ -104,14 +104,14 @@ func (p *Playbook) build(ctx context.Context, task *bobtask.Task) (pt *processed
 		}
 	}
 
-	if !rebuildRequired {
+	if !rebuild.IsRequired {
 		status := StateNoRebuildRequired
 		boblog.Log.V(2).Info(fmt.Sprintf("%-*s\t%s", p.namePad, coloredName, status.Short()))
 		taskSuccessFul = true
 		return pt, p.TaskNoRebuildRequired(task.TaskID)
 	}
 
-	err = task.Clean()
+	err = task.Clean(rebuild.VerifyResult.InvalidFiles)
 	errz.Fatal(err)
 
 	err = task.Run(ctx, p.namePad)

bob/playbook/rebuild.go

@@ -5,73 +5,121 @@ import (
 	"fmt"
 
 	"github.com/benchkram/bob/bobtask"
-	"github.com/benchkram/bob/bobtask/processed"
+	"github.com/benchkram/bob/bobtask/target"
 	"github.com/benchkram/bob/pkg/boblog"
 	"github.com/benchkram/errz"
 )
 
+// RebuildInfo contains information about a task rebuild: if it's required and the cause for it
+type RebuildInfo struct {
+	// IsRequired tells if the task requires rebuild again
+	IsRequired bool
+	// Cause tells why the rebuild is required
+	Cause RebuildCause
+	// VerifyResult is the result of target filesystem verification
+	VerifyResult target.VerifyResult
+}
+
 // TaskNeedsRebuild check if a tasks need a rebuild by looking at its hash value
 // and its child tasks.
-func (p *Playbook) TaskNeedsRebuild(taskID int, pc *processed.Task) (rebuildRequired bool, cause RebuildCause, err error) {
+func (p *Playbook) TaskNeedsRebuild(taskID int) (rebuildInfo RebuildInfo, err error) {
 	task := p.TasksOptimized[taskID]
-
 	coloredName := task.ColoredName()
 
 	// Rebuild strategy set to `always`
 	if task.Rebuild() == bobtask.RebuildAlways {
 		boblog.Log.V(3).Info(fmt.Sprintf("%-*s\tNEEDS REBUILD\t(rebuild set to always)", p.namePad, coloredName))
-		return true, TaskForcedRebuild, nil
+
+		// For a forced rebuild all task targets are marked as invalid
+		invalidFiles := make(map[string][]target.Reason)
+		if task.TargetExists() {
+			t, err := task.Target()
+			errz.Fatal(err)
+			invalidFiles = t.AsInvalidFiles(target.ReasonForcedByNoCache)
+		}
+
+		return RebuildInfo{
+			IsRequired: true,
+			Cause:      TaskForcedRebuild,
+			VerifyResult: target.VerifyResult{
+				TargetIsValid: len(invalidFiles) > 0,
+				InvalidFiles:  invalidFiles,
+			},
+		}, nil
 	}
 
 	// Did a child task change?
-	if p.didChildTaskChange(task.Name(), p.namePad, coloredName) {
+	if p.didChildTaskChange(task.Name()) {
+		// Andrei Boar added this check.
+		// I'm unsure about the reason for it.
+		verifyResult := target.NewVerifyResult()
+		if task.TargetExists() {
+			tt, err := task.Target()
+			errz.Fatal(err)
+			verifyResult = tt.VerifyShallow()
+			if !verifyResult.TargetIsValid {
+				return RebuildInfo{IsRequired: true, Cause: TargetInvalid, VerifyResult: verifyResult}, nil
+			}
+		}
+
 		boblog.Log.V(3).Info(fmt.Sprintf("%-*s\tNEEDS REBUILD\t(dependecy changed)", p.namePad, coloredName))
-		return true, DependencyChanged, nil
+		return RebuildInfo{IsRequired: true, Cause: DependencyChanged}, nil
 	}
 
 	// Did the current task change?
 	// Indicating a cache miss in buildinfostore.
-	rebuildRequired, err = task.DidTaskChange()
+	rebuildRequired, err := task.DidTaskChange()
 	errz.Fatal(err)
 	if rebuildRequired {
 		boblog.Log.V(3).Info(fmt.Sprintf("%-*s\tNEEDS REBUILD\t(input changed)", p.namePad, coloredName))
-		return true, InputNotFoundInBuildInfo, nil
+
+		invalidFiles := make(map[string][]target.Reason)
+		if task.TargetExists() {
+			t, err := task.Target()
+			errz.Fatal(err)
+			invalidFiles = t.AsInvalidFiles(target.ReasonMissing)
+		}
+
+		return RebuildInfo{IsRequired: true, Cause: InputNotFoundInBuildInfo, VerifyResult: target.VerifyResult{
+			TargetIsValid: len(invalidFiles) > 0,
+			InvalidFiles:  invalidFiles,
+		}}, nil
 	}
 
 	// Check rebuild due to invalidated targets
 	target, err := task.Target()
 	if err != nil {
-		return true, "", err
+		return RebuildInfo{IsRequired: true, Cause: ""}, nil
 	}
 	if target != nil {
-		targetValid := target.VerifyShallow()
-		if !targetValid {
+		verifyResult := target.VerifyShallow()
+		if !verifyResult.TargetIsValid {
 			boblog.Log.V(3).Info(fmt.Sprintf("%-*s\tNEEDS REBUILD\t(invalid targets)", p.namePad, coloredName))
-			return true, TargetInvalid, nil
+			return RebuildInfo{IsRequired: true, Cause: TargetInvalid, VerifyResult: verifyResult}, nil
 		}
 
-		// Check if target exists in localstore
+		// Check if target exists in local store
 		hashIn, err := task.HashIn()
 		errz.Fatal(err)
 		if !task.ArtifactExists(hashIn) {
 			boblog.Log.V(3).Info(fmt.Sprintf("%-*s\tNEEDS REBUILD\t(target does not exist in localstore)", p.namePad, coloredName))
-			return true, TargetNotInLocalStore, nil
+			return RebuildInfo{IsRequired: true, Cause: TargetNotInLocalStore, VerifyResult: verifyResult}, nil
 		}
 	}
 
-	return false, "", err
+	return RebuildInfo{IsRequired: false}, err
 }
 
 // didChildTaskChange iterates through all child tasks to verify if any of them changed.
-func (p *Playbook) didChildTaskChange(taskname string, namePad int, coloredName string) bool {
+func (p *Playbook) didChildTaskChange(taskName string) bool {
 	var Done = fmt.Errorf("done")
-	err := p.Tasks.walk(taskname, func(tn string, t *Status, err error) error {
+	err := p.Tasks.walk(taskName, func(tn string, t *Status, err error) error {
 		if err != nil {
 			return err
 		}
 
 		// Ignore the task itself
-		if taskname == tn {
+		if taskName == tn {
 			return nil
 		}
 

bobtask/artifact_create.go

@@ -11,6 +11,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/benchkram/bob/bobtask/target"
 	"github.com/benchkram/errz"
 	"github.com/mholt/archiver/v3"
 	"gopkg.in/yaml.v3"
@@ -44,9 +45,9 @@ func (t *Task) ArtifactCreate(artifactName hash.In) (err error) {
 
 	boblog.Log.V(3).Info(fmt.Sprintf("[task:%s] creating artifact [%s] in localstore", t.name, artifactName))
 
-	target, err := t.Target()
+	tt, err := t.Target()
 	errz.Fatal(err)
-	buildInfo, err := target.BuildInfo()
+	buildInfo, err := tt.BuildInfo()
 
 	dockerTargets := []string{}
 	tempdir := ""
@@ -76,12 +77,19 @@ func (t *Task) ArtifactCreate(artifactName hash.In) (err error) {
 
 	// targets filesystem
 	for fname := range buildInfo.Filesystem.Files {
+		if target.ShouldIgnore(fname) {
+			continue
+		}
 		info, err := os.Lstat(fname)
 		errz.Fatal(err)
 
+		if info.IsDir() {
+			continue
+		}
+
 		// trim the tasks directory from the internal name
 		internalName := strings.TrimPrefix(fname, t.dir)
-		// saved docker images are temporarly stored in the tmp dir,
+		// saved docker images are temporarily stored in the tmp dir,
 		// this assures it's not added as prefix.
 		internalName = strings.TrimPrefix(internalName, os.TempDir())
 		internalName = strings.TrimPrefix(internalName, tempdir)

bobtask/artifact_extract.go

@@ -12,15 +12,19 @@ import (
 	"strings"
 
 	"github.com/benchkram/bob/bobtask/hash"
+	"github.com/benchkram/bob/bobtask/target"
 	"github.com/benchkram/bob/pkg/boblog"
 	"github.com/benchkram/errz"
 )
 
 // ArtifactExtract extract an artifact from the localstore if it exists.
 // Return true on a successful extract operation.
-func (t *Task) ArtifactExtract(artifactName hash.In) (success bool, err error) {
+func (t *Task) ArtifactExtract(artifactName hash.In, invalidFiles map[string][]target.Reason) (success bool, err error) {
 	defer errz.Recover(&err)
 
+	homeDir, err := os.UserHomeDir()
+	errz.Fatal(err)
+
 	artifact, _, err := t.local.GetArtifact(context.TODO(), artifactName.String())
 	if err != nil {
 		_, ok := err.(*fs.PathError)
@@ -32,7 +36,7 @@ func (t *Task) ArtifactExtract(artifactName hash.In) (success bool, err error) {
 	defer artifact.Close()
 
 	// Assure tasks is cleaned up before extracting
-	err = t.Clean()
+	err = t.Clean(invalidFiles)
 	errz.Fatal(err)
 
 	archiveReader := newArchiveReader()
@@ -56,7 +60,6 @@ func (t *Task) ArtifactExtract(artifactName hash.In) (success bool, err error) {
 
 		// targets filesystem
 		if strings.HasPrefix(header.Name, __targetsFilesystem) {
-
 			filename := strings.TrimPrefix(header.Name, __targetsFilesystem+"/")
 
 			// create directory structure
@@ -70,23 +73,31 @@ func (t *Task) ArtifactExtract(artifactName hash.In) (success bool, err error) {
 
 			// symlink
 			if archiveFile.FileInfo.Mode()&os.ModeSymlink == os.ModeSymlink {
+				if dst == "/" || dst == homeDir {
+					return false, fmt.Errorf("Cleanup of %s is not allowed", dst)
+				}
+				err = os.RemoveAll(dst)
+				errz.Fatal(err)
 				err = os.Symlink(header.Linkname, dst)
 				errz.Fatal(err)
 				continue
 			}
 
-			// extract to destination
-			f, err := os.OpenFile(dst, os.O_RDWR|os.O_CREATE|os.O_TRUNC, os.FileMode(header.Mode))
-			errz.Fatal(err)
-			_, err = io.Copy(f, archiveFile)
-			// closing the file right away to reduce the number of open files
-			_ = f.Close()
-			errz.Fatal(err)
+			if shouldFetchFromCache(filename, invalidFiles) {
+				// extract to destination
+				f, err := os.OpenFile(dst, os.O_RDWR|os.O_CREATE|os.O_TRUNC, os.FileMode(header.Mode))
+				errz.Fatal(err)
+
+				_, err = io.Copy(f, archiveFile)
+				errz.Fatal(err)
+
+				// closing the file right away to reduce the number of open files
+				_ = f.Close()
+			}
 		}
 
 		// targets docker
 		if strings.HasPrefix(header.Name, __targetsDocker) {
-
 			filename := strings.TrimPrefix(header.Name, __targetsDocker+"/")
 
 			// create directory structure
@@ -120,3 +131,17 @@ func (t *Task) ArtifactExtract(artifactName hash.In) (success bool, err error) {
 
 	return true, nil
 }
+
+// shouldFetchFromCache checks if a file should be brought back from cache inside the target
+// A file will be brought back from cache if it's missing or was changed
+func shouldFetchFromCache(filename string, invalidFiles map[string][]target.Reason) bool {
+	if _, ok := invalidFiles[filename]; !ok {
+		return false
+	}
+	for _, reason := range invalidFiles[filename] {
+		if reason == target.ReasonSizeChanged || reason == target.ReasonHashChanged || reason == target.ReasonMissing {
+			return true
+		}
+	}
+	return false
+}

bobtask/artifact_metadata.go

@@ -5,7 +5,6 @@ import (
 )
 
 type ArtifactMetadata struct {
-
 	// Project is the project in which the task is defined
 	Project string `yaml:"project,omitempty"`