Merge "java_sdk_library: Allow creation of impl shared library to be disabled"
diff --git a/android/hooks.go b/android/hooks.go
index 604cb9c..0e0f5a4 100644
--- a/android/hooks.go
+++ b/android/hooks.go
@@ -33,24 +33,11 @@
 	CreateModule(ModuleFactory, ...interface{}) Module
 }
 
-// Arch hooks are run after the module has been split into architecture variants, and can be used
-// to add architecture-specific properties.
-type ArchHookContext interface {
-	BaseModuleContext
-	AppendProperties(...interface{})
-	PrependProperties(...interface{})
-}
-
 func AddLoadHook(m blueprint.Module, hook func(LoadHookContext)) {
 	h := &m.(Module).base().hooks
 	h.load = append(h.load, hook)
 }
 
-func AddArchHook(m blueprint.Module, hook func(ArchHookContext)) {
-	h := &m.(Module).base().hooks
-	h.arch = append(h.arch, hook)
-}
-
 func (x *hooks) runLoadHooks(ctx LoadHookContext, m *ModuleBase) {
 	if len(x.load) > 0 {
 		for _, x := range x.load {
@@ -62,17 +49,6 @@
 	}
 }
 
-func (x *hooks) runArchHooks(ctx ArchHookContext, m *ModuleBase) {
-	if len(x.arch) > 0 {
-		for _, x := range x.arch {
-			x(ctx)
-			if ctx.Failed() {
-				return
-			}
-		}
-	}
-}
-
 type InstallHookContext interface {
 	ModuleContext
 	Path() InstallPath
@@ -119,7 +95,6 @@
 
 type hooks struct {
 	load    []func(LoadHookContext)
-	arch    []func(ArchHookContext)
 	install []func(InstallHookContext)
 }
 
@@ -137,12 +112,3 @@
 		m.base().hooks.runLoadHooks(loadHookCtx, m.base())
 	}
 }
-
-func archHookMutator(ctx TopDownMutatorContext) {
-	if m, ok := ctx.Module().(Module); ok {
-		// Cast through *topDownMutatorContext because AppendProperties is implemented
-		// on *topDownMutatorContext but not exposed through TopDownMutatorContext
-		var archHookCtx ArchHookContext = ctx.(*topDownMutatorContext)
-		m.base().hooks.runArchHooks(archHookCtx, m.base())
-	}
-}
diff --git a/android/mutator.go b/android/mutator.go
index c2bae44..709d9c0 100644
--- a/android/mutator.go
+++ b/android/mutator.go
@@ -89,7 +89,6 @@
 	ctx.BottomUp("os", osMutator).Parallel()
 	ctx.BottomUp("image", imageMutator).Parallel()
 	ctx.BottomUp("arch", archMutator).Parallel()
