Merge changes from topics "is_board_platform", "soong_config_namespace"

* changes:
  Convert is-board-platform2/is-board-platform-in-list2/is-vendor-board-qcom macros.
  Fix how the rule is displayed in the error message.
  Implement include path annotation.
  Better Soong config namespace support.

mk2rbc/mk2rbc.go

@@ -40,12 +40,15 @@ import (
 )
 
 const (
-	baseUri = "//build/make/core:product_config.rbc"
+	annotationCommentPrefix = "RBC#"
+	baseUri                 = "//build/make/core:product_config.rbc"
 	// The name of the struct exported by the product_config.rbc
 	// that contains the functions and variables available to
 	// product configuration Starlark files.
 	baseName = "rblf"
 
+	soongNsPrefix = "SOONG_CONFIG_"
+
 	// And here are the functions and variables:
 	cfnGetCfg          = baseName + ".cfg"
 	cfnMain            = baseName + ".product_configuration"
@@ -60,10 +63,14 @@ const (
 const (
 	// Phony makefile functions, they are eventually rewritten
 	// according to knownFunctions map
-	addSoongNamespace      = "add_soong_config_namespace"
+	fileExistsPhony = "$file_exists"
-	addSoongConfigVarValue = "add_soong_config_var_value"
+	// The following two macros are obsolete, and will we deleted once
-	fileExistsPhony        = "$file_exists"
+	// there are deleted from the makefiles:
-	wildcardExistsPhony    = "$wildcard_exists"
+	soongConfigNamespaceOld = "add_soong_config_namespace"
+	soongConfigVarSetOld    = "add_soong_config_var_value"
+	soongConfigAppend       = "soong_config_append"
+	soongConfigAssign       = "soong_config_set"
+	wildcardExistsPhony     = "$wildcard_exists"
 )
 
 const (
@@ -82,8 +89,10 @@ var knownFunctions = map[string]struct {
 	"abspath":                             {baseName + ".abspath", starlarkTypeString, hiddenArgNone},
 	fileExistsPhony:                       {baseName + ".file_exists", starlarkTypeBool, hiddenArgNone},
 	wildcardExistsPhony:                   {baseName + ".file_wildcard_exists", starlarkTypeBool, hiddenArgNone},
-	addSoongNamespace:                     {baseName + ".add_soong_config_namespace", starlarkTypeVoid, hiddenArgGlobal},
+	soongConfigNamespaceOld:               {baseName + ".soong_config_namespace", starlarkTypeVoid, hiddenArgGlobal},
-	addSoongConfigVarValue:                {baseName + ".add_soong_config_var_value", starlarkTypeVoid, hiddenArgGlobal},
+	soongConfigVarSetOld:                  {baseName + ".soong_config_set", starlarkTypeVoid, hiddenArgGlobal},
+	soongConfigAssign:                     {baseName + ".soong_config_set", starlarkTypeVoid, hiddenArgGlobal},
+	soongConfigAppend:                     {baseName + ".soong_config_append", starlarkTypeVoid, hiddenArgGlobal},
 	"add-to-product-copy-files-if-exists": {baseName + ".copy_if_exists", starlarkTypeList, hiddenArgNone},
 	"addprefix":                           {baseName + ".addprefix", starlarkTypeList, hiddenArgNone},
 	"addsuffix":                           {baseName + ".addsuffix", starlarkTypeList, hiddenArgNone},
@@ -102,13 +111,16 @@ var knownFunctions = map[string]struct {
 	"is-android-codename":                 {"!is-android-codename", starlarkTypeBool, hiddenArgNone},         // unused by product config
 	"is-android-codename-in-list":         {"!is-android-codename-in-list", starlarkTypeBool, hiddenArgNone}, // unused by product config
 	"is-board-platform":                   {"!is-board-platform", starlarkTypeBool, hiddenArgNone},
+	"is-board-platform2":                  {baseName + ".board_platform_is", starlarkTypeBool, hiddenArgGlobal},
 	"is-board-platform-in-list":           {"!is-board-platform-in-list", starlarkTypeBool, hiddenArgNone},
+	"is-board-platform-in-list2":          {baseName + ".board_platform_in", starlarkTypeBool, hiddenArgGlobal},
 	"is-chipset-in-board-platform":        {"!is-chipset-in-board-platform", starlarkTypeUnknown, hiddenArgNone},     // unused by product config
 	"is-chipset-prefix-in-board-platform": {"!is-chipset-prefix-in-board-platform", starlarkTypeBool, hiddenArgNone}, // unused by product config
 	"is-not-board-platform":               {"!is-not-board-platform", starlarkTypeBool, hiddenArgNone},               // defined but never used
 	"is-platform-sdk-version-at-least":    {"!is-platform-sdk-version-at-least", starlarkTypeBool, hiddenArgNone},    // unused by product config
 	"is-product-in-list":                  {"!is-product-in-list", starlarkTypeBool, hiddenArgNone},
 	"is-vendor-board-platform":            {"!is-vendor-board-platform", starlarkTypeBool, hiddenArgNone},
+	"is-vendor-board-qcom":                {"!is-vendor-board-qcom", starlarkTypeBool, hiddenArgNone},
 	callLoadAlways:                        {"!inherit-product", starlarkTypeVoid, hiddenArgNone},
 	callLoadIf:                            {"!inherit-product-if-exists", starlarkTypeVoid, hiddenArgNone},
 	"lastword":                            {"!lastword", starlarkTypeString, hiddenArgNone},
@@ -399,6 +411,7 @@ type parseContext struct {
 	outputDir        string
 	dependentModules map[string]*moduleInfo
 	soongNamespaces  map[string]map[string]bool
+	includeTops      []string
 }
 
 func newParseContext(ss *StarlarkScript, nodes []mkparser.Node) *parseContext {
@@ -444,6 +457,7 @@ func newParseContext(ss *StarlarkScript, nodes []mkparser.Node) *parseContext {
 		variables:        make(map[string]variable),
 		dependentModules: make(map[string]*moduleInfo),
 		soongNamespaces:  make(map[string]map[string]bool),
+		includeTops:      []string{"vendor/google-devices"},
 	}
 	ctx.pushVarAssignments()
 	for _, item := range predefined {
@@ -522,7 +536,6 @@ func (ctx *parseContext) handleAssignment(a *mkparser.Assignment) {
 		return
 	}
 	name := a.Name.Strings[0]
-	const soongNsPrefix = "SOONG_CONFIG_"
 	// Soong confuguration
 	if strings.HasPrefix(name, soongNsPrefix) {
 		ctx.handleSoongNsAssignment(strings.TrimPrefix(name, soongNsPrefix), a)
@@ -615,7 +628,7 @@ func (ctx *parseContext) handleSoongNsAssignment(name string, asgn *mkparser.Ass
 		for _, ns := range strings.Fields(s) {
 			ctx.addSoongNamespace(ns)
 			ctx.receiver.newNode(&exprNode{&callExpr{
-				name:       addSoongNamespace,
+				name:       soongConfigNamespaceOld,
 				args:       []starlarkExpr{&stringLiteralExpr{ns}},
 				returnType: starlarkTypeVoid,
 			}})
@@ -665,8 +678,12 @@ func (ctx *parseContext) handleSoongNsAssignment(name string, asgn *mkparser.Ass
 			ctx.errorf(asgn, "no %s variable in %s namespace, please use add_soong_config_var_value instead", varName, namespaceName)
 			return
 		}
+		fname := soongConfigVarSetOld
+		if asgn.Type == "+=" {
+			fname = soongConfigAppend
+		}
 		ctx.receiver.newNode(&exprNode{&callExpr{
-			name:       addSoongConfigVarValue,
+			name:       fname,
 			args:       []starlarkExpr{&stringLiteralExpr{namespaceName}, &stringLiteralExpr{varName}, val},
 			returnType: starlarkTypeVoid,
 		}})
@@ -799,21 +816,15 @@ func (ctx *parseContext) handleSubConfig(
 			pathPattern = append(pathPattern, chunk)
 		}
 	}
-	if pathPattern[0] != "" {
+	if pathPattern[0] == "" {
-		matchingPaths = ctx.findMatchingPaths(pathPattern)
+		// If pattern starts from the top. restrict it to the directories where
-	} else {
+		// we know inherit-product uses dynamically calculated path.
-		// Heuristics -- if pattern starts from top, restrict it to the directories where
+		for _, p := range ctx.includeTops {
-		// we know inherit-product uses dynamically calculated path. Restrict it even further
+			pathPattern[0] = p
-		// for certain path which would yield too many useless matches
+			matchingPaths = append(matchingPaths, ctx.findMatchingPaths(pathPattern)...)
-		if len(varPath.chunks) == 2 && varPath.chunks[1] == "/BoardConfigVendor.mk" {
-			pathPattern[0] = "vendor/google_devices"
-			matchingPaths = ctx.findMatchingPaths(pathPattern)
-		} else {
-			for _, t := range []string{"vendor/qcom", "vendor/google_devices"} {
-				pathPattern[0] = t
-				matchingPaths = append(matchingPaths, ctx.findMatchingPaths(pathPattern)...)
-			}
 		}
+	} else {
+		matchingPaths = ctx.findMatchingPaths(pathPattern)
 	}
 	// Safeguard against $(call inherit-product,$(PRODUCT_PATH))
 	const maxMatchingFiles = 150
@@ -1133,6 +1144,34 @@ func (ctx *parseContext) parseCheckFunctionCallResult(directive *mkparser.Direct
 				list:  &variableRefExpr{ctx.addVariable(s + "_BOARD_PLATFORMS"), true},
 				isNot: negate,
 			}, true
+
+		case "is-board-platform2", "is-board-platform-in-list2":
+			if s, ok := maybeString(xValue); !ok || s != "" {
+				return ctx.newBadExpr(directive,
+					fmt.Sprintf("the result of %s can be compared only to empty", x.name)), true
+			}
+			if len(x.args) != 1 {
+				return ctx.newBadExpr(directive, "%s requires an argument", x.name), true
+			}
+			cc := &callExpr{
+				name:       x.name,
+				args:       []starlarkExpr{x.args[0]},
+				returnType: starlarkTypeBool,
+			}
+			if !negate {
+				return &notExpr{cc}, true
+			}
+			return cc, true
+		case "is-vendor-board-qcom":
+			if s, ok := maybeString(xValue); !ok || s != "" {
+				return ctx.newBadExpr(directive,
+					fmt.Sprintf("the result of %s can be compared only to empty", x.name)), true
+			}
+			return &inExpr{
+				expr:  &variableRefExpr{ctx.addVariable("TARGET_BOARD_PLATFORM"), false},
+				list:  &variableRefExpr{ctx.addVariable("QCOM_BOARD_PLATFORMS"), true},
+				isNot: negate,
+			}, true
 		default:
 			return ctx.newBadExpr(directive, "Unknown function in ifeq: %s", x.name), true
 		}
@@ -1277,6 +1316,10 @@ func (ctx *parseContext) parseReference(node mkparser.Node, ref *mkparser.MakeSt
 				returnType: starlarkTypeUnknown,
 			}
 		}
+		if strings.HasPrefix(refDump, soongNsPrefix) {
+			// TODO (asmundak): if we find many, maybe handle them.
+			return ctx.newBadExpr(node, "SOONG_CONFIG_ variables cannot be referenced: %s", refDump)
+		}
 		if v := ctx.addVariable(refDump); v != nil {
 			return &variableRefExpr{v, ctx.lastAssignment(v.name()) != nil}
 		}
@@ -1438,6 +1481,7 @@ func (ctx *parseContext) handleSimpleStatement(node mkparser.Node) bool {
 	handled := true
 	switch x := node.(type) {
 	case *mkparser.Comment:
+		ctx.maybeHandleAnnotation(x)
 		ctx.insertComment("#" + x.Comment)
 	case *mkparser.Assignment:
 		ctx.handleAssignment(x)
@@ -1453,11 +1497,33 @@ func (ctx *parseContext) handleSimpleStatement(node mkparser.Node) bool {
 			handled = false
 		}
 	default:
-		ctx.errorf(x, "unsupported line %s", x.Dump())
+		ctx.errorf(x, "unsupported line %s", strings.ReplaceAll(x.Dump(), "\n", "\n#"))
 	}
 	return handled
 }
 
+// Processes annotation. An annotation is a comment that starts with #RBC# and provides
+// a conversion hint -- say, where to look for the dynamically calculated inherit/include
+// paths.
+func (ctx *parseContext) maybeHandleAnnotation(cnode *mkparser.Comment) {
+	maybeTrim := func(s, prefix string) (string, bool) {
+		if strings.HasPrefix(s, prefix) {
+			return strings.TrimSpace(strings.TrimPrefix(s, prefix)), true
+		}
+		return s, false
+	}
+	annotation, ok := maybeTrim(cnode.Comment, annotationCommentPrefix)
+	if !ok {
+		return
+	}
+	if p, ok := maybeTrim(annotation, "include_top"); ok {
+		ctx.includeTops = append(ctx.includeTops, p)
+		return
+	}
+	ctx.errorf(cnode, "unsupported annotation %s", cnode.Comment)
+
+}
+
 func (ctx *parseContext) insertComment(s string) {
 	ctx.receiver.newNode(&commentNode{strings.TrimSpace(s)})
 }

mk2rbc/mk2rbc_test.go

@@ -560,6 +560,27 @@ def init(g, handle):
     pass
   elif g["TARGET_PRODUCT"] in g.get("PLATFORM_LIST", []):
     pass
+`,
+	},
+	{
+		desc:   "new is-board calls",
+		mkname: "product.mk",
+		in: `
+ifneq (,$(call is-board-platform-in-list2,msm8998 $(X))
+else ifeq (,$(call is-board-platform2,copper)
+else ifneq (,$(call is-vendor-board-qcom))
+endif
+`,
+		expected: `load("//build/make/core:product_config.rbc", "rblf")
+
+def init(g, handle):
+  cfg = rblf.cfg(handle)
+  if rblf.board_platform_in(g, "msm8998 %s" % g.get("X", "")):
+    pass
+  elif not rblf.board_platform_is(g, "copper"):
+    pass
+  elif g.get("TARGET_BOARD_PLATFORM", "") not in g["QCOM_BOARD_PLATFORMS"]:
+    pass
 `,
 	},
 	{
@@ -681,6 +702,8 @@ $(info $(abspath foo/bar))
 $(info $(notdir foo/bar))
 $(call add_soong_config_namespace,snsconfig)
 $(call add_soong_config_var_value,snsconfig,imagetype,odm_image)
+$(call soong_config_set, snsconfig, foo, foo_value)
+$(call soong_config_append, snsconfig, bar, bar_value)
 PRODUCT_COPY_FILES := $(call copy-files,$(wildcard foo*.mk),etc)
 PRODUCT_COPY_FILES := $(call product-copy-files-by-pattern,from/%,to/%,a b c)
 `,
@@ -700,8 +723,10 @@ def init(g, handle):
   rblf.mkinfo("product.mk", rblf.dir((_foobar).split()[-1]))
   rblf.mkinfo("product.mk", rblf.abspath("foo/bar"))
   rblf.mkinfo("product.mk", rblf.notdir("foo/bar"))
-  rblf.add_soong_config_namespace(g, "snsconfig")
+  rblf.soong_config_namespace(g, "snsconfig")
-  rblf.add_soong_config_var_value(g, "snsconfig", "imagetype", "odm_image")
+  rblf.soong_config_set(g, "snsconfig", "imagetype", "odm_image")
+  rblf.soong_config_set(g, "snsconfig", "foo", "foo_value")
+  rblf.soong_config_append(g, "snsconfig", "bar", "bar_value")
   cfg["PRODUCT_COPY_FILES"] = rblf.copy_files(rblf.expand_wildcard("foo*.mk"), "etc")
   cfg["PRODUCT_COPY_FILES"] = rblf.product_copy_files_by_pattern("from/%", "to/%", "a b c")
 `,
@@ -784,17 +809,21 @@ def init(g, handle):
 		in: `
 SOONG_CONFIG_NAMESPACES += cvd
 SOONG_CONFIG_cvd += launch_configs
-SOONG_CONFIG_cvd_launch_configs += cvd_config_auto.json
+SOONG_CONFIG_cvd_launch_configs = cvd_config_auto.json
 SOONG_CONFIG_cvd += grub_config
 SOONG_CONFIG_cvd_grub_config += grub.cfg
+x := $(SOONG_CONFIG_cvd_grub_config)
 `,
 		expected: `load("//build/make/core:product_config.rbc", "rblf")
 
 def init(g, handle):
   cfg = rblf.cfg(handle)
-  rblf.add_soong_config_namespace(g, "cvd")
+  rblf.soong_config_namespace(g, "cvd")
-  rblf.add_soong_config_var_value(g, "cvd", "launch_configs", "cvd_config_auto.json")
+  rblf.soong_config_set(g, "cvd", "launch_configs", "cvd_config_auto.json")
-  rblf.add_soong_config_var_value(g, "cvd", "grub_config", "grub.cfg")
+  rblf.soong_config_append(g, "cvd", "grub_config", "grub.cfg")
+  # MK2RBC TRANSLATION ERROR: SOONG_CONFIG_ variables cannot be referenced: SOONG_CONFIG_cvd_grub_config
+  # x := $(SOONG_CONFIG_cvd_grub_config)
+  rblf.warning("product.mk", "partially successful conversion")
 `,
 	},
 	{
@@ -911,7 +940,7 @@ def init(g, handle):
 		desc:   "Dynamic inherit path",
 		mkname: "product.mk",
 		in: `
-MY_PATH=foo
+MY_PATH:=foo
 $(call inherit-product,vendor/$(MY_PATH)/cfg.mk)
 `,
 		expected: `load("//build/make/core:product_config.rbc", "rblf")
@@ -929,6 +958,47 @@ def init(g, handle):
   if not _varmod_init:
     rblf.mkerror("cannot")
   rblf.inherit(handle, _varmod, _varmod_init)
+`,
+	},
+	{
+		desc:   "Dynamic inherit with hint",
+		mkname: "product.mk",
+		in: `
+MY_PATH:=foo
+#RBC# include_top vendor/foo1
+$(call inherit-product,$(MY_PATH)/cfg.mk)
+`,
+		expected: `load("//build/make/core:product_config.rbc", "rblf")
+load("//vendor/foo1:cfg.star|init", _cfg_init = "init")
+
+def init(g, handle):
+  cfg = rblf.cfg(handle)
+  g["MY_PATH"] = "foo"
+  #RBC# include_top vendor/foo1
+  _entry = {
+    "vendor/foo1/cfg.mk": ("_cfg", _cfg_init),
+  }.get("%s/cfg.mk" % g["MY_PATH"])
+  (_varmod, _varmod_init) = _entry if _entry else (None, None)
+  if not _varmod_init:
+    rblf.mkerror("cannot")
+  rblf.inherit(handle, _varmod, _varmod_init)
+`,
+	},
+	{
+		desc:   "Ignore make rules",
+		mkname: "product.mk",
+		in: `
+foo: foo.c
+	gcc -o $@ $*`,
+		expected: `# MK2RBC TRANSLATION ERROR: unsupported line rule:       foo: foo.c
+#gcc -o $@ $*
+# rule:       foo: foo.c
+# gcc -o $@ $*
+load("//build/make/core:product_config.rbc", "rblf")
+
+def init(g, handle):
+  cfg = rblf.cfg(handle)
+  rblf.warning("product.mk", "partially successful conversion")
 `,
 	},
 }