Emit unconditional module load only when inherit/include is on the top level.

That is, when a makefile contains
```
ifneq (,$(foo))
  $(call inherit-product,module.mk)
endif
```

module.mk has to be present only if `foo` is set.

Fixes: 200163602
Test: internal
Change-Id: Ic5f10ce8d49d6b87162bfe77922bba5e2cce228b

mk2rbc/mk2rbc.go

@@ -726,7 +726,7 @@ func (ctx *parseContext) buildConcatExpr(a *mkparser.Assignment) *concatExpr {
 func (ctx *parseContext) newDependentModule(path string, optional bool) *moduleInfo {
 	modulePath := ctx.loadedModulePath(path)
 	if mi, ok := ctx.dependentModules[modulePath]; ok {
-		mi.optional = mi.optional || optional
+		mi.optional = mi.optional && optional
 		return mi
 	}
 	moduleName := moduleNameForFile(path)
@@ -753,16 +753,21 @@ func (ctx *parseContext) handleSubConfig(
 
 	// In a simple case, the name of a module to inherit/include is known statically.
 	if path, ok := maybeString(pathExpr); ok {
+		// Note that even if this directive loads a module unconditionally, a module may be
+		// absent without causing any harm if this directive is inside an if/else block.
+		moduleShouldExist := loadAlways && ctx.ifNestLevel == 0
 		if strings.Contains(path, "*") {
 			if paths, err := fs.Glob(ctx.script.sourceFS, path); err == nil {
 				for _, p := range paths {
-					processModule(inheritedStaticModule{ctx.newDependentModule(p, !loadAlways), loadAlways})
+					mi := ctx.newDependentModule(p, !moduleShouldExist)
+					processModule(inheritedStaticModule{mi, loadAlways})
 				}
 			} else {
 				ctx.errorf(v, "cannot glob wildcard argument")
 			}
 		} else {
-			processModule(inheritedStaticModule{ctx.newDependentModule(path, !loadAlways), loadAlways})
+			mi := ctx.newDependentModule(path, !moduleShouldExist)
+			processModule(inheritedStaticModule{mi, loadAlways})
 		}
 		return
 	}
@@ -852,13 +857,13 @@ func (ctx *parseContext) findMatchingPaths(pattern []string) []string {
 
 func (ctx *parseContext) handleInheritModule(v mkparser.Node, pathExpr starlarkExpr, loadAlways bool) {
 	ctx.handleSubConfig(v, pathExpr, loadAlways, func(im inheritedModule) {
-		ctx.receiver.newNode(&inheritNode{im})
+		ctx.receiver.newNode(&inheritNode{im, loadAlways})
 	})
 }
 
 func (ctx *parseContext) handleInclude(v mkparser.Node, pathExpr starlarkExpr, loadAlways bool) {
 	ctx.handleSubConfig(v, pathExpr, loadAlways, func(im inheritedModule) {
-		ctx.receiver.newNode(&includeNode{im})
+		ctx.receiver.newNode(&includeNode{im, loadAlways})
 	})
 }
 

mk2rbc/mk2rbc_test.go

@@ -120,22 +120,25 @@ def init(g, handle):
 		desc:   "Inherit configuration always",
 		mkname: "product.mk",
 		in: `
-ifdef PRODUCT_NAME
 $(call inherit-product, part.mk)
+ifdef PRODUCT_NAME
+$(call inherit-product, part1.mk)
 else # Comment
-$(call inherit-product, $(LOCAL_PATH)/part.mk)
+$(call inherit-product, $(LOCAL_PATH)/part1.mk)
 endif
 `,
 		expected: `load("//build/make/core:product_config.rbc", "rblf")
 load(":part.star", _part_init = "init")
+load(":part1.star|init", _part1_init = "init")
 
 def init(g, handle):
   cfg = rblf.cfg(handle)
-  if g.get("PRODUCT_NAME") != None:
   rblf.inherit(handle, "part", _part_init)
+  if g.get("PRODUCT_NAME") != None:
+    rblf.inherit(handle, "part1", _part1_init)
   else:
     # Comment
-    rblf.inherit(handle, "part", _part_init)
+    rblf.inherit(handle, "part1", _part1_init)
 `,
 	},
 	{
@@ -158,22 +161,25 @@ def init(g, handle):
 		desc:   "Include configuration",
 		mkname: "product.mk",
 		in: `
-ifdef PRODUCT_NAME
 include part.mk
+ifdef PRODUCT_NAME
+include part1.mk
 else
--include $(LOCAL_PATH)/part.mk)
+-include $(LOCAL_PATH)/part1.mk)
 endif
 `,
 		expected: `load("//build/make/core:product_config.rbc", "rblf")
-load(":part.star|init", _part_init = "init")
+load(":part.star", _part_init = "init")
+load(":part1.star|init", _part1_init = "init")
 
 def init(g, handle):
   cfg = rblf.cfg(handle)
+  _part_init(g, handle)
   if g.get("PRODUCT_NAME") != None:
-    _part_init(g, handle)
+    _part1_init(g, handle)
   else:
-    if _part_init != None:
+    if _part1_init != None:
-      _part_init(g, handle)
+      _part1_init(g, handle)
 `,
 	},
 

mk2rbc/node.go

@@ -57,7 +57,7 @@ type inheritedModule interface {
 	name() string
 	entryName() string
 	emitSelect(gctx *generationContext)
-	isLoadAlways() bool
+	shouldExist() bool
 }
 
 type inheritedStaticModule struct {
@@ -72,7 +72,7 @@ func (im inheritedStaticModule) name() string {
 func (im inheritedStaticModule) emitSelect(_ *generationContext) {
 }
 
-func (im inheritedStaticModule) isLoadAlways() bool {
+func (im inheritedStaticModule) shouldExist() bool {
 	return im.loadAlways
 }
 
@@ -115,12 +115,13 @@ func (i inheritedDynamicModule) emitSelect(gctx *generationContext) {
 	}
 }
 
-func (i inheritedDynamicModule) isLoadAlways() bool {
+func (i inheritedDynamicModule) shouldExist() bool {
 	return i.loadAlways
 }
 
 type inheritNode struct {
 	module     inheritedModule
+	loadAlways bool
 }
 
 func (inn *inheritNode) emit(gctx *generationContext) {
@@ -134,7 +135,7 @@ func (inn *inheritNode) emit(gctx *generationContext) {
 	name := inn.module.name()
 	entry := inn.module.entryName()
 	gctx.newLine()
-	if inn.module.isLoadAlways() {
+	if inn.loadAlways {
 		gctx.writef("%s(handle, %s, %s)", cfnInherit, name, entry)
 		return
 	}
@@ -148,13 +149,14 @@ func (inn *inheritNode) emit(gctx *generationContext) {
 
 type includeNode struct {
 	module     inheritedModule
+	loadAlways bool
 }
 
 func (inn *includeNode) emit(gctx *generationContext) {
 	inn.module.emitSelect(gctx)
 	entry := inn.module.entryName()
 	gctx.newLine()
-	if inn.module.isLoadAlways() {
+	if inn.loadAlways {
 		gctx.writef("%s(g, handle)", entry)
 		return
 	}