-	ctx.TopDown("arch_hooks", archHookMutator).Parallel()
 }
 
 var preDeps = []RegisterMutatorFunc{
diff --git a/apex/builder.go b/apex/builder.go
index fe465f5..4a760b9 100644
--- a/apex/builder.go
+++ b/apex/builder.go
@@ -105,6 +105,7 @@
 			`${apexer} --force --manifest ${manifest} ` +
 			`--file_contexts ${file_contexts} ` +
 			`--canned_fs_config ${canned_fs_config} ` +
+			`--include_build_info ` +
 			`--payload_type image ` +
 			`--key ${key} ${opt_flags} ${image_dir} ${out} `,
 		CommandDeps: []string{"${apexer}", "${avbtool}", "${e2fsdroid}", "${merge_zips}",
@@ -378,7 +379,7 @@
 			optFlags = append(optFlags, "--assets_dir "+filepath.Dir(noticeFile.String()))
 		}
 
-		if ctx.ModuleDir() != "system/apex/apexd/apexd_testdata" && a.testOnlyShouldSkipHashtreeGeneration() {
+		if ctx.ModuleDir() != "system/apex/apexd/apexd_testdata" && ctx.ModuleDir() != "system/apex/shim/build" && a.testOnlyShouldSkipHashtreeGeneration() {
 			ctx.PropertyErrorf("test_only_no_hashtree", "not available")
 			return
 		}
diff --git a/build_kzip.bash b/build_kzip.bash
index ccd6bad..02b346d 100755
--- a/build_kzip.bash
+++ b/build_kzip.bash
@@ -19,12 +19,13 @@
 # Build extraction files for C++ and Java. Build `merge_zips` which we use later.
 build/soong/soong_ui.bash --build-mode --all-modules --dir=$PWD -k merge_zips xref_cxx xref_java
 #Build extraction file for Go files in build/soong directory.
+declare -r abspath_out=$(realpath "${out}")
 (cd build/soong;
  ../../prebuilts/build-tools/linux-x86/bin/go_extractor \
     --goroot="${PWD}/../../prebuilts/go/linux-x86" \
     --rules=vnames.go.json \
     --canonicalize_package_corpus \
-    --output "${out}/soong/all.go.kzip" \
+    --output "${abspath_out}/soong/all.go.kzip" \
     ./... )
 
 declare -r kzip_count=$(find "$out" -name '*.kzip' | wc -l)
diff --git a/ui/build/Android.bp b/ui/build/Android.bp
index f212fb6..2a5a51a 100644
--- a/ui/build/Android.bp
+++ b/ui/build/Android.bp
@@ -59,6 +59,7 @@
         "util.go",
     ],
     testSrcs: [
+        "cleanbuild_test.go",
         "config_test.go",
         "environment_test.go",
         "util_test.go",
diff --git a/ui/build/cleanbuild.go b/ui/build/cleanbuild.go
index 0b44b4d..1c4f574 100644
--- a/ui/build/cleanbuild.go
+++ b/ui/build/cleanbuild.go
@@ -15,10 +15,12 @@
 package build
 
 import (
+	"bytes"
 	"fmt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"sort"
 	"strings"
 
 	"android/soong/ui/metrics"
@@ -177,3 +179,78 @@
 
 	writeConfig()
 }
+
+// cleanOldFiles takes an input file (with all paths relative to basePath), and removes files from
+// the filesystem if they were removed from the input file since the last execution.
+func cleanOldFiles(ctx Context, basePath, file string) {
+	file = filepath.Join(basePath, file)
+	oldFile := file + ".previous"
+
+	if _, err := os.Stat(file); err != nil {
+		ctx.Fatalf("Expected %q to be readable", file)
+	}
+
+	if _, err := os.Stat(oldFile); os.IsNotExist(err) {
+		if err := os.Rename(file, oldFile); err != nil {
+			ctx.Fatalf("Failed to rename file list (%q->%q): %v", file, oldFile, err)
+		}
+		return
+	}
+
+	var newPaths, oldPaths []string
+	if newData, err := ioutil.ReadFile(file); err == nil {
+		if oldData, err := ioutil.ReadFile(oldFile); err == nil {
+			// Common case: nothing has changed
+			if bytes.Equal(newData, oldData) {
+				return
+			}
+			newPaths = strings.Fields(string(newData))
+			oldPaths = strings.Fields(string(oldData))
+		} else {
+			ctx.Fatalf("Failed to read list of installable files (%q): %v", oldFile, err)
+		}
+	} else {
+		ctx.Fatalf("Failed to read list of installable files (%q): %v", file, err)
+	}
+
+	// These should be mostly sorted by make already, but better make sure Go concurs
+	sort.Strings(newPaths)
+	sort.Strings(oldPaths)
+
+	for len(oldPaths) > 0 {
+		if len(newPaths) > 0 {
+			if oldPaths[0] == newPaths[0] {
+				// Same file; continue
+				newPaths = newPaths[1:]
+				oldPaths = oldPaths[1:]
+				continue
+			} else if oldPaths[0] > newPaths[0] {
+				// New file; ignore
+				newPaths = newPaths[1:]
+				continue
+			}
+		}
+		// File only exists in the old list; remove if it exists
+		old := filepath.Join(basePath, oldPaths[0])
+		oldPaths = oldPaths[1:]
+		if fi, err := os.Stat(old); err == nil {
+			if fi.IsDir() {
+				if err := os.Remove(old); err == nil {
+					ctx.Println("Removed directory that is no longer installed: ", old)
+				} else {
+					ctx.Println("Failed to remove directory that is no longer installed (%q): %v", old, err)
+					ctx.Println("It's recommended to run `m installclean`")
+				}
+			} else {
+				if err := os.Remove(old); err == nil {
+					ctx.Println("Removed file that is no longer installed: ", old)
+				} else if !os.IsNotExist(err) {
+					ctx.Fatalf("Failed to remove file that is no longer installed (%q): %v", old, err)
+				}
+			}
+		}
+	}
+
+	// Use the new list as the base for the next build
+	os.Rename(file, oldFile)
+}
diff --git a/ui/build/cleanbuild_test.go b/ui/build/cleanbuild_test.go
new file mode 100644
index 0000000..89f4ad9
--- /dev/null
+++ b/ui/build/cleanbuild_test.go
@@ -0,0 +1,100 @@
+// Copyright 2020 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 (
+	"android/soong/ui/logger"
+	"bytes"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"reflect"
+	"sort"
+	"strings"
+	"testing"
+)
+
+func TestCleanOldFiles(t *testing.T) {
+	dir, err := ioutil.TempDir("", "testcleanoldfiles")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(dir)
+
+	ctx := testContext()
+	logBuf := &bytes.Buffer{}
+	ctx.Logger = logger.New(logBuf)
+
+	touch := func(names ...string) {
+		for _, name := range names {
+			if f, err := os.Create(filepath.Join(dir, name)); err != nil {
+				t.Fatal(err)
+			} else {
+				f.Close()
+			}
+		}
+	}
+	runCleanOldFiles := func(names ...string) {
+		data := []byte(strings.Join(names, " "))
+		if err := ioutil.WriteFile(filepath.Join(dir, ".installed"), data, 0666); err != nil {
+			t.Fatal(err)
+		}
+
+		cleanOldFiles(ctx, dir, ".installed")
+	}
+
+	assertFileList := func(names ...string) {
+		t.Helper()
+
+		sort.Strings(names)
+
+		var foundNames []string
+		if foundFiles, err := ioutil.ReadDir(dir); err == nil {
+			for _, fi := range foundFiles {
+				foundNames = append(foundNames, fi.Name())
+			}
+		} else {
+			t.Fatal(err)
+		}
+
+		if !reflect.DeepEqual(names, foundNames) {
+			t.Errorf("Expected a different list of files:\nwant: %v\n got: %v", names, foundNames)
+			t.Error("Log: ", logBuf.String())
+			logBuf.Reset()
+		}
+	}
+
+	// Initial list of potential files
+	runCleanOldFiles("foo", "bar")
+	touch("foo", "bar", "baz")
+	assertFileList("foo", "bar", "baz", ".installed.previous")
+
+	// This should be a no-op, as the list hasn't changed
+	runCleanOldFiles("foo", "bar")
+	assertFileList("foo", "bar", "baz", ".installed", ".installed.previous")
+
+	// This should be a no-op, as only a file was added
+	runCleanOldFiles("foo", "bar", "foo2")
+	assertFileList("foo", "bar", "baz", ".installed.previous")
+
+	// "bar" should be removed, foo2 should be ignored as it was never there
+	runCleanOldFiles("foo")
+	assertFileList("foo", "baz", ".installed.previous")
+
+	// Recreate bar, and create foo2. Ensure that they aren't removed
+	touch("bar", "foo2")
+	runCleanOldFiles("foo", "baz")
+	assertFileList("foo", "bar", "baz", "foo2", ".installed.previous")
+}
diff --git a/ui/build/config.go b/ui/build/config.go
index fae569f..c084171 100644
--- a/ui/build/config.go
+++ b/ui/build/config.go
@@ -55,8 +55,9 @@
 
 	pdkBuild bool
 
-	brokenDupRules    bool
-	brokenUsesNetwork bool
+	brokenDupRules     bool
+	brokenUsesNetwork  bool
+	brokenNinjaEnvVars []string
 
 	pathReplaced bool
 }
