From fce63d3c8fa746e31c486b515b1bcce26760b15d Mon Sep 17 00:00:00 2001 From: Dan Willemsen Date: Tue, 17 Nov 2015 14:21:45 -0800 Subject: [PATCH] Add common implicit deps to Rules For implicit dependencies that will be common to all users of a Rule, add a new field 'CommandDeps' to the RuleParam. This is a list of strings to be prepended to the implicit dependencies in each BuildParam. This lets us have the dependencies declared next to where they are used, instead of duplicated in areas that may be far apart. I looked at passing this information down to ninja too, but it only saves us a few percent of ninja file, and requires a modification to the ninja file format. Change-Id: Ifd910dee1506d4e32a76ed06206f853c4caec622 --- bootstrap/bootstrap.go | 77 +++++++++++++++++++++--------------------- live_tracker.go | 24 ++++++++----- ninja_defs.go | 27 +++++++++++---- 3 files changed, 75 insertions(+), 53 deletions(-) diff --git a/bootstrap/bootstrap.go b/bootstrap/bootstrap.go index ed5debc..14d6bf8 100644 --- a/bootstrap/bootstrap.go +++ b/bootstrap/bootstrap.go @@ -38,6 +38,7 @@ var ( blueprint.RuleParams{ Command: "GOROOT='$goRoot' $compileCmd -o $out -p $pkgPath -complete " + "$incFlags -pack $in", + CommandDeps: []string{"$compileCmd"}, Description: "compile $out", }, "pkgPath", "incFlags") @@ -45,6 +46,7 @@ var ( link = pctx.StaticRule("link", blueprint.RuleParams{ Command: "GOROOT='$goRoot' $linkCmd -o $out $libDirFlags $in", + CommandDeps: []string{"$linkCmd"}, Description: "link $out", }, "libDirFlags") @@ -52,6 +54,7 @@ var ( goTestMain = pctx.StaticRule("gotestmain", blueprint.RuleParams{ Command: "$goTestMainCmd -o $out -pkg $pkg $in", + CommandDeps: []string{"$goTestMainCmd"}, Description: "gotestmain $out", }, "pkg") @@ -59,6 +62,7 @@ var ( pluginGenSrc = pctx.StaticRule("pluginGenSrc", blueprint.RuleParams{ Command: "$pluginGenSrcCmd -o $out -p $pkg $plugins", + CommandDeps: []string{"$pluginGenSrcCmd"}, Description: "create $out", }, "pkg", "plugins") @@ -66,6 +70,7 @@ var ( test = pctx.StaticRule("test", blueprint.RuleParams{ Command: "$goTestRunnerCmd -p $pkgSrcDir -f $out -- $in -test.short", + CommandDeps: []string{"$goTestRunnerCmd"}, Description: "test $pkg", }, "pkg", "pkgSrcDir") @@ -80,6 +85,7 @@ var ( bootstrap = pctx.StaticRule("bootstrap", blueprint.RuleParams{ Command: "BUILDDIR=$buildDir $bootstrapCmd -i $in", + CommandDeps: []string{"$bootstrapCmd"}, Description: "bootstrap $in", Generator: true, }) @@ -87,6 +93,7 @@ var ( chooseStage = pctx.StaticRule("chooseStage", blueprint.RuleParams{ Command: "$chooseStageCmd --current $current --bootstrap $bootstrapManifest -o $out $in", + CommandDeps: []string{"$chooseStageCmd", "$bootstrapManifest"}, Description: "choosing next stage", }, "current", "generator") @@ -409,11 +416,10 @@ func (g *goBinary) GenerateBuildActions(ctx blueprint.ModuleContext) { } ctx.Build(pctx, blueprint.BuildParams{ - Rule: link, - Outputs: []string{aoutFile}, - Inputs: []string{archiveFile}, - Implicits: []string{"$linkCmd"}, - Args: linkArgs, + Rule: link, + Outputs: []string{aoutFile}, + Inputs: []string{archiveFile}, + Args: linkArgs, }) ctx.Build(pctx, blueprint.BuildParams{ @@ -448,9 +454,8 @@ func buildGoPluginLoader(ctx blueprint.ModuleContext, pkgPath, pluginSrc string, }) ctx.Build(pctx, blueprint.BuildParams{ - Rule: pluginGenSrc, - Outputs: []string{pluginSrc}, - Implicits: []string{"$pluginGenSrcCmd"}, + Rule: pluginGenSrc, + Outputs: []string{pluginSrc}, Args: map[string]string{ "pkg": pkgPath, "plugins": strings.Join(pluginPaths, " "), @@ -468,7 +473,7 @@ func buildGoPackage(ctx blueprint.ModuleContext, pkgRoot string, srcFiles = append(srcFiles, genSrcs...) var incFlags []string - deps := []string{"$compileCmd"} + var deps []string ctx.VisitDepsDepthFirstIf(isGoPackageProducer, func(module blueprint.Module) { dep := module.(goPackageProducer) @@ -515,10 +520,9 @@ func buildGoTest(ctx blueprint.ModuleContext, testRoot, testPkgArchive, append(srcs, testSrcs...), genSrcs, nil) ctx.Build(pctx, blueprint.BuildParams{ - Rule: goTestMain, - Outputs: []string{mainFile}, - Inputs: testFiles, - Implicits: []string{"$goTestMainCmd"}, + Rule: goTestMain, + Outputs: []string{mainFile}, + Inputs: testFiles, Args: map[string]string{ "pkg": pkgPath, }, @@ -536,7 +540,7 @@ func buildGoTest(ctx blueprint.ModuleContext, testRoot, testPkgArchive, Rule: compile, Outputs: []string{testArchive}, Inputs: []string{mainFile}, - Implicits: []string{"$compileCmd", testPkgArchive}, + Implicits: []string{testPkgArchive}, Args: map[string]string{ "pkgPath": "main", "incFlags": "-I " + testRoot, @@ -544,20 +548,18 @@ func buildGoTest(ctx blueprint.ModuleContext, testRoot, testPkgArchive, }) ctx.Build(pctx, blueprint.BuildParams{ - Rule: link, - Outputs: []string{testFile}, - Inputs: []string{testArchive}, - Implicits: []string{"$linkCmd"}, + Rule: link, + Outputs: []string{testFile}, + Inputs: []string{testArchive}, Args: map[string]string{ "libDirFlags": strings.Join(libDirFlags, " "), }, }) ctx.Build(pctx, blueprint.BuildParams{ - Rule: test, - Outputs: []string{testPassed}, - Inputs: []string{testFile}, - Implicits: []string{"$goTestRunnerCmd"}, + Rule: test, + Outputs: []string{testPassed}, + Inputs: []string{testFile}, Args: map[string]string{ "pkg": pkgPath, "pkgSrcDir": filepath.Dir(testFiles[0]), @@ -759,6 +761,11 @@ func (s *singleton) GenerateBuildActions(ctx blueprint.SingletonContext) { blueprint.RuleParams{ Command: fmt.Sprintf("%s $runTests -m $bootstrapManifest "+ "-b $buildDir -d $out.d -o $out $in", minibpFile), + // $bootstrapManifest is here so that when it is updated, we + // force a rebuild of bootstrap.ninja.in. chooseStage should + // have already copied the new version over, but kept the old + // timestamps to force this regeneration. + CommandDeps: []string{"$bootstrapManifest", minibpFile}, Description: "minibp $out", Generator: true, Depfile: "$out.d", @@ -775,12 +782,7 @@ func (s *singleton) GenerateBuildActions(ctx blueprint.SingletonContext) { Rule: minibp, Outputs: []string{bootstrapNinjaFile}, Inputs: []string{topLevelBlueprints}, - // $bootstrapManifest is here so that when it is updated, we - // force a rebuild of bootstrap.ninja.in. chooseStage should - // have already copied the new version over, but kept the old - // timestamps to force this regeneration. - Implicits: []string{"$bootstrapManifest", minibpFile}, - Args: args, + Args: args, }) // When the current build.ninja file is a bootstrapper, we always want @@ -797,7 +799,7 @@ func (s *singleton) GenerateBuildActions(ctx blueprint.SingletonContext) { Rule: chooseStage, Outputs: []string{filepath.Join(bootstrapDir, "build.ninja.in")}, Inputs: []string{bootstrapNinjaFile, primaryBuilderNinjaFile}, - Implicits: []string{"$chooseStageCmd", "$bootstrapManifest", notAFile}, + Implicits: []string{notAFile}, Args: map[string]string{ "current": bootstrapNinjaFile, }, @@ -848,13 +850,13 @@ func (s *singleton) GenerateBuildActions(ctx blueprint.SingletonContext) { blueprint.RuleParams{ Command: fmt.Sprintf("%s %s -b $buildDir --docs $out %s", primaryBuilderFile, primaryBuilderExtraFlags, topLevelBlueprints), + CommandDeps: []string{primaryBuilderFile}, Description: fmt.Sprintf("%s docs $out", primaryBuilderName), }) ctx.Build(pctx, blueprint.BuildParams{ - Rule: bigbpDocs, - Outputs: []string{docsFile}, - Implicits: []string{primaryBuilderFile}, + Rule: bigbpDocs, + Outputs: []string{docsFile}, }) // Detect whether we need to rebuild the primary stage by going back to @@ -885,7 +887,7 @@ func (s *singleton) GenerateBuildActions(ctx blueprint.SingletonContext) { Rule: chooseStage, Outputs: []string{filepath.Join(bootstrapDir, "build.ninja.in")}, Inputs: []string{bootstrapNinjaFile, primaryBuilderNinjaFile, mainNinjaFile}, - Implicits: []string{"$chooseStageCmd", "$bootstrapManifest", notAFile, primaryBuilderNinjaTimestampFile}, + Implicits: []string{notAFile, primaryBuilderNinjaTimestampFile}, Args: map[string]string{ "current": primaryBuilderNinjaFile, }, @@ -935,7 +937,7 @@ func (s *singleton) GenerateBuildActions(ctx blueprint.SingletonContext) { Rule: chooseStage, Outputs: []string{filepath.Join(bootstrapDir, "build.ninja.in")}, Inputs: []string{bootstrapNinjaFile, primaryBuilderNinjaFile, mainNinjaFile}, - Implicits: []string{"$chooseStageCmd", "$bootstrapManifest", primaryBuilderNinjaTimestampFile, mainNinjaTimestampFile}, + Implicits: []string{primaryBuilderNinjaTimestampFile, mainNinjaTimestampFile}, Args: map[string]string{ "current": mainNinjaFile, "generator": "true", @@ -962,10 +964,9 @@ func (s *singleton) GenerateBuildActions(ctx blueprint.SingletonContext) { } ctx.Build(pctx, blueprint.BuildParams{ - Rule: bootstrap, - Outputs: []string{"$buildDir/build.ninja"}, - Inputs: []string{filepath.Join(bootstrapDir, "build.ninja.in")}, - Implicits: []string{"$bootstrapCmd"}, + Rule: bootstrap, + Outputs: []string{"$buildDir/build.ninja"}, + Inputs: []string{filepath.Join(bootstrapDir, "build.ninja.in")}, }) } diff --git a/live_tracker.go b/live_tracker.go index 3b41d06..8348988 100644 --- a/live_tracker.go +++ b/live_tracker.go @@ -42,10 +42,11 @@ func (l *liveTracker) AddBuildDefDeps(def *buildDef) error { l.Lock() defer l.Unlock() - err := l.addRule(def.Rule) + ruleDef, err := l.addRule(def.Rule) if err != nil { return err } + def.RuleDef = ruleDef err = l.addNinjaStringListDeps(def.Outputs) if err != nil { @@ -77,36 +78,41 @@ func (l *liveTracker) AddBuildDefDeps(def *buildDef) error { return nil } -func (l *liveTracker) addRule(r Rule) error { - _, ok := l.rules[r] +func (l *liveTracker) addRule(r Rule) (def *ruleDef, err error) { + def, ok := l.rules[r] if !ok { - def, err := r.def(l.config) + def, err = r.def(l.config) if err == errRuleIsBuiltin { // No need to do anything for built-in rules. - return nil + return nil, nil } if err != nil { - return err + return nil, err } if def.Pool != nil { err = l.addPool(def.Pool) if err != nil { - return err + return nil, err } } + err = l.addNinjaStringListDeps(def.CommandDeps) + if err != nil { + return nil, err + } + for _, value := range def.Variables { err = l.addNinjaStringDeps(value) if err != nil { - return err + return nil, err } } l.rules[r] = def } - return nil + return } func (l *liveTracker) addPool(p Pool) error { diff --git a/ninja_defs.go b/ninja_defs.go index 0919ea4..637594f 100644 --- a/ninja_defs.go +++ b/ninja_defs.go @@ -53,10 +53,9 @@ type PoolParams struct { } // A RuleParams object contains the set of parameters that make up a Ninja rule -// definition. Each field except for Comment corresponds with a Ninja variable -// of the same name. +// definition. type RuleParams struct { - Comment string // The comment that will appear above the definition. + // These fields correspond to a Ninja variable of the same name. Command string // The command that Ninja will run for the rule. Depfile string // The dependency file name. Deps Deps // The format of the dependency file. @@ -66,6 +65,10 @@ type RuleParams struct { Restat bool // Whether Ninja should re-stat the rule's outputs. Rspfile string // The response file. RspfileContent string // The response file content. + + // These fields are used internally in Blueprint + CommandDeps []string // Command-specific implicit dependencies to prepend to builds + Comment string // The comment that will appear above the definition. } // A BuildParams object contains the set of parameters that make up a Ninja @@ -120,9 +123,10 @@ func (p *poolDef) WriteTo(nw *ninjaWriter, name string) error { // A ruleDef describes a rule definition. It does not include the name of the // rule. type ruleDef struct { - Comment string - Pool Pool - Variables map[string]*ninjaString + CommandDeps []*ninjaString + Comment string + Pool Pool + Variables map[string]*ninjaString } func parseRuleParams(scope scope, params *RuleParams) (*ruleDef, @@ -194,6 +198,11 @@ func parseRuleParams(scope scope, params *RuleParams) (*ruleDef, r.Variables["rspfile_content"] = value } + r.CommandDeps, err = parseNinjaStrings(scope, params.CommandDeps) + if err != nil { + return nil, fmt.Errorf("error parsing CommandDeps param: %s", err) + } + return r, nil } @@ -239,6 +248,7 @@ func (r *ruleDef) WriteTo(nw *ninjaWriter, name string, type buildDef struct { Comment string Rule Rule + RuleDef *ruleDef Outputs []*ninjaString Inputs []*ninjaString Implicits []*ninjaString @@ -326,6 +336,11 @@ func (b *buildDef) WriteTo(nw *ninjaWriter, pkgNames map[*PackageContext]string) implicitDeps = valueList(b.Implicits, pkgNames, inputEscaper) orderOnlyDeps = valueList(b.OrderOnly, pkgNames, inputEscaper) ) + + if b.RuleDef != nil { + implicitDeps = append(valueList(b.RuleDef.CommandDeps, pkgNames, inputEscaper), implicitDeps...) + } + err := nw.Build(comment, rule, outputs, explicitDeps, implicitDeps, orderOnlyDeps) if err != nil { return err