diff --git a/tests/bootstrap_test.sh b/tests/bootstrap_test.sh index 8c8dc8208..125868490 100755 --- a/tests/bootstrap_test.sh +++ b/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 < /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 diff --git a/tests/lib.sh b/tests/lib.sh index 35ccea973..f1e1efa09 100644 --- a/tests/lib.sh +++ b/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" diff --git a/ui/build/config.go b/ui/build/config.go index 480672171..737062732 100644 --- a/ui/build/config.go +++ b/ui/build/config.go @@ -161,6 +161,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", diff --git a/ui/build/sandbox_linux.go b/ui/build/sandbox_linux.go index b0a674864..5b2046e54 100644 --- a/ui/build/sandbox_linux.go +++ b/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, } diff --git a/ui/status/ninja.go b/ui/status/ninja.go index 765679fb2..2445972fb 100644 --- a/ui/status/ninja.go +++ b/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 := #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 c400c97ee..f3638b323 100644 --- a/ui/status/ninja_test.go +++ b/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) + } + } +}