@@ -907,6 +908,14 @@
 	return c.brokenUsesNetwork
 }
 
+func (c *configImpl) SetBuildBrokenNinjaUsesEnvVars(val []string) {
+	c.brokenNinjaEnvVars = val
+}
+
+func (c *configImpl) BuildBrokenNinjaUsesEnvVars() []string {
+	return c.brokenNinjaEnvVars
+}
+
 func (c *configImpl) SetTargetDeviceDir(dir string) {
 	c.targetDeviceDir = dir
 }
diff --git a/ui/build/dumpvars.go b/ui/build/dumpvars.go
index 4270bb1..c3da38b 100644
--- a/ui/build/dumpvars.go
+++ b/ui/build/dumpvars.go
@@ -216,6 +216,9 @@
 		// Whether to enable the network during the build
 		"BUILD_BROKEN_USES_NETWORK",
 
+		// Extra environment variables to be exported to ninja
+		"BUILD_BROKEN_NINJA_USES_ENV_VARS",
+
 		// Not used, but useful to be in the soong.log
 		"BOARD_VNDK_VERSION",
 
@@ -284,4 +287,5 @@
 	config.SetPdkBuild(make_vars["TARGET_BUILD_PDK"] == "true")
 	config.SetBuildBrokenDupRules(make_vars["BUILD_BROKEN_DUP_RULES"] == "true")
 	config.SetBuildBrokenUsesNetwork(make_vars["BUILD_BROKEN_USES_NETWORK"] == "true")
