Make GENERATE_BAZEL_FILES=true correct.

This is achieved by writing soong.environment.used in Main() instead of
as a side effect of a singleton. This makes a difference because build
actions are not generated when GENERATE_BAZEL_FILES=true is set,
therefore the side effect did not happen.

Arguably, Main() is made worse by this change, but I don't want to
tackle the problem of readably determining which mode soong_build is
running in in this change.

Test: Presubmits + the additional test.
Change-Id: I66af2429aedf008762173eaaa55b828b4cf4328b

android/env.go

@@ -18,12 +18,16 @@ import (
 	"android/soong/shared"
 )
 
-// This file supports dependencies on environment variables.  During build manifest generation,
+// This file supports dependencies on environment variables.  During build
-// any dependency on an environment variable is added to a list.  During the singleton phase
+// manifest generation, any dependency on an environment variable is added to a
-// a JSON file is written containing the current value of all used environment variables.
+// list.  At the end of the build, a JSON file called soong.environment.used is
-// The next time the top-level build script is run, it uses the soong_env executable to
+// written containing the current value of all used environment variables. The
-// compare the contents of the environment variables, rewriting the file if necessary to cause
+// next time the top-level build script is run, soong_ui parses the compare the
-// a manifest regeneration.
+// contents of the used environment variables, then, if they changed, deletes
+// soong.environment.used to cause a rebuild.
+//
+// The dependency of build.ninja on soong.environment.used is declared in
+// build.ninja.d
 
 var originalEnv map[string]string
 
@@ -34,30 +38,3 @@ func InitEnvironment(envFile string) {
 		panic(err)
 	}
 }
-
-func EnvSingleton() Singleton {
-	return &envSingleton{}
-}
-
-type envSingleton struct{}
-
-func (c *envSingleton) GenerateBuildActions(ctx SingletonContext) {
-	envDeps := ctx.Config().EnvDeps()
-
-	envFile := PathForOutput(ctx, "soong.environment.used")
-	if ctx.Failed() {
-		return
-	}
-
-	data, err := shared.EnvFileContents(envDeps)
-	if err != nil {
-		ctx.Errorf(err.Error())
-	}
-
-	err = WriteFileToOutputDir(envFile, data, 0666)
-	if err != nil {
-		ctx.Errorf(err.Error())
-	}
-
-	ctx.AddNinjaFileDeps(envFile.String())
-}

android/register.go

@@ -206,7 +206,6 @@ func collateGloballyRegisteredSingletons() sortableComponents {
 
 		// Register env and ninjadeps last so that they can track all used environment variables and
 		// Ninja file dependencies stored in the config.
-		singleton{false, "env", EnvSingleton},
 		singleton{false, "ninjadeps", ninjaDepsSingletonFactory},
 	)
 

android/testing.go

@@ -401,9 +401,6 @@ func (ctx *TestContext) Register() {
 	globalOrder.mutatorOrder.enforceOrdering(mutators)
 	mutators.registerAll(ctx.Context)
 
-	// Register the env singleton with this context before sorting.
-	ctx.RegisterSingletonType("env", EnvSingleton)
-
 	// Ensure that the singletons used in the test are in the same order as they are used at runtime.
 	globalOrder.singletonOrder.enforceOrdering(ctx.singletons)
 	ctx.singletons.registerAll(ctx.Context)

bootstrap_test.sh

@@ -235,6 +235,86 @@ EOF
   grep -q my_little_library.py out/soong/build.ninja || fail "new file is not in output"
 }
 
