From 06d57c113b46e4a6a9f9094665493257b8130bcc Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Mon, 21 Mar 2022 16:39:09 -0700 Subject: [PATCH] Stop removing unreferenced packages from the topo graph before building the task graph --- .vscode/launch.json | 9 ++++ cli/internal/core/scheduler_test.go | 71 +++++++++++++++++++++++++++++ cli/internal/run/run.go | 16 ------- cli/internal/run/run_test.go | 18 -------- 4 files changed, 80 insertions(+), 34 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 55f314e399e2c..0a74c16062289 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -13,6 +13,15 @@ "cwd": "${workspaceRoot}/examples/basic", "args": ["run", "build"] }, + { + "name": "Kitchen Sink Build", + "type": "go", + "request": "launch", + "mode": "debug", + "program": "${workspaceRoot}/cli/cmd/turbo", + "cwd": "${workspaceRoot}/examples/kitchen-sink", + "args": ["run", "build", "--dry"] + }, { "name": "Turbo Version", "type": "go", diff --git a/cli/internal/core/scheduler_test.go b/cli/internal/core/scheduler_test.go index 0245667c4fa5c..741cc9aa1e461 100644 --- a/cli/internal/core/scheduler_test.go +++ b/cli/internal/core/scheduler_test.go @@ -79,6 +79,77 @@ func TestSchedulerDefault(t *testing.T) { } } +func TestDependenciesOnUnspecifiedPackages(t *testing.T) { + // app1 -> libA + // \ + // > libB -> libD + // / + // app2 < + // \ libC + // + graph := &dag.AcyclicGraph{} + 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")) + + p := NewScheduler(graph) + dependOnBuild := make(util.Set) + dependOnBuild.Add("build") + p.AddTask(&Task{ + Name: "build", + TopoDeps: dependOnBuild, + Deps: make(util.Set), + }) + p.AddTask(&Task{ + Name: "test", + TopoDeps: dependOnBuild, + Deps: make(util.Set), + }) + // We're only requesting one package ("scope"), + // but the combination of that package and task causes + // dependencies to also get run. This is the equivalent of + // turbo run test --filter=app2 + err := p.Prepare(&SchedulerExecutionOptions{ + Packages: []string{"app2"}, + TaskNames: []string{"test"}, + }) + if err != nil { + t.Fatalf("failed to prepare scheduler: %v", err) + } + errs := p.Execute(testVisitor, ExecOpts{ + Concurrency: 10, + }) + for _, err := range errs { + t.Fatalf("error executing tasks: %v", err) + } + expected := ` +___ROOT___ +app2#test + libB#build + libC#build +libB#build + libD#build +libC#build + ___ROOT___ +libD#build + ___ROOT___ +` + expected = strings.TrimSpace(expected) + actual := strings.TrimSpace(p.TaskGraph.String()) + if actual != expected { + t.Errorf("task graph got:\n%v\nwant:\n%v", actual, expected) + } +} + func TestSchedulerTasksOnly(t *testing.T) { var g dag.AcyclicGraph g.Add("a") diff --git a/cli/internal/run/run.go b/cli/internal/run/run.go index 3c7cf891a776a..e5b5429ed0b84 100644 --- a/cli/internal/run/run.go +++ b/cli/internal/run/run.go @@ -259,12 +259,6 @@ func (c *RunCommand) runOperation(g *completeGraph, rs *runSpec, backend *api.La for _, v := range g.TopologicalGraph.Vertices() { vertexSet.Add(v) } - // We remove nodes that aren't in the final filter set - for _, toRemove := range vertexSet.Difference(rs.FilteredPkgs) { - if toRemove != g.RootNode { - g.TopologicalGraph.Remove(toRemove) - } - } // If we are running in parallel, then we remove all the edges in the graph // except for the root @@ -496,11 +490,6 @@ 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:] @@ -574,10 +563,8 @@ 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, "--output-logs="): @@ -621,9 +608,6 @@ 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 d146df212c601..40d9848ba2b5f 100644 --- a/cli/internal/run/run_test.go +++ b/cli/internal/run/run_test.go @@ -160,24 +160,6 @@ func TestParseConfig(t *testing.T) { cacheMissLogsMode: FullLogs, }, }, - { - "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", - cacheHitLogsMode: FullLogs, - cacheMissLogsMode: FullLogs, - }, - }, } ui := &cli.BasicUi{