+	config.SetBuildBrokenNinjaUsesEnvVars(strings.Fields(make_vars["BUILD_BROKEN_NINJA_USES_ENV_VARS"]))
 }
diff --git a/ui/build/kati.go b/ui/build/kati.go
index ac09ce1..a845c5b 100644
--- a/ui/build/kati.go
+++ b/ui/build/kati.go
@@ -153,6 +153,7 @@
 	runKati(ctx, config, katiBuildSuffix, args, func(env *Environment) {})
 
 	cleanCopyHeaders(ctx, config)
+	cleanOldInstalledFiles(ctx, config)
 }
 
 func cleanCopyHeaders(ctx Context, config Config) {
@@ -192,6 +193,23 @@
 		})
 }
 
+func cleanOldInstalledFiles(ctx Context, config Config) {
+	ctx.BeginTrace("clean", "clean old installed files")
+	defer ctx.EndTrace()
+
+	// We shouldn't be removing files from one side of the two-step asan builds
+	var suffix string
+	if v, ok := config.Environment().Get("SANITIZE_TARGET"); ok {
+		if sanitize := strings.Fields(v); inList("address", sanitize) {
+			suffix = "_asan"
+		}
+	}
+
+	cleanOldFiles(ctx, config.ProductOut(), ".installable_files"+suffix)
+
+	cleanOldFiles(ctx, config.HostOut(), ".installable_test_files")
+}
+
 func runKatiPackage(ctx Context, config Config) {
 	ctx.BeginTrace(metrics.RunKati, "kati package")
 	defer ctx.EndTrace()
diff --git a/ui/build/ninja.go b/ui/build/ninja.go
index d5baafe..6775ccf 100644
--- a/ui/build/ninja.go
+++ b/ui/build/ninja.go
@@ -18,6 +18,7 @@
 	"fmt"
 	"os"
 	"path/filepath"
+	"sort"
 	"strconv"
 	"strings"
 	"time"
@@ -65,8 +66,6 @@
 		cmd.Environment.AppendFromKati(config.KatiEnvFile())
 	}
 
