diff --git a/cmd/sbox/sbox.go b/cmd/sbox/sbox.go index 7bd086882..f124e40b0 100644 --- a/cmd/sbox/sbox.go +++ b/cmd/sbox/sbox.go @@ -230,7 +230,7 @@ func runCommand(command *sbox_proto.Command, tempDir string) (depFile string, er } // Copy in any files specified by the manifest. - err = copyFiles(command.CopyBefore, "", tempDir) + err = copyFiles(command.CopyBefore, "", tempDir, false) if err != nil { return "", err } @@ -255,12 +255,11 @@ func runCommand(command *sbox_proto.Command, tempDir string) (depFile string, er return "", err } - commandDescription := rawCommand - cmd := exec.Command("bash", "-c", rawCommand) + buf := &bytes.Buffer{} cmd.Stdin = os.Stdin - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr + cmd.Stdout = buf + cmd.Stderr = buf if command.GetChdir() { cmd.Dir = tempDir @@ -276,9 +275,29 @@ func runCommand(command *sbox_proto.Command, tempDir string) (depFile string, er } err = cmd.Run() + if err != nil { + // The command failed, do a best effort copy of output files out of the sandbox. This is + // especially useful for linters with baselines that print an error message on failure + // with a command to copy the output lint errors to the new baseline. Use a copy instead of + // a move to leave the sandbox intact for manual inspection + copyFiles(command.CopyAfter, tempDir, "", true) + } + + // If the command was executed but failed with an error, print a debugging message before + // the command's output so it doesn't scroll the real error message off the screen. if exit, ok := err.(*exec.ExitError); ok && !exit.Success() { - return "", fmt.Errorf("sbox command failed with err:\n%s\n%w\n", commandDescription, err) - } else if err != nil { + fmt.Fprintf(os.Stderr, + "The failing command was run inside an sbox sandbox in temporary directory\n"+ + "%s\n"+ + "The failing command line was:\n"+ + "%s\n", + tempDir, rawCommand) + } + + // Write the command's combined stdout/stderr. + os.Stdout.Write(buf.Bytes()) + + if err != nil { return "", err } @@ -290,7 +309,7 @@ func runCommand(command *sbox_proto.Command, tempDir string) (depFile string, er // build error message errorMessage := "mismatch between declared and actual outputs\n" - errorMessage += "in sbox command(" + commandDescription + ")\n\n" + errorMessage += "in sbox command(" + rawCommand + ")\n\n" errorMessage += "in sandbox " + tempDir + ",\n" errorMessage += fmt.Sprintf("failed to create %v files:\n", len(missingOutputErrors)) for _, missingOutputError := range missingOutputErrors { @@ -351,12 +370,13 @@ func validateOutputFiles(copies []*sbox_proto.Copy, sandboxDir string) []error { return missingOutputErrors } -// copyFiles copies files in or out of the sandbox. -func copyFiles(copies []*sbox_proto.Copy, fromDir, toDir string) error { +// copyFiles copies files in or out of the sandbox. If allowFromNotExists is true then errors +// caused by a from path not existing are ignored. +func copyFiles(copies []*sbox_proto.Copy, fromDir, toDir string, allowFromNotExists bool) error { for _, copyPair := range copies { fromPath := joinPath(fromDir, copyPair.GetFrom()) toPath := joinPath(toDir, copyPair.GetTo()) - err := copyOneFile(fromPath, toPath, copyPair.GetExecutable()) + err := copyOneFile(fromPath, toPath, copyPair.GetExecutable(), allowFromNotExists) if err != nil { return fmt.Errorf("error copying %q to %q: %w", fromPath, toPath, err) } @@ -364,8 +384,9 @@ func copyFiles(copies []*sbox_proto.Copy, fromDir, toDir string) error { return nil } -// copyOneFile copies a file. -func copyOneFile(from string, to string, executable bool) error { +// copyOneFile copies a file and its permissions. If forceExecutable is true it adds u+x to the +// permissions. If allowFromNotExists is true it returns nil if the from path doesn't exist. +func copyOneFile(from string, to string, forceExecutable, allowFromNotExists bool) error { err := os.MkdirAll(filepath.Dir(to), 0777) if err != nil { return err @@ -373,11 +394,14 @@ func copyOneFile(from string, to string, executable bool) error { stat, err := os.Stat(from) if err != nil { + if os.IsNotExist(err) && allowFromNotExists { + return nil + } return err } perm := stat.Mode() - if executable { + if forceExecutable { perm = perm | 0100 // u+x } @@ -454,7 +478,7 @@ func copyOneRspFile(rspFile *sbox_proto.RspFile, toDir, toDirInSandbox string) e to := applyPathMappings(rspFile.PathMappings, from) // Copy the file into the sandbox. - err := copyOneFile(from, joinPath(toDir, to), false) + err := copyOneFile(from, joinPath(toDir, to), false, false) if err != nil { return err } diff --git a/java/droidstubs.go b/java/droidstubs.go index a9e2749a5..2676f3d6c 100644 --- a/java/droidstubs.go +++ b/java/droidstubs.go @@ -516,9 +516,6 @@ func (d *Droidstubs) GenerateAndroidBuildActions(ctx android.ModuleContext) { d.apiLintTimestamp = android.PathForModuleOut(ctx, "metalava", "api_lint.timestamp") // Note this string includes a special shell quote $' ... ', which decodes the "\n"s. - // However, because $' ... ' doesn't expand environmental variables, we can't just embed - // $PWD, so we have to terminate $'...', use "$PWD", then start $' ... ' again, - // which is why we have '"$PWD"$' in it. // // TODO: metalava also has a slightly different message hardcoded. Should we unify this // message and metalava's one? @@ -539,9 +536,9 @@ func (d *Droidstubs) GenerateAndroidBuildActions(ctx android.ModuleContext) { msg += fmt.Sprintf(``+ `2. You can update the baseline by executing the following\n`+ ` command:\n`+ - ` cp \\\n`+ - ` "'"$PWD"$'/%s" \\\n`+ - ` "'"$PWD"$'/%s"\n`+ + ` (cd $ANDROID_BUILD_TOP && cp \\\n`+ + ` "%s" \\\n`+ + ` "%s")\n`+ ` To submit the revised baseline.txt to the main Android\n`+ ` repository, you will need approval.\n`, updatedBaselineOutput, baselineFile.Path()) } else {