+function test_soong_build_rerun_iff_environment_changes() {
+  setup
+
+  mkdir -p cherry
+  cat > cherry/Android.bp <<'EOF'
+bootstrap_go_package {
+  name: "cherry",
+  pkgPath: "android/soong/cherry",
+  deps: [
+    "blueprint",
+    "soong",
+    "soong-android",
+  ],
+  srcs: [
+    "cherry.go",
+  ],
+  pluginFor: ["soong_build"],
+}
+EOF
+
+  cat > cherry/cherry.go <<'EOF'
+package cherry
+
+import (
+  "android/soong/android"
+  "github.com/google/blueprint"
+)
+
+var (
+  pctx = android.NewPackageContext("cherry")
+)
+
+func init() {
+  android.RegisterSingletonType("cherry", CherrySingleton)
+}
+
+func CherrySingleton() android.Singleton {
+  return &cherrySingleton{}
+}
+
+type cherrySingleton struct{}
+
+func (p *cherrySingleton) GenerateBuildActions(ctx android.SingletonContext) {
+  cherryRule := ctx.Rule(pctx, "cherry",
+    blueprint.RuleParams{
+      Command: "echo CHERRY IS " + ctx.Config().Getenv("CHERRY") + " > ${out}",
+      CommandDeps: []string{},
+      Description: "Cherry",
+    })
+
+  outputFile := android.PathForOutput(ctx, "cherry", "cherry.txt")
+  var deps android.Paths
+
+  ctx.Build(pctx, android.BuildParams{
+    Rule: cherryRule,
+    Output: outputFile,
+    Inputs: deps,
+  })
+}
+EOF
+
+  export CHERRY=TASTY
+  run_soong
+  grep -q "CHERRY IS TASTY" out/soong/build.ninja \
+    || fail "first value of environment variable is not used"
+
+  export CHERRY=RED
+  run_soong
+  grep -q "CHERRY IS RED" out/soong/build.ninja \
+    || fail "second value of environment variable not used"
+  local mtime1=$(stat -c "%y" out/soong/build.ninja)
+
+  run_soong
+  local mtime2=$(stat -c "%y" out/soong/build.ninja)
+  if [[ "$mtime1" != "$mtime2" ]]; then
+    fail "Output Ninja file changed when environment variable did not"
+  fi
+
+}
+
 function test_add_file_to_soong_build() {
   setup
   run_soong
@@ -308,12 +388,28 @@ EOF
   grep -q "Make it so" out/soong/build.ninja || fail "New action not present"
 }
 
+function test_null_build_after_docs {
+  setup
+  run_soong
+  local mtime1=$(stat -c "%y" out/soong/build.ninja)
+
+  prebuilts/build-tools/linux-x86/bin/ninja -f out/soong/build.ninja soong_docs
+  run_soong
+  local mtime2=$(stat -c "%y" out/soong/build.ninja)
+
+  if [[ "$mtime1" != "$mtime2" ]]; then
+    fail "Output Ninja file changed on null build"
+  fi
+}
+
 test_bazel_smoke
 test_smoke
 test_null_build
+test_null_build_after_docs
 test_soong_build_rebuilt_if_blueprint_changes
 test_add_file_to_glob
 test_add_android_bp
 test_change_android_bp
 test_delete_android_bp
 test_add_file_to_soong_build
+test_soong_build_rerun_iff_environment_changes

cmd/soong_build/main.go

