From 29841e7dc4c2799a1a588385ab41c56ae315d7ad Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Thu, 26 May 2022 09:22:03 -0700 Subject: [PATCH 1/8] Use scope.Opts directly in RunOptions --- cli/internal/run/run.go | 64 ++++--------- cli/internal/run/run_test.go | 160 +++++++++++++++++-------------- cli/internal/scope/scope.go | 36 ++++--- cli/internal/scope/scope_test.go | 15 +-- 4 files changed, 133 insertions(+), 142 deletions(-) diff --git a/cli/internal/run/run.go b/cli/internal/run/run.go index 8f13ba2845247..95e464470ea3a 100644 --- a/cli/internal/run/run.go +++ b/cli/internal/run/run.go @@ -197,7 +197,7 @@ func (c *RunCommand) Run(args []string) int { return 1 } } - filteredPkgs, isAllPackages, err := scope.ResolvePackages(runOptions.scopeOpts(), scmInstance, ctx, c.Ui, c.Config.Logger) + filteredPkgs, isAllPackages, err := scope.ResolvePackages(&runOptions.scopeOpts, c.Config.Cwd.ToStringDuringMigration(), scmInstance, ctx, c.Ui, c.Config.Logger) if err != nil { c.logError(c.Config.Logger, "", fmt.Errorf("failed to resolve packages to run: %v", err)) } @@ -377,28 +377,13 @@ func buildTaskGraph(topoGraph *dag.AcyclicGraph, pipeline fs.Pipeline, rs *runSp } // RunOptions holds the current run operations configuration - type RunOptions struct { - // patterns supplied to --filter on the commandline - filterPatterns []string - // Whether to include dependent impacted consumers in execution (defaults to true) - includeDependents bool - // Whether to include includeDependencies (pkg.dependencies) in execution (defaults to false) - includeDependencies bool - // List of globs of file paths to ignore from execution scope calculation - ignore []string // Show a dot graph dotGraph string - // List of globs to global files whose contents will be included in the global hash calculation - globalDeps []string - // Filtered list of package entrypoints - scope []string // Force execution to be serially one-at-a-time concurrency int // Whether to execute in parallel (defaults to false) parallel bool - // Git diff used to calculate changed packages - since string // Current working directory cwd string // Whether to emit a perf profile @@ -413,36 +398,25 @@ type RunOptions struct { cacheOpts cache.Opts runcacheOpts runcache.Opts -} - -func (ro *RunOptions) scopeOpts() *scope.Opts { - return &scope.Opts{ - IncludeDependencies: ro.includeDependencies, - IncludeDependents: ro.includeDependents, - Patterns: ro.scope, - Since: ro.since, - Cwd: ro.cwd, - IgnorePatterns: ro.ignore, - GlobalDepPatterns: ro.globalDeps, - FilterPatterns: ro.filterPatterns, - } + scopeOpts scope.Opts } func getDefaultRunOptions(config *config.Config) *RunOptions { return &RunOptions{ - bail: true, - includeDependents: true, - parallel: false, - concurrency: 10, - dotGraph: "", - includeDependencies: false, - profile: "", // empty string does no tracing - only: false, + bail: true, + parallel: false, + concurrency: 10, + dotGraph: "", + profile: "", // empty string does no tracing + only: false, cacheOpts: cache.Opts{ Dir: cache.DefaultLocation(config.Cwd), Workers: config.Cache.Workers, }, + scopeOpts: scope.Opts{ + IncludeDependents: true, + }, } } @@ -474,23 +448,23 @@ func parseRunArgs(args []string, config *config.Config, output cli.Ui) (*RunOpti case strings.HasPrefix(arg, "--filter="): filterPattern := arg[len("--filter="):] if filterPattern != "" { - runOptions.filterPatterns = append(runOptions.filterPatterns, filterPattern) + runOptions.scopeOpts.FilterPatterns = append(runOptions.scopeOpts.FilterPatterns, filterPattern) } case strings.HasPrefix(arg, "--since="): if len(arg[len("--since="):]) > 0 { - runOptions.since = arg[len("--since="):] + runOptions.scopeOpts.Since = arg[len("--since="):] } case strings.HasPrefix(arg, "--scope="): if len(arg[len("--scope="):]) > 0 { - runOptions.scope = append(runOptions.scope, arg[len("--scope="):]) + runOptions.scopeOpts.Entrypoints = append(runOptions.scopeOpts.Entrypoints, arg[len("--scope="):]) } case strings.HasPrefix(arg, "--ignore="): if len(arg[len("--ignore="):]) > 0 { - runOptions.ignore = append(runOptions.ignore, arg[len("--ignore="):]) + runOptions.scopeOpts.IgnorePatterns = append(runOptions.scopeOpts.IgnorePatterns, arg[len("--ignore="):]) } case strings.HasPrefix(arg, "--global-deps="): if len(arg[len("--global-deps="):]) > 0 { - runOptions.globalDeps = append(runOptions.globalDeps, arg[len("--global-deps="):]) + runOptions.scopeOpts.GlobalDepPatterns = append(runOptions.scopeOpts.GlobalDepPatterns, arg[len("--global-deps="):]) } case strings.HasPrefix(arg, "--parallel"): runOptions.parallel = true @@ -502,7 +476,7 @@ func parseRunArgs(args []string, config *config.Config, output cli.Ui) (*RunOpti runOptions.profile = fmt.Sprintf("%v-profile.json", time.Now().UnixNano()) case strings.HasPrefix(arg, "--no-deps"): - runOptions.includeDependents = false + runOptions.scopeOpts.IncludeDependents = false case strings.HasPrefix(arg, "--no-cache"): runOptions.runcacheOpts.SkipWrites = true case strings.HasPrefix(arg, "--cacheFolder"): @@ -534,9 +508,9 @@ func parseRunArgs(args []string, config *config.Config, output cli.Ui) (*RunOpti } case strings.HasPrefix(arg, "--includeDependencies"): output.Warn("[WARNING] The --includeDependencies flag has renamed to --include-dependencies for consistency. Please use `--include-dependencies` instead") - runOptions.includeDependencies = true + runOptions.scopeOpts.IncludeDependencies = true case strings.HasPrefix(arg, "--include-dependencies"): - runOptions.includeDependencies = true + runOptions.scopeOpts.IncludeDependencies = true case strings.HasPrefix(arg, "--only"): runOptions.only = true case strings.HasPrefix(arg, "--output-logs="): diff --git a/cli/internal/run/run_test.go b/cli/internal/run/run_test.go index e7538c1af4729..3448a1e04c05f 100644 --- a/cli/internal/run/run_test.go +++ b/cli/internal/run/run_test.go @@ -13,6 +13,7 @@ import ( "github.com/vercel/turborepo/cli/internal/config" "github.com/vercel/turborepo/cli/internal/fs" "github.com/vercel/turborepo/cli/internal/runcache" + "github.com/vercel/turborepo/cli/internal/scope" "github.com/vercel/turborepo/cli/internal/util" "github.com/stretchr/testify/assert" @@ -33,103 +34,107 @@ func TestParseConfig(t *testing.T) { "string flags", []string{"foo"}, &RunOptions{ - includeDependents: true, - bail: true, - dotGraph: "", - concurrency: 10, - includeDependencies: false, - profile: "", - cwd: defaultCwd.ToStringDuringMigration(), + bail: true, + dotGraph: "", + concurrency: 10, + profile: "", + cwd: defaultCwd.ToStringDuringMigration(), cacheOpts: cache.Opts{ Dir: defaultCacheFolder, Workers: 10, }, runcacheOpts: runcache.Opts{}, + scopeOpts: scope.Opts{ + IncludeDependents: true, + }, }, }, { "scope", []string{"foo", "--scope=foo", "--scope=blah"}, &RunOptions{ - includeDependents: true, - bail: true, - dotGraph: "", - concurrency: 10, - includeDependencies: false, - profile: "", - scope: []string{"foo", "blah"}, - cwd: defaultCwd.ToStringDuringMigration(), + bail: true, + dotGraph: "", + concurrency: 10, + profile: "", + cwd: defaultCwd.ToStringDuringMigration(), cacheOpts: cache.Opts{ Dir: defaultCacheFolder, Workers: 10, }, runcacheOpts: runcache.Opts{}, + scopeOpts: scope.Opts{ + IncludeDependents: true, + Entrypoints: []string{"foo", "blah"}, + }, }, }, { "concurrency", []string{"foo", "--concurrency=12"}, &RunOptions{ - includeDependents: true, - bail: true, - dotGraph: "", - concurrency: 12, - includeDependencies: false, - profile: "", - cwd: defaultCwd.ToStringDuringMigration(), + bail: true, + dotGraph: "", + concurrency: 12, + profile: "", + cwd: defaultCwd.ToStringDuringMigration(), cacheOpts: cache.Opts{ Dir: defaultCacheFolder, Workers: 10, }, runcacheOpts: runcache.Opts{}, + scopeOpts: scope.Opts{ + IncludeDependents: true, + }, }, }, { "graph", []string{"foo", "--graph=g.png"}, &RunOptions{ - includeDependents: true, - bail: true, - dotGraph: "g.png", - concurrency: 10, - includeDependencies: false, - profile: "", - cwd: defaultCwd.ToStringDuringMigration(), + bail: true, + dotGraph: "g.png", + concurrency: 10, + profile: "", + cwd: defaultCwd.ToStringDuringMigration(), cacheOpts: cache.Opts{ Dir: defaultCacheFolder, Workers: 10, }, runcacheOpts: runcache.Opts{}, + scopeOpts: scope.Opts{ + IncludeDependents: true, + }, }, }, { "passThroughArgs", []string{"foo", "--graph=g.png", "--", "--boop", "zoop"}, &RunOptions{ - includeDependents: true, - bail: true, - dotGraph: "g.png", - concurrency: 10, - includeDependencies: false, - profile: "", - cwd: defaultCwd.ToStringDuringMigration(), - passThroughArgs: []string{"--boop", "zoop"}, + bail: true, + dotGraph: "g.png", + concurrency: 10, + profile: "", + cwd: defaultCwd.ToStringDuringMigration(), + passThroughArgs: []string{"--boop", "zoop"}, cacheOpts: cache.Opts{ Dir: defaultCacheFolder, Workers: 10, }, runcacheOpts: runcache.Opts{}, + scopeOpts: scope.Opts{ + IncludeDependents: true, + }, }, }, { "force", []string{"foo", "--force"}, &RunOptions{ - includeDependents: true, - bail: true, - concurrency: 10, - profile: "", - cwd: defaultCwd.ToStringDuringMigration(), + bail: true, + concurrency: 10, + profile: "", + cwd: defaultCwd.ToStringDuringMigration(), cacheOpts: cache.Opts{ Dir: defaultCacheFolder, Workers: 10, @@ -137,34 +142,38 @@ func TestParseConfig(t *testing.T) { runcacheOpts: runcache.Opts{ SkipReads: true, }, + scopeOpts: scope.Opts{ + IncludeDependents: true, + }, }, }, { "remote-only", []string{"foo", "--remote-only"}, &RunOptions{ - includeDependents: true, - bail: true, - concurrency: 10, - profile: "", - cwd: defaultCwd.ToStringDuringMigration(), + bail: true, + concurrency: 10, + profile: "", + cwd: defaultCwd.ToStringDuringMigration(), cacheOpts: cache.Opts{ Dir: defaultCacheFolder, Workers: 10, SkipFilesystem: true, }, runcacheOpts: runcache.Opts{}, + scopeOpts: scope.Opts{ + IncludeDependents: true, + }, }, }, { "no-cache", []string{"foo", "--no-cache"}, &RunOptions{ - includeDependents: true, - bail: true, - concurrency: 10, - profile: "", - cwd: defaultCwd.ToStringDuringMigration(), + bail: true, + concurrency: 10, + profile: "", + cwd: defaultCwd.ToStringDuringMigration(), cacheOpts: cache.Opts{ Dir: defaultCacheFolder, Workers: 10, @@ -172,41 +181,47 @@ func TestParseConfig(t *testing.T) { runcacheOpts: runcache.Opts{ SkipWrites: true, }, + scopeOpts: scope.Opts{ + IncludeDependents: true, + }, }, }, { "Empty passThroughArgs", []string{"foo", "--graph=g.png", "--"}, &RunOptions{ - includeDependents: true, - bail: true, - dotGraph: "g.png", - concurrency: 10, - includeDependencies: false, - profile: "", - cwd: defaultCwd.ToStringDuringMigration(), - passThroughArgs: []string{}, + bail: true, + dotGraph: "g.png", + concurrency: 10, + profile: "", + cwd: defaultCwd.ToStringDuringMigration(), + passThroughArgs: []string{}, cacheOpts: cache.Opts{ Dir: defaultCacheFolder, Workers: 10, }, runcacheOpts: runcache.Opts{}, + scopeOpts: scope.Opts{ + IncludeDependents: true, + }, }, }, { "can specify filter patterns", []string{"foo", "--filter=bar", "--filter=...[main]"}, &RunOptions{ - includeDependents: true, - filterPatterns: []string{"bar", "...[main]"}, - bail: true, - concurrency: 10, - cwd: defaultCwd.ToStringDuringMigration(), + bail: true, + concurrency: 10, + cwd: defaultCwd.ToStringDuringMigration(), cacheOpts: cache.Opts{ Dir: defaultCacheFolder, Workers: 10, }, runcacheOpts: runcache.Opts{}, + scopeOpts: scope.Opts{ + IncludeDependents: true, + FilterPatterns: []string{"bar", "...[main]"}, + }, }, }, } @@ -244,18 +259,19 @@ func TestParseRunOptionsUsesCWDFlag(t *testing.T) { } cwd := defaultCwd.Join("zop") expected := &RunOptions{ - includeDependents: true, - bail: true, - dotGraph: "", - concurrency: 10, - includeDependencies: false, - profile: "", - cwd: cwd.ToStringDuringMigration(), + bail: true, + dotGraph: "", + concurrency: 10, + profile: "", + cwd: cwd.ToStringDuringMigration(), cacheOpts: cache.Opts{ Dir: cwd.Join("node_modules", ".cache", "turbo"), Workers: 10, }, runcacheOpts: runcache.Opts{}, + scopeOpts: scope.Opts{ + IncludeDependents: true, + }, } ui := &cli.BasicUi{ diff --git a/cli/internal/scope/scope.go b/cli/internal/scope/scope.go index cedc1e76afd33..ce78c6c260ec5 100644 --- a/cli/internal/scope/scope.go +++ b/cli/internal/scope/scope.go @@ -16,14 +16,20 @@ import ( ) type Opts struct { + // IncludeDependencies is whether to include pkg.dependencies in execution (defaults to false) IncludeDependencies bool - IncludeDependents bool - Patterns []string - Since string - Cwd string - IgnorePatterns []string - GlobalDepPatterns []string - FilterPatterns []string + // IncludeDependents is whether to include dependent impacted consumers in execution (defaults to true) + IncludeDependents bool + // Entrypoints is a list of package entrypoints + Entrypoints []string + // Since is the git ref used to calculate changed packages + Since string + // IgnorePatterns is the list of globs of file paths to ignore from execution scope calculation + IgnorePatterns []string + // GlobalDepPatterns is a list of globs to global files whose contents will be included in the global hash calculation + GlobalDepPatterns []string + // Patterns are the filter patterns supplied to --filter on the commandline + FilterPatterns []string } // asFilterPatterns normalizes legacy selectors to filter syntax @@ -42,12 +48,12 @@ func (o *Opts) asFilterPatterns() []string { if o.Since != "" { since = fmt.Sprintf("[%v]", o.Since) } - if len(o.Patterns) > 0 { + if len(o.Entrypoints) > 0 { // --scope implies our tweaked syntax to see if any dependency matches if since != "" { since = "..." + since } - for _, pattern := range o.Patterns { + for _, pattern := range o.Entrypoints { if strings.HasPrefix(pattern, "!") { patterns = append(patterns, pattern) } else { @@ -66,12 +72,12 @@ func (o *Opts) asFilterPatterns() []string { // ResolvePackages translates specified flags to a set of entry point packages for // the selected tasks. Returns the selected packages and whether or not the selected // packages represents a default "all packages". -func ResolvePackages(opts *Opts, scm scm.SCM, ctx *context.Context, tui cli.Ui, logger hclog.Logger) (util.Set, bool, error) { +func ResolvePackages(opts *Opts, cwd string, scm scm.SCM, ctx *context.Context, tui cli.Ui, logger hclog.Logger) (util.Set, bool, error) { filterResolver := &scope_filter.Resolver{ Graph: &ctx.TopologicalGraph, PackageInfos: ctx.PackageInfos, - Cwd: opts.Cwd, - PackagesChangedSince: opts.getPackageChangeFunc(scm, ctx.PackageInfos), + Cwd: cwd, + PackagesChangedSince: opts.getPackageChangeFunc(scm, cwd, ctx.PackageInfos), } filterPatterns := opts.asFilterPatterns() isAllPackages := len(filterPatterns) == 0 @@ -90,7 +96,9 @@ func ResolvePackages(opts *Opts, scm scm.SCM, ctx *context.Context, tui cli.Ui, return filteredPkgs, isAllPackages, nil } -func (o *Opts) getPackageChangeFunc(scm scm.SCM, packageInfos map[interface{}]*fs.PackageJSON) scope_filter.PackagesChangedSince { +func (o *Opts) getPackageChangeFunc(scm scm.SCM, cwd string, packageInfos map[interface{}]*fs.PackageJSON) scope_filter.PackagesChangedSince { + // Note that "since" here is *not* o.Since. Each filter expression can have its own value for + // "since", apart from the value specified via --since. return func(since string) (util.Set, error) { // We could filter changed files at the git level, since it's possible // that the changes we're interested in are scoped, but we need to handle @@ -98,7 +106,7 @@ func (o *Opts) getPackageChangeFunc(scm scm.SCM, packageInfos map[interface{}]*f // scope changed files more deeply if we know there are no global dependencies. var changedFiles []string if since != "" { - scmChangedFiles, err := scm.ChangedFiles(since, true, o.Cwd) + scmChangedFiles, err := scm.ChangedFiles(since, true, cwd) if err != nil { return nil, err } diff --git a/cli/internal/scope/scope_test.go b/cli/internal/scope/scope_test.go index 05e99df5f76b4..9881ce5480ca7 100644 --- a/cli/internal/scope/scope_test.go +++ b/cli/internal/scope/scope_test.go @@ -3,7 +3,6 @@ package scope import ( "fmt" "reflect" - "strings" "testing" "github.com/hashicorp/go-hclog" @@ -18,14 +17,8 @@ type mockSCM struct { changed []string } -func (m *mockSCM) ChangedFiles(fromCommit string, includeUntracked bool, relativeTo string) ([]string, error) { - changed := []string{} - for _, change := range m.changed { - if strings.HasPrefix(change, relativeTo) { - changed = append(changed, change) - } - } - return changed, nil +func (m *mockSCM) ChangedFiles(_fromCommit string, _includeUntracked bool, _relativeTo string) ([]string, error) { + return m.changed, nil } func TestResolvePackages(t *testing.T) { @@ -210,13 +203,13 @@ func TestResolvePackages(t *testing.T) { changed: tc.changed, } pkgs, isAllPackages, err := ResolvePackages(&Opts{ - Patterns: tc.scope, + Entrypoints: tc.scope, Since: tc.since, IgnorePatterns: []string{tc.ignore}, GlobalDepPatterns: tc.globalDeps, IncludeDependencies: tc.includeDependencies, IncludeDependents: tc.includeDependents, - }, scm, &context.Context{ + }, "dummy-repo-root", scm, &context.Context{ PackageInfos: packagesInfos, PackageNames: packageNames, TopologicalGraph: graph, From 25f43fb78ec3804852bf865363a2906f318cdbd2 Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Thu, 26 May 2022 09:43:56 -0700 Subject: [PATCH 2/8] flip boolean for dependents to avoid a default: true in our options struct --- cli/internal/run/run.go | 6 ++--- cli/internal/run/run_test.go | 42 +++++++++----------------------- cli/internal/scope/scope.go | 6 ++--- cli/internal/scope/scope_test.go | 2 +- 4 files changed, 17 insertions(+), 39 deletions(-) diff --git a/cli/internal/run/run.go b/cli/internal/run/run.go index 95e464470ea3a..4437dfbcb8cc4 100644 --- a/cli/internal/run/run.go +++ b/cli/internal/run/run.go @@ -414,9 +414,7 @@ func getDefaultRunOptions(config *config.Config) *RunOptions { Dir: cache.DefaultLocation(config.Cwd), Workers: config.Cache.Workers, }, - scopeOpts: scope.Opts{ - IncludeDependents: true, - }, + scopeOpts: scope.Opts{}, } } @@ -476,7 +474,7 @@ func parseRunArgs(args []string, config *config.Config, output cli.Ui) (*RunOpti runOptions.profile = fmt.Sprintf("%v-profile.json", time.Now().UnixNano()) case strings.HasPrefix(arg, "--no-deps"): - runOptions.scopeOpts.IncludeDependents = false + runOptions.scopeOpts.SkipDependents = true case strings.HasPrefix(arg, "--no-cache"): runOptions.runcacheOpts.SkipWrites = true case strings.HasPrefix(arg, "--cacheFolder"): diff --git a/cli/internal/run/run_test.go b/cli/internal/run/run_test.go index 3448a1e04c05f..928945e9b03ad 100644 --- a/cli/internal/run/run_test.go +++ b/cli/internal/run/run_test.go @@ -44,9 +44,7 @@ func TestParseConfig(t *testing.T) { Workers: 10, }, runcacheOpts: runcache.Opts{}, - scopeOpts: scope.Opts{ - IncludeDependents: true, - }, + scopeOpts: scope.Opts{}, }, }, { @@ -64,8 +62,7 @@ func TestParseConfig(t *testing.T) { }, runcacheOpts: runcache.Opts{}, scopeOpts: scope.Opts{ - IncludeDependents: true, - Entrypoints: []string{"foo", "blah"}, + Entrypoints: []string{"foo", "blah"}, }, }, }, @@ -83,9 +80,7 @@ func TestParseConfig(t *testing.T) { Workers: 10, }, runcacheOpts: runcache.Opts{}, - scopeOpts: scope.Opts{ - IncludeDependents: true, - }, + scopeOpts: scope.Opts{}, }, }, { @@ -102,9 +97,7 @@ func TestParseConfig(t *testing.T) { Workers: 10, }, runcacheOpts: runcache.Opts{}, - scopeOpts: scope.Opts{ - IncludeDependents: true, - }, + scopeOpts: scope.Opts{}, }, }, { @@ -122,9 +115,7 @@ func TestParseConfig(t *testing.T) { Workers: 10, }, runcacheOpts: runcache.Opts{}, - scopeOpts: scope.Opts{ - IncludeDependents: true, - }, + scopeOpts: scope.Opts{}, }, }, { @@ -142,9 +133,7 @@ func TestParseConfig(t *testing.T) { runcacheOpts: runcache.Opts{ SkipReads: true, }, - scopeOpts: scope.Opts{ - IncludeDependents: true, - }, + scopeOpts: scope.Opts{}, }, }, { @@ -161,9 +150,7 @@ func TestParseConfig(t *testing.T) { SkipFilesystem: true, }, runcacheOpts: runcache.Opts{}, - scopeOpts: scope.Opts{ - IncludeDependents: true, - }, + scopeOpts: scope.Opts{}, }, }, { @@ -181,9 +168,7 @@ func TestParseConfig(t *testing.T) { runcacheOpts: runcache.Opts{ SkipWrites: true, }, - scopeOpts: scope.Opts{ - IncludeDependents: true, - }, + scopeOpts: scope.Opts{}, }, }, { @@ -201,9 +186,7 @@ func TestParseConfig(t *testing.T) { Workers: 10, }, runcacheOpts: runcache.Opts{}, - scopeOpts: scope.Opts{ - IncludeDependents: true, - }, + scopeOpts: scope.Opts{}, }, }, { @@ -219,8 +202,7 @@ func TestParseConfig(t *testing.T) { }, runcacheOpts: runcache.Opts{}, scopeOpts: scope.Opts{ - IncludeDependents: true, - FilterPatterns: []string{"bar", "...[main]"}, + FilterPatterns: []string{"bar", "...[main]"}, }, }, }, @@ -269,9 +251,7 @@ func TestParseRunOptionsUsesCWDFlag(t *testing.T) { Workers: 10, }, runcacheOpts: runcache.Opts{}, - scopeOpts: scope.Opts{ - IncludeDependents: true, - }, + scopeOpts: scope.Opts{}, } ui := &cli.BasicUi{ diff --git a/cli/internal/scope/scope.go b/cli/internal/scope/scope.go index ce78c6c260ec5..9a398c1e2e087 100644 --- a/cli/internal/scope/scope.go +++ b/cli/internal/scope/scope.go @@ -18,8 +18,8 @@ import ( type Opts struct { // IncludeDependencies is whether to include pkg.dependencies in execution (defaults to false) IncludeDependencies bool - // IncludeDependents is whether to include dependent impacted consumers in execution (defaults to true) - IncludeDependents bool + // SkipDependents is whether to skip dependent impacted consumers in execution (defaults to false) + SkipDependents bool // Entrypoints is a list of package entrypoints Entrypoints []string // Since is the git ref used to calculate changed packages @@ -37,7 +37,7 @@ func (o *Opts) asFilterPatterns() []string { patterns := make([]string, len(o.FilterPatterns)) copy(patterns, o.FilterPatterns) prefix := "" - if o.IncludeDependents { + if !o.SkipDependents { prefix = "..." } suffix := "" diff --git a/cli/internal/scope/scope_test.go b/cli/internal/scope/scope_test.go index 9881ce5480ca7..051f2673aa74e 100644 --- a/cli/internal/scope/scope_test.go +++ b/cli/internal/scope/scope_test.go @@ -208,7 +208,7 @@ func TestResolvePackages(t *testing.T) { IgnorePatterns: []string{tc.ignore}, GlobalDepPatterns: tc.globalDeps, IncludeDependencies: tc.includeDependencies, - IncludeDependents: tc.includeDependents, + SkipDependents: !tc.includeDependents, }, "dummy-repo-root", scm, &context.Context{ PackageInfos: packagesInfos, PackageNames: packageNames, From 303816b7c7c354f8eac8787b619e08f175a9bd52 Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Thu, 26 May 2022 10:15:45 -0700 Subject: [PATCH 3/8] Move legacy package selection options into their own struct --- cli/internal/run/run.go | 10 +++++----- cli/internal/run/run_test.go | 4 +++- cli/internal/scope/scope.go | 30 +++++++++++++++++++----------- cli/internal/scope/scope_test.go | 14 ++++++++------ 4 files changed, 35 insertions(+), 23 deletions(-) diff --git a/cli/internal/run/run.go b/cli/internal/run/run.go index 4437dfbcb8cc4..5f2febedf6885 100644 --- a/cli/internal/run/run.go +++ b/cli/internal/run/run.go @@ -450,11 +450,11 @@ func parseRunArgs(args []string, config *config.Config, output cli.Ui) (*RunOpti } case strings.HasPrefix(arg, "--since="): if len(arg[len("--since="):]) > 0 { - runOptions.scopeOpts.Since = arg[len("--since="):] + runOptions.scopeOpts.LegacyFilter.Since = arg[len("--since="):] } case strings.HasPrefix(arg, "--scope="): if len(arg[len("--scope="):]) > 0 { - runOptions.scopeOpts.Entrypoints = append(runOptions.scopeOpts.Entrypoints, arg[len("--scope="):]) + runOptions.scopeOpts.LegacyFilter.Entrypoints = append(runOptions.scopeOpts.LegacyFilter.Entrypoints, arg[len("--scope="):]) } case strings.HasPrefix(arg, "--ignore="): if len(arg[len("--ignore="):]) > 0 { @@ -474,7 +474,7 @@ func parseRunArgs(args []string, config *config.Config, output cli.Ui) (*RunOpti runOptions.profile = fmt.Sprintf("%v-profile.json", time.Now().UnixNano()) case strings.HasPrefix(arg, "--no-deps"): - runOptions.scopeOpts.SkipDependents = true + runOptions.scopeOpts.LegacyFilter.SkipDependents = true case strings.HasPrefix(arg, "--no-cache"): runOptions.runcacheOpts.SkipWrites = true case strings.HasPrefix(arg, "--cacheFolder"): @@ -506,9 +506,9 @@ func parseRunArgs(args []string, config *config.Config, output cli.Ui) (*RunOpti } case strings.HasPrefix(arg, "--includeDependencies"): output.Warn("[WARNING] The --includeDependencies flag has renamed to --include-dependencies for consistency. Please use `--include-dependencies` instead") - runOptions.scopeOpts.IncludeDependencies = true + runOptions.scopeOpts.LegacyFilter.IncludeDependencies = true case strings.HasPrefix(arg, "--include-dependencies"): - runOptions.scopeOpts.IncludeDependencies = true + runOptions.scopeOpts.LegacyFilter.IncludeDependencies = true case strings.HasPrefix(arg, "--only"): runOptions.only = true case strings.HasPrefix(arg, "--output-logs="): diff --git a/cli/internal/run/run_test.go b/cli/internal/run/run_test.go index 928945e9b03ad..69110291ebb50 100644 --- a/cli/internal/run/run_test.go +++ b/cli/internal/run/run_test.go @@ -62,7 +62,9 @@ func TestParseConfig(t *testing.T) { }, runcacheOpts: runcache.Opts{}, scopeOpts: scope.Opts{ - Entrypoints: []string{"foo", "blah"}, + LegacyFilter: scope.LegacyFilter{ + Entrypoints: []string{"foo", "blah"}, + }, }, }, }, diff --git a/cli/internal/scope/scope.go b/cli/internal/scope/scope.go index 9a398c1e2e087..03550bce4d416 100644 --- a/cli/internal/scope/scope.go +++ b/cli/internal/scope/scope.go @@ -15,7 +15,9 @@ import ( "github.com/vercel/turborepo/cli/internal/util/filter" ) -type Opts struct { +// LegacyFilter holds the options in use before the filter syntax. They have their own rules +// for how they are compiled into filter expressions. +type LegacyFilter struct { // IncludeDependencies is whether to include pkg.dependencies in execution (defaults to false) IncludeDependencies bool // SkipDependents is whether to skip dependent impacted consumers in execution (defaults to false) @@ -24,6 +26,11 @@ type Opts struct { Entrypoints []string // Since is the git ref used to calculate changed packages Since string +} + +// Opts holds the options for how to select the entrypoint packages for a turbo run +type Opts struct { + LegacyFilter LegacyFilter // IgnorePatterns is the list of globs of file paths to ignore from execution scope calculation IgnorePatterns []string // GlobalDepPatterns is a list of globs to global files whose contents will be included in the global hash calculation @@ -33,27 +40,26 @@ type Opts struct { } // asFilterPatterns normalizes legacy selectors to filter syntax -func (o *Opts) asFilterPatterns() []string { - patterns := make([]string, len(o.FilterPatterns)) - copy(patterns, o.FilterPatterns) +func (l *LegacyFilter) asFilterPatterns() []string { + var patterns []string prefix := "" - if !o.SkipDependents { + if !l.SkipDependents { prefix = "..." } suffix := "" - if o.IncludeDependencies { + if l.IncludeDependencies { suffix = "..." } since := "" - if o.Since != "" { - since = fmt.Sprintf("[%v]", o.Since) + if l.Since != "" { + since = fmt.Sprintf("[%v]", l.Since) } - if len(o.Entrypoints) > 0 { + if len(l.Entrypoints) > 0 { // --scope implies our tweaked syntax to see if any dependency matches if since != "" { since = "..." + since } - for _, pattern := range o.Entrypoints { + for _, pattern := range l.Entrypoints { if strings.HasPrefix(pattern, "!") { patterns = append(patterns, pattern) } else { @@ -79,7 +85,9 @@ func ResolvePackages(opts *Opts, cwd string, scm scm.SCM, ctx *context.Context, Cwd: cwd, PackagesChangedSince: opts.getPackageChangeFunc(scm, cwd, ctx.PackageInfos), } - filterPatterns := opts.asFilterPatterns() + filterPatterns := opts.FilterPatterns + legacyFilterPatterns := opts.LegacyFilter.asFilterPatterns() + filterPatterns = append(filterPatterns, legacyFilterPatterns...) isAllPackages := len(filterPatterns) == 0 filteredPkgs, err := filterResolver.GetPackagesFromPatterns(filterPatterns) if err != nil { diff --git a/cli/internal/scope/scope_test.go b/cli/internal/scope/scope_test.go index 051f2673aa74e..91545a7f93e13 100644 --- a/cli/internal/scope/scope_test.go +++ b/cli/internal/scope/scope_test.go @@ -203,12 +203,14 @@ func TestResolvePackages(t *testing.T) { changed: tc.changed, } pkgs, isAllPackages, err := ResolvePackages(&Opts{ - Entrypoints: tc.scope, - Since: tc.since, - IgnorePatterns: []string{tc.ignore}, - GlobalDepPatterns: tc.globalDeps, - IncludeDependencies: tc.includeDependencies, - SkipDependents: !tc.includeDependents, + LegacyFilter: LegacyFilter{ + Entrypoints: tc.scope, + Since: tc.since, + IncludeDependencies: tc.includeDependencies, + SkipDependents: !tc.includeDependents, + }, + IgnorePatterns: []string{tc.ignore}, + GlobalDepPatterns: tc.globalDeps, }, "dummy-repo-root", scm, &context.Context{ PackageInfos: packagesInfos, PackageNames: packageNames, From 5c1e29213c3f0f9897926edd4ebecddc509e5982 Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Thu, 26 May 2022 10:26:36 -0700 Subject: [PATCH 4/8] Drop cwd from RunOptions --- cli/internal/context/context.go | 3 ++- cli/internal/fs/path.go | 5 ++++ cli/internal/prune/prune.go | 2 +- cli/internal/run/run.go | 41 ++++++++------------------------- cli/internal/run/run_test.go | 11 --------- 5 files changed, 18 insertions(+), 44 deletions(-) diff --git a/cli/internal/context/context.go b/cli/internal/context/context.go index 52de52fe245dd..f9c6b72ebfaab 100644 --- a/cli/internal/context/context.go +++ b/cli/internal/context/context.go @@ -123,8 +123,9 @@ func isWorkspaceReference(packageVersion string, dependencyVersion string, cwd s // WithGraph attaches information about the package dependency graph to the Context instance being // constructed. -func WithGraph(rootpath string, config *config.Config, cacheDir fs.AbsolutePath) Option { +func WithGraph(config *config.Config, cacheDir fs.AbsolutePath) Option { return func(c *Context) error { + rootpath := config.Cwd.ToStringDuringMigration() c.PackageInfos = make(map[interface{}]*fs.PackageJSON) c.RootNode = core.ROOT_NODE_NAME diff --git a/cli/internal/fs/path.go b/cli/internal/fs/path.go index e797a755ebbb8..5c0e0af2199ff 100644 --- a/cli/internal/fs/path.go +++ b/cli/internal/fs/path.go @@ -99,6 +99,11 @@ func (ap AbsolutePath) Create() (*os.File, error) { return os.Create(ap.asString()) } +// Ext implements filepath.Ext for an absolute path +func (ap AbsolutePath) Ext() string { + return filepath.Ext(ap.asString()) +} + // ToString returns the string representation of this absolute path. Used for // interfacing with APIs that require a string func (ap AbsolutePath) ToString() string { diff --git a/cli/internal/prune/prune.go b/cli/internal/prune/prune.go index 9aa253d626407..bdb65772710fb 100644 --- a/cli/internal/prune/prune.go +++ b/cli/internal/prune/prune.go @@ -92,7 +92,7 @@ func (c *PruneCommand) Run(args []string) int { return 1 } cacheDir := cache.DefaultLocation(c.Config.Cwd) - ctx, err := context.New(context.WithGraph(pruneOptions.cwd, c.Config, cacheDir)) + ctx, err := context.New(context.WithGraph(c.Config, cacheDir)) if err != nil { c.logError(c.Config.Logger, "", fmt.Errorf("could not construct graph: %w", err)) diff --git a/cli/internal/run/run.go b/cli/internal/run/run.go index 5f2febedf6885..3387f5401928a 100644 --- a/cli/internal/run/run.go +++ b/cli/internal/run/run.go @@ -1,7 +1,6 @@ package run import ( - "bufio" gocontext "context" "encoding/json" "flag" @@ -170,7 +169,7 @@ func (c *RunCommand) Run(args []string) int { return 1 } - ctx, err := context.New(context.WithGraph(runOptions.cwd, c.Config, runOptions.cacheOpts.Dir)) + ctx, err := context.New(context.WithGraph(c.Config, runOptions.cacheOpts.Dir)) if err != nil { c.logError(c.Config.Logger, "", err) return 1 @@ -188,7 +187,7 @@ func (c *RunCommand) Run(args []string) int { return 1 } - scmInstance, err := scm.FromInRepo(runOptions.cwd) + scmInstance, err := scm.FromInRepo(c.Config.Cwd.ToStringDuringMigration()) if err != nil { if errors.Is(err, scm.ErrFallback) { c.logWarning(c.Config.Logger, "", err) @@ -268,7 +267,7 @@ func (c *RunCommand) runOperation(g *completeGraph, rs *runSpec, packageManager exitCode := 0 if rs.Opts.dotGraph != "" { - err := c.generateDotGraph(engine.TaskGraph, filepath.Join(rs.Opts.cwd, rs.Opts.dotGraph)) + err := c.generateDotGraph(engine.TaskGraph, c.Config.Cwd.Join(rs.Opts.dotGraph)) if err != nil { c.logError(c.Config.Logger, "", err) return 1 @@ -384,8 +383,6 @@ type RunOptions struct { concurrency int // Whether to execute in parallel (defaults to false) parallel bool - // Current working directory - cwd string // Whether to emit a perf profile profile string // Immediately exit on task failure @@ -425,9 +422,7 @@ func parseRunArgs(args []string, config *config.Config, output cli.Ui) (*RunOpti return nil, errors.Errorf("At least one task must be specified.") } - runOptions.cwd = config.Cwd.ToStringDuringMigration() var unresolvedCacheFolder string - // unresolvedCacheFolder := filepath.FromSlash("./node_modules/.cache/turbo") if os.Getenv("TURBO_FORCE") == "true" { runOptions.runcacheOpts.SkipReads = true @@ -756,22 +751,6 @@ func commandLooksLikeTurbo(command string) bool { return _isTurbo.MatchString(command) } -// Replay logs will try to replay logs back to the stdout -func replayLogs(logger hclog.Logger, output cli.Ui, runOptions *RunOptions, logFileName, hash string) { - logger.Debug("start replaying logs") - f, err := os.Open(filepath.Join(runOptions.cwd, logFileName)) - if err != nil { - output.Warn(fmt.Sprintf("error reading logs: %v", err)) - logger.Error(fmt.Sprintf("error reading logs: %v", err.Error())) - } - defer f.Close() - scan := bufio.NewScanner(f) - for scan.Scan() { - output.Output(string(scan.Bytes())) //Writing to Stdout - } - logger.Debug("finish replaying logs") -} - // GetTargetsFromArguments returns a list of targets from the arguments and Turbo config. // Return targets are always unique sorted alphabetically. func getTargetsFromArguments(arguments []string, pipeline fs.Pipeline) ([]string, error) { @@ -950,14 +929,14 @@ func (e *execContext) exec(pt *nodes.PackageTask, deps dag.Set) error { return nil } -func (c *RunCommand) generateDotGraph(taskGraph *dag.AcyclicGraph, outputFilename string) error { +func (c *RunCommand) generateDotGraph(taskGraph *dag.AcyclicGraph, outputFilename fs.AbsolutePath) error { graphString := string(taskGraph.Dot(&dag.DotOpts{ Verbose: true, DrawCycles: true, })) - ext := filepath.Ext(outputFilename) + ext := outputFilename.Ext() if ext == ".html" { - f, err := os.Create(outputFilename) + f, err := outputFilename.Create() if err != nil { return fmt.Errorf("error writing graph: %w", err) } @@ -978,22 +957,22 @@ func (c *RunCommand) generateDotGraph(taskGraph *dag.AcyclicGraph, outputFilenam `) c.Ui.Output("") - c.Ui.Output(fmt.Sprintf("✔ Generated task graph in %s", ui.Bold(outputFilename))) + c.Ui.Output(fmt.Sprintf("✔ Generated task graph in %s", ui.Bold(outputFilename.ToString()))) if ui.IsTTY { - browser.OpenBrowser(outputFilename) + browser.OpenBrowser(outputFilename.ToString()) } return nil } hasDot := hasGraphViz() if hasDot { - dotArgs := []string{"-T" + ext[1:], "-o", outputFilename} + dotArgs := []string{"-T" + ext[1:], "-o", outputFilename.ToString()} cmd := exec.Command("dot", dotArgs...) cmd.Stdin = strings.NewReader(graphString) if err := cmd.Run(); err != nil { return fmt.Errorf("could not generate task graphfile %v: %w", outputFilename, err) } else { c.Ui.Output("") - c.Ui.Output(fmt.Sprintf("✔ Generated task graph in %s", ui.Bold(outputFilename))) + c.Ui.Output(fmt.Sprintf("✔ Generated task graph in %s", ui.Bold(outputFilename.ToString()))) } } else { c.Ui.Output("") diff --git a/cli/internal/run/run_test.go b/cli/internal/run/run_test.go index 69110291ebb50..748ecf66d0234 100644 --- a/cli/internal/run/run_test.go +++ b/cli/internal/run/run_test.go @@ -38,7 +38,6 @@ func TestParseConfig(t *testing.T) { dotGraph: "", concurrency: 10, profile: "", - cwd: defaultCwd.ToStringDuringMigration(), cacheOpts: cache.Opts{ Dir: defaultCacheFolder, Workers: 10, @@ -55,7 +54,6 @@ func TestParseConfig(t *testing.T) { dotGraph: "", concurrency: 10, profile: "", - cwd: defaultCwd.ToStringDuringMigration(), cacheOpts: cache.Opts{ Dir: defaultCacheFolder, Workers: 10, @@ -76,7 +74,6 @@ func TestParseConfig(t *testing.T) { dotGraph: "", concurrency: 12, profile: "", - cwd: defaultCwd.ToStringDuringMigration(), cacheOpts: cache.Opts{ Dir: defaultCacheFolder, Workers: 10, @@ -93,7 +90,6 @@ func TestParseConfig(t *testing.T) { dotGraph: "g.png", concurrency: 10, profile: "", - cwd: defaultCwd.ToStringDuringMigration(), cacheOpts: cache.Opts{ Dir: defaultCacheFolder, Workers: 10, @@ -110,7 +106,6 @@ func TestParseConfig(t *testing.T) { dotGraph: "g.png", concurrency: 10, profile: "", - cwd: defaultCwd.ToStringDuringMigration(), passThroughArgs: []string{"--boop", "zoop"}, cacheOpts: cache.Opts{ Dir: defaultCacheFolder, @@ -127,7 +122,6 @@ func TestParseConfig(t *testing.T) { bail: true, concurrency: 10, profile: "", - cwd: defaultCwd.ToStringDuringMigration(), cacheOpts: cache.Opts{ Dir: defaultCacheFolder, Workers: 10, @@ -145,7 +139,6 @@ func TestParseConfig(t *testing.T) { bail: true, concurrency: 10, profile: "", - cwd: defaultCwd.ToStringDuringMigration(), cacheOpts: cache.Opts{ Dir: defaultCacheFolder, Workers: 10, @@ -162,7 +155,6 @@ func TestParseConfig(t *testing.T) { bail: true, concurrency: 10, profile: "", - cwd: defaultCwd.ToStringDuringMigration(), cacheOpts: cache.Opts{ Dir: defaultCacheFolder, Workers: 10, @@ -181,7 +173,6 @@ func TestParseConfig(t *testing.T) { dotGraph: "g.png", concurrency: 10, profile: "", - cwd: defaultCwd.ToStringDuringMigration(), passThroughArgs: []string{}, cacheOpts: cache.Opts{ Dir: defaultCacheFolder, @@ -197,7 +188,6 @@ func TestParseConfig(t *testing.T) { &RunOptions{ bail: true, concurrency: 10, - cwd: defaultCwd.ToStringDuringMigration(), cacheOpts: cache.Opts{ Dir: defaultCacheFolder, Workers: 10, @@ -247,7 +237,6 @@ func TestParseRunOptionsUsesCWDFlag(t *testing.T) { dotGraph: "", concurrency: 10, profile: "", - cwd: cwd.ToStringDuringMigration(), cacheOpts: cache.Opts{ Dir: cwd.Join("node_modules", ".cache", "turbo"), Workers: 10, From c9acaf74242afe23350b9e25eae2d4c9cbc3bca3 Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Thu, 26 May 2022 10:47:00 -0700 Subject: [PATCH 5/8] Encapsulate runOpts. This part could use future refactoring --- cli/internal/run/run.go | 153 ++++++++++++++++++----------------- cli/internal/run/run_test.go | 118 ++++++++++++++------------- 2 files changed, 140 insertions(+), 131 deletions(-) diff --git a/cli/internal/run/run.go b/cli/internal/run/run.go index 3387f5401928a..1f728aeddf28e 100644 --- a/cli/internal/run/run.go +++ b/cli/internal/run/run.go @@ -64,14 +64,14 @@ type completeGraph struct { type runSpec struct { Targets []string FilteredPkgs util.Set - Opts *RunOptions + Opts *Opts } func (rs *runSpec) ArgsForTask(task string) []string { - passThroughArgs := make([]string, 0, len(rs.Opts.passThroughArgs)) + passThroughArgs := make([]string, 0, len(rs.Opts.runOpts.passThroughArgs)) for _, target := range rs.Targets { if target == task { - passThroughArgs = append(passThroughArgs, rs.Opts.passThroughArgs...) + passThroughArgs = append(passThroughArgs, rs.Opts.runOpts.passThroughArgs...) } } return passThroughArgs @@ -163,13 +163,13 @@ func (c *RunCommand) Run(args []string) int { return 1 } - runOptions, err := parseRunArgs(args, c.Config, c.Ui) + opts, err := parseRunArgs(args, c.Config, c.Ui) if err != nil { c.logError(c.Config.Logger, "", err) return 1 } - ctx, err := context.New(context.WithGraph(c.Config, runOptions.cacheOpts.Dir)) + ctx, err := context.New(context.WithGraph(c.Config, opts.cacheOpts.Dir)) if err != nil { c.logError(c.Config.Logger, "", err) return 1 @@ -196,7 +196,7 @@ func (c *RunCommand) Run(args []string) int { return 1 } } - filteredPkgs, isAllPackages, err := scope.ResolvePackages(&runOptions.scopeOpts, c.Config.Cwd.ToStringDuringMigration(), scmInstance, ctx, c.Ui, c.Config.Logger) + filteredPkgs, isAllPackages, err := scope.ResolvePackages(&opts.scopeOpts, c.Config.Cwd.ToStringDuringMigration(), scmInstance, ctx, c.Ui, c.Config.Logger) if err != nil { c.logError(c.Config.Logger, "", fmt.Errorf("failed to resolve packages to run: %v", err)) } @@ -212,7 +212,7 @@ func (c *RunCommand) Run(args []string) int { } } c.Config.Logger.Debug("global hash", "value", ctx.GlobalHash) - c.Config.Logger.Debug("local cache folder", "path", runOptions.cacheOpts.Dir) + c.Config.Logger.Debug("local cache folder", "path", opts.cacheOpts.Dir) // TODO: consolidate some of these arguments g := &completeGraph{ @@ -225,7 +225,7 @@ func (c *RunCommand) Run(args []string) int { rs := &runSpec{ Targets: targets, FilteredPkgs: filteredPkgs, - Opts: runOptions, + Opts: opts, } packageManager := ctx.PackageManager return c.runOperation(g, rs, packageManager, startAt) @@ -243,7 +243,7 @@ func (c *RunCommand) runOperation(g *completeGraph, rs *runSpec, packageManager return 1 } hashTracker := taskhash.NewTracker(g.RootNode, g.GlobalHash, g.Pipeline, g.PackageInfos) - err = hashTracker.CalculateFileHashes(engine.TaskGraph.Vertices(), rs.Opts.concurrency, c.Config.Cwd) + err = hashTracker.CalculateFileHashes(engine.TaskGraph.Vertices(), rs.Opts.runOpts.concurrency, c.Config.Cwd) if err != nil { c.Ui.Error(fmt.Sprintf("Error hashing package files: %s", err)) return 1 @@ -252,7 +252,7 @@ func (c *RunCommand) runOperation(g *completeGraph, rs *runSpec, packageManager // If we are running in parallel, then we remove all the edges in the graph // except for the root. Rebuild the task graph for backwards compatibility. // We still use dependencies specified by the pipeline configuration. - if rs.Opts.parallel { + if rs.Opts.runOpts.parallel { for _, edge := range g.TopologicalGraph.Edges() { if edge.Target() != g.RootNode { g.TopologicalGraph.RemoveEdge(edge) @@ -266,13 +266,13 @@ func (c *RunCommand) runOperation(g *completeGraph, rs *runSpec, packageManager } exitCode := 0 - if rs.Opts.dotGraph != "" { - err := c.generateDotGraph(engine.TaskGraph, c.Config.Cwd.Join(rs.Opts.dotGraph)) + if rs.Opts.runOpts.dotGraph != "" { + err := c.generateDotGraph(engine.TaskGraph, c.Config.Cwd.Join(rs.Opts.runOpts.dotGraph)) if err != nil { c.logError(c.Config.Logger, "", err) return 1 } - } else if rs.Opts.dryRun { + } else if rs.Opts.runOpts.dryRun { tasksRun, err := c.executeDryRun(engine, g, hashTracker, rs) if err != nil { c.logError(c.Config.Logger, "", err) @@ -280,7 +280,7 @@ func (c *RunCommand) runOperation(g *completeGraph, rs *runSpec, packageManager } packagesInScope := rs.FilteredPkgs.UnsafeListOfStrings() sort.Strings(packagesInScope) - if rs.Opts.dryRunJSON { + if rs.Opts.runOpts.dryRunJSON { dryRun := &struct { Packages []string `json:"packages"` Tasks []hashedTask `json:"tasks"` @@ -363,7 +363,7 @@ func buildTaskGraph(topoGraph *dag.AcyclicGraph, pipeline fs.Pipeline, rs *runSp if err := engine.Prepare(&core.SchedulerExecutionOptions{ Packages: rs.FilteredPkgs.UnsafeListOfStrings(), TaskNames: rs.Targets, - TasksOnly: rs.Opts.only, + TasksOnly: rs.Opts.runOpts.only, }); err != nil { return nil, err } @@ -375,8 +375,16 @@ func buildTaskGraph(topoGraph *dag.AcyclicGraph, pipeline fs.Pipeline, rs *runSp return engine, nil } -// RunOptions holds the current run operations configuration -type RunOptions struct { +// Opts holds the current run operations configuration +type Opts struct { + runOpts runOpts + cacheOpts cache.Opts + runcacheOpts runcache.Opts + scopeOpts scope.Opts +} + +// runOpts holds the options that control the execution of a turbo run +type runOpts struct { // Show a dot graph dotGraph string // Force execution to be serially one-at-a-time @@ -392,21 +400,14 @@ type RunOptions struct { only bool dryRun bool dryRunJSON bool - - cacheOpts cache.Opts - runcacheOpts runcache.Opts - scopeOpts scope.Opts } -func getDefaultRunOptions(config *config.Config) *RunOptions { - return &RunOptions{ - bail: true, - parallel: false, - concurrency: 10, - dotGraph: "", - profile: "", // empty string does no tracing - only: false, - +func getDefaultOptions(config *config.Config) *Opts { + return &Opts{ + runOpts: runOpts{ + bail: true, + concurrency: 10, + }, cacheOpts: cache.Opts{ Dir: cache.DefaultLocation(config.Cwd), Workers: config.Cache.Workers, @@ -415,8 +416,8 @@ func getDefaultRunOptions(config *config.Config) *RunOptions { } } -func parseRunArgs(args []string, config *config.Config, output cli.Ui) (*RunOptions, error) { - runOptions := getDefaultRunOptions(config) +func parseRunArgs(args []string, config *config.Config, output cli.Ui) (*Opts, error) { + opts := getDefaultOptions(config) if len(args) == 0 { return nil, errors.Errorf("At least one task must be specified.") @@ -425,117 +426,117 @@ func parseRunArgs(args []string, config *config.Config, output cli.Ui) (*RunOpti var unresolvedCacheFolder string if os.Getenv("TURBO_FORCE") == "true" { - runOptions.runcacheOpts.SkipReads = true + opts.runcacheOpts.SkipReads = true } if os.Getenv("TURBO_REMOTE_ONLY") == "true" { - runOptions.cacheOpts.SkipFilesystem = true + opts.cacheOpts.SkipFilesystem = true } for argIndex, arg := range args { if arg == "--" { - runOptions.passThroughArgs = args[argIndex+1:] + opts.runOpts.passThroughArgs = args[argIndex+1:] break } else if strings.HasPrefix(arg, "--") { switch { case strings.HasPrefix(arg, "--filter="): filterPattern := arg[len("--filter="):] if filterPattern != "" { - runOptions.scopeOpts.FilterPatterns = append(runOptions.scopeOpts.FilterPatterns, filterPattern) + opts.scopeOpts.FilterPatterns = append(opts.scopeOpts.FilterPatterns, filterPattern) } case strings.HasPrefix(arg, "--since="): if len(arg[len("--since="):]) > 0 { - runOptions.scopeOpts.LegacyFilter.Since = arg[len("--since="):] + opts.scopeOpts.LegacyFilter.Since = arg[len("--since="):] } case strings.HasPrefix(arg, "--scope="): if len(arg[len("--scope="):]) > 0 { - runOptions.scopeOpts.LegacyFilter.Entrypoints = append(runOptions.scopeOpts.LegacyFilter.Entrypoints, arg[len("--scope="):]) + opts.scopeOpts.LegacyFilter.Entrypoints = append(opts.scopeOpts.LegacyFilter.Entrypoints, arg[len("--scope="):]) } case strings.HasPrefix(arg, "--ignore="): if len(arg[len("--ignore="):]) > 0 { - runOptions.scopeOpts.IgnorePatterns = append(runOptions.scopeOpts.IgnorePatterns, arg[len("--ignore="):]) + opts.scopeOpts.IgnorePatterns = append(opts.scopeOpts.IgnorePatterns, arg[len("--ignore="):]) } case strings.HasPrefix(arg, "--global-deps="): if len(arg[len("--global-deps="):]) > 0 { - runOptions.scopeOpts.GlobalDepPatterns = append(runOptions.scopeOpts.GlobalDepPatterns, arg[len("--global-deps="):]) + opts.scopeOpts.GlobalDepPatterns = append(opts.scopeOpts.GlobalDepPatterns, arg[len("--global-deps="):]) } case strings.HasPrefix(arg, "--parallel"): - runOptions.parallel = true + opts.runOpts.parallel = true case strings.HasPrefix(arg, "--profile="): // this one must com before the next if len(arg[len("--profile="):]) > 0 { - runOptions.profile = arg[len("--profile="):] + opts.runOpts.profile = arg[len("--profile="):] } case strings.HasPrefix(arg, "--profile"): - runOptions.profile = fmt.Sprintf("%v-profile.json", time.Now().UnixNano()) + opts.runOpts.profile = fmt.Sprintf("%v-profile.json", time.Now().UnixNano()) case strings.HasPrefix(arg, "--no-deps"): - runOptions.scopeOpts.LegacyFilter.SkipDependents = true + opts.scopeOpts.LegacyFilter.SkipDependents = true case strings.HasPrefix(arg, "--no-cache"): - runOptions.runcacheOpts.SkipWrites = true + opts.runcacheOpts.SkipWrites = true case strings.HasPrefix(arg, "--cacheFolder"): output.Warn("[WARNING] The --cacheFolder flag has been deprecated and will be removed in future versions of turbo. Please use `--cache-dir` instead") unresolvedCacheFolder = arg[len("--cacheFolder="):] case strings.HasPrefix(arg, "--cache-dir"): unresolvedCacheFolder = arg[len("--cache-dir="):] case strings.HasPrefix(arg, "--continue"): - runOptions.bail = false + opts.runOpts.bail = false case strings.HasPrefix(arg, "--force"): - runOptions.runcacheOpts.SkipReads = true + opts.runcacheOpts.SkipReads = true case strings.HasPrefix(arg, "--stream"): output.Warn("[WARNING] The --stream flag is unnecesary and has been deprecated. It will be removed in future versions of turbo.") case strings.HasPrefix(arg, "--graph="): // this one must com before the next if len(arg[len("--graph="):]) > 0 { - runOptions.dotGraph = arg[len("--graph="):] + opts.runOpts.dotGraph = arg[len("--graph="):] } case strings.HasPrefix(arg, "--graph"): - runOptions.dotGraph = fmt.Sprintf("graph-%v.jpg", time.Now().UnixNano()) + opts.runOpts.dotGraph = fmt.Sprintf("graph-%v.jpg", time.Now().UnixNano()) case strings.HasPrefix(arg, "--serial"): output.Warn("[WARNING] The --serial flag has been deprecated and will be removed in future versions of turbo. Please use `--concurrency=1` instead") - runOptions.concurrency = 1 + opts.runOpts.concurrency = 1 case strings.HasPrefix(arg, "--concurrency"): concurrencyRaw := arg[len("--concurrency="):] if concurrency, err := util.ParseConcurrency(concurrencyRaw); err != nil { return nil, err } else { - runOptions.concurrency = concurrency + opts.runOpts.concurrency = concurrency } case strings.HasPrefix(arg, "--includeDependencies"): output.Warn("[WARNING] The --includeDependencies flag has renamed to --include-dependencies for consistency. Please use `--include-dependencies` instead") - runOptions.scopeOpts.LegacyFilter.IncludeDependencies = true + opts.scopeOpts.LegacyFilter.IncludeDependencies = true case strings.HasPrefix(arg, "--include-dependencies"): - runOptions.scopeOpts.LegacyFilter.IncludeDependencies = true + opts.scopeOpts.LegacyFilter.IncludeDependencies = true case strings.HasPrefix(arg, "--only"): - runOptions.only = true + opts.runOpts.only = true case strings.HasPrefix(arg, "--output-logs="): outputLogsMode := arg[len("--output-logs="):] switch outputLogsMode { case "full": - runOptions.runcacheOpts.CacheMissLogsMode = runcache.FullLogs - runOptions.runcacheOpts.CacheHitLogsMode = runcache.FullLogs + opts.runcacheOpts.CacheMissLogsMode = runcache.FullLogs + opts.runcacheOpts.CacheHitLogsMode = runcache.FullLogs case "none": - runOptions.runcacheOpts.CacheMissLogsMode = runcache.NoLogs - runOptions.runcacheOpts.CacheHitLogsMode = runcache.NoLogs + opts.runcacheOpts.CacheMissLogsMode = runcache.NoLogs + opts.runcacheOpts.CacheHitLogsMode = runcache.NoLogs case "hash-only": - runOptions.runcacheOpts.CacheMissLogsMode = runcache.HashLogs - runOptions.runcacheOpts.CacheHitLogsMode = runcache.HashLogs + opts.runcacheOpts.CacheMissLogsMode = runcache.HashLogs + opts.runcacheOpts.CacheHitLogsMode = runcache.HashLogs case "new-only": - runOptions.runcacheOpts.CacheMissLogsMode = runcache.FullLogs - runOptions.runcacheOpts.CacheHitLogsMode = runcache.HashLogs + opts.runcacheOpts.CacheMissLogsMode = runcache.FullLogs + opts.runcacheOpts.CacheHitLogsMode = runcache.HashLogs default: output.Warn(fmt.Sprintf("[WARNING] unknown value %v for --output-logs CLI flag. Falling back to full", outputLogsMode)) } case strings.HasPrefix(arg, "--dry-run"): - runOptions.dryRun = true + opts.runOpts.dryRun = true if strings.HasPrefix(arg, "--dry-run=json") { - runOptions.dryRunJSON = true + opts.runOpts.dryRunJSON = true } case strings.HasPrefix(arg, "--dry"): - runOptions.dryRun = true + opts.runOpts.dryRun = true if strings.HasPrefix(arg, "--dry=json") { - runOptions.dryRunJSON = true + opts.runOpts.dryRunJSON = true } case strings.HasPrefix(arg, "--remote-only"): - runOptions.cacheOpts.SkipFilesystem = true + opts.cacheOpts.SkipFilesystem = true case strings.HasPrefix(arg, "--team"): case strings.HasPrefix(arg, "--token"): case strings.HasPrefix(arg, "--preflight"): @@ -554,13 +555,13 @@ func parseRunArgs(args []string, config *config.Config, output cli.Ui) (*RunOpti // We can only set this cache folder after we know actual cwd if unresolvedCacheFolder != "" { - runOptions.cacheOpts.Dir = fs.ResolveUnknownPath(config.Cwd, unresolvedCacheFolder) + opts.cacheOpts.Dir = fs.ResolveUnknownPath(config.Cwd, unresolvedCacheFolder) } if !config.IsLoggedIn() { - runOptions.cacheOpts.SkipRemote = true + opts.cacheOpts.SkipRemote = true } - return runOptions, nil + return opts, nil } // logError logs an error and outputs it to the UI. @@ -619,7 +620,7 @@ func (c *RunCommand) executeTasks(g *completeGraph, rs *runSpec, engine *core.Sc return 1 } defer turboCache.Shutdown() - runState := NewRunState(startAt, rs.Opts.profile) + runState := NewRunState(startAt, rs.Opts.runOpts.profile) runCache := runcache.New(turboCache, c.Config.Cwd, rs.Opts.runcacheOpts) ec := &execContext{ colorCache: NewColorCache(), @@ -639,8 +640,8 @@ func (c *RunCommand) executeTasks(g *completeGraph, rs *runSpec, engine *core.Sc deps := engine.TaskGraph.DownEdges(pt.TaskID) return ec.exec(pt, deps) }), core.ExecOpts{ - Parallel: rs.Opts.parallel, - Concurrency: rs.Opts.concurrency, + Parallel: rs.Opts.runOpts.parallel, + Concurrency: rs.Opts.runOpts.concurrency, }) // Track if we saw any child with a non-zero exit code @@ -658,7 +659,7 @@ func (c *RunCommand) executeTasks(g *completeGraph, rs *runSpec, engine *core.Sc c.Ui.Error(err.Error()) } - if err := runState.Close(c.Ui, rs.Opts.profile); err != nil { + if err := runState.Close(c.Ui, rs.Opts.runOpts.profile); err != nil { c.Ui.Error(fmt.Sprintf("Error with profiler: %s", err.Error())) return 1 } @@ -858,7 +859,7 @@ func (e *execContext) exec(pt *nodes.PackageTask, deps dag.Set) error { if err != nil { tracer(TargetBuildFailed, err) e.logError(targetLogger, actualPrefix, err) - if e.rs.Opts.bail { + if e.rs.Opts.runOpts.bail { os.Exit(1) } } @@ -904,7 +905,7 @@ func (e *execContext) exec(pt *nodes.PackageTask, deps dag.Set) error { } tracer(TargetBuildFailed, err) targetLogger.Error("Error: command finished with error: %w", err) - if e.rs.Opts.bail { + if e.rs.Opts.runOpts.bail { targetUi.Error(fmt.Sprintf("ERROR: command finished with error: %s", err)) e.processes.Close() } else { diff --git a/cli/internal/run/run_test.go b/cli/internal/run/run_test.go index 748ecf66d0234..8dd18f31e00f8 100644 --- a/cli/internal/run/run_test.go +++ b/cli/internal/run/run_test.go @@ -28,16 +28,16 @@ func TestParseConfig(t *testing.T) { cases := []struct { Name string Args []string - Expected *RunOptions + Expected *Opts }{ { "string flags", []string{"foo"}, - &RunOptions{ - bail: true, - dotGraph: "", - concurrency: 10, - profile: "", + &Opts{ + runOpts: runOpts{ + bail: true, + concurrency: 10, + }, cacheOpts: cache.Opts{ Dir: defaultCacheFolder, Workers: 10, @@ -49,11 +49,11 @@ func TestParseConfig(t *testing.T) { { "scope", []string{"foo", "--scope=foo", "--scope=blah"}, - &RunOptions{ - bail: true, - dotGraph: "", - concurrency: 10, - profile: "", + &Opts{ + runOpts: runOpts{ + bail: true, + concurrency: 10, + }, cacheOpts: cache.Opts{ Dir: defaultCacheFolder, Workers: 10, @@ -69,11 +69,11 @@ func TestParseConfig(t *testing.T) { { "concurrency", []string{"foo", "--concurrency=12"}, - &RunOptions{ - bail: true, - dotGraph: "", - concurrency: 12, - profile: "", + &Opts{ + runOpts: runOpts{ + bail: true, + concurrency: 12, + }, cacheOpts: cache.Opts{ Dir: defaultCacheFolder, Workers: 10, @@ -85,11 +85,12 @@ func TestParseConfig(t *testing.T) { { "graph", []string{"foo", "--graph=g.png"}, - &RunOptions{ - bail: true, - dotGraph: "g.png", - concurrency: 10, - profile: "", + &Opts{ + runOpts: runOpts{ + bail: true, + concurrency: 10, + dotGraph: "g.png", + }, cacheOpts: cache.Opts{ Dir: defaultCacheFolder, Workers: 10, @@ -101,12 +102,13 @@ func TestParseConfig(t *testing.T) { { "passThroughArgs", []string{"foo", "--graph=g.png", "--", "--boop", "zoop"}, - &RunOptions{ - bail: true, - dotGraph: "g.png", - concurrency: 10, - profile: "", - passThroughArgs: []string{"--boop", "zoop"}, + &Opts{ + runOpts: runOpts{ + bail: true, + concurrency: 10, + dotGraph: "g.png", + passThroughArgs: []string{"--boop", "zoop"}, + }, cacheOpts: cache.Opts{ Dir: defaultCacheFolder, Workers: 10, @@ -118,10 +120,11 @@ func TestParseConfig(t *testing.T) { { "force", []string{"foo", "--force"}, - &RunOptions{ - bail: true, - concurrency: 10, - profile: "", + &Opts{ + runOpts: runOpts{ + bail: true, + concurrency: 10, + }, cacheOpts: cache.Opts{ Dir: defaultCacheFolder, Workers: 10, @@ -135,10 +138,11 @@ func TestParseConfig(t *testing.T) { { "remote-only", []string{"foo", "--remote-only"}, - &RunOptions{ - bail: true, - concurrency: 10, - profile: "", + &Opts{ + runOpts: runOpts{ + bail: true, + concurrency: 10, + }, cacheOpts: cache.Opts{ Dir: defaultCacheFolder, Workers: 10, @@ -151,10 +155,11 @@ func TestParseConfig(t *testing.T) { { "no-cache", []string{"foo", "--no-cache"}, - &RunOptions{ - bail: true, - concurrency: 10, - profile: "", + &Opts{ + runOpts: runOpts{ + bail: true, + concurrency: 10, + }, cacheOpts: cache.Opts{ Dir: defaultCacheFolder, Workers: 10, @@ -168,12 +173,13 @@ func TestParseConfig(t *testing.T) { { "Empty passThroughArgs", []string{"foo", "--graph=g.png", "--"}, - &RunOptions{ - bail: true, - dotGraph: "g.png", - concurrency: 10, - profile: "", - passThroughArgs: []string{}, + &Opts{ + runOpts: runOpts{ + bail: true, + concurrency: 10, + dotGraph: "g.png", + passThroughArgs: []string{}, + }, cacheOpts: cache.Opts{ Dir: defaultCacheFolder, Workers: 10, @@ -185,9 +191,11 @@ func TestParseConfig(t *testing.T) { { "can specify filter patterns", []string{"foo", "--filter=bar", "--filter=...[main]"}, - &RunOptions{ - bail: true, - concurrency: 10, + &Opts{ + runOpts: runOpts{ + bail: true, + concurrency: 10, + }, cacheOpts: cache.Opts{ Dir: defaultCacheFolder, Workers: 10, @@ -232,11 +240,11 @@ func TestParseRunOptionsUsesCWDFlag(t *testing.T) { t.Errorf("failed to get cwd: %v", err) } cwd := defaultCwd.Join("zop") - expected := &RunOptions{ - bail: true, - dotGraph: "", - concurrency: 10, - profile: "", + expected := &Opts{ + runOpts: runOpts{ + bail: true, + concurrency: 10, + }, cacheOpts: cache.Opts{ Dir: cwd.Join("node_modules", ".cache", "turbo"), Workers: 10, @@ -377,7 +385,7 @@ func Test_dontSquashTasks(t *testing.T) { rs := &runSpec{ FilteredPkgs: filteredPkgs, Targets: []string{"build"}, - Opts: &RunOptions{}, + Opts: &Opts{}, } engine, err := buildTaskGraph(topoGraph, pipeline, rs) if err != nil { @@ -410,7 +418,7 @@ func Test_taskSelfRef(t *testing.T) { rs := &runSpec{ FilteredPkgs: filteredPkgs, Targets: []string{"build"}, - Opts: &RunOptions{}, + Opts: &Opts{}, } _, err := buildTaskGraph(topoGraph, pipeline, rs) if err == nil { From e53d49f656a9a71b5cb00572a237c94fd61a23d1 Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Thu, 26 May 2022 10:50:56 -0700 Subject: [PATCH 6/8] flip bail -> continueOnError to avoid default: true in runOpts --- cli/internal/run/run.go | 10 +++++----- cli/internal/run/run_test.go | 27 ++++++++++++++++----------- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/cli/internal/run/run.go b/cli/internal/run/run.go index 1f728aeddf28e..fe9f4f0fed6f1 100644 --- a/cli/internal/run/run.go +++ b/cli/internal/run/run.go @@ -394,7 +394,8 @@ type runOpts struct { // Whether to emit a perf profile profile string // Immediately exit on task failure - bail bool + //bail bool + continueOnError bool passThroughArgs []string // Restrict execution to only the listed task names. Default false only bool @@ -405,7 +406,6 @@ type runOpts struct { func getDefaultOptions(config *config.Config) *Opts { return &Opts{ runOpts: runOpts{ - bail: true, concurrency: 10, }, cacheOpts: cache.Opts{ @@ -479,7 +479,7 @@ func parseRunArgs(args []string, config *config.Config, output cli.Ui) (*Opts, e case strings.HasPrefix(arg, "--cache-dir"): unresolvedCacheFolder = arg[len("--cache-dir="):] case strings.HasPrefix(arg, "--continue"): - opts.runOpts.bail = false + opts.runOpts.continueOnError = true case strings.HasPrefix(arg, "--force"): opts.runcacheOpts.SkipReads = true case strings.HasPrefix(arg, "--stream"): @@ -859,7 +859,7 @@ func (e *execContext) exec(pt *nodes.PackageTask, deps dag.Set) error { if err != nil { tracer(TargetBuildFailed, err) e.logError(targetLogger, actualPrefix, err) - if e.rs.Opts.runOpts.bail { + if !e.rs.Opts.runOpts.continueOnError { os.Exit(1) } } @@ -905,7 +905,7 @@ func (e *execContext) exec(pt *nodes.PackageTask, deps dag.Set) error { } tracer(TargetBuildFailed, err) targetLogger.Error("Error: command finished with error: %w", err) - if e.rs.Opts.runOpts.bail { + if !e.rs.Opts.runOpts.continueOnError { targetUi.Error(fmt.Sprintf("ERROR: command finished with error: %s", err)) e.processes.Close() } else { diff --git a/cli/internal/run/run_test.go b/cli/internal/run/run_test.go index 8dd18f31e00f8..0f34bc9b3c800 100644 --- a/cli/internal/run/run_test.go +++ b/cli/internal/run/run_test.go @@ -35,7 +35,6 @@ func TestParseConfig(t *testing.T) { []string{"foo"}, &Opts{ runOpts: runOpts{ - bail: true, concurrency: 10, }, cacheOpts: cache.Opts{ @@ -51,7 +50,6 @@ func TestParseConfig(t *testing.T) { []string{"foo", "--scope=foo", "--scope=blah"}, &Opts{ runOpts: runOpts{ - bail: true, concurrency: 10, }, cacheOpts: cache.Opts{ @@ -71,7 +69,6 @@ func TestParseConfig(t *testing.T) { []string{"foo", "--concurrency=12"}, &Opts{ runOpts: runOpts{ - bail: true, concurrency: 12, }, cacheOpts: cache.Opts{ @@ -87,7 +84,6 @@ func TestParseConfig(t *testing.T) { []string{"foo", "--graph=g.png"}, &Opts{ runOpts: runOpts{ - bail: true, concurrency: 10, dotGraph: "g.png", }, @@ -104,7 +100,6 @@ func TestParseConfig(t *testing.T) { []string{"foo", "--graph=g.png", "--", "--boop", "zoop"}, &Opts{ runOpts: runOpts{ - bail: true, concurrency: 10, dotGraph: "g.png", passThroughArgs: []string{"--boop", "zoop"}, @@ -122,7 +117,6 @@ func TestParseConfig(t *testing.T) { []string{"foo", "--force"}, &Opts{ runOpts: runOpts{ - bail: true, concurrency: 10, }, cacheOpts: cache.Opts{ @@ -140,7 +134,6 @@ func TestParseConfig(t *testing.T) { []string{"foo", "--remote-only"}, &Opts{ runOpts: runOpts{ - bail: true, concurrency: 10, }, cacheOpts: cache.Opts{ @@ -157,7 +150,6 @@ func TestParseConfig(t *testing.T) { []string{"foo", "--no-cache"}, &Opts{ runOpts: runOpts{ - bail: true, concurrency: 10, }, cacheOpts: cache.Opts{ @@ -175,7 +167,6 @@ func TestParseConfig(t *testing.T) { []string{"foo", "--graph=g.png", "--"}, &Opts{ runOpts: runOpts{ - bail: true, concurrency: 10, dotGraph: "g.png", passThroughArgs: []string{}, @@ -193,7 +184,6 @@ func TestParseConfig(t *testing.T) { []string{"foo", "--filter=bar", "--filter=...[main]"}, &Opts{ runOpts: runOpts{ - bail: true, concurrency: 10, }, cacheOpts: cache.Opts{ @@ -206,6 +196,22 @@ func TestParseConfig(t *testing.T) { }, }, }, + { + "continue on errors", + []string{"foo", "--continue"}, + &Opts{ + runOpts: runOpts{ + continueOnError: true, + concurrency: 10, + }, + cacheOpts: cache.Opts{ + Dir: defaultCacheFolder, + Workers: 10, + }, + runcacheOpts: runcache.Opts{}, + scopeOpts: scope.Opts{}, + }, + }, } ui := &cli.BasicUi{ @@ -242,7 +248,6 @@ func TestParseRunOptionsUsesCWDFlag(t *testing.T) { cwd := defaultCwd.Join("zop") expected := &Opts{ runOpts: runOpts{ - bail: true, concurrency: 10, }, cacheOpts: cache.Opts{ From b4368f41559799b0e6a6012ffdca81bc844c0542 Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Thu, 26 May 2022 15:36:55 -0700 Subject: [PATCH 7/8] Lint --- cli/internal/run/run.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cli/internal/run/run.go b/cli/internal/run/run.go index fe9f4f0fed6f1..34940593f29a9 100644 --- a/cli/internal/run/run.go +++ b/cli/internal/run/run.go @@ -960,7 +960,9 @@ func (c *RunCommand) generateDotGraph(taskGraph *dag.AcyclicGraph, outputFilenam c.Ui.Output("") c.Ui.Output(fmt.Sprintf("✔ Generated task graph in %s", ui.Bold(outputFilename.ToString()))) if ui.IsTTY { - browser.OpenBrowser(outputFilename.ToString()) + if err := browser.OpenBrowser(outputFilename.ToString()); err != nil { + c.Ui.Warn(color.New(color.FgYellow, color.Bold, color.ReverseVideo).Sprintf("failed to open browser. Please navigate to file://%v", outputFilename)) + } } return nil } From d45760ba554426180cf95c53db3279d2c5842e81 Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Tue, 31 May 2022 09:32:14 -0700 Subject: [PATCH 8/8] Response to feedback --- cli/internal/run/run.go | 5 ++--- cli/internal/scope/scope.go | 8 ++++---- cli/internal/scope/scope_test.go | 3 ++- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/cli/internal/run/run.go b/cli/internal/run/run.go index 34940593f29a9..00bab8d8e1fce 100644 --- a/cli/internal/run/run.go +++ b/cli/internal/run/run.go @@ -393,8 +393,7 @@ type runOpts struct { parallel bool // Whether to emit a perf profile profile string - // Immediately exit on task failure - //bail bool + // If true, continue task executions even if a task fails. continueOnError bool passThroughArgs []string // Restrict execution to only the listed task names. Default false @@ -961,7 +960,7 @@ func (c *RunCommand) generateDotGraph(taskGraph *dag.AcyclicGraph, outputFilenam c.Ui.Output(fmt.Sprintf("✔ Generated task graph in %s", ui.Bold(outputFilename.ToString()))) if ui.IsTTY { if err := browser.OpenBrowser(outputFilename.ToString()); err != nil { - c.Ui.Warn(color.New(color.FgYellow, color.Bold, color.ReverseVideo).Sprintf("failed to open browser. Please navigate to file://%v", outputFilename)) + c.Ui.Warn(color.New(color.FgYellow, color.Bold, color.ReverseVideo).Sprintf("failed to open browser. Please navigate to file://%v", filepath.ToSlash(outputFilename.ToString()))) } } return nil diff --git a/cli/internal/scope/scope.go b/cli/internal/scope/scope.go index 03550bce4d416..c95e63a6a7a31 100644 --- a/cli/internal/scope/scope.go +++ b/cli/internal/scope/scope.go @@ -105,16 +105,16 @@ func ResolvePackages(opts *Opts, cwd string, scm scm.SCM, ctx *context.Context, } func (o *Opts) getPackageChangeFunc(scm scm.SCM, cwd string, packageInfos map[interface{}]*fs.PackageJSON) scope_filter.PackagesChangedSince { - // Note that "since" here is *not* o.Since. Each filter expression can have its own value for + // Note that "since" here is *not* o.LegacyFilter.Since. Each filter expression can have its own value for // "since", apart from the value specified via --since. - return func(since string) (util.Set, error) { + return func(filterSince string) (util.Set, error) { // We could filter changed files at the git level, since it's possible // that the changes we're interested in are scoped, but we need to handle // global dependencies changing as well. A future optimization might be to // scope changed files more deeply if we know there are no global dependencies. var changedFiles []string - if since != "" { - scmChangedFiles, err := scm.ChangedFiles(since, true, cwd) + if filterSince != "" { + scmChangedFiles, err := scm.ChangedFiles(filterSince, true, cwd) if err != nil { return nil, err } diff --git a/cli/internal/scope/scope_test.go b/cli/internal/scope/scope_test.go index 91545a7f93e13..18ae4689311d7 100644 --- a/cli/internal/scope/scope_test.go +++ b/cli/internal/scope/scope_test.go @@ -2,6 +2,7 @@ package scope import ( "fmt" + "path/filepath" "reflect" "testing" @@ -211,7 +212,7 @@ func TestResolvePackages(t *testing.T) { }, IgnorePatterns: []string{tc.ignore}, GlobalDepPatterns: tc.globalDeps, - }, "dummy-repo-root", scm, &context.Context{ + }, filepath.FromSlash("/dummy/repo/root"), scm, &context.Context{ PackageInfos: packagesInfos, PackageNames: packageNames, TopologicalGraph: graph,