Skip to content

Commit

Permalink
Calculate TaskHash while constructing PackageTask (vercel#4016)
Browse files Browse the repository at this point in the history
We want to calculate this earlier, because it has some important side
effects such as gathering the hashable inputs, etc. By moving this up
the chain, we can ensure that more information is available for run
summary and dry run summary in a consistent way.
  • Loading branch information
mehulkar authored Mar 1, 2023
1 parent 3cd8242 commit e680e72
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 17 deletions.
4 changes: 2 additions & 2 deletions cli/integration_tests/basic_monorepo/verbosity.t
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ Verbosity level 2
\xe2\x80\xa2 Packages in scope: util (esc)
\xe2\x80\xa2 Running build in 1 packages (esc)
\xe2\x80\xa2 Remote caching disabled (esc)
[-0-9:.TWZ+]+ \[DEBUG] turbo.: start (re)
[-0-9:.TWZ+]+ \[DEBUG] turbo: task hash env vars for util:build: vars=\[] (re)
[-0-9:.TWZ+]+ \[DEBUG] turbo.: start (re)
[-0-9:.TWZ+]+ \[DEBUG] turbo: task hash: value=d09a52ea72495c87 (re)
util:build: cache bypass, force executing d09a52ea72495c87
util:build:
Expand Down Expand Up @@ -81,8 +81,8 @@ Verbosity level 2
\xe2\x80\xa2 Packages in scope: util (esc)
\xe2\x80\xa2 Running build in 1 packages (esc)
\xe2\x80\xa2 Remote caching disabled (esc)
[-0-9:.TWZ+]+ \[DEBUG] turbo.: start (re)
[-0-9:.TWZ+]+ \[DEBUG] turbo: task hash env vars for util:build: vars=\[] (re)
[-0-9:.TWZ+]+ \[DEBUG] turbo.: start (re)
[-0-9:.TWZ+]+ \[DEBUG] turbo: task hash: value=d09a52ea72495c87 (re)
util:build: cache bypass, force executing d09a52ea72495c87
util:build:
Expand Down
22 changes: 21 additions & 1 deletion cli/internal/graph/graph.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"path/filepath"

"github.com/hashicorp/go-hclog"
"github.com/pyr-sh/dag"
"github.com/vercel/turbo/cli/internal/fs"
"github.com/vercel/turbo/cli/internal/nodes"
Expand Down Expand Up @@ -42,7 +43,13 @@ type CompleteGraph struct {
// GetPackageTaskVisitor wraps a `visitor` function that is used for walking the TaskGraph
// during execution (or dry-runs). The function returned here does not execute any tasks itself,
// but it helps curry some data from the Complete Graph and pass it into the visitor function.
func (g *CompleteGraph) GetPackageTaskVisitor(ctx gocontext.Context, visitor func(ctx gocontext.Context, packageTask *nodes.PackageTask) error) func(taskID string) error {
func (g *CompleteGraph) GetPackageTaskVisitor(
ctx gocontext.Context,
taskGraph *dag.AcyclicGraph,
getArgs func(taskID string) []string,
logger hclog.Logger,
visitor func(ctx gocontext.Context, packageTask *nodes.PackageTask) error,
) func(taskID string) error {
return func(taskID string) error {
packageName, taskName := util.GetPackageTaskFromId(taskID)
pkg, ok := g.WorkspaceInfos.PackageJSONs[packageName]
Expand All @@ -66,6 +73,19 @@ func (g *CompleteGraph) GetPackageTaskVisitor(ctx gocontext.Context, visitor fun
ExcludedOutputs: taskDefinition.Outputs.Exclusions,
}

hash, err := g.TaskHashTracker.CalculateTaskHash(
packageTask,
taskGraph.DownEdges(taskID),
logger,
getArgs(taskName),
)

if err != nil {
return fmt.Errorf("Hashing error: %v", err)
}

packageTask.Hash = hash

packageTask.ExpandedInputs = g.TaskHashTracker.GetExpandedInputs(packageTask)

if cmd, ok := pkg.Scripts[taskName]; ok {
Expand Down
1 change: 1 addition & 0 deletions cli/internal/nodes/packagetask.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ type PackageTask struct {
ExcludedOutputs []string
LogFile string
ExpandedInputs map[turbopath.AnchoredUnixPath]string
Hash string
}

// OutputPrefix returns the prefix to be used for logging and ui for this task
Expand Down
13 changes: 5 additions & 8 deletions cli/internal/run/dry_run.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,13 +127,7 @@ func executeDryRun(ctx gocontext.Context, engine *core.Engine, g *graph.Complete
taskIDs := []taskSummary{}

dryRunExecFunc := func(ctx gocontext.Context, packageTask *nodes.PackageTask) error {
deps := engine.TaskGraph.DownEdges(packageTask.TaskID)

passThroughArgs := rs.ArgsForTask(packageTask.Task)
hash, err := taskHashTracker.CalculateTaskHash(packageTask, deps, base.Logger, passThroughArgs)
if err != nil {
return err
}
hash := packageTask.Hash

command := missingTaskLabel
if packageTask.Command != "" {
Expand Down Expand Up @@ -191,7 +185,10 @@ func executeDryRun(ctx gocontext.Context, engine *core.Engine, g *graph.Complete
// a visitor function and some hardcoded execOpts.
// Note: we do not currently attempt to parallelize the graph walking
// (as we do in real execution)
visitorFn := g.GetPackageTaskVisitor(ctx, dryRunExecFunc)
getArgs := func(taskID string) []string {
return rs.ArgsForTask(taskID)
}
visitorFn := g.GetPackageTaskVisitor(ctx, engine.TaskGraph, getArgs, base.Logger, dryRunExecFunc)
execOpts := core.EngineExecutionOptions{
Concurrency: 1,
Parallel: false,
Expand Down
12 changes: 6 additions & 6 deletions cli/internal/run/real_run.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,11 @@ func RealRun(
return ec.exec(ctx, packageTask, deps)
}

visitorFn := g.GetPackageTaskVisitor(ctx, execFunc)
getArgs := func(taskID string) []string {
return rs.ArgsForTask(taskID)
}

visitorFn := g.GetPackageTaskVisitor(ctx, engine.TaskGraph, getArgs, base.Logger, execFunc)
errs := engine.Execute(visitorFn, execOpts)

// Track if we saw any child with a non-zero exit code
Expand Down Expand Up @@ -158,12 +162,8 @@ func (ec *execContext) exec(ctx gocontext.Context, packageTask *nodes.PackageTas
tracer := ec.runState.Run(packageTask.TaskID)

passThroughArgs := ec.rs.ArgsForTask(packageTask.Task)
hash, err := ec.taskHashTracker.CalculateTaskHash(packageTask, deps, ec.logger, passThroughArgs)
hash := packageTask.Hash
ec.logger.Debug("task hash", "value", hash)
if err != nil {
ec.ui.Error(fmt.Sprintf("Hashing error: %v", err))
// @TODO probably should abort fatally???
}
// TODO(gsoltis): if/when we fix https://github.com/vercel/turbo/issues/937
// the following block should never get hit. In the meantime, keep it after hashing
// so that downstream tasks can count on the hash existing
Expand Down

0 comments on commit e680e72

Please sign in to comment.