@@ -17,6 +17,7 @@ package main
 import (
 	"flag"
 	"fmt"
+	"io/ioutil"
 	"os"
 	"path/filepath"
 	"strings"
@@ -95,11 +96,15 @@ func main() {
 	android.InitSandbox(topDir)
 	android.InitEnvironment(shared.JoinPath(topDir, outDir, "soong.environment.available"))
 
+	usedVariablesFile := shared.JoinPath(outDir, "soong.environment.used")
 	// The top-level Blueprints file is passed as the first argument.
 	srcDir := filepath.Dir(flag.Arg(0))
 	var ctx *android.Context
 	configuration := newConfig(srcDir)
-	extraNinjaDeps := []string{configuration.ProductVariablesFileName}
+	extraNinjaDeps := []string{
+		configuration.ProductVariablesFileName,
+		shared.JoinPath(outDir, "soong.environment.used"),
+	}
 
 	if configuration.Getenv("ALLOW_MISSING_DEPENDENCIES") == "true" {
 		configuration.SetAllowMissingDependencies()
@@ -115,15 +120,12 @@ func main() {
 		extraNinjaDeps = append(extraNinjaDeps, filepath.Join(configuration.BuildDir(), "always_rerun_for_delve"))
 	}
 
-	if bazelConversionRequested(configuration) {
+	bazelConversionRequested := bazelConversionRequested(configuration)
+	if bazelConversionRequested {
 		// Run the alternate pipeline of bp2build mutators and singleton to convert Blueprint to BUILD files
 		// before everything else.
-		runBp2Build(srcDir, configuration)
+		runBp2Build(srcDir, configuration, extraNinjaDeps)
-		// Short-circuit and return.
+	} else if configuration.BazelContext.BazelEnabled() {
-		return
-	}
-
-	if configuration.BazelContext.BazelEnabled() {
 		// Bazel-enabled mode. Soong runs in two passes.
 		// First pass: Analyze the build tree, but only store all bazel commands
 		// needed to correctly evaluate the tree in the second pass.
@@ -151,7 +153,7 @@ func main() {
 	}
 
 	// Convert the Soong module graph into Bazel BUILD files.
-	if bazelQueryViewDir != "" {
+	if !bazelConversionRequested && bazelQueryViewDir != "" {
 		// Run the code-generation phase to convert BazelTargetModules to BUILD files.
 		codegenContext := bp2build.NewCodegenContext(configuration, *ctx, bp2build.QueryView)
 		absoluteQueryViewDir := shared.JoinPath(topDir, bazelQueryViewDir)
@@ -161,7 +163,7 @@ func main() {
 		}
 	}
 
-	if docFile != "" {
+	if !bazelConversionRequested && docFile != "" {
 		if err := writeDocs(ctx, configuration, docFile); err != nil {
 			fmt.Fprintf(os.Stderr, "%s", err)
 			os.Exit(1)
@@ -170,7 +172,7 @@ func main() {
 
 	// TODO(ccross): make this a command line argument.  Requires plumbing through blueprint
 	//  to affect the command line of the primary builder.
-	if shouldPrepareBuildActions(configuration) {
+	if !bazelConversionRequested && shouldPrepareBuildActions(configuration) {
 		metricsFile := filepath.Join(bootstrap.CmdlineBuildDir(), "soong_build_metrics.pb")
 		err := android.WriteMetrics(configuration, metricsFile)
 		if err != nil {
@@ -178,12 +180,32 @@ func main() {
 			os.Exit(1)
 		}
 	}
+
+	if docFile == "" {
+		// Let's not overwrite the used variables file when generating
+		// documentation
+		writeUsedVariablesFile(shared.JoinPath(topDir, usedVariablesFile), configuration)
+	}
+}
+
+func writeUsedVariablesFile(path string, configuration android.Config) {
+	data, err := shared.EnvFileContents(configuration.EnvDeps())
+	if err != nil {
+		fmt.Fprintf(os.Stderr, "error writing used variables file %s: %s", path, err)
+		os.Exit(1)
+	}
+
+	err = ioutil.WriteFile(path, data, 0666)
+	if err != nil {
+		fmt.Fprintf(os.Stderr, "error writing used variables file %s: %s", path, err)
+		os.Exit(1)
+	}
 }
 
 // Run Soong in the bp2build mode. This creates a standalone context that registers
 // an alternate pipeline of mutators and singletons specifically for generating
 // Bazel BUILD files instead of Ninja files.
-func runBp2Build(srcDir string, configuration android.Config) {
+func runBp2Build(srcDir string, configuration android.Config, extraNinjaDeps []string) {
 	// Register an alternate set of singletons and mutators for bazel
 	// conversion for Bazel conversion.
 	bp2buildCtx := android.NewContext(configuration)
@@ -198,11 +220,13 @@ func runBp2Build(srcDir string, configuration android.Config) {
 	// configurations or variables, since those will generate different BUILD
 	// files based on how the user has configured their tree.
 	bp2buildCtx.SetModuleListFile(bootstrap.CmdlineModuleListFile())
-	extraNinjaDeps, err := bp2buildCtx.ListModulePaths(srcDir)
+	modulePaths, err := bp2buildCtx.ListModulePaths(srcDir)
 	if err != nil {
 		panic(err)
 	}
 
+	extraNinjaDeps = append(extraNinjaDeps, modulePaths...)
+
 	// Run the loading and analysis pipeline to prepare the graph of regular
 	// Modules parsed from Android.bp files, and the BazelTargetModules mapped
 	// from the regular Modules.