Add errorHints to stdout when read-only file system errors are detected

The source tree will eventually be made ReadOnly, and recipes that write
directly to the source tree will fail. Use a pattern-match approach on
the results of stdout/stderr to provide hints to the user in such a
scenario.

If multiple patterns are found in raw output, print error hint
corresponding to first pattern match. first pattern match is chosen
since the failing function will be at the top of the stack, and hence
will be logged first

Test: Wrote a unit test to assert errorhint is added to output.
Wrote an integration test that writes to a file in the source tree
1. When source_tree is RO, the recipe fails and an error hint is printed
to stdout
2. When source tree is RW, the recipe succeeds and no error hint is
printed

Bug: 174726238
Change-Id: Id67b48f8094cdf8a571c239ae469d60464a1e89c

tests/bootstrap_test.sh

@@ -486,6 +486,39 @@ function test_null_build_after_docs {
   fi
 }
 
+function test_write_to_source_tree {
+  setup
+  mkdir -p a
+  cat > a/Android.bp <<EOF
+genrule {
+  name: "write_to_source_tree",
+  out: ["write_to_source_tree"],
+  cmd: "touch file_in_source_tree && touch \$(out)",
+}
+EOF
+  readonly EXPECTED_OUT=out/soong/.intermediates/a/write_to_source_tree/gen/write_to_source_tree
+  readonly ERROR_LOG=${MOCK_TOP}/out/error.log
+  readonly ERROR_MSG="Read-only file system"
+  readonly ERROR_HINT_PATTERN="BUILD_BROKEN_SRC_DIR"
+  # Test in ReadOnly source tree
+  run_ninja BUILD_BROKEN_SRC_DIR_IS_WRITABLE=false ${EXPECTED_OUT} &> /dev/null && \
+    fail "Write to source tree should not work in a ReadOnly source tree"
+
+  if grep -q "${ERROR_MSG}" ${ERROR_LOG} && grep -q "${ERROR_HINT_PATTERN}" ${ERROR_LOG} ; then
+    echo Error message and error hint found in logs >/dev/null
+  else
+    fail "Did not find Read-only error AND error hint in error.log"
+  fi
+
+  # Test in ReadWrite source tree
+  run_ninja BUILD_BROKEN_SRC_DIR_IS_WRITABLE=true ${EXPECTED_OUT} &> /dev/null || \
+    fail "Write to source tree did not succeed in a ReadWrite source tree"
+
+  if  grep -q "${ERROR_MSG}\|${ERROR_HINT_PATTERN}" ${ERROR_LOG} ; then
+    fail "Found read-only error OR error hint in error.log"
+  fi
+}
+
 function test_bp2build_smoke {
   setup
   GENERATE_BAZEL_FILES=1 run_soong
@@ -692,6 +725,7 @@ test_add_file_to_soong_build
 test_glob_during_bootstrapping
 test_soong_build_rerun_iff_environment_changes
 test_dump_json_module_graph
+test_write_to_source_tree
 test_bp2build_smoke
 test_bp2build_generates_fake_ninja_file
 test_bp2build_null_build

tests/lib.sh

@@ -126,6 +126,10 @@ run_bp2build() {
   GENERATE_BAZEL_FILES=true build/soong/soong_ui.bash --make-mode --skip-ninja --skip-make --skip-soong-tests nothing
 }
 
+run_ninja() {
+  build/soong/soong_ui.bash --make-mode --skip-make --skip-soong-tests "$@"
+}
+
 info "Starting Soong integration test suite $(basename $0)"
 info "Mock top: $MOCK_TOP"
 

ui/build/config.go

@@ -158,6 +158,10 @@ func NewConfig(ctx Context, args ...string) Config {
 		ret.distDir = filepath.Join(ret.OutDir(), "dist")
 	}
 
+	if srcDirIsWritable, ok := ret.environ.Get("BUILD_BROKEN_SRC_DIR_IS_WRITABLE"); ok {
+		ret.sandboxConfig.SetSrcDirIsRO(srcDirIsWritable == "false")
+	}
+
 	ret.environ.Unset(
 		// We're already using it
 		"USE_SOONG_UI",

ui/build/sandbox_linux.go

@@ -97,8 +97,11 @@ func (c *Cmd) sandboxSupported() bool {
 			"-u", "nobody",
 			"-g", sandboxConfig.group,
 			"-R", "/",
-			"-B", sandboxConfig.srcDir,
+			// Mount tmp before srcDir
+			// srcDir is /tmp/.* in integration tests, which is a child dir of /tmp
+			// nsjail throws an error if a child dir is mounted before its parent
 			"-B", "/tmp",
+			"-B", sandboxConfig.srcDir,
 			"-B", sandboxConfig.outDir,
 		}
 

ui/status/ninja.go

@@ -19,6 +19,8 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"regexp"
+	"strings"
 	"syscall"
 	"time"
 
@@ -158,9 +160,10 @@ func (n *NinjaReader) run() {
 					err = fmt.Errorf("exited with code: %d", exitCode)
 				}
 
+				outputWithErrorHint := errorHintGenerator.GetOutputWithErrorHint(msg.EdgeFinished.GetOutput(), exitCode)
 				n.status.FinishAction(ActionResult{
 					Action: started,
-					Output: msg.EdgeFinished.GetOutput(),
+					Output: outputWithErrorHint,
 					Error:  err,
 					Stats: ActionResultStats{
 						UserTime:                   msg.EdgeFinished.GetUserTime(),
@@ -219,3 +222,53 @@ func readVarInt(r *bufio.Reader) (int, error) {
 
 	return ret, nil
 }
+
+// key is pattern in stdout/stderr
+// value is error hint
+var allErrorHints = map[string]string{
+	"Read-only file system": `\nWrite to a read-only file system detected. Possible fixes include
+1. Generate file directly to out/ which is ReadWrite, #recommend solution
+2. BUILD_BROKEN_SRC_DIR_RW_ALLOWLIST := <my/path/1> <my/path/2> #discouraged, subset of source tree will be RW
+3. BUILD_BROKEN_SRC_DIR_IS_WRITABLE := true #highly discouraged, entire source tree will be RW
+`,
+}
+var errorHintGenerator = *newErrorHintGenerator(allErrorHints)
+
+type ErrorHintGenerator struct {
+	allErrorHints                map[string]string
+	allErrorHintPatternsCompiled *regexp.Regexp
+}
+
+func newErrorHintGenerator(allErrorHints map[string]string) *ErrorHintGenerator {
+	var allErrorHintPatterns []string
+	for errorHintPattern, _ := range allErrorHints {
+		allErrorHintPatterns = append(allErrorHintPatterns, errorHintPattern)
+	}
+	allErrorHintPatternsRegex := strings.Join(allErrorHintPatterns[:], "|")
+	re := regexp.MustCompile(allErrorHintPatternsRegex)
+	return &ErrorHintGenerator{
+		allErrorHints:                allErrorHints,
+		allErrorHintPatternsCompiled: re,
+	}
+}
+
+func (errorHintGenerator *ErrorHintGenerator) GetOutputWithErrorHint(rawOutput string, buildExitCode int) string {
+	if buildExitCode == 0 {
+		return rawOutput
+	}
+	errorHint := errorHintGenerator.getErrorHint(rawOutput)
+	if errorHint == nil {
+		return rawOutput
+	}
+	return rawOutput + *errorHint
+}
+
+// Returns the error hint corresponding to the FIRST match in raw output
+func (errorHintGenerator *ErrorHintGenerator) getErrorHint(rawOutput string) *string {
+	firstMatch := errorHintGenerator.allErrorHintPatternsCompiled.FindString(rawOutput)
+	if _, found := errorHintGenerator.allErrorHints[firstMatch]; found {
+		errorHint := errorHintGenerator.allErrorHints[firstMatch]
+		return &errorHint
+	}
+	return nil
+}

ui/status/ninja_test.go

@@ -43,3 +43,53 @@ func TestNinjaReader_Close(t *testing.T) {
 		t.Errorf("nr.Close timed out, %s > %s", g, w)
 	}
 }
+
+// Test that error hint is added to output if available
+func TestNinjaReader_CorrectErrorHint(t *testing.T) {
+	errorPattern1 := "pattern-1 in input"
+	errorHint1 := "\n Fix by doing task 1"
+	errorPattern2 := "pattern-2 in input"
+	errorHint2 := "\n Fix by doing task 2"
+	mockErrorHints := make(map[string]string)
+	mockErrorHints[errorPattern1] = errorHint1
+	mockErrorHints[errorPattern2] = errorHint2
+
+	errorHintGenerator := *newErrorHintGenerator(mockErrorHints)
+	testCases := []struct {
+		rawOutput            string
+		buildExitCode        int
+		expectedFinalOutput  string
+		testCaseErrorMessage string
+	}{
+		{
+			rawOutput:            "ninja build was successful",
+			buildExitCode:        0,
+			expectedFinalOutput:  "ninja build was successful",
+			testCaseErrorMessage: "raw output changed when build was successful",
+		},
+		{
+			rawOutput:            "ninja build failed",
+			buildExitCode:        1,
+			expectedFinalOutput:  "ninja build failed",
+			testCaseErrorMessage: "raw output changed even when no error hint pattern was found",
+		},
+		{
+			rawOutput:            "ninja build failed: " + errorPattern1 + "some footnotes",
+			buildExitCode:        1,
+			expectedFinalOutput:  "ninja build failed: " + errorPattern1 + "some footnotes" + errorHint1,
+			testCaseErrorMessage: "error hint not added despite pattern match",
+		},
+		{
+			rawOutput:            "ninja build failed: " + errorPattern2 + errorPattern1,
+			buildExitCode:        1,
+			expectedFinalOutput:  "ninja build failed: " + errorPattern2 + errorPattern1 + errorHint2,
+			testCaseErrorMessage: "error hint should be added for first pattern match in raw output",
+		},
+	}
+	for _, testCase := range testCases {
+		actualFinalOutput := errorHintGenerator.GetOutputWithErrorHint(testCase.rawOutput, testCase.buildExitCode)
+		if actualFinalOutput != testCase.expectedFinalOutput {
+			t.Errorf(testCase.testCaseErrorMessage+"\nexpected: %s\ngot: %s", testCase.expectedFinalOutput, actualFinalOutput)
+		}
+	}
+}