Browse Source

Add a post-build step for dist builds that records what changed in the build.

This will be used to skip tests when related files (or nothing) has changed.

Also minor refactoring of evaluateWhatToRun to put the logic all in one place.

Change-Id: Ia56b251716f6b6640bc5526a597cf7c066231e99
Test: go test, m (with soong tests), manually verify
Joe Onorato 1 year ago
parent
commit
7f29a66586
4 changed files with 473 additions and 18 deletions
  1. 3 1
      ui/build/Android.bp
  2. 36 17
      ui/build/build.go
  3. 246 0
      ui/build/staging_snapshot.go
  4. 188 0
      ui/build/staging_snapshot_test.go

+ 3 - 1
ui/build/Android.bp

@@ -50,6 +50,7 @@ bootstrap_go_package {
         "cleanbuild.go",
         "config.go",
         "context.go",
+        "staging_snapshot.go",
         "dumpvars.go",
         "environment.go",
         "exec.go",
@@ -70,10 +71,11 @@ bootstrap_go_package {
         "cleanbuild_test.go",
         "config_test.go",
         "environment_test.go",
+        "proc_sync_test.go",
         "rbe_test.go",
+        "staging_snapshot_test.go",
         "upload_test.go",
         "util_test.go",
-        "proc_sync_test.go",
     ],
     darwin: {
         srcs: [

+ 36 - 17
ui/build/build.go

@@ -102,9 +102,9 @@ const (
 	// Whether to include the kati-generated ninja file in the combined ninja.
 	RunKatiNinja = 1 << iota
 	// Whether to run ninja on the combined ninja.
-	RunNinja      = 1 << iota
-	RunBuildTests = 1 << iota
-	RunAll        = RunProductConfig | RunSoong | RunKati | RunKatiNinja | RunNinja
+	RunNinja       = 1 << iota
+	RunDistActions = 1 << iota
+	RunBuildTests  = 1 << iota
 )
 
 // checkBazelMode fails the build if there are conflicting arguments for which bazel
@@ -322,34 +322,42 @@ func Build(ctx Context, config Config) {
 
 		runNinjaForBuild(ctx, config)
 	}
+
+	if what&RunDistActions != 0 {
+		runDistActions(ctx, config)
+	}
 }
 
 func evaluateWhatToRun(config Config, verboseln func(v ...interface{})) int {
 	//evaluate what to run
-	what := RunAll
+	what := 0
 	if config.Checkbuild() {
 		what |= RunBuildTests
 	}
-	if config.SkipConfig() {
+	if !config.SkipConfig() {
+		what |= RunProductConfig
+	} else {
 		verboseln("Skipping Config as requested")
-		what = what &^ RunProductConfig
 	}
-	if config.SkipKati() {
+	if !config.SkipSoong() {
+		what |= RunSoong
+	} else {
+		verboseln("Skipping use of Soong as requested")
+	}
+	if !config.SkipKati() {
+		what |= RunKati
+	} else {
 		verboseln("Skipping Kati as requested")
-		what = what &^ RunKati
 	}
-	if config.SkipKatiNinja() {
+	if !config.SkipKatiNinja() {
+		what |= RunKatiNinja
+	} else {
 		verboseln("Skipping use of Kati ninja as requested")
-		what = what &^ RunKatiNinja
 	}
-	if config.SkipSoong() {
-		verboseln("Skipping use of Soong as requested")
-		what = what &^ RunSoong
-	}
-
-	if config.SkipNinja() {
+	if !config.SkipNinja() {
+		what |= RunNinja
+	} else {
 		verboseln("Skipping Ninja as requested")
-		what = what &^ RunNinja
 	}
 
 	if !config.SoongBuildInvocationNeeded() {
@@ -361,6 +369,11 @@ func evaluateWhatToRun(config Config, verboseln func(v ...interface{})) int {
 		what = what &^ RunNinja
 		what = what &^ RunKati
 	}
+
+	if config.Dist() {
+		what |= RunDistActions
+	}
+
 	return what
 }
 
@@ -419,3 +432,9 @@ func distFile(ctx Context, config Config, src string, subDirs ...string) {
 		}
 	}()
 }
+
+// Actions to run on every build where 'dist' is in the actions.
+// Be careful, anything added here slows down EVERY CI build
+func runDistActions(ctx Context, config Config) {
+	runStagingSnapshot(ctx, config)
+}

+ 246 - 0
ui/build/staging_snapshot.go

@@ -0,0 +1,246 @@
+// Copyright 2023 Google Inc. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package build
+
+import (
+	"crypto/sha1"
+	"encoding/hex"
+	"encoding/json"
+	"io"
+	"io/fs"
+	"os"
+	"path/filepath"
+	"sort"
+	"strings"
+
+	"android/soong/shared"
+	"android/soong/ui/metrics"
+)
+
+// Metadata about a staged file
+type fileEntry struct {
+	Name string      `json:"name"`
+	Mode fs.FileMode `json:"mode"`
+	Size int64       `json:"size"`
+	Sha1 string      `json:"sha1"`
+}
+
+func fileEntryEqual(a fileEntry, b fileEntry) bool {
+	return a.Name == b.Name && a.Mode == b.Mode && a.Size == b.Size && a.Sha1 == b.Sha1
+}
+
+func sha1_hash(filename string) (string, error) {
+	f, err := os.Open(filename)
+	if err != nil {
+		return "", err
+	}
+	defer f.Close()
+
+	h := sha1.New()
+	if _, err := io.Copy(h, f); err != nil {
+		return "", err
+	}
+
+	return hex.EncodeToString(h.Sum(nil)), nil
+}
+
+// Subdirs of PRODUCT_OUT to scan
+var stagingSubdirs = []string{
+	"apex",
+	"cache",
+	"coverage",
+	"data",
+	"debug_ramdisk",
+	"fake_packages",
+	"installer",
+	"oem",
+	"product",
+	"ramdisk",
+	"recovery",
+	"root",
+	"sysloader",
+	"system",
+	"system_dlkm",
+	"system_ext",
+	"system_other",
+	"testcases",
+	"test_harness_ramdisk",
+	"vendor",
+	"vendor_debug_ramdisk",
+	"vendor_kernel_ramdisk",
+	"vendor_ramdisk",
+}
+
+// Return an array of stagedFileEntrys, one for each file in the staging directories inside
+// productOut
+func takeStagingSnapshot(ctx Context, productOut string, subdirs []string) ([]fileEntry, error) {
+	var outer_err error
+	if !strings.HasSuffix(productOut, "/") {
+		productOut += "/"
+	}
+	result := []fileEntry{}
+	for _, subdir := range subdirs {
+		filepath.WalkDir(productOut+subdir,
+			func(filename string, dirent fs.DirEntry, err error) error {
+				// Ignore errors. The most common one is that one of the subdirectories
+				// hasn't been built, in which case we just report it as empty.
+				if err != nil {
+					ctx.Verbosef("scanModifiedStagingOutputs error: %s", err)
+					return nil
+				}
+				if dirent.Type().IsRegular() {
+					fileInfo, _ := dirent.Info()
+					relative := strings.TrimPrefix(filename, productOut)
+					sha, err := sha1_hash(filename)
+					if err != nil {
+						outer_err = err
+					}
+					result = append(result, fileEntry{
+						Name: relative,
+						Mode: fileInfo.Mode(),
+						Size: fileInfo.Size(),
+						Sha1: sha,
+					})
+				}
+				return nil
+			})
+	}
+
+	sort.Slice(result, func(l, r int) bool { return result[l].Name < result[r].Name })
+
+	return result, outer_err
+}
+
+// Read json into an array of fileEntry. On error return empty array.
+func readJson(filename string) ([]fileEntry, error) {
+	buf, err := os.ReadFile(filename)
+	if err != nil {
+		// Not an error, just missing, which is empty.
+		return []fileEntry{}, nil
+	}
+
+	var result []fileEntry
+	err = json.Unmarshal(buf, &result)
+	if err != nil {
+		// Bad formatting. This is an error
+		return []fileEntry{}, err
+	}
+
+	return result, nil
+}
+
+// Write obj to filename.
+func writeJson(filename string, obj interface{}) error {
+	buf, err := json.MarshalIndent(obj, "", "  ")
+	if err != nil {
+		return err
+	}
+
+	return os.WriteFile(filename, buf, 0660)
+}
+
+type snapshotDiff struct {
+	Added   []string `json:"added"`
+	Changed []string `json:"changed"`
+	Removed []string `json:"removed"`
+}
+
+// Diff the two snapshots, returning a snapshotDiff.
+func diffSnapshots(previous []fileEntry, current []fileEntry) snapshotDiff {
+	result := snapshotDiff{
+		Added:   []string{},
+		Changed: []string{},
+		Removed: []string{},
+	}
+
+	found := make(map[string]bool)
+
+	prev := make(map[string]fileEntry)
+	for _, pre := range previous {
+		prev[pre.Name] = pre
+	}
+
+	for _, cur := range current {
+		pre, ok := prev[cur.Name]
+		found[cur.Name] = true
+		// Added
+		if !ok {
+			result.Added = append(result.Added, cur.Name)
+			continue
+		}
+		// Changed
+		if !fileEntryEqual(pre, cur) {
+			result.Changed = append(result.Changed, cur.Name)
+		}
+	}
+
+	// Removed
+	for _, pre := range previous {
+		if !found[pre.Name] {
+			result.Removed = append(result.Removed, pre.Name)
+		}
+	}
+
+	// Sort the results
+	sort.Strings(result.Added)
+	sort.Strings(result.Changed)
+	sort.Strings(result.Removed)
+
+	return result
+}
+
+// Write a json files to dist:
+//   - A list of which files have changed in this build.
+//
+// And record in out/soong:
+//   - A list of all files in the staging directories, including their hashes.
+func runStagingSnapshot(ctx Context, config Config) {
+	ctx.BeginTrace(metrics.RunSoong, "runStagingSnapshot")
+	defer ctx.EndTrace()
+
+	snapshotFilename := shared.JoinPath(config.SoongOutDir(), "staged_files.json")
+
+	// Read the existing snapshot file. If it doesn't exist, this is a full
+	// build, so all files will be treated as new.
+	previous, err := readJson(snapshotFilename)
+	if err != nil {
+		ctx.Fatal(err)
+		return
+	}
+
+	// Take a snapshot of the current out directory
+	current, err := takeStagingSnapshot(ctx, config.ProductOut(), stagingSubdirs)
+	if err != nil {
+		ctx.Fatal(err)
+		return
+	}
+
+	// Diff the snapshots
+	diff := diffSnapshots(previous, current)
+
+	// Write the diff (use RealDistDir, not one that might have been faked for bazel)
+	err = writeJson(shared.JoinPath(config.RealDistDir(), "modified_files.json"), diff)
+	if err != nil {
+		ctx.Fatal(err)
+		return
+	}
+
+	// Update the snapshot
+	err = writeJson(snapshotFilename, current)
+	if err != nil {
+		ctx.Fatal(err)
+		return
+	}
+}

+ 188 - 0
ui/build/staging_snapshot_test.go

@@ -0,0 +1,188 @@
+// Copyright 2023 Google Inc. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package build
+
+import (
+	"os"
+	"path/filepath"
+	"reflect"
+	"testing"
+)
+
+func assertDeepEqual(t *testing.T, expected interface{}, actual interface{}) {
+	if !reflect.DeepEqual(actual, expected) {
+		t.Fatalf("expected:\n  %#v\n actual:\n  %#v", expected, actual)
+	}
+}
+
+// Make a temp directory containing the supplied contents
+func makeTempDir(files []string, directories []string, symlinks []string) string {
+	temp, _ := os.MkdirTemp("", "soon_staging_snapshot_test_")
+
+	for _, file := range files {
+		os.MkdirAll(temp+"/"+filepath.Dir(file), 0700)
+		os.WriteFile(temp+"/"+file, []byte(file), 0600)
+	}
+
+	for _, dir := range directories {
+		os.MkdirAll(temp+"/"+dir, 0770)
+	}
+
+	for _, symlink := range symlinks {
+		os.MkdirAll(temp+"/"+filepath.Dir(symlink), 0770)
+		os.Symlink(temp, temp+"/"+symlink)
+	}
+
+	return temp
+}
+
+// If this is a clean build, we won't have any preexisting files, make sure we get back an empty
+// list and not errors.
+func TestEmptyOut(t *testing.T) {
+	ctx := testContext()
+
+	temp := makeTempDir(nil, nil, nil)
+	defer os.RemoveAll(temp)
+
+	actual, _ := takeStagingSnapshot(ctx, temp, []string{"a", "e", "g"})
+
+	expected := []fileEntry{}
+
+	assertDeepEqual(t, expected, actual)
+}
+
+// Make sure only the listed directories are picked up, and only regular files
+func TestNoExtraSubdirs(t *testing.T) {
+	ctx := testContext()
+
+	temp := makeTempDir([]string{"a/b", "a/c", "d", "e/f"}, []string{"g/h"}, []string{"e/symlink"})
+	defer os.RemoveAll(temp)
+
+	actual, _ := takeStagingSnapshot(ctx, temp, []string{"a", "e", "g"})
+
+	expected := []fileEntry{
+		{"a/b", 0600, 3, "3ec69c85a4ff96830024afeef2d4e512181c8f7b"},
+		{"a/c", 0600, 3, "592d70e4e03ee6f6780c71b0bf3b9608dbf1e201"},
+		{"e/f", 0600, 3, "9e164bef74aceede0974b857170100409efe67f1"},
+	}
+
+	assertDeepEqual(t, expected, actual)
+}
+
+// Make sure diff handles empty lists
+func TestDiffEmpty(t *testing.T) {
+	actual := diffSnapshots(nil, []fileEntry{})
+
+	expected := snapshotDiff{
+		Added:   []string{},
+		Changed: []string{},
+		Removed: []string{},
+	}
+
+	assertDeepEqual(t, expected, actual)
+}
+
+// Make sure diff handles adding
+func TestDiffAdd(t *testing.T) {
+	actual := diffSnapshots([]fileEntry{
+		{"a", 0600, 1, "1234"},
+	}, []fileEntry{
+		{"a", 0600, 1, "1234"},
+		{"b", 0700, 2, "5678"},
+	})
+
+	expected := snapshotDiff{
+		Added:   []string{"b"},
+		Changed: []string{},
+		Removed: []string{},
+	}
+
+	assertDeepEqual(t, expected, actual)
+}
+
+// Make sure diff handles changing mode
+func TestDiffChangeMode(t *testing.T) {
+	actual := diffSnapshots([]fileEntry{
+		{"a", 0600, 1, "1234"},
+		{"b", 0700, 2, "5678"},
+	}, []fileEntry{
+		{"a", 0600, 1, "1234"},
+		{"b", 0600, 2, "5678"},
+	})
+
+	expected := snapshotDiff{
+		Added:   []string{},
+		Changed: []string{"b"},
+		Removed: []string{},
+	}
+
+	assertDeepEqual(t, expected, actual)
+}
+
+// Make sure diff handles changing size
+func TestDiffChangeSize(t *testing.T) {
+	actual := diffSnapshots([]fileEntry{
+		{"a", 0600, 1, "1234"},
+		{"b", 0700, 2, "5678"},
+	}, []fileEntry{
+		{"a", 0600, 1, "1234"},
+		{"b", 0700, 3, "5678"},
+	})
+
+	expected := snapshotDiff{
+		Added:   []string{},
+		Changed: []string{"b"},
+		Removed: []string{},
+	}
+
+	assertDeepEqual(t, expected, actual)
+}
+
+// Make sure diff handles changing contents
+func TestDiffChangeContents(t *testing.T) {
+	actual := diffSnapshots([]fileEntry{
+		{"a", 0600, 1, "1234"},
+		{"b", 0700, 2, "5678"},
+	}, []fileEntry{
+		{"a", 0600, 1, "1234"},
+		{"b", 0700, 2, "aaaa"},
+	})
+
+	expected := snapshotDiff{
+		Added:   []string{},
+		Changed: []string{"b"},
+		Removed: []string{},
+	}
+
+	assertDeepEqual(t, expected, actual)
+}
+
+// Make sure diff handles removing
+func TestDiffRemove(t *testing.T) {
+	actual := diffSnapshots([]fileEntry{
+		{"a", 0600, 1, "1234"},
+		{"b", 0700, 2, "5678"},
+	}, []fileEntry{
+		{"a", 0600, 1, "1234"},
+	})
+
+	expected := snapshotDiff{
+		Added:   []string{},
+		Changed: []string{},
+		Removed: []string{"b"},
+	}
+
+	assertDeepEqual(t, expected, actual)
+}