release_config: various cleanup

- Allow scl and textproto data to coexist for now
- Print warnings to stderr instead of stdout.
- Improve formatting of output
- Set displays the new value for all configs, and what file changed.
- Use prettier error messages for better UX
- put build-flag on the path.

Bug: 328495189
Test: manual
Change-Id: I4c38860c2fb24db5111e0cecf790660a4ff2b8b2

5 files changed, 205 insertions, 45 deletions

diff --git a/bin/build-flag b/bin/build-flag
new file mode 100755
index 000000000..dc404bc97
--- /dev/null
+++ b/bin/build-flag
@@ -0,0 +1,28 @@
+#!/bin/bash -eu
+#
+# Copyright 2017 Google Inc. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+source $(cd $(dirname $BASH_SOURCE) &> /dev/null && pwd)/../../make/shell_utils.sh
+require_top
+
+# Save the current PWD for use in soong_ui
+export ORIGINAL_PWD=${PWD}
+export TOP=$(gettop)
+source ${TOP}/build/soong/scripts/microfactory.bash
+
+soong_build_go build-flag android/soong/cmd/release_config/build_flag
+
+cd ${TOP}
+exec "$(getoutdir)/build-flag" "$@"
diff --git a/cmd/release_config/build_flag/main.go b/cmd/release_config/build_flag/main.go
index 67edc3517..ec7d64f1e 100644
--- a/cmd/release_config/build_flag/main.go
+++ b/cmd/release_config/build_flag/main.go
@@ -1,10 +1,12 @@
 package main
 
 import (
+	"cmp"
 	"flag"
 	"fmt"
 	"os"
 	"path/filepath"
+	"slices"
 	"strings"
 
 	rc_lib "android/soong/cmd/release_config/release_config_lib"
@@ -36,6 +38,16 @@ type Flags struct {
 
 	// Disable warning messages
 	quiet bool
+
+	// Show all release configs
+	allReleases bool
+
+	// Call get_build_var PRODUCT_RELEASE_CONFIG_MAPS to get the
+	// product-specific map directories.
+	useGetBuildVar bool
+
+	// Panic on errors.
+	debug bool
 }
 
 type CommandFunc func(*rc_lib.ReleaseConfigs, Flags, string, []string) error
@@ -60,6 +72,14 @@ func GetMapDir(path string) (string, error) {
 	return "", fmt.Errorf("Could not determine directory from %s", path)
 }
 
+func MarshalFlagDefaultValue(config *rc_lib.ReleaseConfig, name string) (ret string, err error) {
+	fa, ok := config.FlagArtifacts[name]
+	if !ok {
+		return "", fmt.Errorf("%s not found in %s", name, config.Name)
+	}
+	return rc_lib.MarshalValue(fa.Traces[0].Value), nil
+}
+
 func MarshalFlagValue(config *rc_lib.ReleaseConfig, name string) (ret string, err error) {
 	fa, ok := config.FlagArtifacts[name]
 	if !ok {
@@ -68,19 +88,41 @@ func MarshalFlagValue(config *rc_lib.ReleaseConfig, name string) (ret string, er
 	return rc_lib.MarshalValue(fa.Value), nil
 }
 
+// Returns a list of ReleaseConfig objects for which to process flags.
 func GetReleaseArgs(configs *rc_lib.ReleaseConfigs, commonFlags Flags) ([]*rc_lib.ReleaseConfig, error) {
 	var all bool
-	relFlags := flag.NewFlagSet("set", flag.ExitOnError)
-	relFlags.BoolVar(&all, "all", false, "Display all flags")
+	relFlags := flag.NewFlagSet("releaseFlags", flag.ExitOnError)
+	relFlags.BoolVar(&all, "all", false, "Display all releases")
 	relFlags.Parse(commonFlags.targetReleases)
 	var ret []*rc_lib.ReleaseConfig
-	if all {
+	if all || commonFlags.allReleases {
+		sortMap := map[string]int{
+			"trunk_staging": 0,
+			"trunk_food":    10,
+			"trunk":         20,
+			// Anything not listed above, uses this for key 1 in the sort.
+			"-default": 100,
+		}
+
 		for _, config := range configs.ReleaseConfigs {
 			ret = append(ret, config)
 		}
+		slices.SortFunc(ret, func(a, b *rc_lib.ReleaseConfig) int {
+			mapValue := func(v *rc_lib.ReleaseConfig) int {
+				if v, ok := sortMap[v.Name]; ok {
+					return v
+				}
+				return sortMap["-default"]
+			}
+			if n := cmp.Compare(mapValue(a), mapValue(b)); n != 0 {
+				return n
+			}
+			return cmp.Compare(a.Name, b.Name)
+		})
 		return ret, nil
 	}
 	for _, arg := range relFlags.Args() {
+		// Return releases in the order that they were given.
 		config, err := configs.GetReleaseConfig(arg)
 		if err != nil {
 			return nil, err
@@ -92,12 +134,17 @@ func GetReleaseArgs(configs *rc_lib.ReleaseConfigs, commonFlags Flags) ([]*rc_li
 
 func GetCommand(configs *rc_lib.ReleaseConfigs, commonFlags Flags, cmd string, args []string) error {
 	isTrace := cmd == "trace"
+	isSet := cmd == "set"
+
 	var all bool
-	getFlags := flag.NewFlagSet("set", flag.ExitOnError)
+	getFlags := flag.NewFlagSet("get", flag.ExitOnError)
 	getFlags.BoolVar(&all, "all", false, "Display all flags")
 	getFlags.Parse(args)
 	args = getFlags.Args()
 
+	if isSet {
+		commonFlags.allReleases = true
+	}
 	releaseConfigList, err := GetReleaseArgs(configs, commonFlags)
 	if err != nil {
 		return err
@@ -113,21 +160,72 @@ func GetCommand(configs *rc_lib.ReleaseConfigs, commonFlags Flags, cmd string, a
 		}
 	}
 
-	showName := len(releaseConfigList) > 1 || len(args) > 1
-	for _, config := range releaseConfigList {
-		var configName string
-		if len(releaseConfigList) > 1 {
-			configName = fmt.Sprintf("%s.", config.Name)
-		}
+	var maxVariableNameLen, maxReleaseNameLen int
+	var releaseNameFormat, variableNameFormat string
+	valueFormat := "%s"
+	showReleaseName := len(releaseConfigList) > 1
+	showVariableName := len(args) > 1
+	if showVariableName {
 		for _, arg := range args {
-			val, err := MarshalFlagValue(config, arg)
-			if err != nil {
-				return err
+			maxVariableNameLen = max(len(arg), maxVariableNameLen)
+		}
+		variableNameFormat = fmt.Sprintf("%%-%ds ", maxVariableNameLen)
+		valueFormat = "'%s'"
+	}
+	if showReleaseName {
+		for _, config := range releaseConfigList {
+			maxReleaseNameLen = max(len(config.Name), maxReleaseNameLen)
+		}
+		releaseNameFormat = fmt.Sprintf("%%-%ds ", maxReleaseNameLen)
+		valueFormat = "'%s'"
+	}
+
+	outputOneLine := func(variable, release, value, valueFormat string) {
+		var outStr string
+		if showVariableName {
+			outStr += fmt.Sprintf(variableNameFormat, variable)
+		}
+		if showReleaseName {
+			outStr += fmt.Sprintf(releaseNameFormat, release)
+		}
+		outStr += fmt.Sprintf(valueFormat, value)
+		fmt.Println(outStr)
+	}
+
+	for _, arg := range args {
+		if _, ok := configs.FlagArtifacts[arg]; !ok {
+			return fmt.Errorf("%s is not a defined build flag", arg)
+		}
+	}
+
+	for _, arg := range args {
+		for _, config := range releaseConfigList {
+			if isSet {
+				// If this is from the set command, format the output as:
+				// <default>           ""
+				// trunk_staging       ""
+				// trunk               ""
+				//
+				// ap1a                ""
+				// ...
+				switch {
+				case config.Name == "trunk_staging":
+					defaultValue, err := MarshalFlagDefaultValue(config, arg)
+					if err != nil {
+						return err
+					}
+					outputOneLine(arg, "<default>", defaultValue, valueFormat)
+				case config.AconfigFlagsOnly:
+					continue
+				case config.Name == "trunk":
+					fmt.Println()
+				}
 			}
-			if showName {
-				fmt.Printf("%s%s=%s\n", configName, arg, val)
+			val, err := MarshalFlagValue(config, arg)
+			if err == nil {
+				outputOneLine(arg, config.Name, val, valueFormat)
 			} else {
-				fmt.Printf("%s\n", val)
+				outputOneLine(arg, config.Name, "REDACTED", "%s")
 			}
 			if isTrace {
 				for _, trace := range config.FlagArtifacts[arg].Traces {
@@ -180,28 +278,48 @@ func SetCommand(configs *rc_lib.ReleaseConfigs, commonFlags Flags, cmd string, a
 		Value: rc_lib.UnmarshalValue(value),
 	}
 	flagPath := filepath.Join(valueDir, "flag_values", targetRelease, fmt.Sprintf("%s.textproto", name))
-	return rc_lib.WriteMessage(flagPath, flagValue)
+	err = rc_lib.WriteMessage(flagPath, flagValue)
+	if err != nil {
+		return err
+	}
+
+	// Reload the release configs.
+	configs, err = rc_lib.ReadReleaseConfigMaps(commonFlags.maps, commonFlags.targetReleases[0], commonFlags.useGetBuildVar)
+	if err != nil {
+		return err
+	}
+	err = GetCommand(configs, commonFlags, cmd, args[0:1])
+	if err != nil {
+		return err
+	}
+	fmt.Printf("Updated: %s\n", flagPath)
+	return nil
 }
 
 func main() {
-	var err error
 	var commonFlags Flags
 	var configs *rc_lib.ReleaseConfigs
-	var useBuildVar bool
+	topDir, err := rc_lib.GetTopDir()
 
-	outEnv := os.Getenv("OUT_DIR")
-	if outEnv == "" {
-		outEnv = "out"
-	}
 	// Handle the common arguments
-	flag.StringVar(&commonFlags.top, "top", ".", "path to top of workspace")
+	flag.StringVar(&commonFlags.top, "top", topDir, "path to top of workspace")
 	flag.BoolVar(&commonFlags.quiet, "quiet", false, "disable warning messages")
 	flag.Var(&commonFlags.maps, "map", "path to a release_config_map.textproto. may be repeated")
-	flag.StringVar(&commonFlags.outDir, "out_dir", rc_lib.GetDefaultOutDir(), "basepath for the output. Multiple formats are created")
+	flag.StringVar(&commonFlags.outDir, "out-dir", rc_lib.GetDefaultOutDir(), "basepath for the output. Multiple formats are created")
 	flag.Var(&commonFlags.targetReleases, "release", "TARGET_RELEASE for this build")
-	flag.BoolVar(&useBuildVar, "use_get_build_var", true, "use get_build_var PRODUCT_RELEASE_CONFIG_MAPS")
+	flag.BoolVar(&commonFlags.allReleases, "all-releases", false, "operate on all releases. (Ignored for set command)")
+	flag.BoolVar(&commonFlags.useGetBuildVar, "use-get-build-var", true, "use get_build_var PRODUCT_RELEASE_CONFIG_MAPS to get needed maps")
+	flag.BoolVar(&commonFlags.debug, "debug", false, "turn on debugging output for errors")
 	flag.Parse()
 
+	errorExit := func(err error) {
+		if commonFlags.debug {
+			panic(err)
+		}
+		fmt.Fprintf(os.Stderr, "%s\n", err)
+		os.Exit(1)
+	}
+
 	if commonFlags.quiet {
 		rc_lib.DisableWarnings()
 	}
@@ -211,24 +329,23 @@ func main() {
 	}
 
 	if err = os.Chdir(commonFlags.top); err != nil {
-		panic(err)
+		errorExit(err)
 	}
 
 	// Get the current state of flagging.
 	relName := commonFlags.targetReleases[0]
 	if relName == "--all" || relName == "-all" {
-		// If the users said `--release --all`, grab trunk staging for simplicity.
-		relName = "trunk_staging"
+		commonFlags.allReleases = true
 	}
-	configs, err = rc_lib.ReadReleaseConfigMaps(commonFlags.maps, relName, true)
+	configs, err = rc_lib.ReadReleaseConfigMaps(commonFlags.maps, relName, commonFlags.useGetBuildVar)
 	if err != nil {
-		panic(err)
+		errorExit(err)
 	}
 
 	if cmd, ok := commandMap[flag.Arg(0)]; ok {
 		args := flag.Args()
 		if err = cmd(configs, commonFlags, args[0], args[1:]); err != nil {
-			panic(err)
+			errorExit(err)
 		}
 	}
 }
diff --git a/cmd/release_config/crunch_flags/main.go b/cmd/release_config/crunch_flags/main.go
index e107b9fbb..95342b1d0 100644
--- a/cmd/release_config/crunch_flags/main.go
+++ b/cmd/release_config/crunch_flags/main.go
@@ -216,7 +216,7 @@ func ProcessBuildConfigs(dir, name string, paths []string, releaseProto *rc_prot
 				fmt.Printf("%s: Unexpected value %s=%s\n", path, valName, valValue)
 			}
 			if flagValue != nil {
-				if releaseProto.AconfigFlagsOnly {
+				if releaseProto.GetAconfigFlagsOnly() {
 					return fmt.Errorf("%s does not allow build flag overrides", RenameNext(name))
 				}
 				valPath := filepath.Join(dir, "flag_values", RenameNext(name), fmt.Sprintf("%s.textproto", valName))
@@ -300,7 +300,7 @@ func ProcessReleaseConfigMap(dir string, descriptionMap map[string]string) error
 			Name: proto.String(RenameNext(name)),
 		}
 		if aconfigFlagsOnlyConfigs[name] {
-			releaseConfig.AconfigFlagsOnly = true
+			releaseConfig.AconfigFlagsOnly = proto.Bool(true)
 		}
 		configFiles := config[configRegexp.SubexpIndex("files")]
 		files := strings.Split(strings.ReplaceAll(configFiles, "$(local_dir)", dir+"/"), " ")
@@ -328,16 +328,26 @@ func main() {
 	var dirs rc_lib.StringList
 	var namespacesFile string
 	var descriptionsFile string
+	var debug bool
 	defaultTopDir, err := rc_lib.GetTopDir()
 
 	flag.StringVar(&top, "top", defaultTopDir, "path to top of workspace")
 	flag.Var(&dirs, "dir", "directory to process, relative to the top of the workspace")
 	flag.StringVar(&namespacesFile, "namespaces", "", "location of file with 'flag_name namespace' information")
 	flag.StringVar(&descriptionsFile, "descriptions", "", "location of file with 'directory description' information")
+	flag.BoolVar(&debug, "debug", false, "turn on debugging output for errors")
 	flag.Parse()
 
+	errorExit := func(err error) {
+		if debug {
+			panic(err)
+		}
+		fmt.Fprintf(os.Stderr, "%s\n", err)
+		os.Exit(1)
+	}
+
 	if err = os.Chdir(top); err != nil {
-		panic(err)
+		errorExit(err)
 	}
 	if len(dirs) == 0 {
 		dirs = rc_lib.StringList{"build/release", "vendor/google_shared/build/release", "vendor/google/release"}
@@ -347,12 +357,12 @@ func main() {
 	if namespacesFile != "" {
 		data, err := os.ReadFile(namespacesFile)
 		if err != nil {
-			panic(err)
+			errorExit(err)
 		}
 		for idx, line := range strings.Split(string(data), "\n") {
 			fields := strings.Split(line, " ")
 			if len(fields) > 2 {
-				panic(fmt.Errorf("line %d: too many fields: %s", idx, line))
+				errorExit(fmt.Errorf("line %d: too many fields: %s", idx, line))
 			}
 			namespaceMap[fields[0]] = fields[1]
 		}
@@ -364,7 +374,7 @@ func main() {
 	if descriptionsFile != "" {
 		data, err := os.ReadFile(descriptionsFile)
 		if err != nil {
-			panic(err)
+			errorExit(err)
 		}
 		for _, line := range strings.Split(string(data), "\n") {
 			if strings.TrimSpace(line) != "" {
@@ -378,12 +388,12 @@ func main() {
 	for _, dir := range dirs {
 		err = ProcessBuildFlags(dir, namespaceMap)
 		if err != nil {
-			panic(err)
+			errorExit(err)
 		}
 
 		err = ProcessReleaseConfigMap(dir, descriptionMap)
 		if err != nil {
-			panic(err)
+			errorExit(err)
 		}
 	}
 }
diff --git a/cmd/release_config/release_config_lib/release_configs.go b/cmd/release_config/release_config_lib/release_configs.go
index e5fd99e7c..cedf247f8 100644
--- a/cmd/release_config/release_config_lib/release_configs.go
+++ b/cmd/release_config/release_config_lib/release_configs.go
@@ -117,9 +117,12 @@ func ReleaseConfigMapFactory(protoPath string) (m *ReleaseConfigMap) {
 }
 
 func (configs *ReleaseConfigs) LoadReleaseConfigMap(path string, ConfigDirIndex int) error {
+	if _, err := os.Stat(path); err != nil {
+		return fmt.Errorf("%s does not exist\n", path)
+	}
 	m := ReleaseConfigMapFactory(path)
 	if m.proto.DefaultContainers == nil {
-		return fmt.Errorf("Release config map %s lacks default_container", path)
+		return fmt.Errorf("Release config map %s lacks default_containers", path)
 	}
 	for _, container := range m.proto.DefaultContainers {
 		if !validContainer(container) {
@@ -379,7 +382,7 @@ func ReadReleaseConfigMaps(releaseConfigMapPaths StringList, targetRelease strin
 		if len(releaseConfigMapPaths) == 0 {
 			return nil, fmt.Errorf("No maps found")
 		}
-		fmt.Printf("No --map argument provided.  Using: --map %s\n", strings.Join(releaseConfigMapPaths, " --map "))
+		warnf("No --map argument provided.  Using: --map %s\n", strings.Join(releaseConfigMapPaths, " --map "))
 	}
 
 	configs := ReleaseConfigsFactory()
@@ -393,6 +396,8 @@ func ReadReleaseConfigMaps(releaseConfigMapPaths StringList, targetRelease strin
 		mapsRead[configDir] = true
 		configs.configDirIndexes[configDir] = idx
 		configs.configDirs = append(configs.configDirs, configDir)
+		// Force the path to be the textproto path, so that both the scl and textproto formats can coexist.
+		releaseConfigMapPath = filepath.Join(configDir, "release_config_map.textproto")
 		err = configs.LoadReleaseConfigMap(releaseConfigMapPath, idx)
 		if err != nil {
 			return nil, err
diff --git a/cmd/release_config/release_config_lib/util.go b/cmd/release_config/release_config_lib/util.go
index 52a1f85de..c0ea7897b 100644
--- a/cmd/release_config/release_config_lib/util.go
+++ b/cmd/release_config/release_config_lib/util.go
@@ -128,7 +128,7 @@ func DisableWarnings() {
 
 func warnf(format string, args ...any) (n int, err error) {
 	if !disableWarnings {
-		return fmt.Printf(format, args...)
+		return fmt.Fprintf(os.Stderr, format, args...)
 	}
 	return 0, nil
 }
@@ -164,8 +164,8 @@ func GetTopDir() (topDir string, err error) {
 }
 
 // Return the default list of map files to use.
-func GetDefaultMapPaths(queryMaps bool) (defaultLocations StringList, err error) {
-	var defaultMapPaths StringList
+func GetDefaultMapPaths(queryMaps bool) (defaultMapPaths StringList, err error) {
+	var defaultLocations StringList
 	workingDir, err := os.Getwd()
 	if err != nil {
 		return