Commit b73daa58 authored Nov 25, 2020 by Ulf Adams

Soong: use deterministic temp dir names in sbox

The sbox tool is used to wrap genrule actions, and run them in a
temporary directory. Afterwards, the outputs are moved into their
proper location in the output tree. However, some tools embed the file
name of the output file (as passed to the tool) into the output file.

For example, the perfetto code generator script uses the output file
name to generate a C header guard when it generates
perfetto_src_base_version_gen_h/gen/perfetto_version.gen.h.

When using remote execution / remote caching, these genrule actions are
run locally whenever the output file doesn't exist, effectively making
them unique (across time and between users).

They then cause cache misses on all actions depending on these output
files as well as on transitive actions. In the above example, this
causes libperfetto.so to differ, which then causes all actions
depending on libperfetto.so to be rerun unnecessarily.

As a fix, this commit changes the sbox tool to use the sha1 hash of the
manifest path. The tool already seems to assume that there are no
concurrent runs of the same tool with the same manifest (otherwise
it would fail flakily depending on exact timing). It seems therefore
safe to use a temporary path deterministically derived from the
manifest path.

Test: ran multiple builds w/ remote execution; observe proper caching
Change-Id: I5b73ffd3b7f85cbb0336dfa1675de7ac0e2fd1a8

parent b0ce5757

cmd/sbox/sbox.go

+18 −1

Original line number	Diff line number	Diff line
		@@ -16,6 +16,8 @@ package main

		import (
		"bytes"
		"crypto/sha1"
		"encoding/hex"
		"errors"
		"flag"
		"fmt"
		@@ -121,7 +123,22 @@ func run() error {
		return fmt.Errorf("failed to create %q: %w", sandboxesRoot, err)
		}

		tempDir, err := ioutil.TempDir(sandboxesRoot, "sbox")
		// This tool assumes that there are no two concurrent runs with the same
		// manifestFile. It should therefore be safe to use the hash of the
		// manifestFile as the temporary directory name. We do this because it
		// makes the temporary directory name deterministic. There are some
		// tools that embed the name of the temporary output in the output, and
		// they otherwise cause non-determinism, which then poisons actions
		// depending on this one.
		hash := sha1.New()
		hash.Write([]byte(manifestFile))
		tempDir := filepath.Join(sandboxesRoot, "sbox", hex.EncodeToString(hash.Sum(nil)))

		err = os.RemoveAll(tempDir)
		if err != nil {
		return err
		}
		err = os.MkdirAll(tempDir, 0777)
		if err != nil {
		return fmt.Errorf("failed to create temporary dir in %q: %w", sandboxesRoot, err)
		}