-	cmd.Environment.Set("DIST_DIR", config.DistDir())
-
 	// Allow both NINJA_ARGS and NINJA_EXTRA_ARGS, since both have been
 	// used in the past to specify extra ninja arguments.
 	if extra, ok := cmd.Environment.Get("NINJA_ARGS"); ok {
@@ -85,6 +84,72 @@
 			ninjaHeartbeatDuration = overrideDuration
 		}
 	}
+
+	// Filter the environment, as ninja does not rebuild files when environment variables change.
+	//
+	// Anything listed here must not change the output of rules/actions when the value changes,
+	// otherwise incremental builds may be unsafe. Vars explicitly set to stable values
+	// elsewhere in soong_ui are fine.
+	//
+	// For the majority of cases, either Soong or the makefiles should be replicating any
+	// necessary environment variables in the command line of each action that needs it.
+	if cmd.Environment.IsEnvTrue("ALLOW_NINJA_ENV") {
+		ctx.Println("Allowing all environment variables during ninja; incremental builds may be unsafe.")
+	} else {
+		cmd.Environment.Allow(append([]string{
+			"ASAN_SYMBOLIZER_PATH",
+			"HOME",
+			"JAVA_HOME",
+			"LANG",
+			"LC_MESSAGES",
+			"OUT_DIR",
+			"PATH",
+			"PWD",
+			"PYTHONDONTWRITEBYTECODE",
+			"TMPDIR",
+			"USER",
+
+			// TODO: remove these carefully
+			"ASAN_OPTIONS",
+			"TARGET_BUILD_APPS",
+			"TARGET_BUILD_VARIANT",
+			"TARGET_PRODUCT",
+
+			// Goma -- gomacc may not need all of these
+			"GOMA_DIR",
+			"GOMA_DISABLED",
+			"GOMA_FAIL_FAST",
+			"GOMA_FALLBACK",
+			"GOMA_GCE_SERVICE_ACCOUNT",
+			"GOMA_TMP_DIR",
+			"GOMA_USE_LOCAL",
+
+			// RBE client
+			"FLAG_exec_root",
+			"FLAG_exec_strategy",
+			"FLAG_invocation_id",
+			"FLAG_log_dir",
+			"FLAG_platform",
+			"FLAG_server_address",
+
+			// ccache settings
+			"CCACHE_COMPILERCHECK",
+			"CCACHE_SLOPPINESS",
+			"CCACHE_BASEDIR",
+			"CCACHE_CPP2",
+		}, config.BuildBrokenNinjaUsesEnvVars()...)...)
+	}
+
+	cmd.Environment.Set("DIST_DIR", config.DistDir())
+	cmd.Environment.Set("SHELL", "/bin/bash")
+
+	ctx.Verboseln("Ninja environment: ")
+	envVars := cmd.Environment.Environ()
+	sort.Strings(envVars)
+	for _, envVar := range envVars {
+		ctx.Verbosef("  %s", envVar)
+	}
+
 	// Poll the ninja log for updates; if it isn't updated enough, then we want to show some diagnostics
 	done := make(chan struct{})
 	defer close(done)
diff --git a/ui/status/critical_path.go b/ui/status/critical_path.go
index 444327b..8065c60 100644
--- a/ui/status/critical_path.go
+++ b/ui/status/critical_path.go
@@ -112,8 +112,10 @@
 		if !cp.start.IsZero() {
 			elapsedTime := cp.end.Sub(cp.start).Round(time.Second)
 			cp.log.Verbosef("elapsed time %s", elapsedTime.String())
-			cp.log.Verbosef("perfect parallelism ratio %d%%",
-				int(float64(criticalTime)/float64(elapsedTime)*100))
+			if elapsedTime > 0 {
+				cp.log.Verbosef("perfect parallelism ratio %d%%",
+					int(float64(criticalTime)/float64(elapsedTime)*100))
+			}
 		}
 		cp.log.Verbose("critical path:")
 		for i := len(criticalPath) - 1; i >= 0; i-- {