Clean up the API for release config in starlark

Also gets the json summaries working again.

Bug: 283477392
Test: Manual
Merged-In: Iaeb840e96bd7fef2fa1492cb55d9688f7ca75858
Change-Id: I05d9b816ad7ad30c1d238d01df811426d9aeddcb
diff --git a/core/Makefile b/core/Makefile
index ab80516..ccd4e17 100644
--- a/core/Makefile
+++ b/core/Makefile
@@ -27,26 +27,28 @@
 $(eval $(strip $(1)): PRIVATE_FLAG_NAMES := $(strip $(2)))
 $(strip $(1)):
 	mkdir -p $$(dir $$(PRIVATE_OUT))
-	( \
-		echo '{' ; \
-		echo 'flags: [' ; \
-		$$(foreach flag, $$(PRIVATE_FLAG_NAMES), \
+	echo '{' > $$(PRIVATE_OUT)
+	echo '"flags": [' >> $$(PRIVATE_OUT)
+	$$(foreach flag, $$(PRIVATE_FLAG_NAMES), \
+		( \
 			printf '  { "name": "%s", "value": "%s", ' \
 					'$$(flag)' \
 					'$$(_ALL_RELEASE_FLAGS.$$(flag).VALUE)' \
 					; \
-			printf '"set": "%s", "default": "%s", "declared": "%s", }' \
+			printf '"set": "%s", "default": "%s", "declared": "%s" }' \
 					'$$(_ALL_RELEASE_FLAGS.$$(flag).SET_IN)' \
 					'$$(_ALL_RELEASE_FLAGS.$$(flag).DEFAULT)' \
 					'$$(_ALL_RELEASE_FLAGS.$$(flag).DECLARED_IN)' \
 					; \
 			printf '$$(if $$(filter $$(lastword $$(PRIVATE_FLAG_NAMES)),$$(flag)),,$$(comma))\n' ; \
-		) \
-		echo "]" ; \
-		echo "}" \
-	) >> $$(PRIVATE_OUT)
+		) >> $$(PRIVATE_OUT) \
+	)
+	echo "]" >> $$(PRIVATE_OUT)
+	echo "}" >> $$(PRIVATE_OUT)
 endef
 
+_FLAG_PARTITIONS := product system system_ext vendor
+
 $(foreach partition, $(_FLAG_PARTITIONS), \
 	$(eval BUILD_FLAG_SUMMARIES.$(partition) \
 			:= $(TARGET_OUT_FLAGS)/$(partition)/etc/build_flags.json) \
diff --git a/core/release_config.bzl b/core/release_config.bzl
index ecb9ec6..1346508 100644
--- a/core/release_config.bzl
+++ b/core/release_config.bzl
@@ -20,71 +20,101 @@
     "vendor",
 ]
 
-def _combine_dicts_no_duplicate_keys(dicts):
-    result = {}
-    for d in dicts:
-        for k, v in d.items():
-            if k in result:
-                fail("Duplicate key: " + k)
-            result[k] = v
-    return result
+ALL = ["all"]
+PRODUCT = ["product"]
+SYSTEM = ["system"]
+SYSTEM_EXT = ["system_ext"]
+VENDOR = ["vendor"]
 
