From 7940c6a5b5d0be18da17e515a1cdceb84b3be3c7 Mon Sep 17 00:00:00 2001
From: Cole Faust <colefaust@google.com>
Date: Mon, 31 Jan 2022 15:54:05 -0800
Subject: [PATCH] Remove RBC hints from generated starlark

The hints no longer need to be there in the generated code.

Fixes: 217269465
Test: go test
Change-Id: If2049780a874b42eb9df9351dda4f29c85482470
---
 mk2rbc/mk2rbc.go      | 17 +++++++++--------
 mk2rbc/mk2rbc_test.go |  8 ++------
 2 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/mk2rbc/mk2rbc.go b/mk2rbc/mk2rbc.go
index 90dc46b3a..927427a43 100644
--- a/mk2rbc/mk2rbc.go
+++ b/mk2rbc/mk2rbc.go
@@ -1733,8 +1733,9 @@ func (ctx *parseContext) parseMakeString(node mkparser.Node, mk *mkparser.MakeSt
 func (ctx *parseContext) handleSimpleStatement(node mkparser.Node) {
 	switch x := node.(type) {
 	case *mkparser.Comment:
-		ctx.maybeHandleAnnotation(x)
-		ctx.insertComment("#" + x.Comment)
+		if !ctx.maybeHandleAnnotation(x) {
+			ctx.insertComment("#" + x.Comment)
+		}
 	case *mkparser.Assignment:
 		ctx.handleAssignment(x)
 	case *mkparser.Variable:
@@ -1764,8 +1765,8 @@ func (ctx *parseContext) handleSimpleStatement(node mkparser.Node) {
 
 // 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) {
+// paths. Returns true if the comment was a successfully-handled annotation.
+func (ctx *parseContext) maybeHandleAnnotation(cnode *mkparser.Comment) bool {
 	maybeTrim := func(s, prefix string) (string, bool) {
 		if strings.HasPrefix(s, prefix) {
 			return strings.TrimSpace(strings.TrimPrefix(s, prefix)), true
@@ -1774,21 +1775,21 @@ func (ctx *parseContext) maybeHandleAnnotation(cnode *mkparser.Comment) {
 	}
 	annotation, ok := maybeTrim(cnode.Comment, annotationCommentPrefix)
 	if !ok {
-		return
+		return false
 	}
 	if p, ok := maybeTrim(annotation, "include_top"); ok {
 		// Don't allow duplicate include tops, because then we will generate
 		// invalid starlark code. (duplicate keys in the _entry dictionary)
 		for _, top := range ctx.includeTops {
 			if top == p {
-				return
+				return true
 			}
 		}
 		ctx.includeTops = append(ctx.includeTops, p)
-		return
+		return true
 	}
 	ctx.errorf(cnode, "unsupported annotation %s", cnode.Comment)
-
+	return true
 }
 
 func (ctx *parseContext) insertComment(s string) {
diff --git a/mk2rbc/mk2rbc_test.go b/mk2rbc/mk2rbc_test.go
index 376ee5e31..1b0753662 100644
--- a/mk2rbc/mk2rbc_test.go
+++ b/mk2rbc/mk2rbc_test.go
@@ -1071,7 +1071,6 @@ 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
   rblf.inherit(handle, "vendor/foo1/cfg", _cfg_init)
 `,
 	},
@@ -1083,6 +1082,7 @@ MY_PATH:=foo
 #RBC# include_top vendor/foo1
 $(call inherit-product,$(MY_PATH)/cfg.mk)
 #RBC# include_top vendor/foo1
+#RBC# include_top vendor/foo1
 $(call inherit-product,$(MY_PATH)/cfg.mk)
 `,
 		expected: `load("//build/make/core:product_config.rbc", "rblf")
@@ -1091,9 +1091,7 @@ 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
   rblf.inherit(handle, "vendor/foo1/cfg", _cfg_init)
-  #RBC# include_top vendor/foo1
   rblf.inherit(handle, "vendor/foo1/cfg", _cfg_init)
 `,
 	},
@@ -1112,15 +1110,13 @@ $(call inherit-product,$(MY_VAR)/font.mk)
 
 $(call inherit-product,$(MY_VAR)/font.mk)
 `,
-		expected: `#RBC# include_top foo
-load("//build/make/core:product_config.rbc", "rblf")
+		expected: `load("//build/make/core:product_config.rbc", "rblf")
 load("//foo:font.star|init", _font_init = "init")
 load("//bar:font.star|init", _font1_init = "init")
 
 def init(g, handle):
   cfg = rblf.cfg(handle)
   rblf.inherit(handle, "foo/font", _font_init)
-  #RBC# include_top foo
   # There's some space and even this comment between the include_top and the inherit-product
   rblf.inherit(handle, "foo/font", _font_init)
   rblf.mkwarning("product.mk:11", "Including a path with a non-constant prefix, please convert this to a simple literal to generate cleaner starlark.")