Skip to content

Commit

Permalink
parallel filter inputs [tests failing]
Browse files Browse the repository at this point in the history
  • Loading branch information
Equanox committed Jan 4, 2023
1 parent 8c1d161 commit dee75b3
Show file tree
Hide file tree
Showing 10 changed files with 91 additions and 51 deletions.
4 changes: 2 additions & 2 deletions bob/aggregate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
5 changes: 5 additions & 0 deletions bob/playbook/build_internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
71 changes: 46 additions & 25 deletions bobtask/input.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package bobtask

import (
"fmt"
"log"
"os"
"path/filepath"
"sort"
Expand All @@ -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 {
Expand All @@ -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, "!")
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
20 changes: 10 additions & 10 deletions bobtask/map.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
15 changes: 7 additions & 8 deletions bobtask/sanitize.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"os"
"path/filepath"
"strings"
"sync"

"errors"
)
Expand Down Expand Up @@ -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.
Expand All @@ -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
}

Expand All @@ -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
}

Expand Down
6 changes: 3 additions & 3 deletions bobtask/target/target.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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...)
}
2 changes: 1 addition & 1 deletion pkg/filepathutil/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func listDir(path string) ([]string, error) {
return fs.SkipDir
}

// Skip dirs
// Append files
if !fi.IsDir() {
all = append(all, p)
}
Expand Down
2 changes: 2 additions & 0 deletions test/e2e/input/input_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -23,6 +24,7 @@ var (
)

var _ = BeforeSuite(func() {
boblog.SetLogLevel(10)
ctx = context.Background()

version = bob.Version
Expand Down
1 change: 1 addition & 0 deletions test/e2e/target/target_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ var (

var _ = BeforeSuite(func() {
boblog.SetLogLevel(10)

var err error
var storageDir string
dir, storageDir, cleanup, err = setup.TestDirs("target")
Expand Down
16 changes: 14 additions & 2 deletions test/e2e/target/target_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())

Expand All @@ -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())

Expand Down Expand Up @@ -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())

Expand All @@ -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())

Expand Down

0 comments on commit dee75b3

Please sign in to comment.