From 9ece72b0550811f743d2e05b4cb0ca374a9a2b02 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Thu, 9 Jul 2020 14:24:56 -0700 Subject: [PATCH 1/2] Add Validations support to Blueprint The Android fork of Ninja supports "validations", a node that is added to the top level of the build graph whenever another node is in the build graph. Add support in Blueprint to specify them on build statements and write them to the ninja. Test: ninja_writer_test.go Change-Id: I87cd1281dbd2ed113cc26a265c50d20c65118c91 --- live_tracker.go | 5 +++++ ninja_defs.go | 10 +++++++++- ninja_writer.go | 10 +++++++++- ninja_writer_test.go | 6 +++--- 4 files changed, 26 insertions(+), 5 deletions(-) diff --git a/live_tracker.go b/live_tracker.go index 40e1930..1d48e58 100644 --- a/live_tracker.go +++ b/live_tracker.go @@ -68,6 +68,11 @@ func (l *liveTracker) AddBuildDefDeps(def *buildDef) error { return err } + err = l.addNinjaStringListDeps(def.Validations) + if err != nil { + return err + } + for _, value := range def.Variables { err = l.addNinjaStringDeps(value) if err != nil { diff --git a/ninja_defs.go b/ninja_defs.go index c5d0e4b..8c5db57 100644 --- a/ninja_defs.go +++ b/ninja_defs.go @@ -87,6 +87,7 @@ type BuildParams struct { Inputs []string // The list of explicit input dependencies. Implicits []string // The list of implicit input dependencies. OrderOnly []string // The list of order-only dependencies. + Validations []string // The list of validations to run when this rule runs. Args map[string]string // The variable/value pairs to set. Optional bool // Skip outputting a default statement } @@ -257,6 +258,7 @@ type buildDef struct { Inputs []ninjaString Implicits []ninjaString OrderOnly []ninjaString + Validations []ninjaString Args map[Variable]ninjaString Variables map[string]ninjaString Optional bool @@ -314,6 +316,11 @@ func parseBuildParams(scope scope, params *BuildParams) (*buildDef, return nil, fmt.Errorf("error parsing OrderOnly param: %s", err) } + b.Validations, err = parseNinjaStrings(scope, params.Validations) + if err != nil { + return nil, fmt.Errorf("error parsing Validations param: %s", err) + } + b.Optional = params.Optional if params.Depfile != "" { @@ -373,6 +380,7 @@ func (b *buildDef) WriteTo(nw *ninjaWriter, pkgNames map[*packageContext]string) explicitDeps = valueList(b.Inputs, pkgNames, inputEscaper) implicitDeps = valueList(b.Implicits, pkgNames, inputEscaper) orderOnlyDeps = valueList(b.OrderOnly, pkgNames, inputEscaper) + validations = valueList(b.Validations, pkgNames, inputEscaper) ) if b.RuleDef != nil { @@ -380,7 +388,7 @@ func (b *buildDef) WriteTo(nw *ninjaWriter, pkgNames map[*packageContext]string) orderOnlyDeps = append(valueList(b.RuleDef.CommandOrderOnly, pkgNames, inputEscaper), orderOnlyDeps...) } - err := nw.Build(comment, rule, outputs, implicitOuts, explicitDeps, implicitDeps, orderOnlyDeps) + err := nw.Build(comment, rule, outputs, implicitOuts, explicitDeps, implicitDeps, orderOnlyDeps, validations) if err != nil { return err } diff --git a/ninja_writer.go b/ninja_writer.go index 5366f3f..5e69c08 100644 --- a/ninja_writer.go +++ b/ninja_writer.go @@ -104,7 +104,7 @@ func (n *ninjaWriter) Rule(name string) error { } func (n *ninjaWriter) Build(comment string, rule string, outputs, implicitOuts, - explicitDeps, implicitDeps, orderOnlyDeps []string) error { + explicitDeps, implicitDeps, orderOnlyDeps, validations []string) error { n.justDidBlankLine = false @@ -161,6 +161,14 @@ func (n *ninjaWriter) Build(comment string, rule string, outputs, implicitOuts, } } + if len(validations) > 0 { + wrapper.WriteStringWithSpace("|@") + + for _, dep := range validations { + wrapper.WriteStringWithSpace(dep) + } + } + return wrapper.Flush() } diff --git a/ninja_writer_test.go b/ninja_writer_test.go index cc880e5..48ecb7c 100644 --- a/ninja_writer_test.go +++ b/ninja_writer_test.go @@ -50,9 +50,9 @@ var ninjaWriterTestCases = []struct { { input: func(w *ninjaWriter) { ck(w.Build("foo comment", "foo", []string{"o1", "o2"}, []string{"io1", "io2"}, - []string{"e1", "e2"}, []string{"i1", "i2"}, []string{"oo1", "oo2"})) + []string{"e1", "e2"}, []string{"i1", "i2"}, []string{"oo1", "oo2"}, []string{"v1", "v2"})) }, - output: "# foo comment\nbuild o1 o2 | io1 io2: foo e1 e2 | i1 i2 || oo1 oo2\n", + output: "# foo comment\nbuild o1 o2 | io1 io2: foo e1 e2 | i1 i2 || oo1 oo2 |@ v1 v2\n", }, { input: func(w *ninjaWriter) { @@ -94,7 +94,7 @@ var ninjaWriterTestCases = []struct { ck(w.ScopedAssign("command", "echo out: $out in: $in _arg: $_arg")) ck(w.ScopedAssign("pool", "p")) ck(w.BlankLine()) - ck(w.Build("r comment", "r", []string{"foo.o"}, nil, []string{"foo.in"}, nil, nil)) + ck(w.Build("r comment", "r", []string{"foo.o"}, nil, []string{"foo.in"}, nil, nil, nil)) ck(w.ScopedAssign("_arg", "arg value")) }, output: `pool p From 0cdec99c81b5c3c47f61645ae56eee886a855a23 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Thu, 9 Jul 2020 14:24:56 -0700 Subject: [PATCH 2/2] Add flag to use validations for tests Using validations for tests ensures the tests run without blocking the critical path. Change-Id: Icb21a52e96f70d815f7df86882351c13f5575cf5 --- bootstrap.bash | 7 ++++++- bootstrap/bootstrap.go | 46 ++++++++++++++++++++++++++++-------------- bootstrap/command.go | 3 +++ bootstrap/config.go | 1 + 4 files changed, 41 insertions(+), 16 deletions(-) diff --git a/bootstrap.bash b/bootstrap.bash index 08b85b5..b08bf1e 100755 --- a/bootstrap.bash +++ b/bootstrap.bash @@ -67,12 +67,14 @@ usage() { echo " -h: print a help message and exit" echo " -b : set the build directory" echo " -t: run tests" + echo " -n: use validations to depend on tests" } # Parse the command line flags. -while getopts ":b:ht" opt; do +while getopts ":b:hnt" opt; do case $opt in b) BUILDDIR="$OPTARG";; + n) USE_VALIDATIONS=true;; t) RUN_TESTS=true;; h) usage @@ -93,6 +95,9 @@ done # If RUN_TESTS is set, behave like -t was passed in as an option. [ ! -z "$RUN_TESTS" ] && EXTRA_ARGS="${EXTRA_ARGS} -t" +# If $USE_VALIDATIONS is set, pass --use-validations. +[ ! -z "$USE_VALIDATIONS" ] && EXTRA_ARGS="${EXTRA_ARGS} --use-validations" + # If EMPTY_NINJA_FILE is set, have the primary build write out a 0-byte ninja # file instead of a full length one. Useful if you don't plan on executing the # build, but want to verify the primary builder execution. diff --git a/bootstrap/bootstrap.go b/bootstrap/bootstrap.go index bb85e7d..8a9a5f8 100644 --- a/bootstrap/bootstrap.go +++ b/bootstrap/bootstrap.go @@ -338,7 +338,7 @@ func (g *goPackage) GenerateBuildActions(ctx blueprint.ModuleContext) { filepath.FromSlash(g.properties.PkgPath)+".a") g.testResultFile = buildGoTest(ctx, testRoot(ctx, g.config), testArchiveFile, g.properties.PkgPath, srcs, genSrcs, - testSrcs) + testSrcs, g.config.useValidations) } buildGoPackage(ctx, g.pkgRoot, g.properties.PkgPath, g.archiveFile, @@ -421,7 +421,7 @@ func (g *goBinary) GenerateBuildActions(ctx blueprint.ModuleContext) { genSrcs = append(genSrcs, pluginSrc) } - var deps []string + var testDeps []string if hasPlugins && !buildGoPluginLoader(ctx, "main", pluginSrc) { return @@ -437,8 +437,8 @@ func (g *goBinary) GenerateBuildActions(ctx blueprint.ModuleContext) { } if g.config.runGoTests { - deps = buildGoTest(ctx, testRoot(ctx, g.config), testArchiveFile, - name, srcs, genSrcs, testSrcs) + testDeps = buildGoTest(ctx, testRoot(ctx, g.config), testArchiveFile, + name, srcs, genSrcs, testSrcs, g.config.useValidations) } buildGoPackage(ctx, objDir, "main", archiveFile, srcs, genSrcs) @@ -451,7 +451,7 @@ func (g *goBinary) GenerateBuildActions(ctx blueprint.ModuleContext) { linkDeps = append(linkDeps, dep.GoPackageTarget()) libDir := dep.GoPkgRoot() libDirFlags = append(libDirFlags, "-L "+libDir) - deps = append(deps, dep.GoTestTargets()...) + testDeps = append(testDeps, dep.GoTestTargets()...) }) linkArgs := map[string]string{} @@ -468,12 +468,20 @@ func (g *goBinary) GenerateBuildActions(ctx blueprint.ModuleContext) { Optional: true, }) + var orderOnlyDeps, validationDeps []string + if g.config.useValidations { + validationDeps = testDeps + } else { + orderOnlyDeps = testDeps + } + ctx.Build(pctx, blueprint.BuildParams{ - Rule: cp, - Outputs: []string{g.installPath}, - Inputs: []string{aoutFile}, - OrderOnly: deps, - Optional: !g.properties.Default, + Rule: cp, + Outputs: []string{g.installPath}, + Inputs: []string{aoutFile}, + OrderOnly: orderOnlyDeps, + Validations: validationDeps, + Optional: !g.properties.Default, }) } @@ -538,7 +546,7 @@ func buildGoPackage(ctx blueprint.ModuleContext, pkgRoot string, } func buildGoTest(ctx blueprint.ModuleContext, testRoot, testPkgArchive, - pkgPath string, srcs, genSrcs, testSrcs []string) []string { + pkgPath string, srcs, genSrcs, testSrcs []string, useValidations bool) []string { if len(testSrcs) == 0 { return nil @@ -600,11 +608,19 @@ func buildGoTest(ctx blueprint.ModuleContext, testRoot, testPkgArchive, Optional: true, }) + var orderOnlyDeps, validationDeps []string + if useValidations { + validationDeps = testDeps + } else { + orderOnlyDeps = testDeps + } + ctx.Build(pctx, blueprint.BuildParams{ - Rule: test, - Outputs: []string{testPassed}, - Inputs: []string{testFile}, - OrderOnly: testDeps, + Rule: test, + Outputs: []string{testPassed}, + Inputs: []string{testFile}, + OrderOnly: orderOnlyDeps, + Validations: validationDeps, Args: map[string]string{ "pkg": pkgPath, "pkgSrcDir": filepath.Dir(testFiles[0]), diff --git a/bootstrap/command.go b/bootstrap/command.go index 1f73ce3..dc655e8 100644 --- a/bootstrap/command.go +++ b/bootstrap/command.go @@ -40,6 +40,7 @@ var ( memprofile string traceFile string runGoTests bool + useValidations bool noGC bool emptyNinjaFile bool BuildDir string @@ -61,6 +62,7 @@ func init() { flag.StringVar(&memprofile, "memprofile", "", "write memory profile to file") flag.BoolVar(&noGC, "nogc", false, "turn off GC for debugging") flag.BoolVar(&runGoTests, "t", false, "build and run go tests during bootstrap") + flag.BoolVar(&useValidations, "use-validations", false, "use validations to depend on go tests") flag.StringVar(&ModuleListFile, "l", "", "file that lists filepaths to parse") flag.BoolVar(&emptyNinjaFile, "empty-ninja-file", false, "write out a 0-byte ninja file") } @@ -131,6 +133,7 @@ func Main(ctx *blueprint.Context, config interface{}, extraNinjaFileDeps ...stri topLevelBlueprintsFile: flag.Arg(0), emptyNinjaFile: emptyNinjaFile, runGoTests: runGoTests, + useValidations: useValidations, moduleListFile: ModuleListFile, } diff --git a/bootstrap/config.go b/bootstrap/config.go index 9499aeb..ace0ae6 100644 --- a/bootstrap/config.go +++ b/bootstrap/config.go @@ -107,5 +107,6 @@ type Config struct { emptyNinjaFile bool runGoTests bool + useValidations bool moduleListFile string }