Merge "Add errorHints to stdout when read-only file system errors are detected"
diff --git a/tests/bootstrap_test.sh b/tests/bootstrap_test.sh
index 8c8dc82..1258684 100755
--- a/tests/bootstrap_test.sh
+++ b/tests/bootstrap_test.sh
@@ -486,6 +486,39 @@
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_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
diff --git a/tests/lib.sh b/tests/lib.sh
index 35ccea9..f1e1efa 100644
--- a/tests/lib.sh
+++ b/tests/lib.sh
@@ -126,6 +126,10 @@
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"
diff --git a/ui/build/config.go b/ui/build/config.go
index 4806721..7370627 100644
--- a/ui/build/config.go
+++ b/ui/build/config.go
@@ -161,6 +161,10 @@
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",
diff --git a/ui/build/sandbox_linux.go b/ui/build/sandbox_linux.go
index b0a6748..5b2046e 100644
--- a/ui/build/sandbox_linux.go
+++ b/ui/build/sandbox_linux.go
@@ -97,8 +97,11 @@
"-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,
}
diff --git a/ui/status/ninja.go b/ui/status/ninja.go
index 765679f..2445972 100644
--- a/ui/status/ninja.go
+++ b/ui/status/ninja.go
@@ -19,6 +19,8 @@
"fmt"
"io"
"os"
+ "regexp"
+ "strings"
"syscall"
"time"
@@ -158,9 +160,10 @@
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 @@
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
+}
diff --git a/ui/status/ninja_test.go b/ui/status/ninja_test.go
index c400c97..f3638b3 100644
--- a/ui/status/ninja_test.go
+++ b/ui/status/ninja_test.go
@@ -43,3 +43,53 @@
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)
+ }
+ }
+}