From cba2330e84f6be16b2fc3c17f68c95792dbf2738 Mon Sep 17 00:00:00 2001
From: LaMont Jones <lamontjones@google.com>
Date: Wed, 17 Jul 2024 13:53:59 -0700
Subject: [PATCH] release-config: disallow new duplicate flag declarations

Flags must only be declared in one place in the tree.  This change
prevents new ones from being added while we fix the bad flags already
present.

Bug: 352105274
Test: manual
Merged-In: I8c4bf2b2852685e84177500f28fe8908016082b9
Change-Id: I8c4bf2b2852685e84177500f28fe8908016082b9
---
 .../release_config_lib/flag_declaration.go    | 11 ++++++++
 .../release_config_lib/release_configs.go     | 26 ++++++++++++-------
 2 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/cmd/release_config/release_config_lib/flag_declaration.go b/cmd/release_config/release_config_lib/flag_declaration.go
index 97d4d4c76..22001bf09 100644
--- a/cmd/release_config/release_config_lib/flag_declaration.go
+++ b/cmd/release_config/release_config_lib/flag_declaration.go
@@ -18,10 +18,21 @@ import (
 	rc_proto "android/soong/cmd/release_config/release_config_proto"
 )
 
+var (
+	// Allowlist: these flags may have duplicate (identical) declarations
+	// without generating an error.  This will be removed once all such
+	// declarations have been fixed.
+	DuplicateDeclarationAllowlist = map[string]bool{}
+)
+
 func FlagDeclarationFactory(protoPath string) (fd *rc_proto.FlagDeclaration) {
 	fd = &rc_proto.FlagDeclaration{}
 	if protoPath != "" {
 		LoadMessage(protoPath, fd)
 	}
+	// If the input didn't specify a value, create one (== UnspecifiedValue).
+	if fd.Value == nil {
+		fd.Value = &rc_proto.Value{Val: &rc_proto.Value_UnspecifiedValue{false}}
+	}
 	return fd
 }
diff --git a/cmd/release_config/release_config_lib/release_configs.go b/cmd/release_config/release_config_lib/release_configs.go
index a4c884ce3..97eb8f156 100644
--- a/cmd/release_config/release_config_lib/release_configs.go
+++ b/cmd/release_config/release_config_lib/release_configs.go
@@ -276,6 +276,20 @@ func (configs *ReleaseConfigs) LoadReleaseConfigMap(path string, ConfigDirIndex
 		configs.Aliases[name] = alias.Target
 	}
 	var err error
+	// Temporarily allowlist duplicate flag declaration files to prevent
+	// more from entering the tree while we work to clean up the duplicates
+	// that already exist.
+	dupFlagFile := filepath.Join(dir, "duplicate_allowlist.txt")
+	data, err := os.ReadFile(dupFlagFile)
+	if err == nil {
+		for _, flag := range strings.Split(string(data), "\n") {
+			flag = strings.TrimSpace(flag)
+			if strings.HasPrefix(flag, "//") || strings.HasPrefix(flag, "#") {
+				continue
+			}
+			DuplicateDeclarationAllowlist[flag] = true
+		}
+	}
 	err = WalkTextprotoFiles(dir, "flag_declarations", func(path string, d fs.DirEntry, err error) error {
 		flagDeclaration := FlagDeclarationFactory(path)
 		// Container must be specified.
@@ -289,14 +303,6 @@ func (configs *ReleaseConfigs) LoadReleaseConfigMap(path string, ConfigDirIndex
 			}
 		}
 
-		// TODO: once we have namespaces initialized, we can throw an error here.
-		if flagDeclaration.Namespace == nil {
-			flagDeclaration.Namespace = proto.String("android_UNKNOWN")
-		}
-		// If the input didn't specify a value, create one (== UnspecifiedValue).
-		if flagDeclaration.Value == nil {
-			flagDeclaration.Value = &rc_proto.Value{Val: &rc_proto.Value_UnspecifiedValue{false}}
-		}
 		m.FlagDeclarations = append(m.FlagDeclarations, *flagDeclaration)
 		name := *flagDeclaration.Name
 		if name == "RELEASE_ACONFIG_VALUE_SETS" {
@@ -304,8 +310,8 @@ func (configs *ReleaseConfigs) LoadReleaseConfigMap(path string, ConfigDirIndex
 		}
 		if def, ok := configs.FlagArtifacts[name]; !ok {
 			configs.FlagArtifacts[name] = &FlagArtifact{FlagDeclaration: flagDeclaration, DeclarationIndex: ConfigDirIndex}
-		} else if !proto.Equal(def.FlagDeclaration, flagDeclaration) {
-			return fmt.Errorf("Duplicate definition of %s", *flagDeclaration.Name)
+		} else if !proto.Equal(def.FlagDeclaration, flagDeclaration) || !DuplicateDeclarationAllowlist[name] {
+			return fmt.Errorf("Duplicate definition of %s in %s", *flagDeclaration.Name, path)
 		}
 		// Set the initial value in the flag artifact.
 		configs.FilesUsedMap[path] = true