Make PackageVarContext implement PathGlobContext

Make PackageVarContext implement PathGlobContext by implementing
GlobWithDeps.  This will allow calls to ExistentPathForSource
inside a VariableFunc to use optimized glob dependencies instead of
falling back to AddNinjaFileDeps, which is resulting in extra
dependencies from soong_build on top level directories, triggering
extra Soong regenerations.

Remove the fallback path in ExistentPathForSource by making it take
a PathGlobContext, which is now a superset of PathContext.

Rewrite TestNinjaDeps to not rely on the unoptimized glob dependencies
in VariableFuncs and instead call ctx.Config().AddNinjaFileDeps
directly.

Bug: 257079828
Test: test_create_global_include_directory
Change-Id: I48cf189157d78b9252d339dbc9baeb27e4694807

5 files changed, 59 insertions, 55 deletions

diff --git a/android/config.go b/android/config.go
index df2c7673a..50f63ea46 100644
--- a/android/config.go
+++ b/android/config.go
@@ -1109,7 +1109,7 @@ func (c *deviceConfig) WithDexpreopt() bool {
 	return c.config.productVariables.WithDexpreopt
 }
 
-func (c *config) FrameworksBaseDirExists(ctx PathContext) bool {
+func (c *config) FrameworksBaseDirExists(ctx PathGlobContext) bool {
 	return ExistentPathForSource(ctx, "frameworks", "base", "Android.bp").Valid()
 }
 
diff --git a/android/ninja_deps_test.go b/android/ninja_deps_test.go
index 947c25726..d6afcc0de 100644
--- a/android/ninja_deps_test.go
+++ b/android/ninja_deps_test.go
@@ -18,21 +18,6 @@ import (
 	"testing"
 )
 
-func init() {
-	// This variable uses ExistentPathForSource on a PackageVarContext, which is a PathContext
-	// that is not a PathGlobContext.  That requires the deps to be stored in the Config.
-	pctx.VariableFunc("test_ninja_deps_variable", func(ctx PackageVarContext) string {
-		// Using ExistentPathForSource to look for a file that does not exist in a directory that
-		// does exist (test_ninja_deps) from a PackageVarContext adds a dependency from build.ninja
-		// to the directory.
-		if ExistentPathForSource(ctx, "test_ninja_deps/does_not_exist").Valid() {
-			return "true"
-		} else {
-			return "false"
-		}
-	})
-}
-
 func testNinjaDepsSingletonFactory() Singleton {
 	return testNinjaDepsSingleton{}
 }
@@ -40,33 +25,19 @@ func testNinjaDepsSingletonFactory() Singleton {
 type testNinjaDepsSingleton struct{}
 
 func (testNinjaDepsSingleton) GenerateBuildActions(ctx SingletonContext) {
-	// Reference the test_ninja_deps_variable in a build statement so Blueprint is forced to
-	// evaluate it.
-	ctx.Build(pctx, BuildParams{
-		Rule:   Cp,
-		Input:  PathForTesting("foo"),
-		Output: PathForOutput(ctx, "test_ninja_deps_out"),
-		Args: map[string]string{
-			"cpFlags": "${test_ninja_deps_variable}",
-		},
-	})
+	ctx.Config().addNinjaFileDeps("foo")
 }
 
 func TestNinjaDeps(t *testing.T) {
-	fs := MockFS{
-		"test_ninja_deps/exists": nil,
-	}
-
 	result := GroupFixturePreparers(
 		FixtureRegisterWithContext(func(ctx RegistrationContext) {
 			ctx.RegisterSingletonType("test_ninja_deps_singleton", testNinjaDepsSingletonFactory)
 			ctx.RegisterSingletonType("ninja_deps_singleton", ninjaDepsSingletonFactory)
 		}),
-		fs.AddToFixture(),
 	).RunTest(t)
 
 	// Verify that the ninja file has a dependency on the test_ninja_deps directory.
-	if g, w := result.NinjaDeps, "test_ninja_deps"; !InList(w, g) {
+	if g, w := result.NinjaDeps, "foo"; !InList(w, g) {
 		t.Errorf("expected %q in %q", w, g)
 	}
 }
diff --git a/android/package_ctx.go b/android/package_ctx.go
index f354db83f..c348c82b6 100644
--- a/android/package_ctx.go
+++ b/android/package_ctx.go
@@ -48,7 +48,7 @@ type configErrorWrapper struct {
 
 var _ PathContext = &configErrorWrapper{}
 var _ errorfContext = &configErrorWrapper{}
-var _ PackageVarContext = &configErrorWrapper{}
+var _ PackageVarContext = &variableFuncContextWrapper{}
 var _ PackagePoolContext = &configErrorWrapper{}
 var _ PackageRuleContext = &configErrorWrapper{}
 
@@ -62,21 +62,33 @@ func (e *configErrorWrapper) AddNinjaFileDeps(deps ...string) {
 	e.config.addNinjaFileDeps(deps...)
 }
 
-type PackageVarContext interface {
+type variableFuncContextWrapper struct {
+	configErrorWrapper
+	blueprint.VariableFuncContext
+}
+
+type PackagePoolContext interface {
 	PathContext
 	errorfContext
 }
 
-type PackagePoolContext PackageVarContext
-type PackageRuleContext PackageVarContext
+type PackageRuleContext PackagePoolContext
+
+type PackageVarContext interface {
+	PackagePoolContext
+	PathGlobContext
+}
 
 // VariableFunc wraps blueprint.PackageContext.VariableFunc, converting the interface{} config
 // argument to a PackageVarContext.
 func (p PackageContext) VariableFunc(name string,
 	f func(PackageVarContext) string) blueprint.Variable {
 
-	return p.PackageContext.VariableFunc(name, func(config interface{}) (string, error) {
-		ctx := &configErrorWrapper{p, config.(Config), nil}
+	return p.PackageContext.VariableFunc(name, func(bpctx blueprint.VariableFuncContext, config interface{}) (string, error) {
+		ctx := &variableFuncContextWrapper{
+			configErrorWrapper:  configErrorWrapper{p, config.(Config), nil},
+			VariableFuncContext: bpctx,
+		}
 		ret := f(ctx)
 		if len(ctx.errors) > 0 {
 			return "", ctx.errors[0]
diff --git a/android/paths.go b/android/paths.go
index 375297f6a..a6a54fab4 100644
--- a/android/paths.go
+++ b/android/paths.go
@@ -39,6 +39,7 @@ type PathContext interface {
 }
 
 type PathGlobContext interface {
+	PathContext
 	GlobWithDeps(globPattern string, excludes []string) ([]string, error)
 }
 
@@ -56,7 +57,6 @@ func (ctx NullPathContext) Config() Config         { return ctx.config }
 // EarlyModulePathContext is a subset of EarlyModuleContext methods required by the
 // Path methods. These path methods can be called before any mutators have run.
 type EarlyModulePathContext interface {
-	PathContext
 	PathGlobContext
 
 	ModuleDir() string
@@ -375,7 +375,7 @@ func PathsForSource(ctx PathContext, paths []string) Paths {
 // ExistentPathsForSources returns a list of Paths rooted from SrcDir, *not* rooted from the
 // module's local source directory, that are found in the tree. If any are not found, they are
 // omitted from the list, and dependencies are added so that we're re-run when they are added.
-func ExistentPathsForSources(ctx PathContext, paths []string) Paths {
+func ExistentPathsForSources(ctx PathGlobContext, paths []string) Paths {
 	ret := make(Paths, 0, len(paths))
 	for _, path := range paths {
 		p := ExistentPathForSource(ctx, path)
@@ -1087,21 +1087,12 @@ func pathForSource(ctx PathContext, pathComponents ...string) (SourcePath, error
 
 // existsWithDependencies returns true if the path exists, and adds appropriate dependencies to rerun if the
 // path does not exist.
-func existsWithDependencies(ctx PathContext, path SourcePath) (exists bool, err error) {
+func existsWithDependencies(ctx PathGlobContext, path SourcePath) (exists bool, err error) {
 	var files []string
 
-	if gctx, ok := ctx.(PathGlobContext); ok {
-		// Use glob to produce proper dependencies, even though we only want
-		// a single file.
-		files, err = gctx.GlobWithDeps(path.String(), nil)
-	} else {
-		var result pathtools.GlobResult
-		// We cannot add build statements in this context, so we fall back to
-		// AddNinjaFileDeps
-		result, err = ctx.Config().fs.Glob(path.String(), nil, pathtools.FollowSymlinks)
-		ctx.AddNinjaFileDeps(result.Deps...)
-		files = result.Matches
-	}
+	// Use glob to produce proper dependencies, even though we only want
+	// a single file.
+	files, err = ctx.GlobWithDeps(path.String(), nil)
 
 	if err != nil {
 		return false, fmt.Errorf("glob: %s", err.Error())
@@ -1124,7 +1115,7 @@ func PathForSource(ctx PathContext, pathComponents ...string) SourcePath {
 	}
 
 	if modCtx, ok := ctx.(ModuleMissingDepsPathContext); ok && ctx.Config().AllowMissingDependencies() {
-		exists, err := existsWithDependencies(ctx, path)
+		exists, err := existsWithDependencies(modCtx, path)
 		if err != nil {
 			reportPathError(ctx, err)
 		}
@@ -1158,7 +1149,7 @@ func MaybeExistentPathForSource(ctx PathContext, pathComponents ...string) Sourc
 // rooted from the module's local source directory, if the path exists, or an empty OptionalPath if
 // it doesn't exist. Dependencies are added so that the ninja file will be regenerated if the state
 // of the path changes.
-func ExistentPathForSource(ctx PathContext, pathComponents ...string) OptionalPath {
+func ExistentPathForSource(ctx PathGlobContext, pathComponents ...string) OptionalPath {
 	path, err := pathForSource(ctx, pathComponents...)
 	if err != nil {
 		reportPathError(ctx, err)
diff --git a/tests/bootstrap_test.sh b/tests/bootstrap_test.sh
index ba5ba8bf9..d89e6b702 100755
--- a/tests/bootstrap_test.sh
+++ b/tests/bootstrap_test.sh
@@ -285,6 +285,35 @@ EOF
 
 }
 
+function test_create_global_include_directory() {
+  setup
+  run_soong
+  local mtime1=$(stat -c "%y" out/soong/build.ninja)
+
+  # Soong needs to know if top level directories like hardware/ exist for use
+  # as global include directories.  Make sure that doesn't cause regens for
+  # unrelated changes to the top level directory.
+  mkdir -p system/core
+
+  run_soong
+  local mtime2=$(stat -c "%y" out/soong/build.ninja)
+  if [[ "$mtime1" != "$mtime2" ]]; then
+    fail "Output Ninja file changed when top level directory changed"
+  fi
+
+  # Make sure it does regen if a missing directory in the path of a global
+  # include directory is added.
+  mkdir -p system/core/include
+
+  run_soong
+  local mtime3=$(stat -c "%y" out/soong/build.ninja)
+  if [[ "$mtime2" = "$mtime3" ]]; then
+    fail "Output Ninja file did not change when global include directory created"
+  fi
+
+}
+
+
 function test_add_file_to_soong_build() {
   setup
   run_soong
@@ -874,6 +903,7 @@ test_delete_android_bp
 test_add_file_to_soong_build
 test_glob_during_bootstrapping
 test_soong_build_rerun_iff_environment_changes
+test_create_global_include_directory
 test_multiple_soong_build_modes
 test_dump_json_module_graph
 test_json_module_graph_back_and_forth_null_build