Fix a re-bootstrapping issue.

This change fixes an issue where the re-bootstrapping process would overwrite a
newer bootstrap manifest with one that it generates using its older minibp.  It
fixes the issue by only generating a new bootstrap manifest right after
rebuilding minibp (as part of the bootstrap process).  It then uses an
additional rebootstrap iteration to replace the old bootstrap manifest with the
new one.

Change-Id: I16bad2f30f6ad7f10da07d77105e8745adec3650

blueprint/bootstrap/bootstrap.go

@@ -38,7 +38,8 @@ var (
 		blueprint.RuleParams{
 			Command:     "cp $in $out",
 			Description: "cp $out",
-		})
+		},
+		"generator")
 
 	bootstrap = blueprint.StaticRule("bootstrap",
 		blueprint.RuleParams{
@@ -65,17 +66,6 @@ var (
 			Generator:   true,
 		})
 
-	minibp = blueprint.StaticRule("minibp",
-		blueprint.RuleParams{
-			Command: fmt.Sprintf("%s -d %s -o $out $in",
-				minibpFile, minibpDepFile),
-			Description: "minibp $out",
-			Generator:   true,
-			Restat:      true,
-			Depfile:     minibpDepFile,
-			Deps:        blueprint.DepsGCC,
-		})
-
 	// Work around a Ninja issue.  See https://github.com/martine/ninja/pull/634
 	phony = blueprint.StaticRule("phony",
 		blueprint.RuleParams{
@@ -88,9 +78,8 @@ var (
 	goPackageModule = blueprint.MakeModuleType("goPackageModule", newGoPackage)
 	goBinaryModule  = blueprint.MakeModuleType("goBinaryModule", newGoBinary)
 
-	binDir        = filepath.Join("bootstrap", "bin")
+	binDir     = filepath.Join("bootstrap", "bin")
-	minibpFile    = filepath.Join(binDir, "minibp")
+	minibpFile = filepath.Join(binDir, "minibp")
-	minibpDepFile = filepath.Join("bootstrap", "bootstrap_manifest.d")
 )
 
 type goPackageProducer interface {
@@ -364,11 +353,19 @@ func (s *singleton) GenerateBuildActions(ctx blueprint.SingletonContext) {
 
 	tmpNinjaFile := filepath.Join("bootstrap", "build.ninja.in")
 	tmpNinjaDepFile := tmpNinjaFile + ".d"
+	tmpBootstrapFile := filepath.Join("bootstrap", "bootstrap.ninja.in")
 
 	if generatingBootstrapper(ctx.Config()) {
 		// We're generating a bootstrapper Ninja file, so we need to set things
 		// up to rebuild the build.ninja file using the primary builder.
 
+		// Because the non-bootstrap build.ninja file manually re-invokes Ninja,
+		// its builddir must be different than that of the bootstrap build.ninja
+		// file.  Otherwise we occasionally get "warning: bad deps log signature
+		// or version; starting over" messages from Ninja, presumably because
+		// two Ninja processes try to write to the same log concurrently.
+		ctx.SetBuildDir("bootstrap")
+
 		// We generate the depfile here that includes the dependencies for all
 		// the Blueprints files that contribute to generating the big build
 		// manifest (build.ninja file).  This depfile will be used by the non-
@@ -397,10 +394,8 @@ func (s *singleton) GenerateBuildActions(ctx blueprint.SingletonContext) {
 		// accomplish that we depend on a file that should never exist and
 		// "build" it using Ninja's built-in phony rule.
 		//
-		// We also need to add an implicit dependency on the minibp binary so
+		// We also need to add an implicit dependency on tmpBootstrapFile so
-		// that it actually gets built.  Nothing in the bootstrap build.ninja
+		// that it gets generated as part of the bootstrap process.
-		// file actually requires minibp, but the non-bootstrap build.ninja
-		// requires that it have been built during the bootstrapping.
 		notAFile := filepath.Join("bootstrap", "notAFile")
 		ctx.Build(blueprint.BuildParams{
 			Rule:    blueprint.Phony,
@@ -411,15 +406,34 @@ func (s *singleton) GenerateBuildActions(ctx blueprint.SingletonContext) {
 			Rule:      bootstrap,
 			Outputs:   []string{"build.ninja"},
 			Inputs:    []string{tmpNinjaFile},
-			Implicits: []string{"$Bootstrap", notAFile, minibpFile},
+			Implicits: []string{"$Bootstrap", notAFile, tmpBootstrapFile},
 		})
 
-		// Because the non-bootstrap build.ninja file manually re-invokes Ninja,
+		// Rebuild the bootstrap Ninja file using the minibp that we just built.
-		// its builddir must be different than that of the bootstrap build.ninja
+		// The checkFile tells minibp to compare the new bootstrap file to the
-		// file.  Otherwise we occasionally get "warning: bad deps log signature
+		// current one.  If the files are the same then minibp sets the new
-		// or version; starting over" messages from Ninja, presumably because
+		// file's mtime to match that of the current one.  If they're different
-		// two Ninja processes try to write to the same log concurrently.
+		// then the new file will have a newer timestamp than the current one
-		ctx.SetBuildDir("bootstrap")
+		// and it will trigger a reboostrap by the non-boostrap build manifest.
+		minibp := ctx.Rule("minibp",
+			blueprint.RuleParams{
+				Command: fmt.Sprintf("%s -c $checkFile -d $out.d -o $out $in",
+					minibpFile),
+				Description: "minibp $out",
+				Generator:   true,
+				Depfile:     "$out.d",
+			},
+			"checkFile")
+
+		ctx.Build(blueprint.BuildParams{
+			Rule:      minibp,
+			Outputs:   []string{tmpBootstrapFile},
+			Inputs:    []string{topLevelBlueprints},
+			Implicits: []string{minibpFile},
+			Args: map[string]string{
+				"checkFile": "$BootstrapManifest",
+			},
+		})
 	} else {
 		// We're generating a non-bootstrapper Ninja file, so we need to set it
 		// up to depend on the bootstrapper Ninja file.  The build.ninja target
@@ -447,13 +461,18 @@ func (s *singleton) GenerateBuildActions(ctx blueprint.SingletonContext) {
 			},
 		})
 
-		// Rebuild the bootstrap Ninja file using minibp, passing it all the
+		// If the bootstrap Ninja invocation caused a new tmpBootstrapFile to be
-		// Blueprint files that define a bootstrap_* module.
+		// generated then that means we need to rebootstrap using it instead of
+		// the current bootstrap manifest.  We enable the Ninja "generator"
+		// behavior so that Ninja doesn't invoke this build just because it's
+		// missing a command line log entry for the bootstrap manifest.
 		ctx.Build(blueprint.BuildParams{
-			Rule:      minibp,
+			Rule:    cp,
-			Outputs:   []string{"$BootstrapManifest"},
+			Outputs: []string{"$BootstrapManifest"},
-			Inputs:    []string{topLevelBlueprints},
+			Inputs:  []string{tmpBootstrapFile},
-			Implicits: []string{minibpFile},
+			Args: map[string]string{
+				"generator": "true",
+			},
 		})
 	}
 }

blueprint/bootstrap/command.go

@@ -12,7 +12,7 @@ import (
 
 var outFile string
 var depFile string
-var depTarget string
+var checkFile string
 
 // topLevelBlueprintsFile is set by Main as a way to pass this information on to
 // the bootstrap build manifest generators.  This information was not passed via
@@ -23,8 +23,7 @@ var topLevelBlueprintsFile string
 func init() {
 	flag.StringVar(&outFile, "o", "build.ninja.in", "the Ninja file to output")
 	flag.StringVar(&depFile, "d", "", "the dependency file to output")
-	flag.StringVar(&depTarget, "t", "", "the target name for the dependency "+
+	flag.StringVar(&checkFile, "c", "", "the existing file to check against")
-		"file")
 }
 
 func Main(ctx *blueprint.Context, config blueprint.Config) {
@@ -58,23 +57,51 @@ func Main(ctx *blueprint.Context, config blueprint.Config) {
 		fatalf("error generating Ninja file contents: %s", err)
 	}
 
-	err = writeFileIfChanged(outFile, buf.Bytes(), 0666)
+	const outFilePermissions = 0666
+	err = ioutil.WriteFile(outFile, buf.Bytes(), outFilePermissions)
 	if err != nil {
 		fatalf("error writing %s: %s", outFile, err)
 	}
 
+	if checkFile != "" {
+		checkData, err := ioutil.ReadFile(checkFile)
+		if err != nil {
+			fatalf("error reading %s: %s", checkFile, err)
+		}
+
+		matches := buf.Len() == len(checkData)
+		if !matches {
+			for i, value := range buf.Bytes() {
+				if value != checkData[i] {
+					matches = false
+					break
+				}
+			}
+		}
+
+		if matches {
+			// The new file content matches the check-file content, so we set
+			// the new file's mtime and atime to match that of the check-file.
+			checkFileInfo, err := os.Stat(checkFile)
+			if err != nil {
+				fatalf("error stat'ing %s: %s", checkFile, err)
+			}
+
+			time := checkFileInfo.ModTime()
+			err = os.Chtimes(outFile, time, time)
+			if err != nil {
+				fatalf("error setting timestamps for %s: %s", outFile, err)
+			}
+		}
+	}
+
 	if depFile != "" {
 		f, err := os.Create(depFile)
 		if err != nil {
 			fatalf("error creating depfile: %s", err)
 		}
 
-		target := depTarget
+		_, err = fmt.Fprintf(f, "%s: \\\n %s\n", outFile,
-		if target == "" {
-			target = outFile
-		}
-
-		_, err = fmt.Fprintf(f, "%s: \\\n %s\n", target,
 			strings.Join(deps, " \\\n "))
 		if err != nil {
 			fatalf("error writing depfile: %s", err)
@@ -87,7 +114,7 @@ func Main(ctx *blueprint.Context, config blueprint.Config) {
 }
 
 func fatalf(format string, args ...interface{}) {
-	fmt.Fprintf(os.Stderr, format, args...)
+	fmt.Printf(format, args...)
 	os.Exit(1)
 }
 
@@ -95,9 +122,9 @@ func fatalErrors(errs []error) {
 	for _, err := range errs {
 		switch err.(type) {
 		case *blueprint.Error:
-			_, _ = fmt.Fprintf(os.Stderr, "%s\n", err.Error())
+			_, _ = fmt.Printf("%s\n", err.Error())
 		default:
-			_, _ = fmt.Fprintf(os.Stderr, "internal error: %s\n", err)
+			_, _ = fmt.Printf("internal error: %s\n", err)
 		}
 	}
 	os.Exit(1)

blueprint/module_ctx.go

@@ -18,7 +18,7 @@ type ModuleContext interface {
 	PropertyErrorf(property, fmt string, args ...interface{})
 
 	Variable(name, value string)
-	Rule(name string, params RuleParams) Rule
+	Rule(name string, params RuleParams, argNames ...string) Rule
 	Build(params BuildParams)
 
 	VisitDepsDepthFirst(visit func(Module))
@@ -81,10 +81,12 @@ func (m *moduleContext) Variable(name, value string) {
 	m.actionDefs.variables = append(m.actionDefs.variables, v)
 }
 
-func (m *moduleContext) Rule(name string, params RuleParams) Rule {
+func (m *moduleContext) Rule(name string, params RuleParams,
+	argNames ...string) Rule {
+
 	// TODO: Verify that params.Pool is accessible in this module's scope.
 
-	r, err := m.scope.AddLocalRule(name, &params)
+	r, err := m.scope.AddLocalRule(name, &params, argNames...)
 	if err != nil {
 		panic(err)
 	}

blueprint/singleton_ctx.go

@@ -20,7 +20,7 @@ type SingletonContext interface {
 	Errorf(format string, args ...interface{})
 
 	Variable(name, value string)
-	Rule(name string, params RuleParams) Rule
+	Rule(name string, params RuleParams, argNames ...string) Rule
 	Build(params BuildParams)
 	RequireNinjaVersion(major, minor, micro int)
 
@@ -89,10 +89,12 @@ func (s *singletonContext) Variable(name, value string) {
 	s.actionDefs.variables = append(s.actionDefs.variables, v)
 }
 
-func (s *singletonContext) Rule(name string, params RuleParams) Rule {
+func (s *singletonContext) Rule(name string, params RuleParams,
+	argNames ...string) Rule {
+
 	// TODO: Verify that params.Pool is accessible in this module's scope.
 
-	r, err := s.scope.AddLocalRule(name, &params)
+	r, err := s.scope.AddLocalRule(name, &params, argNames...)
 	if err != nil {
 		panic(err)
 	}

build.ninja.in

@@ -148,9 +148,19 @@ rule s.bootstrap.bigbp
     depfile = bootstrap/build.ninja.in.d
     description = minibp ${out}
 
+rule s.bootstrap.minibp
+    command = bootstrap/bin/minibp -c ${checkFile} -d ${out}.d -o ${out} ${in}
+    depfile = ${out}.d
+    description = minibp ${out}
+    generator = true
+
 build bootstrap/build.ninja.in: s.bootstrap.bigbp $
         ${g.bootstrap.SrcDir}/Blueprints | bootstrap/bin/minibp
 build bootstrap/notAFile: phony
 build build.ninja: g.bootstrap.bootstrap bootstrap/build.ninja.in | $
-        ${g.bootstrap.Bootstrap} bootstrap/notAFile bootstrap/bin/minibp
+        ${g.bootstrap.Bootstrap} bootstrap/notAFile $
+        bootstrap/bootstrap.ninja.in
+build bootstrap/bootstrap.ninja.in: s.bootstrap.minibp $
+        ${g.bootstrap.SrcDir}/Blueprints | bootstrap/bin/minibp
+    checkFile = ${g.bootstrap.BootstrapManifest}