Merge changes from topic "lamont-flagging-flags" into main

* changes:
  RELEASE_ACONFIG_VALUE_SETS is a reserved flag
  Guard with RELEASE_BUILD_FLAGS_IN_PROTOBUF

7 files changed, 126 insertions, 43 deletions

diff --git a/cmd/release_config/build_flag/main.go b/cmd/release_config/build_flag/main.go
index ec7d64f1e..56c49d883 100644
--- a/cmd/release_config/build_flag/main.go
+++ b/cmd/release_config/build_flag/main.go
@@ -325,7 +325,12 @@ func main() {
 	}
 
 	if len(commonFlags.targetReleases) == 0 {
-		commonFlags.targetReleases = rc_lib.StringList{"trunk_staging"}
+		release, ok := os.LookupEnv("TARGET_RELEASE")
+		if ok {
+			commonFlags.targetReleases = rc_lib.StringList{release}
+		} else {
+			commonFlags.targetReleases = rc_lib.StringList{"trunk_staging"}
+		}
 	}
 
 	if err = os.Chdir(commonFlags.top); err != nil {
diff --git a/cmd/release_config/crunch_flags/main.go b/cmd/release_config/crunch_flags/main.go
index 95342b1d0..4d763c8d7 100644
--- a/cmd/release_config/crunch_flags/main.go
+++ b/cmd/release_config/crunch_flags/main.go
@@ -21,6 +21,7 @@ var (
 	manualFlagNamePrefixes []string = []string{
 		"RELEASE_ACONFIG_",
 		"RELEASE_PLATFORM_",
+		"RELEASE_BUILD_FLAGS_",
 	}
 
 	// Set `aconfig_flags_only: true` in these release configs.
diff --git a/cmd/release_config/release_config/main.go b/cmd/release_config/release_config/main.go
index c443257c6..fac31f092 100644
--- a/cmd/release_config/release_config/main.go
+++ b/cmd/release_config/release_config/main.go
@@ -35,6 +35,7 @@ func main() {
 	var product string
 	var allMake bool
 	var useBuildVar bool
+	var guard bool
 
 	defaultRelease := os.Getenv("TARGET_RELEASE")
 	if defaultRelease == "" {
@@ -52,6 +53,7 @@ func main() {
 	flag.BoolVar(&pb, "pb", true, "write artifacts as binary protobuf")
 	flag.BoolVar(&allMake, "all_make", true, "write makefiles for all release configs")
 	flag.BoolVar(&useBuildVar, "use_get_build_var", false, "use get_build_var PRODUCT_RELEASE_CONFIG_MAPS")
+	flag.BoolVar(&guard, "guard", true, "whether to guard with RELEASE_BUILD_FLAGS_IN_PROTOBUF")
 
 	flag.Parse()
 
@@ -76,20 +78,21 @@ func main() {
 		panic(err)
 	}
 
-	if err = config.WritePartitionBuildFlags(outputDir, product, targetRelease); err != nil {
-		panic(err)
-	}
-
-	if allMake {
+	makefilePath := filepath.Join(outputDir, fmt.Sprintf("release_config-%s-%s.mk", product, releaseName))
+	useProto, ok := config.FlagArtifacts["RELEASE_BUILD_FLAGS_IN_PROTOBUF"]
+	if guard && (!ok || rc_lib.MarshalValue(useProto.Value) == "") {
+		// We were told to guard operation and either we have no build flag, or it is False.
+		// Write an empty file so that release_config.mk will use the old process.
+		os.WriteFile(makefilePath, []byte{}, 0644)
+	} else if allMake {
 		for k, _ := range configs.ReleaseConfigs {
-			makefilePath := filepath.Join(outputDir, fmt.Sprintf("release_config-%s-%s.mk", product, k))
+			makefilePath = filepath.Join(outputDir, fmt.Sprintf("release_config-%s-%s.mk", product, k))
 			err = configs.WriteMakefile(makefilePath, k)
 			if err != nil {
 				panic(err)
 			}
 		}
 	} else {
-		makefilePath := filepath.Join(outputDir, fmt.Sprintf("release_config-%s-%s.mk", product, releaseName))
 		err = configs.WriteMakefile(makefilePath, targetRelease)
 		if err != nil {
 			panic(err)
@@ -113,4 +116,8 @@ func main() {
 			panic(err)
 		}
 	}
+	if err = config.WritePartitionBuildFlags(outputDir, product, targetRelease); err != nil {
+		panic(err)
+	}
+
 }
diff --git a/cmd/release_config/release_config_lib/flag_value.go b/cmd/release_config/release_config_lib/flag_value.go
index e155e7782..59021e260 100644
--- a/cmd/release_config/release_config_lib/flag_value.go
+++ b/cmd/release_config/release_config_lib/flag_value.go
@@ -52,6 +52,9 @@ func UnmarshalValue(str string) *rc_proto.Value {
 }
 
 func MarshalValue(value *rc_proto.Value) string {
+	if value == nil {
+		return ""
+	}
 	switch val := value.Val.(type) {
 	case *rc_proto.Value_UnspecifiedValue:
 		// Value was never set.
diff --git a/cmd/release_config/release_config_lib/flag_value_test.go b/cmd/release_config/release_config_lib/flag_value_test.go
index aaa4cafeb..8a98baf6a 100644
--- a/cmd/release_config/release_config_lib/flag_value_test.go
+++ b/cmd/release_config/release_config_lib/flag_value_test.go
@@ -24,7 +24,7 @@ import (
 	"google.golang.org/protobuf/proto"
 )
 
-type testCaseFlagValue struct {
+type testCaseFlagValueFactory struct {
 	protoPath string
 	name      string
 	data      []byte
@@ -32,14 +32,14 @@ type testCaseFlagValue struct {
 	err       error
 }
 
-func (tc testCaseFlagValue) assertProtoEqual(t *testing.T, expected, actual proto.Message) {
+func (tc testCaseFlagValueFactory) assertProtoEqual(t *testing.T, expected, actual proto.Message) {
 	if !proto.Equal(expected, actual) {
 		t.Errorf("Expected %q found %q", expected, actual)
 	}
 }
 
-func TestFlagValue(t *testing.T) {
-	testCases := []testCaseFlagValue{
+func TestFlagValueFactory(t *testing.T) {
+	testCases := []testCaseFlagValueFactory{
 		{
 			name:      "stringVal",
 			protoPath: "build/release/flag_values/test/RELEASE_FOO.textproto",
@@ -65,3 +65,50 @@ func TestFlagValue(t *testing.T) {
 		tc.assertProtoEqual(t, &tc.expected, &actual.proto)
 	}
 }
+
+type testCaseMarshalValue struct {
+	name     string
+	value    *rc_proto.Value
+	expected string
+}
+
+func TestMarshalValue(t *testing.T) {
+	testCases := []testCaseMarshalValue{
+		{
+			name:     "nil",
+			value:    nil,
+			expected: "",
+		},
+		{
+			name:     "unspecified",
+			value:    &rc_proto.Value{},
+			expected: "",
+		},
+		{
+			name:     "false",
+			value:    &rc_proto.Value{Val: &rc_proto.Value_BoolValue{false}},
+			expected: "",
+		},
+		{
+			name:     "true",
+			value:    &rc_proto.Value{Val: &rc_proto.Value_BoolValue{true}},
+			expected: "true",
+		},
+		{
+			name:     "string",
+			value:    &rc_proto.Value{Val: &rc_proto.Value_StringValue{"BAR"}},
+			expected: "BAR",
+		},
+		{
+			name:     "obsolete",
+			value:    &rc_proto.Value{Val: &rc_proto.Value_Obsolete{true}},
+			expected: " #OBSOLETE",
+		},
+	}
+	for _, tc := range testCases {
+		actual := MarshalValue(tc.value)
+		if actual != tc.expected {
+			t.Errorf("Expected %q found %q", tc.expected, actual)
+		}
+	}
+}
diff --git a/cmd/release_config/release_config_lib/release_config.go b/cmd/release_config/release_config_lib/release_config.go
index e51ff08b2..f25cc6e66 100644
--- a/cmd/release_config/release_config_lib/release_config.go
+++ b/cmd/release_config/release_config_lib/release_config.go
@@ -90,7 +90,13 @@ func (config *ReleaseConfig) InheritConfig(iConfig *ReleaseConfig) error {
 		if !ok {
 			return fmt.Errorf("Could not inherit flag %s from %s", name, iConfig.Name)
 		}
-		if len(fa.Traces) > 1 {
+		if name == "RELEASE_ACONFIG_VALUE_SETS" {
+			if len(fa.Traces) > 0 {
+				myFa.Traces = append(myFa.Traces, fa.Traces...)
+				myFa.Value = &rc_proto.Value{Val: &rc_proto.Value_StringValue{
+					myFa.Value.GetStringValue() + " " + fa.Value.GetStringValue()}}
+			}
+		} else if len(fa.Traces) > 1 {
 			// A value was assigned. Set our value.
 			myFa.Traces = append(myFa.Traces, fa.Traces[1:]...)
 			myFa.Value = fa.Value
@@ -111,21 +117,7 @@ func (config *ReleaseConfig) GenerateReleaseConfig(configs *ReleaseConfigs) erro
 
 	// Start with only the flag declarations.
 	config.FlagArtifacts = configs.FlagArtifacts.Clone()
-	// Add RELEASE_ACONFIG_VALUE_SETS
-	workflowManual := rc_proto.Workflow(rc_proto.Workflow_MANUAL)
-	releaseAconfigValueSets := FlagArtifact{
-		FlagDeclaration: &rc_proto.FlagDeclaration{
-			Name:        proto.String("RELEASE_ACONFIG_VALUE_SETS"),
-			Namespace:   proto.String("android_UNKNOWN"),
-			Description: proto.String("Aconfig value sets assembled by release-config"),
-			Workflow:    &workflowManual,
-			Containers:  []string{"system", "system_ext", "product", "vendor"},
-			Value:       &rc_proto.Value{Val: &rc_proto.Value_UnspecifiedValue{false}},
-		},
-		DeclarationIndex: -1,
-		Traces:           []*rc_proto.Tracepoint{},
-	}
-	config.FlagArtifacts["RELEASE_ACONFIG_VALUE_SETS"] = &releaseAconfigValueSets
+	releaseAconfigValueSets := config.FlagArtifacts["RELEASE_ACONFIG_VALUE_SETS"]
 
 	// Generate any configs we need to inherit.  This will detect loops in
 	// the config.
@@ -154,27 +146,22 @@ func (config *ReleaseConfig) GenerateReleaseConfig(configs *ReleaseConfigs) erro
 	}
 	contributionsToApply = append(contributionsToApply, config.Contributions...)
 
-	myAconfigValueSets := strings.Split(releaseAconfigValueSets.Value.GetStringValue(), " ")
-	myAconfigValueSetsMap := map[string]bool{}
-	for _, v := range myAconfigValueSets {
-		myAconfigValueSetsMap[v] = true
-	}
+	workflowManual := rc_proto.Workflow(rc_proto.Workflow_MANUAL)
 	myDirsMap := make(map[int]bool)
 	for _, contrib := range contributionsToApply {
 		contribAconfigValueSets := []string{}
-		// Gather the aconfig_value_sets from this contribution that are not already in the list.
+		// Gather the aconfig_value_sets from this contribution, allowing duplicates for simplicity.
 		for _, v := range contrib.proto.AconfigValueSets {
-			if _, ok := myAconfigValueSetsMap[v]; !ok {
-				contribAconfigValueSets = append(contribAconfigValueSets, v)
-				myAconfigValueSetsMap[v] = true
-			}
+			contribAconfigValueSets = append(contribAconfigValueSets, v)
 		}
-		myAconfigValueSets = append(myAconfigValueSets, contribAconfigValueSets...)
+		contribAconfigValueSetsString := strings.Join(contribAconfigValueSets, " ")
+		releaseAconfigValueSets.Value = &rc_proto.Value{Val: &rc_proto.Value_StringValue{
+			releaseAconfigValueSets.Value.GetStringValue() + " " + contribAconfigValueSetsString}}
 		releaseAconfigValueSets.Traces = append(
 			releaseAconfigValueSets.Traces,
 			&rc_proto.Tracepoint{
 				Source: proto.String(contrib.path),
-				Value:  &rc_proto.Value{Val: &rc_proto.Value_StringValue{strings.Join(contribAconfigValueSets, " ")}},
+				Value:  &rc_proto.Value{Val: &rc_proto.Value_StringValue{contribAconfigValueSetsString}},
 			})
 
 		myDirsMap[contrib.DeclarationIndex] = true
@@ -204,6 +191,16 @@ func (config *ReleaseConfig) GenerateReleaseConfig(configs *ReleaseConfigs) erro
 			}
 		}
 	}
+	// Now remove any duplicates from the actual value of RELEASE_ACONFIG_VALUE_SETS
+	myAconfigValueSets := []string{}
+	myAconfigValueSetsMap := map[string]bool{}
+	for _, v := range strings.Split(releaseAconfigValueSets.Value.GetStringValue(), " ") {
+		if myAconfigValueSetsMap[v] {
+			continue
+		}
+		myAconfigValueSetsMap[v] = true
+		myAconfigValueSets = append(myAconfigValueSets, v)
+	}
 	releaseAconfigValueSets.Value = &rc_proto.Value{Val: &rc_proto.Value_StringValue{strings.TrimSpace(strings.Join(myAconfigValueSets, " "))}}
 
 	directories := []string{}
diff --git a/cmd/release_config/release_config_lib/release_configs.go b/cmd/release_config/release_config_lib/release_configs.go
index cedf247f8..34294002c 100644
--- a/cmd/release_config/release_config_lib/release_configs.go
+++ b/cmd/release_config/release_config_lib/release_configs.go
@@ -95,7 +95,7 @@ func (configs *ReleaseConfigs) WriteArtifact(outDir, product, format string) err
 }
 
 func ReleaseConfigsFactory() (c *ReleaseConfigs) {
-	return &ReleaseConfigs{
+	configs := ReleaseConfigs{
 		Aliases:              make(map[string]*string),
 		FlagArtifacts:        make(map[string]*FlagArtifact),
 		ReleaseConfigs:       make(map[string]*ReleaseConfig),
@@ -103,6 +103,21 @@ func ReleaseConfigsFactory() (c *ReleaseConfigs) {
 		configDirs:           []string{},
 		configDirIndexes:     make(ReleaseConfigDirMap),
 	}
+	workflowManual := rc_proto.Workflow(rc_proto.Workflow_MANUAL)
+	releaseAconfigValueSets := FlagArtifact{
+		FlagDeclaration: &rc_proto.FlagDeclaration{
+			Name:        proto.String("RELEASE_ACONFIG_VALUE_SETS"),
+			Namespace:   proto.String("android_UNKNOWN"),
+			Description: proto.String("Aconfig value sets assembled by release-config"),
+			Workflow:    &workflowManual,
+			Containers:  []string{"system", "system_ext", "product", "vendor"},
+			Value:       &rc_proto.Value{Val: &rc_proto.Value_UnspecifiedValue{false}},
+		},
+		DeclarationIndex: -1,
+		Traces:           []*rc_proto.Tracepoint{},
+	}
+	configs.FlagArtifacts["RELEASE_ACONFIG_VALUE_SETS"] = &releaseAconfigValueSets
+	return &configs
 }
 
 func ReleaseConfigMapFactory(protoPath string) (m *ReleaseConfigMap) {
@@ -166,6 +181,9 @@ func (configs *ReleaseConfigs) LoadReleaseConfigMap(path string, ConfigDirIndex
 		}
 		m.FlagDeclarations = append(m.FlagDeclarations, *flagDeclaration)
 		name := *flagDeclaration.Name
+		if name == "RELEASE_ACONFIG_VALUE_SETS" {
+			return fmt.Errorf("%s: %s is a reserved build flag", path, name)
+		}
 		if def, ok := configs.FlagArtifacts[name]; !ok {
 			configs.FlagArtifacts[name] = &FlagArtifact{FlagDeclaration: flagDeclaration, DeclarationIndex: ConfigDirIndex}
 		} else if !proto.Equal(def.FlagDeclaration, flagDeclaration) {
@@ -176,7 +194,7 @@ func (configs *ReleaseConfigs) LoadReleaseConfigMap(path string, ConfigDirIndex
 			FlagValue{path: path, proto: rc_proto.FlagValue{
 				Name: proto.String(name), Value: flagDeclaration.Value}})
 		if configs.FlagArtifacts[name].Redacted {
-			return fmt.Errorf("%s may not be redacted by default.", *flagDeclaration.Name)
+			return fmt.Errorf("%s may not be redacted by default.", name)
 		}
 		return nil
 	})
@@ -203,6 +221,9 @@ func (configs *ReleaseConfigs) LoadReleaseConfigMap(path string, ConfigDirIndex
 			if fmt.Sprintf("%s.textproto", *flagValue.proto.Name) != filepath.Base(path) {
 				return fmt.Errorf("%s incorrectly sets value for flag %s", path, *flagValue.proto.Name)
 			}
+			if *flagValue.proto.Name == "RELEASE_ACONFIG_VALUE_SETS" {
+				return fmt.Errorf("%s: %s is a reserved build flag", path, *flagValue.proto.Name)
+			}
 			releaseConfigContribution.FlagValues = append(releaseConfigContribution.FlagValues, flagValue)
 			return nil
 		})
@@ -382,7 +403,9 @@ func ReadReleaseConfigMaps(releaseConfigMapPaths StringList, targetRelease strin
 		if len(releaseConfigMapPaths) == 0 {
 			return nil, fmt.Errorf("No maps found")
 		}
-		warnf("No --map argument provided.  Using: --map %s\n", strings.Join(releaseConfigMapPaths, " --map "))
+		if !useBuildVar {
+			warnf("No --map argument provided.  Using: --map %s\n", strings.Join(releaseConfigMapPaths, " --map "))
+		}
 	}
 
 	configs := ReleaseConfigsFactory()