tequilaOS/platform_build_soong
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Merge "Enable restat for sbox rules" am: 209844ce1c

Browse source 
Original change: https://android-review.googlesource.com/c/platform/build/soong/+/2046151

Change-Id: If9c767c20491d72b95034082afc2e0033afd6149
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
This commit is contained in:
Colin Cross 2022-04-05 23:12:59 +00:00 committed by Automerger Merge Worker
commit 55d2f88cc8
4 changed files with 239 additions and 73 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

21
android/rule_builder.go
View file

@ -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.

16
android/rule_builder_test.go
View file

@ -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),

257
cmd/sbox/sbox.go
View file

@ -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))
}
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 {
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 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 {

10
java/droidstubs.go
View file

@ -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)