Merge changes Iefe133ce,I893f3dd0,I604a11c9

* changes:
  Fix metalava api baseline update command
  sbox: print failing command line before output
  sbox: best-effort copy output files on failure

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
 		}

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 {