Add option to limit environment variables given to ninja

Ninja does not track changes in environment variables, so we get
potentially incorrect builds when environment variables change during
incremental builds if some action was using one of them.

Add a variable to limit exposure of these variables to ninja, and thus,
all actions run by ninja. Kati and Soong can still read environment
variables, they explicitly track which ones they read so that we can
re-run them appropriately.

This list is just the beginning, there's no good way to detect which
environment variables are currently being used and to pass them through.
So this initial change won't have a behavioral change, and we'll flip
the switch and see what fails or who complains, flipping it off and on
and adding to the list until we can make this always happen.

Also adds a board-specific `BUILD_BROKEN_NINJA_USES_ENV_VARS := ...`
list so that we can temporarily allow board-specific variables until
they're fixed.

Test: check out/soong.log
Test: ALLOW_NINJA_ENV=false m nothing; check out/soong.log
Test: set BUILD_BROKEN_NINJA_USES_ENV_VARS := OLDPWD
      ALLOW_NINJA_ENV=false m nothing; check out/soong.log
Change-Id: I08e4834ce12100a577ef7d6a9a21b9e9d345cb93

3 files changed, 79 insertions, 4 deletions

diff --git a/ui/build/config.go b/ui/build/config.go
index fae569f0..c0841713 100644
--- a/ui/build/config.go
+++ b/ui/build/config.go
@@ -55,8 +55,9 @@ type configImpl struct {
 
 	pdkBuild bool
 
-	brokenDupRules    bool
-	brokenUsesNetwork bool
+	brokenDupRules     bool
+	brokenUsesNetwork  bool
+	brokenNinjaEnvVars []string
 
 	pathReplaced bool
 }
@@ -907,6 +908,14 @@ func (c *configImpl) BuildBrokenUsesNetwork() bool {
 	return c.brokenUsesNetwork
 }
 
+func (c *configImpl) SetBuildBrokenNinjaUsesEnvVars(val []string) {
+	c.brokenNinjaEnvVars = val
+}
+
+func (c *configImpl) BuildBrokenNinjaUsesEnvVars() []string {
+	return c.brokenNinjaEnvVars
+}
+
 func (c *configImpl) SetTargetDeviceDir(dir string) {
 	c.targetDeviceDir = dir
 }
diff --git a/ui/build/dumpvars.go b/ui/build/dumpvars.go
index 4270bb19..c3da38bc 100644
--- a/ui/build/dumpvars.go
+++ b/ui/build/dumpvars.go
@@ -216,6 +216,9 @@ func runMakeProductConfig(ctx Context, config Config) {
 		// Whether to enable the network during the build
 		"BUILD_BROKEN_USES_NETWORK",
 
+		// Extra environment variables to be exported to ninja
+		"BUILD_BROKEN_NINJA_USES_ENV_VARS",
+
 		// Not used, but useful to be in the soong.log
 		"BOARD_VNDK_VERSION",
 
@@ -284,4 +287,5 @@ func runMakeProductConfig(ctx Context, config Config) {
 	config.SetPdkBuild(make_vars["TARGET_BUILD_PDK"] == "true")
 	config.SetBuildBrokenDupRules(make_vars["BUILD_BROKEN_DUP_RULES"] == "true")
 	config.SetBuildBrokenUsesNetwork(make_vars["BUILD_BROKEN_USES_NETWORK"] == "true")
+	config.SetBuildBrokenNinjaUsesEnvVars(strings.Fields(make_vars["BUILD_BROKEN_NINJA_USES_ENV_VARS"]))
 }
diff --git a/ui/build/ninja.go b/ui/build/ninja.go
index d5baafe7..5357d44c 100644
--- a/ui/build/ninja.go
+++ b/ui/build/ninja.go
@@ -18,6 +18,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"sort"
 	"strconv"
 	"strings"
 	"time"
@@ -65,8 +66,6 @@ func runNinja(ctx Context, config Config) {
 		cmd.Environment.AppendFromKati(config.KatiEnvFile())
 	}
 
-	cmd.Environment.Set("DIST_DIR", config.DistDir())
-
 	// Allow both NINJA_ARGS and NINJA_EXTRA_ARGS, since both have been
 	// used in the past to specify extra ninja arguments.
 	if extra, ok := cmd.Environment.Get("NINJA_ARGS"); ok {
@@ -85,6 +84,69 @@ func runNinja(ctx Context, config Config) {
 			ninjaHeartbeatDuration = overrideDuration
 		}
 	}
+
+	// Filter the environment, as ninja does not rebuild files when environment variables change.
+	//
+	// Anything listed here must not change the output of rules/actions when the value changes,
+	// otherwise incremental builds may be unsafe. Vars explicitly set to stable values
+	// elsewhere in soong_ui are fine.
+	//
+	// For the majority of cases, either Soong or the makefiles should be replicating any
+	// necessary environment variables in the command line of each action that needs it.
+	if cmd.Environment.IsFalse("ALLOW_NINJA_ENV") {
+		cmd.Environment.Allow(append([]string{
+			"ASAN_SYMBOLIZER_PATH",
+			"HOME",
+			"JAVA_HOME",
+			"LANG",
+			"LC_MESSAGES",
+			"OUT_DIR",
+			"PATH",
+			"PWD",
+			"PYTHONDONTWRITEBYTECODE",
+			"TMPDIR",
+			"USER",
+
+			// TODO: remove these carefully
+			"TARGET_BUILD_APPS",
+			"TARGET_BUILD_VARIANT",
+			"TARGET_PRODUCT",
+
+			// Goma -- gomacc may not need all of these
+			"GOMA_DIR",
+			"GOMA_DISABLED",
+			"GOMA_FAIL_FAST",
+			"GOMA_FALLBACK",
+			"GOMA_GCE_SERVICE_ACCOUNT",
+			"GOMA_TMP_DIR",
+			"GOMA_USE_LOCAL",
+
+			// RBE client
+			"FLAG_exec_root",
+			"FLAG_exec_strategy",
+			"FLAG_invocation_id",
+			"FLAG_log_dir",
+			"FLAG_platform",
+			"FLAG_server_address",
+
+			// ccache settings
+			"CCACHE_COMPILERCHECK",
+			"CCACHE_SLOPPINESS",
+			"CCACHE_BASEDIR",
+			"CCACHE_CPP2",
+		}, config.BuildBrokenNinjaUsesEnvVars()...)...)
+	}
+
+	cmd.Environment.Set("DIST_DIR", config.DistDir())
+	cmd.Environment.Set("SHELL", "/bin/bash")
+
+	ctx.Verboseln("Ninja environment: ")
+	envVars := cmd.Environment.Environ()
+	sort.Strings(envVars)
+	for _, envVar := range envVars {
+		ctx.Verbosef("  %s", envVar)
+	}
+
 	// Poll the ninja log for updates; if it isn't updated enough, then we want to show some diagnostics
 	done := make(chan struct{})
 	defer close(done)