From afd3e1d3d50210a4f9d28998074c6461d22eaa6f Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Thu, 3 Mar 2022 14:19:20 -0800 Subject: [PATCH 1/6] scope packages doesn't need the whole context --- cli/internal/run/run.go | 13 +++++-------- cli/internal/run/run_test.go | 29 ++++++++++------------------- 2 files changed, 15 insertions(+), 27 deletions(-) diff --git a/cli/internal/run/run.go b/cli/internal/run/run.go index 0c5a4b8ae4c46..259f300ae41e7 100644 --- a/cli/internal/run/run.go +++ b/cli/internal/run/run.go @@ -220,7 +220,7 @@ func (c *RunCommand) Run(args []string) int { // Scoped packages // Unwind scope globs - scopePkgs, err := getScopedPackages(ctx, runOptions.scope) + scopePkgs, err := getScopedPackages(ctx.PackageNames, runOptions.scope) if err != nil { c.logError(c.Config.Logger, "", fmt.Errorf("invalid scope: %w", err)) return 1 @@ -932,13 +932,10 @@ func parseRunArgs(args []string, output cli.Ui) (*RunOptions, error) { } // getScopedPackages returns a set of package names in scope for a given list of glob patterns -func getScopedPackages(ctx *context.Context, scopePatterns []string) (scopePkgs util.Set, err error) { - if err != nil { - return nil, fmt.Errorf("invalid glob pattern %w", err) - } - var scopedPkgs = make(util.Set) +func getScopedPackages(packageNames []string, scopePatterns []string) (util.Set, error) { + scopedPkgs := make(util.Set) if len(scopePatterns) == 0 { - return scopePkgs, nil + return scopedPkgs, nil } include := make([]string, 0, len(scopePatterns)) @@ -956,7 +953,7 @@ func getScopedPackages(ctx *context.Context, scopePatterns []string) (scopePkgs if err != nil { return nil, err } - for _, f := range ctx.PackageNames { + for _, f := range packageNames { if glob.Match(f) { scopedPkgs.Add(f) } diff --git a/cli/internal/run/run_test.go b/cli/internal/run/run_test.go index 334cc8b6587b5..bab12deefae48 100644 --- a/cli/internal/run/run_test.go +++ b/cli/internal/run/run_test.go @@ -8,7 +8,6 @@ import ( "testing" "github.com/mitchellh/cli" - "github.com/vercel/turborepo/cli/internal/context" "github.com/vercel/turborepo/cli/internal/fs" "github.com/vercel/turborepo/cli/internal/util" @@ -170,40 +169,32 @@ func TestParseConfig(t *testing.T) { func TestScopedPackages(t *testing.T) { cases := []struct { - Name string - Ctx *context.Context - Pattern []string - Expected util.Set + Name string + PackageNames []string + Pattern []string + Expected util.Set }{ { "starts with @", - &context.Context{ - PackageNames: []string{"@sample/app", "sample-app", "jared"}, - }, + []string{"@sample/app", "sample-app", "jared"}, []string{"@sample/*"}, util.Set{"@sample/app": "@sample/app"}, }, { "return an array of matches", - &context.Context{ - PackageNames: []string{"foo", "bar", "baz"}, - }, + []string{"foo", "bar", "baz"}, []string{"f*"}, util.Set{"foo": "foo"}, }, { "return an array of matches", - &context.Context{ - PackageNames: []string{"foo", "bar", "baz"}, - }, + []string{"foo", "bar", "baz"}, []string{"f*", "bar"}, util.Set{"bar": "bar", "foo": "foo"}, }, { "return matches in the order the list were defined", - &context.Context{ - PackageNames: []string{"foo", "bar", "baz"}, - }, + []string{"foo", "bar", "baz"}, []string{"*a*", "!f*"}, util.Set{"bar": "bar", "baz": "baz"}, }, @@ -211,7 +202,7 @@ func TestScopedPackages(t *testing.T) { for i, tc := range cases { t.Run(fmt.Sprintf("%d-%s", i, tc.Name), func(t *testing.T) { - actual, err := getScopedPackages(tc.Ctx, tc.Pattern) + actual, err := getScopedPackages(tc.PackageNames, tc.Pattern) if err != nil { t.Fatalf("invalid scope parse: %#v", err) } @@ -220,7 +211,7 @@ func TestScopedPackages(t *testing.T) { } t.Run(fmt.Sprintf("%d-%s", len(cases), "throws an error if no package matches the provided scope pattern"), func(t *testing.T) { - _, err := getScopedPackages(&context.Context{PackageNames: []string{"foo", "bar"}}, []string{"baz"}) + _, err := getScopedPackages([]string{"foo", "bar"}, []string{"baz"}) assert.Error(t, err) }) } From a4596eccbbc3df997515f2eab48b2796826299e2 Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Thu, 3 Mar 2022 14:13:50 -0800 Subject: [PATCH 2/6] Needs cleanup, but have extracted the filtered packages logic --- cli/internal/run/run.go | 143 +-------------------------- cli/internal/run/scope.go | 202 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 204 insertions(+), 141 deletions(-) create mode 100644 cli/internal/run/scope.go diff --git a/cli/internal/run/run.go b/cli/internal/run/run.go index 259f300ae41e7..09d1bfa4d6703 100644 --- a/cli/internal/run/run.go +++ b/cli/internal/run/run.go @@ -26,7 +26,6 @@ import ( "github.com/vercel/turborepo/cli/internal/globby" "github.com/vercel/turborepo/cli/internal/logstreamer" "github.com/vercel/turborepo/cli/internal/process" - "github.com/vercel/turborepo/cli/internal/scm" "github.com/vercel/turborepo/cli/internal/ui" "github.com/vercel/turborepo/cli/internal/util" "github.com/vercel/turborepo/cli/internal/util/browser" @@ -154,147 +153,9 @@ func (c *RunCommand) Run(args []string) int { return 1 } - gitRepoRoot, err := fs.FindupFrom(".git", runOptions.cwd) + filteredPkgs, err := resolvePackagesInScope(runOptions, ctx, c.Ui, c.Config.Logger) if err != nil { - c.logWarning(c.Config.Logger, "Cannot find a .git folder in current working directory or in any parent directories. Falling back to manual file hashing (which may be slower). If you are running this build in a pruned directory, you can ignore this message. Otherwise, please initialize a git repository in the root of your monorepo.", err) - } - git, err := scm.NewFallback(filepath.Dir(gitRepoRoot)) - if err != nil { - c.logWarning(c.Config.Logger, "", err) - } - - ignoreGlob, err := filter.Compile(runOptions.ignore) - if err != nil { - c.logError(c.Config.Logger, "", fmt.Errorf("invalid ignore globs: %w", err)) - return 1 - } - globalDepsGlob, err := filter.Compile(runOptions.globalDeps) - if err != nil { - c.logError(c.Config.Logger, "", fmt.Errorf("invalid global deps glob: %w", err)) - return 1 - } - hasRepoGlobalFileChanged := false - var changedFiles []string - if runOptions.since != "" { - changedFiles = git.ChangedFiles(runOptions.since, true, runOptions.cwd) - } - - ignoreSet := make(util.Set) - if globalDepsGlob != nil { - for _, f := range changedFiles { - if globalDepsGlob.Match(f) { - hasRepoGlobalFileChanged = true - break - } - } - } - - if ignoreGlob != nil { - for _, f := range changedFiles { - if ignoreGlob.Match(f) { - ignoreSet.Add(f) - } - } - } - - filteredChangedFiles := make(util.Set) - // Ignore any changed files in the ignore set - for _, c := range changedFiles { - if !ignoreSet.Includes(c) { - filteredChangedFiles.Add(c) - } - } - - changedPackages := make(util.Set) - // Be specific with the changed packages only if no repo-wide changes occurred - if !hasRepoGlobalFileChanged { - for k, pkgInfo := range ctx.PackageInfos { - partialPath := pkgInfo.Dir - if filteredChangedFiles.Some(func(v interface{}) bool { - return strings.HasPrefix(fmt.Sprintf("%v", v), partialPath) // true - }) { - changedPackages.Add(k) - } - } - } - - // Scoped packages - // Unwind scope globs - scopePkgs, err := getScopedPackages(ctx.PackageNames, runOptions.scope) - if err != nil { - c.logError(c.Config.Logger, "", fmt.Errorf("invalid scope: %w", err)) - return 1 - } - - // Filter Packages - filteredPkgs := make(util.Set) - - // If both scoped and since are specified, we have to merge two lists: - // 1. changed packages that ARE themselves the scoped packages - // 2. changed package consumers (package dependents) that are within the scoped subgraph - if scopePkgs.Len() > 0 && changedPackages.Len() > 0 { - filteredPkgs = scopePkgs.Intersection(changedPackages) - for _, changed := range changedPackages { - descenders, err := ctx.TopologicalGraph.Descendents(changed) - if err != nil { - c.logError(c.Config.Logger, "", fmt.Errorf("could not determine dependency: %w", err)) - return 1 - } - - filteredPkgs.Add(changed) - for _, d := range descenders { - filteredPkgs.Add(d) - } - } - c.Ui.Output(fmt.Sprintf(ui.Dim("• Packages changed since %s in scope: %s"), runOptions.since, strings.Join(filteredPkgs.UnsafeListOfStrings(), ", "))) - } else if changedPackages.Len() > 0 { - filteredPkgs = changedPackages - c.Ui.Output(fmt.Sprintf(ui.Dim("• Packages changed since %s: %s"), runOptions.since, strings.Join(filteredPkgs.UnsafeListOfStrings(), ", "))) - } else if scopePkgs.Len() > 0 { - filteredPkgs = scopePkgs - } else if runOptions.since == "" { - for _, f := range ctx.PackageNames { - filteredPkgs.Add(f) - } - } - - if runOptions.includeDependents { - // perf??? this is duplicative from the step above - for _, pkg := range filteredPkgs { - descenders, err := ctx.TopologicalGraph.Descendents(pkg) - if err != nil { - c.logError(c.Config.Logger, "", fmt.Errorf("error calculating affected packages: %w", err)) - return 1 - } - c.Config.Logger.Debug("dependents", "pkg", pkg, "value", descenders.List()) - for _, d := range descenders { - // we need to exclude the fake root node - // since it is not a real package - if d != ctx.RootNode { - filteredPkgs.Add(d) - } - } - } - c.Config.Logger.Debug("running with dependents") - } - - if runOptions.includeDependencies { - for _, pkg := range filteredPkgs { - ancestors, err := ctx.TopologicalGraph.Ancestors(pkg) - if err != nil { - c.logError(c.Config.Logger, "", fmt.Errorf("error getting dependency %v", err)) - return 1 - } - c.Config.Logger.Debug("dependencies", "pkg", pkg, "value", ancestors.List()) - for _, d := range ancestors { - // we need to exclude the fake root node - // since it is not a real package - if d != ctx.RootNode { - filteredPkgs.Add(d) - } - } - } - c.Config.Logger.Debug(ui.Dim("running with dependencies")) + c.logError(c.Config.Logger, "", fmt.Errorf("failed resolve packages to run %v", err)) } c.Config.Logger.Debug("global hash", "value", ctx.GlobalHash) packagesInScope := filteredPkgs.UnsafeListOfStrings() diff --git a/cli/internal/run/scope.go b/cli/internal/run/scope.go new file mode 100644 index 0000000000000..6baf900f19c21 --- /dev/null +++ b/cli/internal/run/scope.go @@ -0,0 +1,202 @@ +package run + +import ( + "fmt" + "path/filepath" + "strings" + + "github.com/hashicorp/go-hclog" + "github.com/mitchellh/cli" + "github.com/pkg/errors" + "github.com/vercel/turborepo/cli/internal/context" + "github.com/vercel/turborepo/cli/internal/fs" + "github.com/vercel/turborepo/cli/internal/scm" + "github.com/vercel/turborepo/cli/internal/ui" + "github.com/vercel/turborepo/cli/internal/util" + "github.com/vercel/turborepo/cli/internal/util/filter" +) + +func resolvePackagesInScope(runOptions *RunOptions, ctx *context.Context, tui cli.Ui, logger hclog.Logger) (util.Set, error) { + changedFiles, err := getChangedFiles(runOptions) + if err != nil { + return nil, err + } + + // Note that we do this calculation *before* filtering the changed files. + // The user can technically specify both that a file is a global dependency and + // that it should be ignored, and currently we treat a change to that file as a + // global change. + hasRepoGlobalFileChanged, err := repoGlobalFileHasChanged(runOptions, changedFiles) + if err != nil { + return nil, err + } + filteredChangedFiles, err := filterIgnoredFiles(runOptions, changedFiles) + if err != nil { + return nil, err + } + + changedPackages := make(util.Set) + // Be specific with the changed packages only if no repo-wide changes occurred + if !hasRepoGlobalFileChanged { + changedPackages = getChangedPackages(filteredChangedFiles, ctx.PackageInfos) + } + + // Scoped packages + // Unwind scope globs + scopePkgs, err := getScopedPackages(ctx.PackageNames, runOptions.scope) + if err != nil { + return nil, errors.Wrap(err, "invalid scope") + } + + // Filter Packages + filteredPkgs := make(util.Set) + includeDependencies := runOptions.includeDependencies + includeDependents := runOptions.includeDependents + // If there has been a global change, changedPackages.Len() will be 0 + // If both scoped and since are specified, we have to merge two lists: + // 1. changed packages that ARE themselves the scoped packages + // 2. changed package consumers (package dependents) that are within the scoped subgraph + if scopePkgs.Len() > 0 && changedPackages.Len() > 0 { + filteredPkgs = scopePkgs.Intersection(changedPackages) + for _, changed := range changedPackages { + filteredPkgs.Add(changed) + includeDependents = true + + } + tui.Output(fmt.Sprintf(ui.Dim("• Packages changed since %s in scope: %s"), runOptions.since, strings.Join(filteredPkgs.UnsafeListOfStrings(), ", "))) + } else if changedPackages.Len() > 0 { + // --since was specified, there are changes, but no scope was specified. + // Run the packages that have changed + filteredPkgs = changedPackages + tui.Output(fmt.Sprintf(ui.Dim("• Packages changed since %s: %s"), runOptions.since, strings.Join(filteredPkgs.UnsafeListOfStrings(), ", "))) + } else if scopePkgs.Len() > 0 { + // There was either a global change, or no changes, or no --since flag + // There was a --scope flag, run the desired scopes + filteredPkgs = scopePkgs + } else if runOptions.since == "" { + // No scope was specified, and no diff base was specified + // Run every package + for _, f := range ctx.PackageNames { + filteredPkgs.Add(f) + } + } + + if includeDependents { + // TODO(gsoltis): we're concurrently iterating and adding to a map, potentially + // resulting in a bunch of duplicate work as we look for descendents of something + // that has already had all of its descendents included. + for _, pkg := range filteredPkgs { + err = addDependents(filteredPkgs, pkg, ctx, logger) + if err != nil { + return nil, err + } + } + logger.Debug("running with dependents") + } + + if includeDependencies { + // TODO(gsoltis): we're concurrently iterating and adding to a map, potentially + // resulting in a bunch of duplicate work as we look for dependencies of something + // that has already had all of its dependencies included. + for _, pkg := range filteredPkgs { + err = addDependencies(filteredPkgs, pkg, ctx, logger) + if err != nil { + return nil, err + } + } + logger.Debug(ui.Dim("running with dependencies")) + } + return filteredPkgs, nil +} + +func getChangedFiles(runOptions *RunOptions) ([]string, error) { + if runOptions.since == "" { + return []string{}, nil + } + gitRepoRoot, err := fs.FindupFrom(".git", runOptions.cwd) + if err != nil { + errors.Wrap(err, "Cannot find a .git folder in current working directory or in any parent directories. Falling back to manual file hashing (which may be slower). If you are running this build in a pruned directory, you can ignore this message. Otherwise, please initialize a git repository in the root of your monorepo.") + } + git, err := scm.NewFallback(filepath.Dir(gitRepoRoot)) + if err != nil { + return nil, err + } + return git.ChangedFiles(runOptions.since, true, runOptions.cwd), nil +} + +func repoGlobalFileHasChanged(runOptions *RunOptions, changedFiles []string) (bool, error) { + globalDepsGlob, err := filter.Compile(runOptions.globalDeps) + if err != nil { + return false, errors.Wrap(err, "invalid global deps glob") + } + + if globalDepsGlob != nil { + for _, f := range changedFiles { + if globalDepsGlob.Match(f) { + return true, nil + } + } + } + return false, nil +} + +func filterIgnoredFiles(runOptions *RunOptions, changedFiles []string) (util.Set, error) { + ignoreGlob, err := filter.Compile(runOptions.ignore) + if err != nil { + return nil, errors.Wrap(err, "invalid ignore globs") + } + filteredChanges := make(util.Set) + for _, file := range changedFiles { + // If we don't have anything to ignore, or if this file doesn't match the ignore pattern, + // keep it as a changed file. + if ignoreGlob == nil || !ignoreGlob.Match(file) { + filteredChanges.Add(file) + } + } + return filteredChanges, nil +} + +func getChangedPackages(changedFiles util.Set, packageInfos map[interface{}]*fs.PackageJSON) util.Set { + changedPackages := make(util.Set) + for k, pkgInfo := range packageInfos { + partialPath := pkgInfo.Dir + if changedFiles.Some(func(v interface{}) bool { + return strings.HasPrefix(fmt.Sprintf("%v", v), partialPath) // true + }) { + changedPackages.Add(k) + } + } + return changedPackages +} + +func addDependents(deps util.Set, pkg interface{}, ctx *context.Context, logger hclog.Logger) error { + descenders, err := ctx.TopologicalGraph.Descendents(pkg) + if err != nil { + return errors.Wrap(err, "error calculating affected packages") + } + logger.Debug("dependents", "pkg", pkg, "value", descenders.List()) + for _, d := range descenders { + // we need to exclude the fake root node + // since it is not a real package + if d != ctx.RootNode { + deps.Add(d) + } + } + return nil +} + +func addDependencies(deps util.Set, pkg interface{}, ctx *context.Context, logger hclog.Logger) error { + ancestors, err := ctx.TopologicalGraph.Ancestors(pkg) + if err != nil { + return errors.Wrap(err, "error getting dependency") + } + logger.Debug("dependencies", "pkg", pkg, "value", ancestors.List()) + for _, d := range ancestors { + // we need to exclude the fake root node + // since it is not a real package + if d != ctx.RootNode { + deps.Add(d) + } + } + return nil +} From 5aaf196a88e8582152582d61c6d712966fc3b7b0 Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Fri, 4 Mar 2022 12:46:07 -0800 Subject: [PATCH 3/6] Move to scope opts --- cli/internal/core/scheduler.go | 4 +- cli/internal/run/run.go | 51 +++++------------ cli/internal/run/run_test.go | 50 ----------------- cli/internal/{run => scope}/scope.go | 83 +++++++++++++++++++++------- cli/internal/scope/scope_test.go | 58 +++++++++++++++++++ 5 files changed, 139 insertions(+), 107 deletions(-) rename cli/internal/{run => scope}/scope.go (71%) create mode 100644 cli/internal/scope/scope_test.go diff --git a/cli/internal/core/scheduler.go b/cli/internal/core/scheduler.go index a1f14bb55d063..a2506932823b5 100644 --- a/cli/internal/core/scheduler.go +++ b/cli/internal/core/scheduler.go @@ -2,9 +2,10 @@ package core import ( "fmt" - "github.com/vercel/turborepo/cli/internal/util" "strings" + "github.com/vercel/turborepo/cli/internal/util" + "github.com/pyr-sh/dag" ) @@ -63,6 +64,7 @@ type SchedulerExecutionOptions struct { func (p *scheduler) Prepare(options *SchedulerExecutionOptions) error { pkgs := options.Packages if len(pkgs) == 0 { + // TODO(gsoltis): Is this behavior only used in tests? for _, v := range p.TopologicGraph.Vertices() { pkgs = append(pkgs, dag.VertexName(v)) } diff --git a/cli/internal/run/run.go b/cli/internal/run/run.go index 09d1bfa4d6703..5a98b32865629 100644 --- a/cli/internal/run/run.go +++ b/cli/internal/run/run.go @@ -26,10 +26,10 @@ import ( "github.com/vercel/turborepo/cli/internal/globby" "github.com/vercel/turborepo/cli/internal/logstreamer" "github.com/vercel/turborepo/cli/internal/process" + "github.com/vercel/turborepo/cli/internal/scope" "github.com/vercel/turborepo/cli/internal/ui" "github.com/vercel/turborepo/cli/internal/util" "github.com/vercel/turborepo/cli/internal/util/browser" - "github.com/vercel/turborepo/cli/internal/util/filter" "github.com/pyr-sh/dag" @@ -153,7 +153,7 @@ func (c *RunCommand) Run(args []string) int { return 1 } - filteredPkgs, err := resolvePackagesInScope(runOptions, ctx, c.Ui, c.Config.Logger) + filteredPkgs, err := scope.ResolvePackages(runOptions.ScopeOpts(), ctx, c.Ui, c.Config.Logger) if err != nil { c.logError(c.Config.Logger, "", fmt.Errorf("failed resolve packages to run %v", err)) } @@ -659,6 +659,18 @@ type RunOptions struct { only bool } +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, + } +} + func getDefaultRunOptions() *RunOptions { return &RunOptions{ bail: true, @@ -792,41 +804,6 @@ func parseRunArgs(args []string, output cli.Ui) (*RunOptions, error) { return runOptions, nil } -// getScopedPackages returns a set of package names in scope for a given list of glob patterns -func getScopedPackages(packageNames []string, scopePatterns []string) (util.Set, error) { - scopedPkgs := make(util.Set) - if len(scopePatterns) == 0 { - return scopedPkgs, nil - } - - include := make([]string, 0, len(scopePatterns)) - exclude := make([]string, 0, len(scopePatterns)) - - for _, pattern := range scopePatterns { - if strings.HasPrefix(pattern, "!") { - exclude = append(exclude, pattern[1:]) - } else { - include = append(include, pattern) - } - } - - glob, err := filter.NewIncludeExcludeFilter(include, exclude) - if err != nil { - return nil, err - } - for _, f := range packageNames { - if glob.Match(f) { - scopedPkgs.Add(f) - } - } - - if len(include) > 0 && scopedPkgs.Len() == 0 { - return nil, errors.Errorf("No packages found matching the provided scope pattern.") - } - - return scopedPkgs, nil -} - // logError logs an error and outputs it to the UI. func (c *RunCommand) logError(log hclog.Logger, prefix string, err error) { log.Error(prefix, "error", err) diff --git a/cli/internal/run/run_test.go b/cli/internal/run/run_test.go index bab12deefae48..662c18abedb80 100644 --- a/cli/internal/run/run_test.go +++ b/cli/internal/run/run_test.go @@ -9,7 +9,6 @@ import ( "github.com/mitchellh/cli" "github.com/vercel/turborepo/cli/internal/fs" - "github.com/vercel/turborepo/cli/internal/util" "github.com/stretchr/testify/assert" ) @@ -167,55 +166,6 @@ func TestParseConfig(t *testing.T) { } } -func TestScopedPackages(t *testing.T) { - cases := []struct { - Name string - PackageNames []string - Pattern []string - Expected util.Set - }{ - { - "starts with @", - []string{"@sample/app", "sample-app", "jared"}, - []string{"@sample/*"}, - util.Set{"@sample/app": "@sample/app"}, - }, - { - "return an array of matches", - []string{"foo", "bar", "baz"}, - []string{"f*"}, - util.Set{"foo": "foo"}, - }, - { - "return an array of matches", - []string{"foo", "bar", "baz"}, - []string{"f*", "bar"}, - util.Set{"bar": "bar", "foo": "foo"}, - }, - { - "return matches in the order the list were defined", - []string{"foo", "bar", "baz"}, - []string{"*a*", "!f*"}, - util.Set{"bar": "bar", "baz": "baz"}, - }, - } - - for i, tc := range cases { - t.Run(fmt.Sprintf("%d-%s", i, tc.Name), func(t *testing.T) { - actual, err := getScopedPackages(tc.PackageNames, tc.Pattern) - if err != nil { - t.Fatalf("invalid scope parse: %#v", err) - } - assert.EqualValues(t, tc.Expected, actual) - }) - } - - t.Run(fmt.Sprintf("%d-%s", len(cases), "throws an error if no package matches the provided scope pattern"), func(t *testing.T) { - _, err := getScopedPackages([]string{"foo", "bar"}, []string{"baz"}) - assert.Error(t, err) - }) -} - func TestGetTargetsFromArguments(t *testing.T) { type args struct { arguments []string diff --git a/cli/internal/run/scope.go b/cli/internal/scope/scope.go similarity index 71% rename from cli/internal/run/scope.go rename to cli/internal/scope/scope.go index 6baf900f19c21..1dd669b044e74 100644 --- a/cli/internal/run/scope.go +++ b/cli/internal/scope/scope.go @@ -1,4 +1,4 @@ -package run +package scope import ( "fmt" @@ -16,8 +16,18 @@ import ( "github.com/vercel/turborepo/cli/internal/util/filter" ) -func resolvePackagesInScope(runOptions *RunOptions, ctx *context.Context, tui cli.Ui, logger hclog.Logger) (util.Set, error) { - changedFiles, err := getChangedFiles(runOptions) +type Opts struct { + IncludeDependencies bool + IncludeDependents bool + Patterns []string + Since string + Cwd string + IgnorePatterns []string + GlobalDepPatterns []string +} + +func ResolvePackages(opts *Opts, ctx *context.Context, tui cli.Ui, logger hclog.Logger) (util.Set, error) { + changedFiles, err := getChangedFiles(opts) if err != nil { return nil, err } @@ -26,11 +36,11 @@ func resolvePackagesInScope(runOptions *RunOptions, ctx *context.Context, tui cl // The user can technically specify both that a file is a global dependency and // that it should be ignored, and currently we treat a change to that file as a // global change. - hasRepoGlobalFileChanged, err := repoGlobalFileHasChanged(runOptions, changedFiles) + hasRepoGlobalFileChanged, err := repoGlobalFileHasChanged(opts, changedFiles) if err != nil { return nil, err } - filteredChangedFiles, err := filterIgnoredFiles(runOptions, changedFiles) + filteredChangedFiles, err := filterIgnoredFiles(opts, changedFiles) if err != nil { return nil, err } @@ -43,15 +53,15 @@ func resolvePackagesInScope(runOptions *RunOptions, ctx *context.Context, tui cl // Scoped packages // Unwind scope globs - scopePkgs, err := getScopedPackages(ctx.PackageNames, runOptions.scope) + scopePkgs, err := getScopedPackages(ctx.PackageNames, opts.Patterns) if err != nil { return nil, errors.Wrap(err, "invalid scope") } // Filter Packages filteredPkgs := make(util.Set) - includeDependencies := runOptions.includeDependencies - includeDependents := runOptions.includeDependents + includeDependencies := opts.IncludeDependencies + includeDependents := opts.IncludeDependents // If there has been a global change, changedPackages.Len() will be 0 // If both scoped and since are specified, we have to merge two lists: // 1. changed packages that ARE themselves the scoped packages @@ -63,17 +73,17 @@ func resolvePackagesInScope(runOptions *RunOptions, ctx *context.Context, tui cl includeDependents = true } - tui.Output(fmt.Sprintf(ui.Dim("• Packages changed since %s in scope: %s"), runOptions.since, strings.Join(filteredPkgs.UnsafeListOfStrings(), ", "))) + tui.Output(fmt.Sprintf(ui.Dim("• Packages changed since %s in scope: %s"), opts.Since, strings.Join(filteredPkgs.UnsafeListOfStrings(), ", "))) } else if changedPackages.Len() > 0 { // --since was specified, there are changes, but no scope was specified. // Run the packages that have changed filteredPkgs = changedPackages - tui.Output(fmt.Sprintf(ui.Dim("• Packages changed since %s: %s"), runOptions.since, strings.Join(filteredPkgs.UnsafeListOfStrings(), ", "))) + tui.Output(fmt.Sprintf(ui.Dim("• Packages changed since %s: %s"), opts.Since, strings.Join(filteredPkgs.UnsafeListOfStrings(), ", "))) } else if scopePkgs.Len() > 0 { // There was either a global change, or no changes, or no --since flag // There was a --scope flag, run the desired scopes filteredPkgs = scopePkgs - } else if runOptions.since == "" { + } else if opts.Since == "" { // No scope was specified, and no diff base was specified // Run every package for _, f := range ctx.PackageNames { @@ -109,11 +119,11 @@ func resolvePackagesInScope(runOptions *RunOptions, ctx *context.Context, tui cl return filteredPkgs, nil } -func getChangedFiles(runOptions *RunOptions) ([]string, error) { - if runOptions.since == "" { +func getChangedFiles(opts *Opts) ([]string, error) { + if opts.Since == "" { return []string{}, nil } - gitRepoRoot, err := fs.FindupFrom(".git", runOptions.cwd) + gitRepoRoot, err := fs.FindupFrom(".git", opts.Cwd) if err != nil { errors.Wrap(err, "Cannot find a .git folder in current working directory or in any parent directories. Falling back to manual file hashing (which may be slower). If you are running this build in a pruned directory, you can ignore this message. Otherwise, please initialize a git repository in the root of your monorepo.") } @@ -121,11 +131,11 @@ func getChangedFiles(runOptions *RunOptions) ([]string, error) { if err != nil { return nil, err } - return git.ChangedFiles(runOptions.since, true, runOptions.cwd), nil + return git.ChangedFiles(opts.Since, true, opts.Cwd), nil } -func repoGlobalFileHasChanged(runOptions *RunOptions, changedFiles []string) (bool, error) { - globalDepsGlob, err := filter.Compile(runOptions.globalDeps) +func repoGlobalFileHasChanged(opts *Opts, changedFiles []string) (bool, error) { + globalDepsGlob, err := filter.Compile(opts.GlobalDepPatterns) if err != nil { return false, errors.Wrap(err, "invalid global deps glob") } @@ -140,8 +150,8 @@ func repoGlobalFileHasChanged(runOptions *RunOptions, changedFiles []string) (bo return false, nil } -func filterIgnoredFiles(runOptions *RunOptions, changedFiles []string) (util.Set, error) { - ignoreGlob, err := filter.Compile(runOptions.ignore) +func filterIgnoredFiles(opts *Opts, changedFiles []string) (util.Set, error) { + ignoreGlob, err := filter.Compile(opts.IgnorePatterns) if err != nil { return nil, errors.Wrap(err, "invalid ignore globs") } @@ -200,3 +210,38 @@ func addDependencies(deps util.Set, pkg interface{}, ctx *context.Context, logge } return nil } + +// getScopedPackages returns a set of package names in scope for a given list of glob patterns +func getScopedPackages(packageNames []string, scopePatterns []string) (util.Set, error) { + scopedPkgs := make(util.Set) + if len(scopePatterns) == 0 { + return scopedPkgs, nil + } + + include := make([]string, 0, len(scopePatterns)) + exclude := make([]string, 0, len(scopePatterns)) + + for _, pattern := range scopePatterns { + if strings.HasPrefix(pattern, "!") { + exclude = append(exclude, pattern[1:]) + } else { + include = append(include, pattern) + } + } + + glob, err := filter.NewIncludeExcludeFilter(include, exclude) + if err != nil { + return nil, err + } + for _, f := range packageNames { + if glob.Match(f) { + scopedPkgs.Add(f) + } + } + + if len(include) > 0 && scopedPkgs.Len() == 0 { + return nil, errors.Errorf("No packages found matching the provided scope pattern.") + } + + return scopedPkgs, nil +} diff --git a/cli/internal/scope/scope_test.go b/cli/internal/scope/scope_test.go new file mode 100644 index 0000000000000..b1e94d1095be0 --- /dev/null +++ b/cli/internal/scope/scope_test.go @@ -0,0 +1,58 @@ +package scope + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/vercel/turborepo/cli/internal/util" +) + +func TestScopedPackages(t *testing.T) { + cases := []struct { + Name string + PackageNames []string + Pattern []string + Expected util.Set + }{ + { + "starts with @", + []string{"@sample/app", "sample-app", "jared"}, + []string{"@sample/*"}, + util.Set{"@sample/app": "@sample/app"}, + }, + { + "return an array of matches", + []string{"foo", "bar", "baz"}, + []string{"f*"}, + util.Set{"foo": "foo"}, + }, + { + "return an array of matches", + []string{"foo", "bar", "baz"}, + []string{"f*", "bar"}, + util.Set{"bar": "bar", "foo": "foo"}, + }, + { + "return matches in the order the list were defined", + []string{"foo", "bar", "baz"}, + []string{"*a*", "!f*"}, + util.Set{"bar": "bar", "baz": "baz"}, + }, + } + + for i, tc := range cases { + t.Run(fmt.Sprintf("%d-%s", i, tc.Name), func(t *testing.T) { + actual, err := getScopedPackages(tc.PackageNames, tc.Pattern) + if err != nil { + t.Fatalf("invalid scope parse: %#v", err) + } + assert.EqualValues(t, tc.Expected, actual) + }) + } + + t.Run(fmt.Sprintf("%d-%s", len(cases), "throws an error if no package matches the provided scope pattern"), func(t *testing.T) { + _, err := getScopedPackages([]string{"foo", "bar"}, []string{"baz"}) + assert.Error(t, err) + }) +} From 06fb2e8378bf768d1760cfe1ccabb972fe4f7e43 Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Sun, 6 Mar 2022 20:17:41 -0800 Subject: [PATCH 4/6] Add ResolveScope test. Does not pass yet --- cli/internal/run/run.go | 8 +- cli/internal/scm/scm.go | 28 +++--- cli/internal/scope/scope.go | 17 +--- cli/internal/scope/scope_test.go | 149 +++++++++++++++++++++++++++++++ 4 files changed, 170 insertions(+), 32 deletions(-) diff --git a/cli/internal/run/run.go b/cli/internal/run/run.go index 5a98b32865629..8bb615425e4e4 100644 --- a/cli/internal/run/run.go +++ b/cli/internal/run/run.go @@ -26,6 +26,7 @@ import ( "github.com/vercel/turborepo/cli/internal/globby" "github.com/vercel/turborepo/cli/internal/logstreamer" "github.com/vercel/turborepo/cli/internal/process" + "github.com/vercel/turborepo/cli/internal/scm" "github.com/vercel/turborepo/cli/internal/scope" "github.com/vercel/turborepo/cli/internal/ui" "github.com/vercel/turborepo/cli/internal/util" @@ -153,7 +154,12 @@ func (c *RunCommand) Run(args []string) int { return 1 } - filteredPkgs, err := scope.ResolvePackages(runOptions.ScopeOpts(), ctx, c.Ui, c.Config.Logger) + scm, err := scm.FromInRepo(runOptions.cwd) + if err != nil { + c.logError(c.Config.Logger, "", fmt.Errorf("failed to create SCM: %w", err)) + return 1 + } + filteredPkgs, err := scope.ResolvePackages(runOptions.ScopeOpts(), scm, ctx, c.Ui, c.Config.Logger) if err != nil { c.logError(c.Config.Logger, "", fmt.Errorf("failed resolve packages to run %v", err)) } diff --git a/cli/internal/scm/scm.go b/cli/internal/scm/scm.go index cd6b287fdd799..35b29210dba9e 100644 --- a/cli/internal/scm/scm.go +++ b/cli/internal/scm/scm.go @@ -5,30 +5,14 @@ package scm import ( "fmt" "path/filepath" + "github.com/vercel/turborepo/cli/internal/fs" ) // An SCM represents an SCM implementation that we can ask for various things. type SCM interface { - // DescribeIdentifier returns the string that is a "human-readable" identifier of the given revision. - DescribeIdentifier(revision string) string - // CurrentRevIdentifier returns the string that specifies what the current revision is. - CurrentRevIdentifier() string - // ChangesIn returns a list of modified files in the given diffSpec. - ChangesIn(diffSpec string, relativeTo string) []string - // ChangedFiles returns a list of modified files since the given commit, optionally including untracked files. + // ChangedFiles returns a list of modified files since the given commit, optionally including untracked files.*/ ChangedFiles(fromCommit string, includeUntracked bool, relativeTo string) []string - // IgnoreFile marks a file to be ignored by the SCM. - IgnoreFiles(gitignore string, files []string) error - // Remove deletes the given files from the SCM. - Remove(names []string) error - // ChangedLines returns the set of lines that have been modified, - // as a map of filename -> affected line numbers. - ChangedLines() (map[string][]int, error) - // Checkout checks out the given revision. - Checkout(revision string) error - // CurrentRevDate returns the commit date of the current revision, formatted according to the given format string. - CurrentRevDate(format string) string } // New returns a new SCM instance for this repo root. @@ -49,3 +33,11 @@ func NewFallback(repoRoot string) (SCM, error) { return &stub{}, fmt.Errorf("cannot find a .git folder. Falling back to manual file hashing (which may be slower). If you are running this build in a pruned directory, you can ignore this message. Otherwise, please initialize a git repository in the root of your monorepo") } + +func FromInRepo(cwd string) (SCM, error) { + repoRoot, err := fs.FindupFrom(".git", cwd) + if err != nil { + return nil, err + } + return NewFallback(repoRoot) +} diff --git a/cli/internal/scope/scope.go b/cli/internal/scope/scope.go index 1dd669b044e74..e8a3f07a27b6e 100644 --- a/cli/internal/scope/scope.go +++ b/cli/internal/scope/scope.go @@ -2,7 +2,6 @@ package scope import ( "fmt" - "path/filepath" "strings" "github.com/hashicorp/go-hclog" @@ -26,8 +25,8 @@ type Opts struct { GlobalDepPatterns []string } -func ResolvePackages(opts *Opts, ctx *context.Context, tui cli.Ui, logger hclog.Logger) (util.Set, error) { - changedFiles, err := getChangedFiles(opts) +func ResolvePackages(opts *Opts, scm scm.SCM, ctx *context.Context, tui cli.Ui, logger hclog.Logger) (util.Set, error) { + changedFiles, err := getChangedFiles(opts, scm) if err != nil { return nil, err } @@ -119,19 +118,11 @@ func ResolvePackages(opts *Opts, ctx *context.Context, tui cli.Ui, logger hclog. return filteredPkgs, nil } -func getChangedFiles(opts *Opts) ([]string, error) { +func getChangedFiles(opts *Opts, scm scm.SCM) ([]string, error) { if opts.Since == "" { return []string{}, nil } - gitRepoRoot, err := fs.FindupFrom(".git", opts.Cwd) - if err != nil { - errors.Wrap(err, "Cannot find a .git folder in current working directory or in any parent directories. Falling back to manual file hashing (which may be slower). If you are running this build in a pruned directory, you can ignore this message. Otherwise, please initialize a git repository in the root of your monorepo.") - } - git, err := scm.NewFallback(filepath.Dir(gitRepoRoot)) - if err != nil { - return nil, err - } - return git.ChangedFiles(opts.Since, true, opts.Cwd), nil + return scm.ChangedFiles(opts.Since, true, opts.Cwd), nil } func repoGlobalFileHasChanged(opts *Opts, changedFiles []string) (bool, error) { diff --git a/cli/internal/scope/scope_test.go b/cli/internal/scope/scope_test.go index b1e94d1095be0..494263d1dec2b 100644 --- a/cli/internal/scope/scope_test.go +++ b/cli/internal/scope/scope_test.go @@ -2,9 +2,16 @@ package scope import ( "fmt" + "reflect" + "strings" "testing" + "github.com/hashicorp/go-hclog" + "github.com/pyr-sh/dag" "github.com/stretchr/testify/assert" + "github.com/vercel/turborepo/cli/internal/context" + "github.com/vercel/turborepo/cli/internal/fs" + "github.com/vercel/turborepo/cli/internal/ui" "github.com/vercel/turborepo/cli/internal/util" ) @@ -56,3 +63,145 @@ func TestScopedPackages(t *testing.T) { assert.Error(t, err) }) } + +type mockSCM struct { + changed []string +} + +func (m *mockSCM) ChangedFiles(fromCommit string, includeUntracked bool, relativeTo string) []string { + changed := []string{} + for _, change := range m.changed { + if strings.HasPrefix(change, relativeTo) { + changed = append(changed, change) + } + } + return changed +} + +func TestResolvePackages(t *testing.T) { + tui := ui.Default() + logger := hclog.Default() + // app1 -> libA -> libB + // + // / libB + // app2 < + // \ libC + // + graph := dag.AcyclicGraph{} + graph.Add("app1") + graph.Add("app2") + graph.Add("libA") + graph.Add("libB") + graph.Add("libC") + graph.Connect(dag.BasicEdge("libA", "libB")) + graph.Connect(dag.BasicEdge("app1", "libA")) + graph.Connect(dag.BasicEdge("app2", "libB")) + graph.Connect(dag.BasicEdge("app2", "libC")) + scc := dag.StronglyConnected(&graph.Graph) + packagesInfos := map[interface{}]*fs.PackageJSON{ + "app1": { + Dir: "app/app1", + }, + "app2": { + Dir: "app/app2", + }, + "libA": { + Dir: "libs/libA", + }, + "libB": { + Dir: "libs/libB", + }, + "libC": { + Dir: "libs/libC", + }, + } + packageNames := []string{} + for name := range packagesInfos { + packageNames = append(packageNames, name.(string)) + } + + testCases := []struct { + changed []string + expected []string + scope []string + since string + ignore string + globalDeps []string + includeDependencies bool + includeDependents bool + }{ + { + changed: []string{"libs/libB/src/index.ts"}, + expected: []string{"libB"}, + since: "dummy", + }, + { + changed: []string{"libs/libB/src/index.ts"}, + expected: []string{}, + since: "dummy", + ignore: "libs/libB/**/*.ts", + }, + { + // a non-dependent lib changed + changed: []string{"libs/libC/src/index.ts"}, + expected: []string{}, + since: "dummy", + scope: []string{"app1"}, + }, + { + changed: []string{"libs/libB/src/index.ts"}, + // expect everything, global changed, no scope + expected: []string{"app1", "app2", "libA", "libB", "libC"}, + since: "dummy", + ignore: "libs/libB/**/*.ts", + globalDeps: []string{"libs/**/*.ts"}, + }, + { + changed: []string{"app/app2/src/index.ts"}, + since: "dummy", + includeDependencies: true, + expected: []string{"app2", "libB", "libC"}, + }, + { + changed: []string{"libs/libB"}, + since: "dummy", + includeDependents: true, + expected: []string{"app1", "app2", "libA", "libB"}, + }, + { + // no changes, no base to compare against, defaults to everything + since: "", + expected: []string{"app1", "app2", "libA", "libB", "libC"}, + }, + } + for i, tc := range testCases { + t.Run(fmt.Sprintf("test #%v", i), func(t *testing.T) { + scm := &mockSCM{ + changed: tc.changed, + } + pkgs, err := ResolvePackages(&Opts{ + Patterns: tc.scope, + Since: tc.since, + IgnorePatterns: []string{tc.ignore}, + GlobalDepPatterns: tc.globalDeps, + IncludeDependencies: tc.includeDependencies, + IncludeDependents: tc.includeDependents, + }, scm, &context.Context{ + PackageInfos: packagesInfos, + PackageNames: packageNames, + TopologicalGraph: graph, + SCC: scc, + }, tui, logger) + if err != nil { + t.Errorf("expected no error, got %v", err) + } + expected := make(util.Set) + for _, pkg := range tc.expected { + expected.Add(pkg) + } + if !reflect.DeepEqual(pkgs, expected) { + t.Errorf("ResolvePackages got %v, want %v", pkgs, expected) + } + }) + } +} From f9a1abbf046b00cdb7e279edcf4f46178c26e771 Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Mon, 7 Mar 2022 14:01:31 -0800 Subject: [PATCH 5/6] More tests for scope, as well as some tweaks to run options for backwards compatibility --- cli/internal/run/run.go | 22 +++++-- cli/internal/run/run_test.go | 16 +++++ cli/internal/scm/scm.go | 7 ++- cli/internal/scope/scope.go | 57 ++++++++++++++--- cli/internal/scope/scope_test.go | 102 ++++++++++++++++++++++++++----- 5 files changed, 173 insertions(+), 31 deletions(-) diff --git a/cli/internal/run/run.go b/cli/internal/run/run.go index 8bb615425e4e4..02e77620226fd 100644 --- a/cli/internal/run/run.go +++ b/cli/internal/run/run.go @@ -154,12 +154,16 @@ func (c *RunCommand) Run(args []string) int { return 1 } - scm, err := scm.FromInRepo(runOptions.cwd) + scmInstance, err := scm.FromInRepo(runOptions.cwd) if err != nil { - c.logError(c.Config.Logger, "", fmt.Errorf("failed to create SCM: %w", err)) - return 1 + if errors.Is(err, scm.ErrFallback) { + c.logWarning(c.Config.Logger, "", err) + } else { + c.logError(c.Config.Logger, "", fmt.Errorf("failed to create SCM: %w", err)) + return 1 + } } - filteredPkgs, err := scope.ResolvePackages(runOptions.ScopeOpts(), scm, ctx, c.Ui, c.Config.Logger) + filteredPkgs, err := scope.ResolvePackages(runOptions.ScopeOpts(), scmInstance, ctx, c.Ui, c.Config.Logger) if err != nil { c.logError(c.Config.Logger, "", fmt.Errorf("failed resolve packages to run %v", err)) } @@ -708,6 +712,11 @@ func parseRunArgs(args []string, output cli.Ui) (*RunOptions, error) { unresolvedCacheFolder := filepath.FromSlash("./node_modules/.cache/turbo") + // --scope and --since implies --include-dependencies for backwards compatibility + // When we switch to cobra we will need to track if it's been set manually. Currently + // it's only possible to set to true, but in the future a user could theoretically set + // it to false and override the default behavior. + includDepsSet := false for argIndex, arg := range args { if arg == "--" { runOptions.passThroughArgs = args[argIndex+1:] @@ -781,8 +790,10 @@ func parseRunArgs(args []string, output cli.Ui) (*RunOptions, error) { 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 + includDepsSet = true case strings.HasPrefix(arg, "--include-dependencies"): runOptions.includeDependencies = true + includDepsSet = true case strings.HasPrefix(arg, "--only"): runOptions.only = true case strings.HasPrefix(arg, "--team"): @@ -798,6 +809,9 @@ func parseRunArgs(args []string, output cli.Ui) (*RunOptions, error) { } } } + if len(runOptions.scope) != 0 && runOptions.since != "" && !includDepsSet { + runOptions.includeDependencies = true + } // Force streaming output in CI/CD non-interactive mode if !ui.IsTTY || ui.IsCI { diff --git a/cli/internal/run/run_test.go b/cli/internal/run/run_test.go index 662c18abedb80..aaeb9ca43538b 100644 --- a/cli/internal/run/run_test.go +++ b/cli/internal/run/run_test.go @@ -146,6 +146,22 @@ func TestParseConfig(t *testing.T) { passThroughArgs: []string{}, }, }, + { + "since and scope imply including dependencies for backwards compatibility", + []string{"foo", "--scope=bar", "--since=some-ref"}, + &RunOptions{ + includeDependents: true, + stream: true, + bail: true, + concurrency: 10, + includeDependencies: true, + cache: true, + cwd: defaultCwd, + cacheFolder: defaultCacheFolder, + scope: []string{"bar"}, + since: "some-ref", + }, + }, } ui := &cli.BasicUi{ diff --git a/cli/internal/scm/scm.go b/cli/internal/scm/scm.go index 35b29210dba9e..0f837d468bcf6 100644 --- a/cli/internal/scm/scm.go +++ b/cli/internal/scm/scm.go @@ -3,12 +3,15 @@ package scm import ( - "fmt" "path/filepath" + "github.com/pkg/errors" + "github.com/vercel/turborepo/cli/internal/fs" ) +var ErrFallback = errors.New("cannot find a .git folder. Falling back to manual file hashing (which may be slower). If you are running this build in a pruned directory, you can ignore this message. Otherwise, please initialize a git repository in the root of your monorepo") + // An SCM represents an SCM implementation that we can ask for various things. type SCM interface { // ChangedFiles returns a list of modified files since the given commit, optionally including untracked files.*/ @@ -31,7 +34,7 @@ func NewFallback(repoRoot string) (SCM, error) { return scm, nil } - return &stub{}, fmt.Errorf("cannot find a .git folder. Falling back to manual file hashing (which may be slower). If you are running this build in a pruned directory, you can ignore this message. Otherwise, please initialize a git repository in the root of your monorepo") + return &stub{}, ErrFallback } func FromInRepo(cwd string) (SCM, error) { diff --git a/cli/internal/scope/scope.go b/cli/internal/scope/scope.go index e8a3f07a27b6e..c648677c1f59c 100644 --- a/cli/internal/scope/scope.go +++ b/cli/internal/scope/scope.go @@ -61,16 +61,55 @@ func ResolvePackages(opts *Opts, scm scm.SCM, ctx *context.Context, tui cli.Ui, filteredPkgs := make(util.Set) includeDependencies := opts.IncludeDependencies includeDependents := opts.IncludeDependents - // If there has been a global change, changedPackages.Len() will be 0 - // If both scoped and since are specified, we have to merge two lists: - // 1. changed packages that ARE themselves the scoped packages - // 2. changed package consumers (package dependents) that are within the scoped subgraph - if scopePkgs.Len() > 0 && changedPackages.Len() > 0 { - filteredPkgs = scopePkgs.Intersection(changedPackages) - for _, changed := range changedPackages { - filteredPkgs.Add(changed) - includeDependents = true + // If there has been a global change, run everything in scope + // (this may be every package if no scope is provider) + if hasRepoGlobalFileChanged { + // If a global dependency has changed, run everything in scope. + // If no scope was provided, run everything + if scopePkgs.Len() > 0 { + filteredPkgs = scopePkgs + } else { + for _, f := range ctx.PackageNames { + filteredPkgs.Add(f) + } + } + } else if scopePkgs.Len() > 0 && changedPackages.Len() > 0 { + // If we have both a scope and changed packages: + // We want the intersection of two sets: + // 1. the scopes and all of their dependencies + // 2. the changed packages and all of their dependents + // + // Note that other commandline flags can cause including dependents / dependencies + // beyond this set + + // scopes and all deps + rootsAndDeps := make(util.Set) + for _, pkg := range scopePkgs { + rootsAndDeps.Add(pkg) + deps, err := ctx.TopologicalGraph.Ancestors(pkg) + if err != nil { + return nil, err + } + for _, dep := range deps { + rootsAndDeps.Add(dep) + } + } + // changed packages and all dependents + for _, pkg := range changedPackages { + // do the intersection inline, rather than building up the set + if rootsAndDeps.Includes(pkg) { + filteredPkgs.Add(pkg) + } + dependents, err := ctx.TopologicalGraph.Descendents(pkg) + if err != nil { + return nil, err + } + for _, dependent := range dependents { + if rootsAndDeps.Includes(dependent) { + filteredPkgs.Add(dependent) + } + } } tui.Output(fmt.Sprintf(ui.Dim("• Packages changed since %s in scope: %s"), opts.Since, strings.Join(filteredPkgs.UnsafeListOfStrings(), ", "))) } else if changedPackages.Len() > 0 { diff --git a/cli/internal/scope/scope_test.go b/cli/internal/scope/scope_test.go index 494263d1dec2b..7244cfa939f1c 100644 --- a/cli/internal/scope/scope_test.go +++ b/cli/internal/scope/scope_test.go @@ -81,24 +81,35 @@ func (m *mockSCM) ChangedFiles(fromCommit string, includeUntracked bool, relativ func TestResolvePackages(t *testing.T) { tui := ui.Default() logger := hclog.Default() - // app1 -> libA -> libB // - // / libB - // app2 < - // \ libC + // app0 - + // \ + // app1 -> libA + // \ + // > libB -> libD + // / + // app2 < + // \ libC // graph := dag.AcyclicGraph{} + graph.Add("app0") graph.Add("app1") graph.Add("app2") graph.Add("libA") graph.Add("libB") graph.Add("libC") + graph.Add("libD") graph.Connect(dag.BasicEdge("libA", "libB")) + graph.Connect(dag.BasicEdge("libB", "libD")) + graph.Connect(dag.BasicEdge("app0", "libA")) graph.Connect(dag.BasicEdge("app1", "libA")) graph.Connect(dag.BasicEdge("app2", "libB")) graph.Connect(dag.BasicEdge("app2", "libC")) scc := dag.StronglyConnected(&graph.Graph) packagesInfos := map[interface{}]*fs.PackageJSON{ + "app0": { + Dir: "app/app0", + }, "app1": { Dir: "app/app1", }, @@ -114,6 +125,9 @@ func TestResolvePackages(t *testing.T) { "libC": { Dir: "libs/libC", }, + "libD": { + Dir: "libs/libD", + }, } packageNames := []string{} for name := range packagesInfos { @@ -121,6 +135,7 @@ func TestResolvePackages(t *testing.T) { } testCases := []struct { + name string changed []string expected []string scope []string @@ -131,51 +146,106 @@ func TestResolvePackages(t *testing.T) { includeDependents bool }{ { + name: "One package changed", changed: []string{"libs/libB/src/index.ts"}, expected: []string{"libB"}, since: "dummy", }, { + name: "An ignored package changed", changed: []string{"libs/libB/src/index.ts"}, expected: []string{}, since: "dummy", ignore: "libs/libB/**/*.ts", }, { - // a non-dependent lib changed - changed: []string{"libs/libC/src/index.ts"}, - expected: []string{}, - since: "dummy", - scope: []string{"app1"}, + // nothing in scope depends on the change + name: "unrelated library changed", + changed: []string{"libs/libC/src/index.ts"}, + expected: []string{}, + since: "dummy", + scope: []string{"app1"}, + includeDependencies: true, // scope implies include-dependencies + }, + { + // a dependent lib changed, scope implies include-dependencies, + // so all deps of app1 get built + name: "dependency of scope changed", + changed: []string{"libs/libA/src/index.ts"}, + expected: []string{"libA", "libB", "libD", "app1"}, + since: "dummy", + scope: []string{"app1"}, + includeDependencies: true, // scope implies include-dependencies + }, + { + // a dependent lib changed, user explicitly asked to not build dependencies + // note: this is not yet supported by the CLI, as you cannot specify --include-dependencies=false + name: "dependency of scope changed, user asked to not include depedencies", + changed: []string{"libs/libA/src/index.ts"}, + expected: []string{"libA", "app1"}, + since: "dummy", + scope: []string{"app1"}, + includeDependencies: false, }, { - changed: []string{"libs/libB/src/index.ts"}, - // expect everything, global changed, no scope - expected: []string{"app1", "app2", "libA", "libB", "libC"}, + // a nested dependent lib changed, user explicitly asked to not build dependencies + // note: this is not yet supported by the CLI, as you cannot specify --include-dependencies=false + name: "nested dependency of scope changed, user asked to not include dependencies", + changed: []string{"libs/libB/src/index.ts"}, + expected: []string{"libA", "libB", "app1"}, + since: "dummy", + scope: []string{"app1"}, + includeDependencies: false, + }, + { + name: "global dependency changed, even though it was ignored, forcing a build of everything", + changed: []string{"libs/libB/src/index.ts"}, + expected: []string{"app0", "app1", "app2", "libA", "libB", "libC", "libD"}, since: "dummy", ignore: "libs/libB/**/*.ts", globalDeps: []string{"libs/**/*.ts"}, }, { + name: "an app changed, user asked for dependencies to build", changed: []string{"app/app2/src/index.ts"}, since: "dummy", includeDependencies: true, - expected: []string{"app2", "libB", "libC"}, + expected: []string{"app2", "libB", "libC", "libD"}, }, { + name: "a library changed, user asked for dependents to be built", changed: []string{"libs/libB"}, since: "dummy", includeDependents: true, - expected: []string{"app1", "app2", "libA", "libB"}, + expected: []string{"app0", "app1", "app2", "libA", "libB"}, }, { // no changes, no base to compare against, defaults to everything + name: "no changes or scope specified, build everything", since: "", - expected: []string{"app1", "app2", "libA", "libB", "libC"}, + expected: []string{"app0", "app1", "app2", "libA", "libB", "libC", "libD"}, + }, + { + // a dependent library changed, no deps beyond the scope are build + // "libB" is still built because it is a dependent within the scope, but libB's dependents + // are skipped + name: "a dependent library changed, build up to scope", + changed: []string{"libs/libD/src/index.ts"}, + since: "dummy", + scope: []string{"libB"}, + expected: []string{"libB", "libD"}, + includeDependencies: true, // scope implies include-dependencies + }, + { + name: "library change, no scope", + changed: []string{"libs/libA/src/index.ts"}, + expected: []string{"libA", "app0", "app1"}, + includeDependents: true, + since: "dummy", }, } for i, tc := range testCases { - t.Run(fmt.Sprintf("test #%v", i), func(t *testing.T) { + t.Run(fmt.Sprintf("test #%v %v", i, tc.name), func(t *testing.T) { scm := &mockSCM{ changed: tc.changed, } From 5e9a26dbe3f3787b4dde3f7b979c55cca940a91a Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Mon, 7 Mar 2022 14:19:19 -0800 Subject: [PATCH 6/6] Fix scm root path --- cli/internal/scm/scm.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cli/internal/scm/scm.go b/cli/internal/scm/scm.go index 0f837d468bcf6..530e120143101 100644 --- a/cli/internal/scm/scm.go +++ b/cli/internal/scm/scm.go @@ -38,9 +38,9 @@ func NewFallback(repoRoot string) (SCM, error) { } func FromInRepo(cwd string) (SCM, error) { - repoRoot, err := fs.FindupFrom(".git", cwd) + dotGitDir, err := fs.FindupFrom(".git", cwd) if err != nil { return nil, err } - return NewFallback(repoRoot) + return NewFallback(filepath.Dir(dotGitDir)) }