From fb618c387a487413885382ab1a0534c81dba4669 Mon Sep 17 00:00:00 2001
From: Colin Cross <ccross@android.com>
Date: Wed, 9 Jun 2021 12:48:53 -0700
Subject: [PATCH] Always propagate some environment variables to RBE

Always propagate LANG, LC_MESSAGES and PYTHONDONTWRITEBYTECODE
to RBE to get more consistent behavior between local actions and
RBE.

Bug: 182415460
Bug: 190593001
Test: treehugger
Change-Id: I726e6f02fd3ef77e158baf6fde77ffb7247a1375
Merged-In: I726e6f02fd3ef77e158baf6fde77ffb7247a1375
---
 java/lint.go                  |  5 +----
 remoteexec/remoteexec.go      | 16 ++++++++++++----
 remoteexec/remoteexec_test.go |  8 ++++----
 3 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/java/lint.go b/java/lint.go
index 9f769dfe3..1511cfe26 100644
--- a/java/lint.go
+++ b/java/lint.go
@@ -361,10 +361,7 @@ func (l *linter) lint(ctx android.ModuleContext) {
 			Labels:          map[string]string{"type": "tool", "name": "lint"},
 			ExecStrategy:    lintRBEExecStrategy(ctx),
 			ToolchainInputs: []string{config.JavaCmd(ctx).String()},
-			EnvironmentVariables: []string{
-				"LANG",
-			},
-			Platform: map[string]string{remoteexec.PoolKey: pool},
+			Platform:        map[string]string{remoteexec.PoolKey: pool},
 		})
 	}
 
diff --git a/remoteexec/remoteexec.go b/remoteexec/remoteexec.go
index ef4672a73..9e7a0f1e3 100644
--- a/remoteexec/remoteexec.go
+++ b/remoteexec/remoteexec.go
@@ -50,8 +50,14 @@ const (
 )
 
 var (
-	defaultLabels       = map[string]string{"type": "tool"}
-	defaultExecStrategy = LocalExecStrategy
+	defaultLabels               = map[string]string{"type": "tool"}
+	defaultExecStrategy         = LocalExecStrategy
+	defaultEnvironmentVariables = []string{
+		// This is a subset of the allowlist in ui/build/ninja.go that makes sense remotely.
+		"LANG",
+		"LC_MESSAGES",
+		"PYTHONDONTWRITEBYTECODE",
+	}
 )
 
 // REParams holds information pertinent to the remote execution of a rule.
@@ -149,8 +155,10 @@ func (r *REParams) wrapperArgs() string {
 		args += " --toolchain_inputs=" + strings.Join(r.ToolchainInputs, ",")
 	}
 
-	if len(r.EnvironmentVariables) > 0 {
-		args += " --env_var_allowlist=" + strings.Join(r.EnvironmentVariables, ",")
+	envVarAllowlist := append(r.EnvironmentVariables, defaultEnvironmentVariables...)
+
+	if len(envVarAllowlist) > 0 {
+		args += " --env_var_allowlist=" + strings.Join(envVarAllowlist, ",")
 	}
 
 	return args + " -- "
diff --git a/remoteexec/remoteexec_test.go b/remoteexec/remoteexec_test.go
index b117b8915..368631609 100644
--- a/remoteexec/remoteexec_test.go
+++ b/remoteexec/remoteexec_test.go
@@ -36,7 +36,7 @@ func TestTemplate(t *testing.T) {
 					PoolKey:           "default",
 				},
 			},
-			want: fmt.Sprintf("${android.RBEWrapper} --labels=compiler=clang,lang=cpp,type=compile --platform=\"Pool=default,container-image=%s\" --exec_strategy=local --inputs=$in --output_files=$out -- ", DefaultImage),
+			want: fmt.Sprintf("${android.RBEWrapper} --labels=compiler=clang,lang=cpp,type=compile --platform=\"Pool=default,container-image=%s\" --exec_strategy=local --inputs=$in --output_files=$out --env_var_allowlist=LANG,LC_MESSAGES,PYTHONDONTWRITEBYTECODE -- ", DefaultImage),
 		},
 		{
 			name: "all params",
@@ -52,7 +52,7 @@ func TestTemplate(t *testing.T) {
 					PoolKey:           "default",
 				},
 			},
-			want: fmt.Sprintf("${android.RBEWrapper} --labels=compiler=clang,lang=cpp,type=compile --platform=\"Pool=default,container-image=%s\" --exec_strategy=remote --inputs=$in --input_list_paths=$out.rsp,out2.rsp --output_files=$out --toolchain_inputs=clang++ -- ", DefaultImage),
+			want: fmt.Sprintf("${android.RBEWrapper} --labels=compiler=clang,lang=cpp,type=compile --platform=\"Pool=default,container-image=%s\" --exec_strategy=remote --inputs=$in --input_list_paths=$out.rsp,out2.rsp --output_files=$out --toolchain_inputs=clang++ --env_var_allowlist=LANG,LC_MESSAGES,PYTHONDONTWRITEBYTECODE -- ", DefaultImage),
 		},
 	}
 	for _, test := range tests {
@@ -74,7 +74,7 @@ func TestNoVarTemplate(t *testing.T) {
 			PoolKey:           "default",
 		},
 	}
-	want := fmt.Sprintf("prebuilts/remoteexecution-client/live/rewrapper --labels=compiler=clang,lang=cpp,type=compile --platform=\"Pool=default,container-image=%s\" --exec_strategy=local --inputs=$in --output_files=$out -- ", DefaultImage)
+	want := fmt.Sprintf("prebuilts/remoteexecution-client/live/rewrapper --labels=compiler=clang,lang=cpp,type=compile --platform=\"Pool=default,container-image=%s\" --exec_strategy=local --inputs=$in --output_files=$out --env_var_allowlist=LANG,LC_MESSAGES,PYTHONDONTWRITEBYTECODE -- ", DefaultImage)
 	if got := params.NoVarTemplate(DefaultWrapperPath); got != want {
 		t.Errorf("NoVarTemplate() returned\n%s\nwant\n%s", got, want)
 	}
@@ -90,7 +90,7 @@ func TestTemplateDeterminism(t *testing.T) {
 			PoolKey:           "default",
 		},
 	}
-	want := fmt.Sprintf("${android.RBEWrapper} --labels=compiler=clang,lang=cpp,type=compile --platform=\"Pool=default,container-image=%s\" --exec_strategy=local --inputs=$in --output_files=$out -- ", DefaultImage)
+	want := fmt.Sprintf("${android.RBEWrapper} --labels=compiler=clang,lang=cpp,type=compile --platform=\"Pool=default,container-image=%s\" --exec_strategy=local --inputs=$in --output_files=$out --env_var_allowlist=LANG,LC_MESSAGES,PYTHONDONTWRITEBYTECODE -- ", DefaultImage)
 	for i := 0; i < 1000; i++ {
 		if got := r.Template(); got != want {
 			t.Fatalf("Template() returned\n%s\nwant\n%s", got, want)