From 74ac02796253f6d4de323766ad8e6c18c679b18a Mon Sep 17 00:00:00 2001
From: Cole Faust <colefaust@google.com>
Date: Tue, 14 Jun 2022 12:45:26 -0700
Subject: [PATCH] Allow using include_top to filter results even when there is
 a constant prefix

Previously, when seeing a `include foo/$(VAR)/bar.mk`, the include
tops wouldn't apply to filter down the results any further.

Allowing include_tops is useful here to prevent cycles in the
load graph.

Fixes: 236025787
Test: go test
Change-Id: I3ea445f222ee2283a68d4aca5002df750204076f
---
 mk2rbc/mk2rbc.go      | 40 +++++++++++++++++++++++-----------------
 mk2rbc/mk2rbc_test.go |  9 +++++++++
 2 files changed, 32 insertions(+), 17 deletions(-)

diff --git a/mk2rbc/mk2rbc.go b/mk2rbc/mk2rbc.go
index e59146be1..2707f0c5e 100644
--- a/mk2rbc/mk2rbc.go
+++ b/mk2rbc/mk2rbc.go
@@ -830,21 +830,13 @@ func (ctx *parseContext) handleSubConfig(
 				pathPattern = append(pathPattern, chunk)
 			}
 		}
-		if pathPattern[0] == "" && len(ctx.includeTops) > 0 {
-			// If pattern starts from the top. restrict it to the directories where
-			// we know inherit-product uses dynamically calculated path.
-			for _, p := range ctx.includeTops {
-				pathPattern[0] = p
-				matchingPaths = append(matchingPaths, ctx.findMatchingPaths(pathPattern)...)
-			}
-		} else {
-			matchingPaths = ctx.findMatchingPaths(pathPattern)
+		if len(pathPattern) == 1 {
+			pathPattern = append(pathPattern, "")
 		}
+		matchingPaths = ctx.findMatchingPaths(pathPattern)
 		needsWarning = pathPattern[0] == "" && len(ctx.includeTops) == 0
 	} else if len(ctx.includeTops) > 0 {
-		for _, p := range ctx.includeTops {
-			matchingPaths = append(matchingPaths, ctx.findMatchingPaths([]string{p, ""})...)
-		}
+		matchingPaths = append(matchingPaths, ctx.findMatchingPaths([]string{"", ""})...)
 	} else {
 		return []starlarkNode{ctx.newBadNode(v, "inherit-product/include argument is too complex")}
 	}
@@ -872,17 +864,31 @@ func (ctx *parseContext) findMatchingPaths(pattern []string) []string {
 	}
 
 	// Create regular expression from the pattern
-	s_regexp := "^" + regexp.QuoteMeta(pattern[0])
+	regexString := "^" + regexp.QuoteMeta(pattern[0])
 	for _, s := range pattern[1:] {
-		s_regexp += ".*" + regexp.QuoteMeta(s)
+		regexString += ".*" + regexp.QuoteMeta(s)
 	}
-	s_regexp += "$"
-	rex := regexp.MustCompile(s_regexp)
+	regexString += "$"
+	rex := regexp.MustCompile(regexString)
+
+	includeTopRegexString := ""
+	if len(ctx.includeTops) > 0 {
+		for i, top := range ctx.includeTops {
+			if i > 0 {
+				includeTopRegexString += "|"
+			}
+			includeTopRegexString += "^" + regexp.QuoteMeta(top)
+		}
+	} else {
+		includeTopRegexString = ".*"
+	}
+
+	includeTopRegex := regexp.MustCompile(includeTopRegexString)
 
 	// Now match
 	var res []string
 	for _, p := range files {
-		if rex.MatchString(p) {
+		if rex.MatchString(p) && includeTopRegex.MatchString(p) {
 			res = append(res, p)
 		}
 	}
diff --git a/mk2rbc/mk2rbc_test.go b/mk2rbc/mk2rbc_test.go
index a09764c17..31555d3f5 100644
--- a/mk2rbc/mk2rbc_test.go
+++ b/mk2rbc/mk2rbc_test.go
@@ -1157,6 +1157,8 @@ $(call inherit-product,$(MY_PATH)/cfg.mk)
 #RBC# include_top vendor/foo1
 $(call inherit-product,$(MY_OTHER_PATH))
 #RBC# include_top vendor/foo1
+$(call inherit-product,vendor/$(MY_OTHER_PATH))
+#RBC# include_top vendor/foo1
 $(foreach f,$(MY_MAKEFILES), \
 	$(call inherit-product,$(f)))
 `,
@@ -1180,6 +1182,13 @@ def init(g, handle):
   if not _varmod_init:
     rblf.mkerror("product.mk", "Cannot find %s" % (g.get("MY_OTHER_PATH", "")))
   rblf.inherit(handle, _varmod, _varmod_init)
+  _entry = {
+    "vendor/foo1/cfg.mk": ("vendor/foo1/cfg", _cfg_init),
+  }.get("vendor/%s" % g.get("MY_OTHER_PATH", ""))
+  (_varmod, _varmod_init) = _entry if _entry else (None, None)
+  if not _varmod_init:
+    rblf.mkerror("product.mk", "Cannot find %s" % ("vendor/%s" % g.get("MY_OTHER_PATH", "")))
+  rblf.inherit(handle, _varmod, _varmod_init)
   for f in rblf.words(g.get("MY_MAKEFILES", "")):
     _entry = {
       "vendor/foo1/cfg.mk": ("vendor/foo1/cfg", _cfg_init),