Merge "Enable restat for sbox rules" (209844ce) · Commits · e / os / android_build_soong

android/rule_builder.go

+7 −14

Original line number	Diff line number	Diff line
		@@ -101,12 +101,7 @@ func (r *RuleBuilder) MissingDeps(missingDeps []string) {
		}

		// Restat marks the rule as a restat rule, which will be passed to ModuleContext.Rule in BuildParams.Restat.
		//
		// Restat is not compatible with Sbox()
		func (r RuleBuilder) Restat() RuleBuilder {
		if r.sbox {
		panic("Restat() is not compatible with Sbox()")
		}
		r.restat = true
		return r
		}
		@@ -141,8 +136,6 @@ func (r RuleBuilder) Rewrapper(params remoteexec.REParams) *RuleBuilder {
		// point to a location where sbox's manifest will be written and must be outside outputDir. sbox
		// will ensure that all outputs have been written, and will discard any output files that were not
		// specified.
		//
		// Sbox is not compatible with Restat()
		func (r RuleBuilder) Sbox(outputDir WritablePath, manifestPath WritablePath) RuleBuilder {
		if r.sbox {
		panic("Sbox() may not be called more than once")
		@@ -150,9 +143,6 @@ func (r RuleBuilder) Sbox(outputDir WritablePath, manifestPath WritablePath) R
		if len(r.commands) > 0 {
		panic("Sbox() may not be called after Command()")
		}
		if r.restat {
		panic("Sbox() is not compatible with Restat()")
		}
		r.sbox = true
		r.outDir = outputDir
		r.sboxManifestPath = manifestPath
		@@ -636,11 +626,14 @@ func (r *RuleBuilder) Build(name string, desc string) {
		ctx: r.ctx,
		},
		}
		sboxCmd.Text("rm -rf").Output(r.outDir)
		sboxCmd.Text("&&")
		sboxCmd.builtToolWithoutDeps("sbox").
		Flag("--sandbox-path").Text(shared.TempDirForOutDir(PathForOutput(r.ctx).String())).
		Flag("--manifest").Input(r.sboxManifestPath)
		FlagWithArg("--sandbox-path ", shared.TempDirForOutDir(PathForOutput(r.ctx).String())).
		FlagWithArg("--output-dir ", r.outDir.String()).
		FlagWithInput("--manifest ", r.sboxManifestPath)

		if r.restat {
		sboxCmd.Flag("--write-if-changed")
		}

		// Replace the command string, and add the sbox tool and manifest textproto to the
		// dependencies of the final sbox rule.

android/rule_builder_test.go

+8 −8

Original line number	Diff line number	Diff line
		@@ -678,32 +678,32 @@ func TestRuleBuilder_Build(t *testing.T) {
		})
		t.Run("sbox", func(t *testing.T) {
		outDir := "out/soong/.intermediates/foo_sbox"
		outFile := filepath.Join(outDir, "gen/foo_sbox")
		depFile := filepath.Join(outDir, "gen/foo_sbox.d")
		sboxOutDir := filepath.Join(outDir, "gen")
		outFile := filepath.Join(sboxOutDir, "foo_sbox")
		depFile := filepath.Join(sboxOutDir, "foo_sbox.d")
		rspFile := filepath.Join(outDir, "rsp")
		rspFile2 := filepath.Join(outDir, "rsp2")
		manifest := filepath.Join(outDir, "sbox.textproto")
		sbox := filepath.Join("out", "soong", "host", result.Config.PrebuiltOS(), "bin/sbox")
		sandboxPath := shared.TempDirForOutDir("out/soong")

		cmd := `rm -rf ` + outDir + `/gen && ` +
		sbox + ` --sandbox-path ` + sandboxPath + ` --manifest ` + manifest
		cmd := sbox + ` --sandbox-path ` + sandboxPath + ` --output-dir ` + sboxOutDir + ` --manifest ` + manifest
		module := result.ModuleForTests("foo_sbox", "")
		check(t, module.Output("gen/foo_sbox"), module.Output(rspFile2),
		cmd, outFile, depFile, rspFile, rspFile2, false, []string{manifest}, []string{sbox})
		})
		t.Run("sbox_inputs", func(t *testing.T) {
		outDir := "out/soong/.intermediates/foo_sbox_inputs"
		outFile := filepath.Join(outDir, "gen/foo_sbox_inputs")
		depFile := filepath.Join(outDir, "gen/foo_sbox_inputs.d")
		sboxOutDir := filepath.Join(outDir, "gen")
		outFile := filepath.Join(sboxOutDir, "foo_sbox_inputs")
		depFile := filepath.Join(sboxOutDir, "foo_sbox_inputs.d")
		rspFile := filepath.Join(outDir, "rsp")
		rspFile2 := filepath.Join(outDir, "rsp2")
		manifest := filepath.Join(outDir, "sbox.textproto")
		sbox := filepath.Join("out", "soong", "host", result.Config.PrebuiltOS(), "bin/sbox")
		sandboxPath := shared.TempDirForOutDir("out/soong")

		cmd := `rm -rf ` + outDir + `/gen && ` +
		sbox + ` --sandbox-path ` + sandboxPath + ` --manifest ` + manifest
		cmd := sbox + ` --sandbox-path ` + sandboxPath + ` --output-dir ` + sboxOutDir + ` --manifest ` + manifest

		module := result.ModuleForTests("foo_sbox_inputs", "")
		check(t, module.Output("gen/foo_sbox_inputs"), module.Output(rspFile2),

cmd/sbox/sbox.go

+216 −49

Original line number	Diff line number	Diff line
		@@ -39,8 +39,10 @@ import (

		var (
		sandboxesRoot string
		outputDir string
		manifestFile string
		keepOutDir bool
		writeIfChanged bool
		)

		const (
		@@ -51,10 +53,14 @@ const (
		func init() {
		flag.StringVar(&sandboxesRoot, "sandbox-path", "",
		"root of temp directory to put the sandbox into")
		flag.StringVar(&outputDir, "output-dir", "",
		"directory which will contain all output files and only output files")
		flag.StringVar(&manifestFile, "manifest", "",
		"textproto manifest describing the sandboxed command(s)")
		flag.BoolVar(&keepOutDir, "keep-out-dir", false,
		"whether to keep the sandbox directory when done")
		flag.BoolVar(&writeIfChanged, "write-if-changed", false,
		"only write the output files if they have changed")
		}

		func usageViolation(violation string) {
		@@ -241,6 +247,12 @@ func runCommand(command *sbox_proto.Command, tempDir string, commandIndex int) (
		return "", fmt.Errorf("command is required")
		}

		// Remove files from the output directory
		err = clearOutputDirectory(command.CopyAfter, outputDir, writeType(writeIfChanged))
		if err != nil {
		return "", err
		}

		pathToTempDirInSbox := tempDir
		if command.GetChdir() {
		pathToTempDirInSbox = "."
		@@ -252,7 +264,7 @@ func runCommand(command *sbox_proto.Command, tempDir string, commandIndex int) (
		}

		// Copy in any files specified by the manifest.
		err = copyFiles(command.CopyBefore, "", tempDir, false)
		err = copyFiles(command.CopyBefore, "", tempDir, requireFromExists, alwaysWrite)
		if err != nil {
		return "", err
		}
		@@ -306,7 +318,7 @@ func runCommand(command *sbox_proto.Command, tempDir string, commandIndex int) (
		// 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)
		copyFiles(command.CopyAfter, tempDir, "", allowFromNotExists, writeType(writeIfChanged))
		}

		// If the command was executed but failed with an error, print a debugging message before
		@@ -327,39 +339,16 @@ func runCommand(command *sbox_proto.Command, tempDir string, commandIndex int) (
		return "", err
		}

		missingOutputErrors := validateOutputFiles(command.CopyAfter, tempDir)

		if len(missingOutputErrors) > 0 {
		// find all created files for making a more informative error message
		createdFiles := findAllFilesUnder(tempDir)

		// build error message
		errorMessage := "mismatch between declared and actual outputs\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 {
		errorMessage += " " + missingOutputError.Error() + "\n"
		}
		if len(createdFiles) < 1 {
		errorMessage += "created 0 files."
		} else {
		errorMessage += fmt.Sprintf("did create %v files:\n", len(createdFiles))
		creationMessages := createdFiles
		maxNumCreationLines := 10
		if len(creationMessages) > maxNumCreationLines {
		creationMessages = creationMessages[:maxNumCreationLines]
		creationMessages = append(creationMessages, fmt.Sprintf("...%v more", len(createdFiles)-maxNumCreationLines))
		}
		for _, creationMessage := range creationMessages {
		errorMessage += " " + creationMessage + "\n"
		}
		err = validateOutputFiles(command.CopyAfter, tempDir, outputDir, rawCommand)
		if err != nil {
		return "", err
		}

		return "", errors.New(errorMessage)
		}
		// the created files match the declared files; now move them
		err = moveFiles(command.CopyAfter, tempDir, "")
		err = moveFiles(command.CopyAfter, tempDir, "", writeType(writeIfChanged))
		if err != nil {
		return "", err
		}

		return depFile, nil
		}
		@@ -380,8 +369,9 @@ func makeOutputDirs(copies []*sbox_proto.Copy, sandboxDir string) error {

		// validateOutputFiles verifies that all files that have a rule to be copied out of the sandbox
		// were created by the command.
		func validateOutputFiles(copies []*sbox_proto.Copy, sandboxDir string) []error {
		func validateOutputFiles(copies []*sbox_proto.Copy, sandboxDir, outputDir, rawCommand string) error {
		var missingOutputErrors []error
		var incorrectOutputDirectoryErrors []error
		for _, copyPair := range copies {
		fromPath := joinPath(sandboxDir, copyPair.GetFrom())
		fileInfo, err := os.Stat(fromPath)
		@@ -392,17 +382,91 @@ func validateOutputFiles(copies []*sbox_proto.Copy, sandboxDir string) []error {
		if fileInfo.IsDir() {
		missingOutputErrors = append(missingOutputErrors, fmt.Errorf("%s: not a file", fromPath))
		}

		toPath := copyPair.GetTo()
		if rel, err := filepath.Rel(outputDir, toPath); err != nil {
		return err
		} else if strings.HasPrefix(rel, "../") {
		incorrectOutputDirectoryErrors = append(incorrectOutputDirectoryErrors,
		fmt.Errorf("%s is not under %s", toPath, outputDir))
		}
		}

		const maxErrors = 10

		if len(incorrectOutputDirectoryErrors) > 0 {
		errorMessage := ""
		more := 0
		if len(incorrectOutputDirectoryErrors) > maxErrors {
		more = len(incorrectOutputDirectoryErrors) - maxErrors
		incorrectOutputDirectoryErrors = incorrectOutputDirectoryErrors[:maxErrors]
		}

		for _, err := range incorrectOutputDirectoryErrors {
		errorMessage += err.Error() + "\n"
		}
		if more > 0 {
		errorMessage += fmt.Sprintf("...%v more", more)
		}

		return errors.New(errorMessage)
		}

		if len(missingOutputErrors) > 0 {
		// find all created files for making a more informative error message
		createdFiles := findAllFilesUnder(sandboxDir)

		// build error message
		errorMessage := "mismatch between declared and actual outputs\n"
		errorMessage += "in sbox command(" + rawCommand + ")\n\n"
		errorMessage += "in sandbox " + sandboxDir + ",\n"
		errorMessage += fmt.Sprintf("failed to create %v files:\n", len(missingOutputErrors))
		for _, missingOutputError := range missingOutputErrors {
		errorMessage += " " + missingOutputError.Error() + "\n"
		}
		if len(createdFiles) < 1 {
		errorMessage += "created 0 files."
		} else {
		errorMessage += fmt.Sprintf("did create %v files:\n", len(createdFiles))
		creationMessages := createdFiles
		if len(creationMessages) > maxErrors {
		creationMessages = creationMessages[:maxErrors]
		creationMessages = append(creationMessages, fmt.Sprintf("...%v more", len(createdFiles)-maxErrors))
		}
		for _, creationMessage := range creationMessages {
		errorMessage += " " + creationMessage + "\n"
		}
		return missingOutputErrors
		}

		// 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 {
		return errors.New(errorMessage)
		}

		return nil
		}

		type existsType bool

		const (
		requireFromExists existsType = false
		allowFromNotExists = true
		)

		type writeType bool

		const (
		alwaysWrite writeType = false
		onlyWriteIfChanged = true
		)

		// copyFiles copies files in or out of the sandbox. If exists is allowFromNotExists then errors
		// caused by a from path not existing are ignored. If write is onlyWriteIfChanged then the output
		// file is compared to the input file and not written to if it is the same, avoiding updating
		// the timestamp.
		func copyFiles(copies []*sbox_proto.Copy, fromDir, toDir string, exists existsType, write writeType) error {
		for _, copyPair := range copies {
		fromPath := joinPath(fromDir, copyPair.GetFrom())
		toPath := joinPath(toDir, copyPair.GetTo())
		err := copyOneFile(fromPath, toPath, copyPair.GetExecutable(), allowFromNotExists)
		err := copyOneFile(fromPath, toPath, copyPair.GetExecutable(), exists, write)
		if err != nil {
		return fmt.Errorf("error copying %q to %q: %w", fromPath, toPath, err)
		}
		@@ -411,8 +475,11 @@ func copyFiles(copies []*sbox_proto.Copy, fromDir, toDir string, allowFromNotExi
		}

		// 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 {
		// permissions. If exists is allowFromNotExists it returns nil if the from path doesn't exist.
		// If write is onlyWriteIfChanged then the output file is compared to the input file and not written to
		// if it is the same, avoiding updating the timestamp.
		func copyOneFile(from string, to string, forceExecutable bool, exists existsType,
		write writeType) error {
		err := os.MkdirAll(filepath.Dir(to), 0777)
		if err != nil {
		return err
		@@ -420,7 +487,7 @@ func copyOneFile(from string, to string, forceExecutable, allowFromNotExists boo

		stat, err := os.Stat(from)
		if err != nil {
		if os.IsNotExist(err) && allowFromNotExists {
		if os.IsNotExist(err) && exists == allowFromNotExists {
		return nil
		}
		return err
		@@ -431,6 +498,10 @@ func copyOneFile(from string, to string, forceExecutable, allowFromNotExists boo
		perm = perm \| 0100 // u+x
		}

		if write == onlyWriteIfChanged && filesHaveSameContents(from, to) {
		return nil
		}

		in, err := os.Open(from)
		if err != nil {
		return err
		@@ -504,7 +575,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, false)
		err := copyOneFile(from, joinPath(toDir, to), false, requireFromExists, alwaysWrite)
		if err != nil {
		return err
		}
		@@ -551,9 +622,10 @@ func applyPathMappings(pathMappings []*sbox_proto.PathMapping, path string) stri

		// moveFiles moves files specified by a set of copy rules. It uses os.Rename, so it is restricted
		// to moving files where the source and destination are in the same filesystem. This is OK for
		// sbox because the temporary directory is inside the out directory. It updates the timestamp
		// of the new file.
		func moveFiles(copies []*sbox_proto.Copy, fromDir, toDir string) error {
		// sbox because the temporary directory is inside the out directory. If write is onlyWriteIfChanged
		// then the output file is compared to the input file and not written to if it is the same, avoiding
		// updating the timestamp. Otherwise it always updates the timestamp of the new file.
		func moveFiles(copies []*sbox_proto.Copy, fromDir, toDir string, write writeType) error {
		for _, copyPair := range copies {
		fromPath := joinPath(fromDir, copyPair.GetFrom())
		toPath := joinPath(toDir, copyPair.GetTo())
		@@ -562,6 +634,10 @@ func moveFiles(copies []*sbox_proto.Copy, fromDir, toDir string) error {
		return err
		}

		if write == onlyWriteIfChanged && filesHaveSameContents(fromPath, toPath) {
		continue
		}

		err = os.Rename(fromPath, toPath)
		if err != nil {
		return err
		@@ -578,6 +654,37 @@ func moveFiles(copies []*sbox_proto.Copy, fromDir, toDir string) error {
		return nil
		}

		// clearOutputDirectory removes all files in the output directory if write is alwaysWrite, or
		// any files not listed in copies if write is onlyWriteIfChanged
		func clearOutputDirectory(copies []*sbox_proto.Copy, outputDir string, write writeType) error {
		if outputDir == "" {
		return fmt.Errorf("output directory must be set")
		}

		if write == alwaysWrite {
		// When writing all the output files remove the whole output directory
		return os.RemoveAll(outputDir)
		}

		outputFiles := make(map[string]bool, len(copies))
		for _, copyPair := range copies {
		outputFiles[copyPair.GetTo()] = true
		}

		existingFiles := findAllFilesUnder(outputDir)
		for _, existingFile := range existingFiles {
		fullExistingFile := filepath.Join(outputDir, existingFile)
		if !outputFiles[fullExistingFile] {
		err := os.Remove(fullExistingFile)
		if err != nil {
		return fmt.Errorf("failed to remove obsolete output file %s: %w", fullExistingFile, err)
		}
		}
		}

		return nil
		}

		// Rewrite one or more depfiles so that it doesn't include the (randomized) sandbox directory
		// to an output file.
		func rewriteDepFiles(ins []string, out string) error {
		@@ -621,6 +728,66 @@ func joinPath(dir, file string) string {
		return filepath.Join(dir, file)
		}

		// filesHaveSameContents compares the contents if two files, returning true if they are the same
		// and returning false if they are different or any errors occur.
		func filesHaveSameContents(a, b string) bool {
		// Compare the sizes of the two files
		statA, err := os.Stat(a)
		if err != nil {
		return false
		}
		statB, err := os.Stat(b)
		if err != nil {
		return false
		}

		if statA.Size() != statB.Size() {
		return false
		}

		// Open the two files
		fileA, err := os.Open(a)
		if err != nil {
		return false
		}
		defer fileA.Close()
		fileB, err := os.Open(a)
		if err != nil {
		return false
		}
		defer fileB.Close()

		// Compare the files 1MB at a time
		const bufSize = 1 * 1024 * 1024
		bufA := make([]byte, bufSize)
		bufB := make([]byte, bufSize)

		remain := statA.Size()
		for remain > 0 {
		toRead := int64(bufSize)
		if toRead > remain {
		toRead = remain
		}

		_, err = io.ReadFull(fileA, bufA[:toRead])
		if err != nil {
		return false
		}
		_, err = io.ReadFull(fileB, bufB[:toRead])
		if err != nil {
		return false
		}

		if bytes.Compare(bufA[:toRead], bufB[:toRead]) != 0 {
		return false
		}

		remain -= toRead
		}

		return true
		}

		func makeAbsPathEnv(pathEnv string) (string, error) {
		pathEnvElements := filepath.SplitList(pathEnv)
		for i, p := range pathEnvElements {

java/droidstubs.go

+8 −2

Original line number	Diff line number	Diff line
		@@ -433,6 +433,10 @@ func (d Droidstubs) apiLevelsAnnotationsFlags(ctx android.ModuleContext, cmd a
		}
		}

		func metalavaUseRbe(ctx android.ModuleContext) bool {
		return ctx.Config().UseRBE() && ctx.Config().IsEnvTrue("RBE_METALAVA")
		}

		func metalavaCmd(ctx android.ModuleContext, rule *android.RuleBuilder, javaVersion javaVersion, srcs android.Paths,
		srcJarList android.Path, bootclasspath, classpath classpath, homeDir android.WritablePath) *android.RuleBuilderCommand {
		rule.Command().Text("rm -rf").Flag(homeDir.String())
		@@ -441,7 +445,7 @@ func metalavaCmd(ctx android.ModuleContext, rule *android.RuleBuilder, javaVersi
		cmd := rule.Command()
		cmd.FlagWithArg("ANDROID_PREFS_ROOT=", homeDir.String())

		if ctx.Config().UseRBE() && ctx.Config().IsEnvTrue("RBE_METALAVA") {
		if metalavaUseRbe(ctx) {
		rule.Remoteable(android.RemoteRuleSupports{RBE: true})
		execStrategy := ctx.Config().GetenvWithDefault("RBE_METALAVA_EXEC_STRATEGY", remoteexec.LocalExecStrategy)
		labels := map[string]string{"type": "tool", "name": "metalava"}
		@@ -665,7 +669,9 @@ func (d *Droidstubs) GenerateAndroidBuildActions(ctx android.ModuleContext) {
		}

		// TODO(b/183630617): rewrapper doesn't support restat rules
		// rule.Restat()
		if !metalavaUseRbe(ctx) {
		rule.Restat()
		}

		zipSyncCleanupCmd(rule, srcJarDir)