From dee75b3f9691ad06aa219af89c032b56203a98dd Mon Sep 17 00:00:00 2001 From: equanox Date: Wed, 4 Jan 2023 12:01:24 +0100 Subject: [PATCH] parallel filter inputs [tests failing] --- bob/aggregate.go | 4 +- bob/playbook/build_internal.go | 5 ++ bobtask/input.go | 71 ++++++++++++++++++---------- bobtask/map.go | 20 ++++---- bobtask/sanitize.go | 15 +++--- bobtask/target/target.go | 6 +-- pkg/filepathutil/list.go | 2 +- test/e2e/input/input_suite_test.go | 2 + test/e2e/target/target_suite_test.go | 1 + test/e2e/target/target_test.go | 16 ++++++- 10 files changed, 91 insertions(+), 51 deletions(-) diff --git a/bob/aggregate.go b/bob/aggregate.go index a6f6b1a0..a415b768 100644 --- a/bob/aggregate.go +++ b/bob/aggregate.go @@ -224,8 +224,8 @@ func (b *B) Aggregate() (aggregate *bobfile.Bobfile, err error) { err = aggregate.BTasks.IgnoreChildTargets() errz.Fatal(err) - err = aggregate.BTasks.FilterInputs() - errz.Fatal(err) + // err = aggregate.BTasks.FilterInputs() + // errz.Fatal(err) return aggregate, nil } diff --git a/bob/playbook/build_internal.go b/bob/playbook/build_internal.go index 695fd193..19067e51 100644 --- a/bob/playbook/build_internal.go +++ b/bob/playbook/build_internal.go @@ -43,6 +43,11 @@ func (p *Playbook) build(ctx context.Context, task *bobtask.Task) (err error) { } }() + // Filter inputs populates the task input member by reading and validating + // inputs with the filesystem. + err = task.FilterInputs() + errz.Fatal(err) + rebuildRequired, rebuildCause, err := p.TaskNeedsRebuild(task.Name()) errz.Fatal(err) boblog.Log.V(2).Info(fmt.Sprintf("TaskNeedsRebuild [rebuildRequired: %t] [cause:%s]", rebuildRequired, rebuildCause)) diff --git a/bobtask/input.go b/bobtask/input.go index 13c7ee76..9e36a949 100644 --- a/bobtask/input.go +++ b/bobtask/input.go @@ -2,7 +2,6 @@ package bobtask import ( "fmt" - "log" "os" "path/filepath" "sort" @@ -11,6 +10,7 @@ import ( "github.com/benchkram/bob/bob/global" "github.com/benchkram/bob/pkg/file" "github.com/benchkram/bob/pkg/filepathutil" + "github.com/benchkram/errz" ) func (t *Task) Inputs() []string { @@ -28,34 +28,44 @@ var ( ) ) +func (t *Task) FilterInputs() (err error) { + defer errz.Recover(&err) + + inputs, err := t.FilteredInputs() + errz.Fatal(err) + t.inputs = inputs + + return nil +} + // filteredInputs returns inputs filtered by ignores and file targets. // Calls sanitize on the result. -func (t *Task) filteredInputs() ([]string, error) { +func (t *Task) FilteredInputs() ([]string, error) { - wd, err := filepath.Abs(t.dir) + wd, err := filepath.Abs(".") if err != nil { return nil, err } - owd, err := os.Getwd() - if err != nil { - return nil, fmt.Errorf("failed to get current working directory: %w", err) - } - if err := os.Chdir(wd); err != nil { - return nil, fmt.Errorf("failed to change current working directory to %s: %w", t.dir, err) - } - defer func() { - if err := os.Chdir(owd); err != nil { - log.Printf("failed to change current working directory back to %s: %v\n", owd, err) - } - }() - inputDirty := fmt.Sprintf("%s\n%s", t.InputDirty, defaultIgnores) + inputDirtyUnique := appendUnique([]string{}, split(inputDirty)...) + inputDirtyRooted := inputDirtyUnique + if t.dir != "." { + inputDirtyRooted = make([]string, len(inputDirtyUnique)) + for i, input := range inputDirtyUnique { + // keep ignored in tact + if strings.HasPrefix(input, "!") { + inputDirtyRooted[i] = "!" + filepath.Join(t.dir, strings.TrimPrefix(input, "!")) + continue + } + inputDirtyRooted[i] = filepath.Join(t.dir, input) + } + } // Determine inputs and files to be ignored var inputs []string var ignores []string - for _, input := range appendUnique([]string{}, split(inputDirty)...) { + for _, input := range inputDirtyRooted { // Ignore starts with ! if strings.HasPrefix(input, "!") { input = strings.TrimPrefix(input, "!") @@ -76,9 +86,10 @@ func (t *Task) filteredInputs() ([]string, error) { inputs = appendUnique(inputs, list...) } - // Also ignore file & dir targets stored in the same directory + // Ignore file & dir targets stored in the same directory if t.target != nil { - for _, path := range t.target.FilesystemEntriesRawPlain() { + + for _, path := range rooted(t.target.FilesystemEntriesRawPlain(), t.dir) { if file.Exists(path) { info, err := os.Stat(path) if err != nil { @@ -97,9 +108,9 @@ func (t *Task) filteredInputs() ([]string, error) { } } - // Also ignore additional ignores found during aggregation. + // Ignore additional items found during aggregation. // Usually the targets of child tasks. - for _, path := range t.InputAdditionalIgnores { + for _, path := range rooted(t.InputAdditionalIgnores, t.dir) { if file.Exists(path) { info, err := os.Stat(path) if err != nil { @@ -142,16 +153,26 @@ func (t *Task) filteredInputs() ([]string, error) { return nil, fmt.Errorf("failed to sanitize inputs: %w", err) } - sortedInputs := sanitizedInputs sort.Strings(sanitizedInputs) - // fmt.Println("Inputs:", inputs) + //fmt.Println(t.name) + //fmt.Println("Inputs:", inputs) // fmt.Println("Ignores:", ignores) // fmt.Println("Filtered:", filteredInputs) - // fmt.Println("Sanitized:", sanitizedInputs) + //fmt.Println("Sanitized:", sanitizedInputs) // fmt.Println("Sorted:", sortedInputs) - return sortedInputs, nil + return sanitizedInputs, nil +} + +func rooted(ss []string, prefix string) []string { + if prefix == "." { + return ss + } + for i, s := range ss { + ss[i] = filepath.Join(prefix, s) + } + return ss } func appendUnique(a []string, xx ...string) []string { diff --git a/bobtask/map.go b/bobtask/map.go index e2becd6c..c0b15d19 100644 --- a/bobtask/map.go +++ b/bobtask/map.go @@ -47,19 +47,19 @@ func (tm Map) Walk(root string, parentLevel string, fn func(taskname string, _ T return nil } -func (tm Map) FilterInputs() (err error) { - defer errz.Recover(&err) +// func (tm Map) FilterInputs() (err error) { +// defer errz.Recover(&err) - for key, task := range tm { - inputs, err := task.filteredInputs() - errz.Fatal(err) - task.inputs = inputs +// for key, task := range tm { +// inputs, err := task.FilteredInputs() +// errz.Fatal(err) +// task.inputs = inputs - tm[key] = task - } +// tm[key] = task +// } - return nil -} +// return nil +// } // Sanitize task map and write filtered & sanitized // properties from dirty members to plain (e.g. dirtyInputs -> filter&sanitize -> inputs) diff --git a/bobtask/sanitize.go b/bobtask/sanitize.go index 4523ce95..6a67eabc 100644 --- a/bobtask/sanitize.go +++ b/bobtask/sanitize.go @@ -6,6 +6,7 @@ import ( "os" "path/filepath" "strings" + "sync" "errors" ) @@ -60,9 +61,7 @@ type absolutePathOrError struct { } // absPathMap caches already resolved absolute paths. -// FIXME: in case of asynchronous calls this should be a sync map -// or use a lock. -var absPathMap = make(map[string]absolutePathOrError, 10000) +var absPathMap = sync.Map{} // resolve is a very basic implementation only preventing the inclusion of files outside of the project. // It is very likely still possible to include other files with malicious intention. @@ -84,8 +83,8 @@ func resolve(path string, opts optimisationOptions) (_ string, err error) { } - aoe, ok := absPathMap[abs] - if ok { + if aoe, ok := absPathMap.Load(abs); ok { + aoe := aoe.(absolutePathOrError) return aoe.abs, aoe.err } @@ -99,14 +98,14 @@ func resolve(path string, opts optimisationOptions) (_ string, err error) { sym, err := filepath.EvalSymlinks(abs) if err != nil { a := absolutePathOrError{"", fmt.Errorf("failed to follow symlink of %q: %w", abs, err)} - absPathMap[abs] = a + absPathMap.Store(abs, a) return a.abs, a.err } - absPathMap[abs] = absolutePathOrError{abs: sym, err: nil} + absPathMap.Store(abs, absolutePathOrError{abs: sym, err: nil}) return sym, nil } - absPathMap[abs] = absolutePathOrError{abs: abs, err: nil} + absPathMap.Store(abs, absolutePathOrError{abs: abs, err: nil}) return abs, nil } diff --git a/bobtask/target/target.go b/bobtask/target/target.go index b256ab59..0da64c0a 100644 --- a/bobtask/target/target.go +++ b/bobtask/target/target.go @@ -104,11 +104,11 @@ func (t *T) FilesystemEntriesRaw() []string { // FilesystemEntriesPlain does return the pure path // as given in the bobfile. func (t *T) FilesystemEntriesPlain() []string { - return *t.filesystemEntries + return append([]string{}, *t.filesystemEntries...) } func (t *T) FilesystemEntriesRawPlain() []string { - return t.filesystemEntriesRaw + return append([]string{}, t.filesystemEntriesRaw...) } func (t *T) WithExpected(expected *buildinfo.Targets) *T { @@ -117,5 +117,5 @@ func (t *T) WithExpected(expected *buildinfo.Targets) *T { } func (t *T) DockerImages() []string { - return t.dockerImages + return append([]string{}, t.dockerImages...) } diff --git a/pkg/filepathutil/list.go b/pkg/filepathutil/list.go index 997b9949..760328b8 100644 --- a/pkg/filepathutil/list.go +++ b/pkg/filepathutil/list.go @@ -98,7 +98,7 @@ func listDir(path string) ([]string, error) { return fs.SkipDir } - // Skip dirs + // Append files if !fi.IsDir() { all = append(all, p) } diff --git a/test/e2e/input/input_suite_test.go b/test/e2e/input/input_suite_test.go index 2f8b2d10..7f543fe3 100644 --- a/test/e2e/input/input_suite_test.go +++ b/test/e2e/input/input_suite_test.go @@ -9,6 +9,7 @@ import ( "github.com/benchkram/bob/bob" "github.com/benchkram/bob/bob/bobfile" + "github.com/benchkram/bob/pkg/boblog" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" ) @@ -23,6 +24,7 @@ var ( ) var _ = BeforeSuite(func() { + boblog.SetLogLevel(10) ctx = context.Background() version = bob.Version diff --git a/test/e2e/target/target_suite_test.go b/test/e2e/target/target_suite_test.go index c1097832..1cdae4a1 100644 --- a/test/e2e/target/target_suite_test.go +++ b/test/e2e/target/target_suite_test.go @@ -27,6 +27,7 @@ var ( var _ = BeforeSuite(func() { boblog.SetLogLevel(10) + var err error var storageDir string dir, storageDir, cleanup, err = setup.TestDirs("target") diff --git a/test/e2e/target/target_test.go b/test/e2e/target/target_test.go index 01ec9f78..90cd7fde 100644 --- a/test/e2e/target/target_test.go +++ b/test/e2e/target/target_test.go @@ -33,6 +33,9 @@ var _ = Describe("Test bob's file target handling", func() { task, ok := aggregate.BTasks[globaltaskname] Expect(ok).To(BeTrue()) + err = task.FilterInputs() + Expect(err).NotTo(HaveOccurred()) + hashIn, err := task.HashInAlways() Expect(err).NotTo(HaveOccurred()) @@ -57,6 +60,9 @@ var _ = Describe("Test bob's file target handling", func() { task, ok := aggregate.BTasks[globaltaskname] Expect(ok).To(BeTrue()) + err = task.FilterInputs() + Expect(err).NotTo(HaveOccurred()) + hashIn, err := task.HashInAlways() Expect(err).NotTo(HaveOccurred()) @@ -97,11 +103,14 @@ var _ = Describe("Test bob's file target handling", func() { globaltaskname := "second-level/third-level/build3" err = b.Nix().BuildNixDependenciesInPipeline(aggregate, globaltaskname) - errz.Fatal(err) + Expect(err).NotTo(HaveOccurred()) task, ok := aggregate.BTasks[globaltaskname] Expect(ok).To(BeTrue()) + err = task.FilterInputs() + Expect(err).NotTo(HaveOccurred()) + hashIn, err := task.HashIn() Expect(err).NotTo(HaveOccurred()) @@ -117,11 +126,14 @@ var _ = Describe("Test bob's file target handling", func() { globaltaskname := "second-level/third-level/print" err = b.Nix().BuildNixDependenciesInPipeline(aggregate, globaltaskname) - errz.Fatal(err) + Expect(err).NotTo(HaveOccurred()) task, ok := aggregate.BTasks[globaltaskname] Expect(ok).To(BeTrue()) + err = task.FilterInputs() + Expect(err).NotTo(HaveOccurred()) + hashIn, err := task.HashIn() Expect(err).NotTo(HaveOccurred())