From ab223a512bbf0e8ca3012a7425e15c0210c7b1a6 Mon Sep 17 00:00:00 2001 From: Dan Willemsen Date: Thu, 5 Jul 2018 21:56:59 -0700 Subject: [PATCH] Run globs during earlier bootstrap phases Instead of sometimes re-running minibp/the primary builder during the next phase, run bpglob earlier to check dependencies. We've run into issues where the environment is slightly different between bootstrapping phase and the main build phase. It's also a problem because our primary builder (Soong) exports information used by another tool (Kati) that runs in between the bootstrapping phases and the main phase. When Soong would run in the main phase, it could get out of sync, and would require the build to be run again. To do this, add a "subninja" include a build-globs.ninja file to each build.ninja file. The first time, this will be an empty file, but we'll always run minibp / the primary builder anyway. When the builder runs, in addition to writing a dependency file, write out the build-globs.ninja file with the rules to run bpglob. Since bpglob may need to be run very early, before it would normally be built, build it with microfactory. Change-Id: I89fcd849a8729e892f163d40060ab90b5d4dfa5d --- blueprint_impl.bash | 2 ++ bootstrap.bash | 4 +++ bootstrap/bootstrap.go | 53 ++++++++++++++++++++------------------ bootstrap/build.ninja | 5 +++- bootstrap/cleanup.go | 8 ++++-- bootstrap/command.go | 18 +++++++++++-- bootstrap/config.go | 9 ++++--- bootstrap/doc.go | 12 ++++++--- bootstrap/glob.go | 55 ++++++++++++++++++++++++++++++++++------ bootstrap/minibp/main.go | 8 ++++-- context.go | 14 ++++++++++ ninja_writer.go | 6 +++++ ninja_writer_test.go | 6 +++++ singleton_ctx.go | 8 ++++++ 14 files changed, 160 insertions(+), 48 deletions(-) diff --git a/blueprint_impl.bash b/blueprint_impl.bash index 514ba31..6f5abba 100644 --- a/blueprint_impl.bash +++ b/blueprint_impl.bash @@ -28,6 +28,8 @@ source "${BLUEPRINTDIR}/microfactory/microfactory.bash" BUILDDIR="${BUILDDIR}/.minibootstrap" build_go minibp github.com/google/blueprint/bootstrap/minibp +BUILDDIR="${BUILDDIR}/.minibootstrap" build_go bpglob github.com/google/blueprint/bootstrap/bpglob + # Build the bootstrap build.ninja "${NINJA}" -w dupbuild=err -f "${BUILDDIR}/.minibootstrap/build.ninja" diff --git a/bootstrap.bash b/bootstrap.bash index 24f16c2..c02fe81 100755 --- a/bootstrap.bash +++ b/bootstrap.bash @@ -107,6 +107,10 @@ echo "extraArgs = $EXTRA_ARGS" >> $BUILDDIR/.minibootstrap/build.ninja echo "builddir = $NINJA_BUILDDIR" >> $BUILDDIR/.minibootstrap/build.ninja echo "include $BLUEPRINTDIR/bootstrap/build.ninja" >> $BUILDDIR/.minibootstrap/build.ninja +if [ ! -f "$BUILDDIR/.minibootstrap/build-globs.ninja" ]; then + touch "$BUILDDIR/.minibootstrap/build-globs.ninja" +fi + echo "BLUEPRINT_BOOTSTRAP_VERSION=2" > $BUILDDIR/.blueprint.bootstrap echo "SRCDIR=\"${SRCDIR}\"" >> $BUILDDIR/.blueprint.bootstrap echo "BLUEPRINTDIR=\"${BLUEPRINTDIR}\"" >> $BUILDDIR/.blueprint.bootstrap diff --git a/bootstrap/bootstrap.go b/bootstrap/bootstrap.go index e9a8f01..92b8a32 100644 --- a/bootstrap/bootstrap.go +++ b/bootstrap/bootstrap.go @@ -17,6 +17,8 @@ package bootstrap import ( "fmt" "go/build" + "io/ioutil" + "os" "path/filepath" "runtime" "strings" @@ -118,14 +120,14 @@ var ( generateBuildNinja = pctx.StaticRule("build.ninja", blueprint.RuleParams{ - Command: "$builder $extra -b $buildDir -n $ninjaBuildDir -d $out.d -o $out $in", + Command: "$builder $extra -b $buildDir -n $ninjaBuildDir -d $out.d -globFile $globFile -o $out $in", CommandDeps: []string{"$builder"}, Description: "$builder $out", Deps: blueprint.DepsGCC, Depfile: "$out.d", Restat: true, }, - "builder", "extra", "generator") + "builder", "extra", "generator", "globFile") // Work around a Ninja issue. See https://github.com/martine/ninja/pull/634 phony = pctx.StaticRule("phony", @@ -668,32 +670,33 @@ func (s *singleton) GenerateBuildActions(ctx blueprint.SingletonContext) { topLevelBlueprints := filepath.Join("$srcDir", filepath.Base(s.config.topLevelBlueprintsFile)) - mainNinjaFile := filepath.Join("$buildDir", "build.ninja") - primaryBuilderNinjaFile := filepath.Join(bootstrapDir, "build.ninja") - ctx.SetNinjaBuildDir(pctx, "${ninjaBuildDir}") - // Build the main build.ninja - ctx.Build(pctx, blueprint.BuildParams{ - Rule: generateBuildNinja, - Outputs: []string{mainNinjaFile}, - Inputs: []string{topLevelBlueprints}, - Args: map[string]string{ - "builder": primaryBuilderFile, - "extra": primaryBuilderExtraFlags, - }, - }) + if s.config.stage == StagePrimary { + mainNinjaFile := filepath.Join("$buildDir", "build.ninja") + primaryBuilderNinjaGlobFile := filepath.Join(BuildDir, bootstrapSubDir, "build-globs.ninja") - // Add a way to rebuild the primary build.ninja so that globs works - ctx.Build(pctx, blueprint.BuildParams{ - Rule: generateBuildNinja, - Outputs: []string{primaryBuilderNinjaFile}, - Inputs: []string{topLevelBlueprints}, - Args: map[string]string{ - "builder": minibpFile, - "extra": extraSharedFlagString, - }, - }) + if _, err := os.Stat(primaryBuilderNinjaGlobFile); os.IsNotExist(err) { + err = ioutil.WriteFile(primaryBuilderNinjaGlobFile, nil, 0666) + if err != nil { + ctx.Errorf("Failed to create empty ninja file: %s", err) + } + } + + ctx.AddSubninja(primaryBuilderNinjaGlobFile) + + // Build the main build.ninja + ctx.Build(pctx, blueprint.BuildParams{ + Rule: generateBuildNinja, + Outputs: []string{mainNinjaFile}, + Inputs: []string{topLevelBlueprints}, + Args: map[string]string{ + "builder": primaryBuilderFile, + "extra": primaryBuilderExtraFlags, + "globFile": primaryBuilderNinjaGlobFile, + }, + }) + } if s.config.stage == StageMain { if primaryBuilderName == "minibp" { diff --git a/bootstrap/build.ninja b/bootstrap/build.ninja index b338843..5787c72 100644 --- a/bootstrap/build.ninja +++ b/bootstrap/build.ninja @@ -7,8 +7,11 @@ ninja_required_version = 1.7.0 +myGlobs = ${bootstrapBuildDir}/.minibootstrap/build-globs.ninja +subninja ${myGlobs} + rule build.ninja - command = ${builder} ${extraArgs} -b ${bootstrapBuildDir} -n ${builddir} -d ${out}.d -o ${out} ${in} + command = ${builder} ${extraArgs} -b ${bootstrapBuildDir} -n ${builddir} -d ${out}.d -globFile ${myGlobs} -o ${out} ${in} deps = gcc depfile = ${out}.d description = ${builder} ${out} diff --git a/bootstrap/cleanup.go b/bootstrap/cleanup.go index a4a18c7..4a8ce25 100644 --- a/bootstrap/cleanup.go +++ b/bootstrap/cleanup.go @@ -30,9 +30,9 @@ const logFileName = ".ninja_log" // removeAbandonedFilesUnder removes any files that appear in the Ninja log, and // are prefixed with one of the `under` entries, but that are not currently -// build targets. +// build targets, or in `exempt` func removeAbandonedFilesUnder(ctx *blueprint.Context, config *Config, - srcDir string, under []string) error { + srcDir string, under, exempt []string) error { if len(under) == 0 { return nil @@ -57,6 +57,10 @@ func removeAbandonedFilesUnder(ctx *blueprint.Context, config *Config, replacedTarget := replacer.Replace(target) targets[filepath.Clean(replacedTarget)] = true } + for _, target := range exempt { + replacedTarget := replacer.Replace(target) + targets[filepath.Clean(replacedTarget)] = true + } filePaths, err := parseNinjaLog(ninjaBuildDir, under) if err != nil { diff --git a/bootstrap/command.go b/bootstrap/command.go index 04eb535..ac3fd00 100644 --- a/bootstrap/command.go +++ b/bootstrap/command.go @@ -32,6 +32,7 @@ import ( var ( outFile string + globFile string depFile string docFile string cpuprofile string @@ -48,6 +49,7 @@ var ( func init() { flag.StringVar(&outFile, "o", "build.ninja", "the Ninja file to output") + flag.StringVar(&globFile, "globFile", "build-globs.ninja", "the Ninja file of globs to output") flag.StringVar(&BuildDir, "b", ".", "the build output directory") flag.StringVar(&NinjaBuildDir, "n", "", "the ninja builddir directory") flag.StringVar(&depFile, "d", "", "the dependency file to output") @@ -179,6 +181,18 @@ func Main(ctx *blueprint.Context, config interface{}, extraNinjaFileDeps ...stri fatalf("error writing %s: %s", outFile, err) } + if globFile != "" { + buffer, errs := generateGlobNinjaFile(ctx.Globs) + if len(errs) > 0 { + fatalErrors(errs) + } + + err = ioutil.WriteFile(globFile, buffer, outFilePermissions) + if err != nil { + fatalf("error writing %s: %s", outFile, err) + } + } + if depFile != "" { err := deptools.WriteDepFile(depFile, outFile, deps) if err != nil { @@ -187,8 +201,8 @@ func Main(ctx *blueprint.Context, config interface{}, extraNinjaFileDeps ...stri } if c, ok := config.(ConfigRemoveAbandonedFilesUnder); ok { - under := c.RemoveAbandonedFilesUnder() - err := removeAbandonedFilesUnder(ctx, bootstrapConfig, SrcDir, under) + under, except := c.RemoveAbandonedFilesUnder() + err := removeAbandonedFilesUnder(ctx, bootstrapConfig, SrcDir, under, except) if err != nil { fatalf("error removing abandoned files: %s", err) } diff --git a/bootstrap/config.go b/bootstrap/config.go index 5785ea7..790ac4b 100644 --- a/bootstrap/config.go +++ b/bootstrap/config.go @@ -69,10 +69,11 @@ type ConfigInterface interface { } type ConfigRemoveAbandonedFilesUnder interface { - // RemoveAbandonedFilesUnder should return a slice of path prefixes that - // will be cleaned of files that are no longer active targets, but are - // listed in the .ninja_log. - RemoveAbandonedFilesUnder() []string + // RemoveAbandonedFilesUnder should return two slices: + // - a slice of path prefixes that will be cleaned of files that are no + // longer active targets, but are listed in the .ninja_log. + // - a slice of paths that are exempt from cleaning + RemoveAbandonedFilesUnder() (under, except []string) } type ConfigBlueprintToolLocation interface { diff --git a/bootstrap/doc.go b/bootstrap/doc.go index 3c7108e..69a1784 100644 --- a/bootstrap/doc.go +++ b/bootstrap/doc.go @@ -120,23 +120,27 @@ // - Runs .bootstrap/build.ninja to build and run the primary builder // - Runs build.ninja to build your code // -// Microfactory takes care of building an up to date version of `minibp` under -// the .minibootstrap/ directory. +// Microfactory takes care of building an up to date version of `minibp` and +// `bpglob` under the .minibootstrap/ directory. // // During /.minibootstrap/build.ninja, the following actions are // taken, if necessary: // // - Run minibp to generate .bootstrap/build.ninja (Primary stage) +// - Includes .minibootstrap/build-globs.ninja, which defines rules to +// run bpglob during incremental builds. These outputs are listed in +// the dependency file output by minibp. // // During the /.bootstrap/build.ninja, the following actions are // taken, if necessary: // -// - Rebuild .bootstrap/build.ninja, usually due to globs changing -- -// other dependencies will trigger it to be built during minibootstrap // - Build the primary builder, anything marked `default: true`, and // any dependencies. // - Run the primary builder to generate build.ninja // - Run the primary builder to extract documentation +// - Includes .bootstrap/build-globs.ninja, which defines rules to run +// bpglob during incremental builds. These outputs are listed in the +// dependency file output by the primary builder. // // Then the main stage is at /build.ninja, and will contain all the // rules generated by the primary builder. In addition, the bootstrap code diff --git a/bootstrap/glob.go b/bootstrap/glob.go index 160ef58..9841611 100644 --- a/bootstrap/glob.go +++ b/bootstrap/glob.go @@ -15,6 +15,7 @@ package bootstrap import ( + "bytes" "fmt" "path/filepath" "strings" @@ -40,7 +41,7 @@ import ( // in a build failure with a "missing and no known rule to make it" error. var ( - globCmd = filepath.Join("$BinDir", "bpglob") + globCmd = filepath.Join(miniBootstrapDir, "bpglob") // globRule rule traverses directories to produce a list of files that match $glob // and writes it to $out if it has changed, and writes the directories to $out.d @@ -111,6 +112,7 @@ func joinWithPrefixAndQuote(strs []string, prefix string) string { // primary builder if the results change. type globSingleton struct { globLister func() []blueprint.GlobPath + writeRule bool } func globSingletonFactory(ctx *blueprint.Context) func() blueprint.Singleton { @@ -124,15 +126,52 @@ func globSingletonFactory(ctx *blueprint.Context) func() blueprint.Singleton { func (s *globSingleton) GenerateBuildActions(ctx blueprint.SingletonContext) { for _, g := range s.globLister() { fileListFile := filepath.Join(BuildDir, ".glob", g.Name) - depFile := fileListFile + ".d" - fileList := strings.Join(g.Files, "\n") + "\n" - pathtools.WriteFileIfChanged(fileListFile, []byte(fileList), 0666) - deptools.WriteDepFile(depFile, fileListFile, g.Deps) + if s.writeRule { + depFile := fileListFile + ".d" - GlobFile(ctx, g.Pattern, g.Excludes, fileListFile, depFile) + fileList := strings.Join(g.Files, "\n") + "\n" + pathtools.WriteFileIfChanged(fileListFile, []byte(fileList), 0666) + deptools.WriteDepFile(depFile, fileListFile, g.Deps) - // Make build.ninja depend on the fileListFile - ctx.AddNinjaFileDeps(fileListFile) + GlobFile(ctx, g.Pattern, g.Excludes, fileListFile, depFile) + } else { + // Make build.ninja depend on the fileListFile + ctx.AddNinjaFileDeps(fileListFile) + } } } + +func generateGlobNinjaFile(globLister func() []blueprint.GlobPath) ([]byte, []error) { + ctx := blueprint.NewContext() + ctx.RegisterSingletonType("glob", func() blueprint.Singleton { + return &globSingleton{ + globLister: globLister, + writeRule: true, + } + }) + + extraDeps, errs := ctx.ResolveDependencies(nil) + if len(extraDeps) > 0 { + return nil, []error{fmt.Errorf("shouldn't have extra deps")} + } + if len(errs) > 0 { + return nil, errs + } + + extraDeps, errs = ctx.PrepareBuildActions(nil) + if len(extraDeps) > 0 { + return nil, []error{fmt.Errorf("shouldn't have extra deps")} + } + if len(errs) > 0 { + return nil, errs + } + + buf := bytes.NewBuffer(nil) + err := ctx.WriteBuildFile(buf) + if err != nil { + return nil, []error{err} + } + + return buf.Bytes(), nil +} diff --git a/bootstrap/minibp/main.go b/bootstrap/minibp/main.go index 72ed9f6..1714739 100644 --- a/bootstrap/minibp/main.go +++ b/bootstrap/minibp/main.go @@ -37,8 +37,12 @@ func (c Config) GeneratingPrimaryBuilder() bool { return c.generatingPrimaryBuilder } -func (c Config) RemoveAbandonedFilesUnder() []string { - return []string{filepath.Join(bootstrap.BuildDir, ".bootstrap")} +func (c Config) RemoveAbandonedFilesUnder() (under, exempt []string) { + if c.generatingPrimaryBuilder { + under = []string{filepath.Join(bootstrap.BuildDir, ".bootstrap")} + exempt = []string{filepath.Join(bootstrap.BuildDir, ".bootstrap", "build.ninja")} + } + return } func main() { diff --git a/context.go b/context.go index 919377c..69f26f6 100644 --- a/context.go +++ b/context.go @@ -102,6 +102,8 @@ type Context struct { requiredNinjaMinor int // For the ninja_required_version variable requiredNinjaMicro int // For the ninja_required_version variable + subninjas []string + // set lazily by sortedModuleGroups cachedSortedModuleGroups []*moduleGroup @@ -2939,6 +2941,11 @@ func (c *Context) WriteBuildFile(w io.Writer) error { return err } + err = c.writeSubninjas(nw) + if err != nil { + return err + } + // TODO: Group the globals by package. err = c.writeGlobalVariables(nw) @@ -3048,6 +3055,13 @@ func (c *Context) writeNinjaRequiredVersion(nw *ninjaWriter) error { return nw.BlankLine() } +func (c *Context) writeSubninjas(nw *ninjaWriter) error { + for _, subninja := range c.subninjas { + nw.Subninja(subninja) + } + return nw.BlankLine() +} + func (c *Context) writeBuildDir(nw *ninjaWriter) error { if c.ninjaBuildDir != nil { err := nw.Assign("builddir", c.ninjaBuildDir.Value(c.pkgNames)) diff --git a/ninja_writer.go b/ninja_writer.go index a61667d..5902986 100644 --- a/ninja_writer.go +++ b/ninja_writer.go @@ -193,6 +193,12 @@ func (n *ninjaWriter) Default(targets ...string) error { return wrapper.Flush() } +func (n *ninjaWriter) Subninja(file string) error { + n.justDidBlankLine = false + _, err := fmt.Fprintf(n.writer, "subninja %s\n", file) + return err +} + func (n *ninjaWriter) BlankLine() (err error) { // We don't output multiple blank lines in a row. if !n.justDidBlankLine { diff --git a/ninja_writer_test.go b/ninja_writer_test.go index 44e4ff8..cc880e5 100644 --- a/ninja_writer_test.go +++ b/ninja_writer_test.go @@ -72,6 +72,12 @@ var ninjaWriterTestCases = []struct { }, output: " foo = bar\n", }, + { + input: func(w *ninjaWriter) { + ck(w.Subninja("build.ninja")) + }, + output: "subninja build.ninja\n", + }, { input: func(w *ninjaWriter) { ck(w.BlankLine()) diff --git a/singleton_ctx.go b/singleton_ctx.go index bbfce00..333811d 100644 --- a/singleton_ctx.go +++ b/singleton_ctx.go @@ -47,6 +47,10 @@ type SingletonContext interface { // set at most one time for a single build, later calls are ignored. SetNinjaBuildDir(pctx PackageContext, value string) + // AddSubninja adds a ninja file to include with subninja. This should likely + // only ever be used inside bootstrap to handle glob rules. + AddSubninja(file string) + // Eval takes a string with embedded ninja variables, and returns a string // with all of the variables recursively expanded. Any variables references // are expanded in the scope of the PackageContext. @@ -203,6 +207,10 @@ func (s *singletonContext) SetNinjaBuildDir(pctx PackageContext, value string) { s.context.setNinjaBuildDir(ninjaValue) } +func (s *singletonContext) AddSubninja(file string) { + s.context.subninjas = append(s.context.subninjas, file) +} + func (s *singletonContext) VisitAllModules(visit func(Module)) { s.context.VisitAllModules(visit) }