-def release_config(target_release, flag_definitions, config_maps, fail_if_no_release_config = True):
-    result = {
-        "_ALL_RELEASE_FLAGS": [flag.name for flag in flag_definitions],
+_valid_types = ["NoneType", "bool", "list", "string", "int"]
+
+def flag(name, partitions, default):
+    "Declare a flag."
+    if not partitions:
+        fail("At least 1 partition is required")
+    if not name.startswith("RELEASE_"):
+        fail("Release flag names must start with RELEASE_")
+    if " " in name or "\t" in name or "\n" in name:
+        fail("Flag names must not contain whitespace: \"" + name + "\"")
+    for partition in partitions:
+        if partition == "all":
+            if len(partitions) > 1:
+                fail("\"all\" can't be combined with other partitions: " + str(partitions))
+        elif partition not in _flag_partitions:
+            fail("Invalid partition: " + partition + ", allowed partitions: " +
+                 str(_flag_partitions))
+    if type(default) not in _valid_types:
+        fail("Invalid type of default for flag \"" + name + "\" (" + type(default) + ")")
+    return {
+        "name": name,
+        "partitions": partitions,
+        "default": default,
     }
-    all_flags = {}
-    for flag in flag_definitions:
-        if sorted(dir(flag)) != ["default", "name", "partitions"]:
-            fail("Flag structs must contain 3 fields: name, partitions, and default")
-        if not flag.partitions:
-            fail("At least 1 partition is required")
-        for partition in flag.partitions:
-            if partition == "all":
-                if len(flag.partitions) > 1:
-                    fail("\"all\" can't be combined with other partitions: " + str(flag.partitions))
-            elif partition not in _flag_partitions:
-                fail("Invalid partition: " + flag.partition + ", allowed partitions: " + str(_flag_partitions))
-        if not flag.name.startswith("RELEASE_"):
-            fail("Release flag names must start with RELEASE_")
-        if " " in flag.name or "\t" in flag.name or "\n" in flag.name:
-            fail("Flag names must not contain whitespace.")
-        if flag.name in all_flags:
-            fail("Duplicate declaration of flag " + flag.name)
-        all_flags[flag.name] = True
 
-        default = flag.default
-        if type(default) == "bool":
-            default = "true" if default else ""
+def value(name, value):
+    "Define the flag value for a particular configuration."
+    return {
+        "name": name,
+        "value": value,
+    }
 
-        result["_ALL_RELEASE_FLAGS." + flag.name + ".PARTITIONS"] = flag.partitions
-        result["_ALL_RELEASE_FLAGS." + flag.name + ".DEFAULT"] = default
-        result["_ALL_RELEASE_FLAGS." + flag.name + ".VALUE"] = default
-
-    # If TARGET_RELEASE is set, fail if there is no matching release config
-    # If it isn't set, no release config files will be included and all flags
-    # will get their default values.
-    if target_release:
-        config_map = _combine_dicts_no_duplicate_keys(config_maps)
-        if target_release not in config_map:
-            fail("No release config found for TARGET_RELEASE: " + target_release + ". Available releases are: " + str(config_map.keys()))
-        release_config = config_map[target_release]
-        if sorted(dir(release_config)) != ["flags", "release_version"]:
-            fail("A release config must be a struct with a flags and release_version fields")
-        result["_RELEASE_VERSION"] = release_config.release_version
-        for flag in release_config.flags:
-            if sorted(dir(flag)) != ["name", "value"]:
-                fail("A flag must be a struct with name and value fields, got: " + str(sorted(dir(flag))))
-            if flag.name not in all_flags:
-                fail("Undeclared build flag: " + flag.name)
-            value = flag.value
-            if type(value) == "bool":
-                value = "true" if value else ""
-            result["_ALL_RELEASE_FLAGS." + flag.name + ".VALUE"] = value
-    elif fail_if_no_release_config:
-        fail("FAIL_IF_NO_RELEASE_CONFIG was set and TARGET_RELEASE was not")
+def _format_value(val):
+    "Format the starlark type correctly for make"
+    if type(val) == "NoneType":
+        return ""
+    elif type(val) == "bool":
+        return "true" if val else ""
     else:
-        # No TARGET_RELEASE means release version 0
-        result["_RELEASE_VERSION"] = 0
+        return val
+
+def release_config(all_flags, all_values):
+    "Return the make variables that should be set for this release config."
+
+    # Validate flags
+    flag_names = []
+    for flag in all_flags:
+        if flag["name"] in flag_names:
+            fail(flag["declared_in"] + ": Duplicate declaration of flag " + flag["name"])
+        flag_names.append(flag["name"])
+
+    # Record which flags go on which partition
+    partitions = {}
+    for flag in all_flags:
+        for partition in flag["partitions"]:
+            if partition == "all":
+                for partition in _flag_partitions:
+                    partitions.setdefault(partition, []).append(flag["name"])
+            else:
+                partitions.setdefault(partition, []).append(flag["name"])
+
+    # Validate values
+    values = {}
+    for value in all_values:
+        if value["name"] not in flag_names:
+            fail(value["set_in"] + ": Value set for undeclared build flag: " + value["name"])
+        values[value["name"]] = value
+
+    # Collect values
+    result = {
+        "_ALL_RELEASE_FLAGS": sorted(flag_names),
+    }
+    for partition, names in partitions.items():
+        result["_ALL_RELEASE_FLAGS.PARTITIONS." + partition] = names
+    for flag in all_flags:
+        if flag["name"] in values:
+            val = values[flag["name"]]["value"]
+            set_in = values[flag["name"]]["set_in"]
+            if type(val) not in _valid_types:
+                fail("Invalid type of value for flag \"" + flag["name"] + "\" (" + type(val) + ")")
+        else:
+            val = flag["default"]
+            set_in = flag["declared_in"]
+        val = _format_value(val)
+        result[flag["name"]] = val
+        result["_ALL_RELEASE_FLAGS." + flag["name"] + ".PARTITIONS"] = flag["partitions"]
+        result["_ALL_RELEASE_FLAGS." + flag["name"] + ".DEFAULT"] = _format_value(flag["default"])
+        result["_ALL_RELEASE_FLAGS." + flag["name"] + ".VALUE"] = val
+        result["_ALL_RELEASE_FLAGS." + flag["name"] + ".DECLARED_IN"] = flag["declared_in"]
+        result["_ALL_RELEASE_FLAGS." + flag["name"] + ".SET_IN"] = set_in
 
     return result
diff --git a/core/release_config.mk b/core/release_config.mk
index ce57855..3cd8b41 100644
--- a/core/release_config.mk
+++ b/core/release_config.mk
@@ -12,6 +12,104 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+
+# -----------------------------------------------------------------
+# Choose the flag files
+# -----------------------------------------------------------------
+# Do this first, because we're going to unset TARGET_RELEASE before
+# including anyone, so they don't start making conditionals based on it.
+# This logic is in make because starlark doesn't understand optional
+# vendor files.
+
+# If this is a google source tree, restrict it to only the one file
+# which has OWNERS control.  If it isn't let others define their own.
+# TODO: Remove wildcard for build/release one when all branch manifests
+# have updated.
+config_map_files := $(wildcard build/release/release_config_map.mk) \
+    $(if $(wildcard vendor/google/release/release_config_map.mk), \
+        vendor/google/release/release_config_map.mk, \
+        $(sort \
+            $(wildcard device/*/release/release_config_map.mk) \
+            $(wildcard device/*/*/release/release_config_map.mk) \
+            $(wildcard vendor/*/release/release_config_map.mk) \
+            $(wildcard vendor/*/*/release/release_config_map.mk) \
+        ) \
+    )
+
+# $1 config name
+# $2 release config files
+define declare-release-config
+    $(eval # No duplicates)
+    $(if $(filter $(_all_release_configs), $(strip $(1))), \
+        $(error declare-release-config: config $(strip $(1)) declared in: $(_included) Previously declared here: $(_all_release_configs.$(strip $(1)).DECLARED_IN)) \
+    )
+    $(eval # Must have release config files)
+    $(if $(strip $(2)),,  \
+        $(error declare-release-config: config $(strip $(1)) must have release config files) \
+    )
+    $(eval _all_release_configs := $(sort $(_all_release_configs) $(strip $(1))))
+    $(eval _all_release_configs.$(strip $(1)).DECLARED_IN := $(_included))
+    $(eval _all_release_configs.$(strip $(1)).FILES := $(strip $(2)))
+endef
+
+# Include the config map files
+$(foreach f, $(config_map_files), \
+    $(eval _included := $(f)) \
+    $(eval include $(f)) \
+)
+
+# If TARGET_RELEASE is set, fail if there is no matching release config
+# If it isn't set, no release config files will be included and all flags
+# will get their default values.
+ifneq ($(TARGET_RELEASE),)
+ifeq ($(filter $(_all_release_configs), $(TARGET_RELEASE)),)
+    $(error No release config found for TARGET_RELEASE: $(TARGET_RELEASE). Available releases are: $(_all_release_configs))
+else
+    # Choose flag files
+    # Don't sort this, use it in the order they gave us.
+    flag_value_files := $(_all_release_configs.$(TARGET_RELEASE).FILES)
+endif
+else
+# Useful for finding scripts etc that aren't passing or setting TARGET_RELEASE
+ifneq ($(FAIL_IF_NO_RELEASE_CONFIG),)
+    $(error FAIL_IF_NO_RELEASE_CONFIG was set and TARGET_RELEASE was not)
+endif
+flag_value_files :=
+endif
+
+# Unset variables so they can't use them
+define declare-release-config
+$(error declare-release-config can only be called from inside release_config_map.mk files)
+endef
+
+# TODO: Remove this check after enough people have sourced lunch that we don't
+# need to worry about it trying to do get_build_vars TARGET_RELEASE. Maybe after ~9/2023
+ifneq ($(CALLED_FROM_SETUP),true)
+define TARGET_RELEASE
+$(error TARGET_RELEASE may not be accessed directly. Use individual flags.)
+endef
+else
+TARGET_RELEASE:=
+endif
+.KATI_READONLY := TARGET_RELEASE
+
+
+$(foreach config, $(_all_release_configs), \
+    $(eval _all_release_configs.$(config).DECLARED_IN:= ) \
+    $(eval _all_release_configs.$(config).FILES:= ) \
+)
+_all_release_configs:=
+config_map_files:=
+
+
+# -----------------------------------------------------------------
+# Flag declarations and values
+# -----------------------------------------------------------------
+# This part is in starlark.  We generate a root starlark file that loads
+# all of the flags declaration files that we found, and the flag_value_files
+# that we chose from the config map above.  Then we run that, and load the
+# results of that into the make environment.
+
 # If this is a google source tree, restrict it to only the one file
 # which has OWNERS control.  If it isn't let others define their own.
 # TODO: Remove wildcard for build/release one when all branch manifests
@@ -26,43 +124,25 @@
             $(wildcard vendor/*/*/release/build_flags.bzl) \
         ) \
     )
-config_map_files := $(wildcard build/release/release_config_map.bzl) \
-    $(if $(wildcard vendor/google/release/release_config_map.bzl), \
-        vendor/google/release/release_config_map.bzl, \
-        $(sort \
-            $(wildcard device/*/release/release_config_map.bzl) \
-            $(wildcard device/*/*/release/release_config_map.bzl) \
-            $(wildcard vendor/*/release/release_config_map.bzl) \
-            $(wildcard vendor/*/*/release/release_config_map.bzl) \
-        ) \
-    )
+
 
 # Because starlark can't find files with $(wildcard), write an entrypoint starlark script that
 # contains the result of the above wildcards for the starlark code to use.
 filename_to_starlark=$(subst /,_,$(subst .,_,$(1)))
 _c:=load("//build/make/core/release_config.bzl", "release_config")
-_c+=$(foreach f,$(flag_declaration_files),$(newline)load("//$(f)", flags_$(call filename_to_starlark,$(f)) = "flags"))
-_c+=$(foreach f,$(config_map_files),$(newline)load("//$(f)", config_maps_$(call filename_to_starlark,$(f)) = "config_maps"))
-_c+=$(newline)all_flags = [] $(foreach f,$(flag_declaration_files),+ flags_$(call filename_to_starlark,$(f)))
-_c+=$(newline)all_config_maps = [$(foreach f,$(config_map_files),config_maps_$(call filename_to_starlark,$(f))$(comma))]
-_c+=$(newline)target_release = "$(TARGET_RELEASE)"
-_c+=$(newline)fail_if_no_release_config = True if "$(FAIL_IF_NO_RELEASE_CONFIG)" else False
-_c+=$(newline)variables_to_export_to_make = release_config(target_release, all_flags, all_config_maps, fail_if_no_release_config)
+_c+=$(newline)def add(d, k, v):
+_c+=$(newline)$(space)d = dict(d)
+_c+=$(newline)$(space)d[k] = v
+_c+=$(newline)$(space)return d
+_c+=$(foreach f,$(flag_declaration_files),$(newline)load("$(f)", flags_$(call filename_to_starlark,$(f)) = "flags"))
+_c+=$(newline)all_flags = [] $(foreach f,$(flag_declaration_files),+ [add(x, "declared_in", "$(f)") for x in flags_$(call filename_to_starlark,$(f))])
+_c+=$(foreach f,$(flag_value_files),$(newline)load("//$(f)", values_$(call filename_to_starlark,$(f)) = "values"))
+_c+=$(newline)all_values = [] $(foreach f,$(flag_value_files),+ [add(x, "set_in", "$(f)") for x in values_$(call filename_to_starlark,$(f))])
+_c+=$(newline)variables_to_export_to_make = release_config(all_flags, all_values)
 $(file >$(OUT_DIR)/release_config_entrypoint.bzl,$(_c))
 _c:=
 filename_to_starlark:=
 
-# TODO: Remove this check after enough people have sourced lunch that we don't
-# need to worry about it trying to do get_build_vars TARGET_RELEASE. Maybe after ~9/2023
-ifneq ($(CALLED_FROM_SETUP),true)
-define TARGET_RELEASE
-$(error TARGET_RELEASE may not be accessed directly. Use individual flags.)
-endef
-else
-TARGET_RELEASE:=
-endif
-.KATI_READONLY := TARGET_RELEASE
-
 # Exclude the entrypoint file as a dependency (by passing it as the 2nd argument) so that we don't
 # rerun kati every build. Kati will replay the $(file) command that generates it every build,
 # updating its timestamp.
@@ -71,9 +151,3 @@
 # outside of the source tree.
 $(call run-starlark,$(OUT_DIR)/release_config_entrypoint.bzl,$(OUT_DIR)/release_config_entrypoint.bzl,--allow_external_entrypoint)
 
-# Set the flag values, and don't allow any one to modify them.
-$(foreach flag, $(_ALL_RELEASE_FLAGS), \
-    $(eval $(flag) := $(_ALL_RELEASE_FLAGS.$(flag).VALUE)) \
-    $(eval .KATI_READONLY := $(flag)) \
-)
-