From 8e15f69709f70fce745bcd281a1a2bc04468f95d Mon Sep 17 00:00:00 2001
From: Cole Faust <colefaust@google.com>
Date: Mon, 9 Oct 2023 12:26:21 -0700
Subject: [PATCH] Remove ?= assignements to product variables

In make, all product variables are assigned to an empty string before
including the product config makefiles. In starlark, we don't do that
just so the memory usage is smaller. This means that in make, using
?= to assign a product config variable has no effect. Replicate this
behavior in starlark by not emitting any code for ?= assignments of
product variables.

Fixes: 304324003
Test: go test
Change-Id: Id31531506ac9e372a1bea92ce9ae8e17ae0ca70c
---
 mk2rbc/mk2rbc.go      |  7 +++++++
 mk2rbc/mk2rbc_test.go | 20 ++++++++------------
 mk2rbc/variable.go    | 13 +++++--------
 3 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/mk2rbc/mk2rbc.go b/mk2rbc/mk2rbc.go
index 3a6854c29..78ab771b9 100644
--- a/mk2rbc/mk2rbc.go
+++ b/mk2rbc/mk2rbc.go
@@ -635,6 +635,13 @@ func (ctx *parseContext) handleAssignment(a *mkparser.Assignment) []starlarkNode
 	case "+=":
 		asgn.flavor = asgnAppend
 	case "?=":
+		if _, ok := lhs.(*productConfigVariable); ok {
+			// Make sets all product configuration variables to empty strings before running product
+			// config makefiles. ?= will have no effect on a variable that has been assigned before,
+			// even if assigned to an empty string. So just skip emitting any code for this
+			// assignment.
+			return nil
+		}
 		asgn.flavor = asgnMaybeSet
 	default:
 		panic(fmt.Errorf("unexpected assignment type %s", a.Type))
diff --git a/mk2rbc/mk2rbc_test.go b/mk2rbc/mk2rbc_test.go
index a9b61978a..0c4d21375 100644
--- a/mk2rbc/mk2rbc_test.go
+++ b/mk2rbc/mk2rbc_test.go
@@ -923,8 +923,6 @@ def init(g, handle):
   cfg["PRODUCT_LIST2"] += ["a"]
   cfg["PRODUCT_LIST1"] += ["b"]
   cfg["PRODUCT_LIST2"] += ["b"]
-  if cfg.get("PRODUCT_LIST3") == None:
-    cfg["PRODUCT_LIST3"] = ["a"]
   cfg["PRODUCT_LIST1"] = ["c"]
   g.setdefault("PLATFORM_LIST", [])
   g["PLATFORM_LIST"] += ["x"]
@@ -966,9 +964,10 @@ PRODUCT_LIST1 = a $(PRODUCT_LIST1)
 PRODUCT_LIST2 ?= a $(PRODUCT_LIST2)
 PRODUCT_LIST3 += a
 
-# Now doing them again should not have a setdefault because they've already been set
+# Now doing them again should not have a setdefault because they've already been set, except 2
+# which did not emit an assignment before
 PRODUCT_LIST1 = a $(PRODUCT_LIST1)
-PRODUCT_LIST2 ?= a $(PRODUCT_LIST2)
+PRODUCT_LIST2 = a $(PRODUCT_LIST2)
 PRODUCT_LIST3 += a
 `,
 		expected: `# All of these should have a setdefault because they're self-referential and not defined before
@@ -979,18 +978,15 @@ def init(g, handle):
   rblf.setdefault(handle, "PRODUCT_LIST1")
   cfg["PRODUCT_LIST1"] = (["a"] +
       cfg.get("PRODUCT_LIST1", []))
-  if cfg.get("PRODUCT_LIST2") == None:
-    rblf.setdefault(handle, "PRODUCT_LIST2")
-    cfg["PRODUCT_LIST2"] = (["a"] +
-        cfg.get("PRODUCT_LIST2", []))
   rblf.setdefault(handle, "PRODUCT_LIST3")
   cfg["PRODUCT_LIST3"] += ["a"]
-  # Now doing them again should not have a setdefault because they've already been set
+  # Now doing them again should not have a setdefault because they've already been set, except 2
+  # which did not emit an assignment before
   cfg["PRODUCT_LIST1"] = (["a"] +
       cfg["PRODUCT_LIST1"])
-  if cfg.get("PRODUCT_LIST2") == None:
-    cfg["PRODUCT_LIST2"] = (["a"] +
-        cfg["PRODUCT_LIST2"])
+  rblf.setdefault(handle, "PRODUCT_LIST2")
+  cfg["PRODUCT_LIST2"] = (["a"] +
+      cfg.get("PRODUCT_LIST2", []))
   cfg["PRODUCT_LIST3"] += ["a"]
 `,
 	},
diff --git a/mk2rbc/variable.go b/mk2rbc/variable.go
index 0a26ed8b2..95e1f8ec7 100644
--- a/mk2rbc/variable.go
+++ b/mk2rbc/variable.go
@@ -109,14 +109,11 @@ func (pcv productConfigVariable) emitSet(gctx *generationContext, asgn *assignme
 		}
 		emitAppend()
 	case asgnMaybeSet:
-		gctx.writef("if cfg.get(%q) == None:", pcv.nam)
-		gctx.indentLevel++
-		gctx.newLine()
-		if needsSetDefault {
-			emitSetDefault()
-		}
-		emitAssignment()
-		gctx.indentLevel--
+		// In mk2rbc.go we never emit a maybeSet assignment for product config variables, because
+		// they are set to empty strings before running product config.
+		panic("Should never get here")
+	default:
+		panic("Unknown assignment flavor")
 	}
 
 	gctx.setHasBeenAssigned(&pcv)