Make crosstool to starlark converter error out if it comes across multiple expand_if_all_available or expand_if_none_available in a single flag_group
PiperOrigin-RevId: 232499399
diff --git a/tools/migration/crosstool_to_starlark_lib.go b/tools/migration/crosstool_to_starlark_lib.go
index a3e230c..01fe45e 100644
--- a/tools/migration/crosstool_to_starlark_lib.go
+++ b/tools/migration/crosstool_to_starlark_lib.go
@@ -8,6 +8,7 @@
import (
"bytes"
+ "errors"
"fmt"
"sort"
"strings"
@@ -352,7 +353,7 @@
}
func getFeatures(crosstool *crosstoolpb.CrosstoolRelease) (
- map[string][]string, map[string]map[string][]string) {
+ map[string][]string, map[string]map[string][]string, error) {
featureNameToFeature := make(map[string]map[string][]string)
toolchainToFeatures := make(map[string][]string)
for _, toolchain := range crosstool.GetToolchain() {
@@ -364,7 +365,11 @@
featureName := strings.ToLower(feature.GetName()) + "_feature"
featureName = strings.Replace(featureName, "+", "p", -1)
featureName = strings.Replace(featureName, ".", "_", -1)
- stringFeature := parseFeature(feature, 1)
+ stringFeature, err := parseFeature(feature, 1)
+ if err != nil {
+ return nil, nil, fmt.Errorf(
+ "Error in feature '%s': %v", feature.GetName(), err)
+ }
if _, ok := featureNameToFeature[featureName]; !ok {
featureNameToFeature[featureName] = make(map[string][]string)
}
@@ -373,7 +378,7 @@
toolchainToFeatures[id] = append(toolchainToFeatures[id], featureName)
}
}
- return toolchainToFeatures, featureNameToFeature
+ return toolchainToFeatures, featureNameToFeature, nil
}
func getFeaturesDeclaration(crosstool *crosstoolpb.CrosstoolRelease,
@@ -412,7 +417,7 @@
}
func getActions(crosstool *crosstoolpb.CrosstoolRelease) (
- map[string][]string, map[string]map[string][]string) {
+ map[string][]string, map[string]map[string][]string, error) {
actionNameToAction := make(map[string]map[string][]string)
toolchainToActions := make(map[string][]string)
for _, toolchain := range crosstool.GetToolchain() {
@@ -429,7 +434,11 @@
actionName = strings.Replace(actionName, "+", "p", -1)
actionName = strings.Replace(actionName, ".", "_", -1)
}
- stringAction := parseAction(action, 1)
+ stringAction, err := parseAction(action, 1)
+ if err != nil {
+ return nil, nil, fmt.Errorf(
+ "Error in action_config '%s': %v", action.GetActionName(), err)
+ }
if _, ok := actionNameToAction[actionName]; !ok {
actionNameToAction[actionName] = make(map[string][]string)
}
@@ -440,7 +449,7 @@
strings.TrimPrefix(strings.ToLower(actionName), "action_names.")+"_action")
}
}
- return toolchainToActions, actionNameToAction
+ return toolchainToActions, actionNameToAction, nil
}
func getActionConfigsDeclaration(
@@ -482,7 +491,7 @@
return strings.Join(res, "\n")
}
-func parseAction(action *crosstoolpb.CToolchain_ActionConfig, depth int) string {
+func parseAction(action *crosstoolpb.CToolchain_ActionConfig, depth int) (string, error) {
actionName := action.GetActionName()
aName := ""
if val, ok := actionNames[actionName]; ok {
@@ -496,9 +505,11 @@
fields = append(fields, "enabled = True")
}
if len(action.GetFlagSet()) != 0 {
- flagSets := "flag_sets = " +
- parseFlagSets(action.GetFlagSet(), depth+1)
- fields = append(fields, flagSets)
+ flagSets, err := parseFlagSets(action.GetFlagSet(), depth+1)
+ if err != nil {
+ return "", err
+ }
+ fields = append(fields, "flag_sets = "+flagSets)
}
if len(action.GetImplies()) != 0 {
implies := "implies = " +
@@ -509,7 +520,7 @@
tools := "tools = " + parseTools(action.GetTool(), depth+1)
fields = append(fields, tools)
}
- return createObject("action_config", fields, depth)
+ return createObject("action_config", fields, depth), nil
}
func getStringArrStatement(attr string, arrValToIds map[string][]string,
@@ -660,7 +671,7 @@
return createObject("artifact_name_pattern", fields, depth)
}
-func parseFeature(feature *crosstoolpb.CToolchain_Feature, depth int) string {
+func parseFeature(feature *crosstoolpb.CToolchain_Feature, depth int) (string, error) {
name := fmt.Sprintf("name = \"%s\"", feature.GetName())
fields := []string{name}
@@ -669,9 +680,11 @@
}
if len(feature.GetFlagSet()) > 0 {
- flagSets := "flag_sets = " +
- parseFlagSets(feature.GetFlagSet(), depth+1)
- fields = append(fields, flagSets)
+ flagSets, err := parseFlagSets(feature.GetFlagSet(), depth+1)
+ if err != nil {
+ return "", err
+ }
+ fields = append(fields, "flag_sets = "+flagSets)
}
if len(feature.GetEnvSet()) > 0 {
envSets := "env_sets = " + parseEnvSets(feature.GetEnvSet(), depth+1)
@@ -691,19 +704,22 @@
makeStringArr(feature.GetProvides(), depth+1 /* isPlainString= */, true)
fields = append(fields, provides)
}
- return createObject("feature", fields, depth)
+ return createObject("feature", fields, depth), nil
}
-func parseFlagSets(flagSets []*crosstoolpb.CToolchain_FlagSet, depth int) string {
+func parseFlagSets(flagSets []*crosstoolpb.CToolchain_FlagSet, depth int) (string, error) {
var res []string
for _, flagSet := range flagSets {
- parsedFlagset := parseFlagSet(flagSet, depth+1)
+ parsedFlagset, err := parseFlagSet(flagSet, depth+1)
+ if err != nil {
+ return "", err
+ }
res = append(res, parsedFlagset)
}
- return makeStringArr(res, depth /* isPlainString= */, false)
+ return makeStringArr(res, depth /* isPlainString= */, false), nil
}
-func parseFlagSet(flagSet *crosstoolpb.CToolchain_FlagSet, depth int) string {
+func parseFlagSet(flagSet *crosstoolpb.CToolchain_FlagSet, depth int) (string, error) {
var fields []string
if len(flagSet.GetAction()) > 0 {
actionArr := processActions(flagSet.GetAction(), depth)
@@ -711,27 +727,33 @@
fields = append(fields, actions)
}
if len(flagSet.GetFlagGroup()) > 0 {
- flagGroups := "flag_groups = " +
- parseFlagGroups(flagSet.GetFlagGroup(), depth+1)
- fields = append(fields, flagGroups)
+ flagGroups, err := parseFlagGroups(flagSet.GetFlagGroup(), depth+1)
+ if err != nil {
+ return "", err
+ }
+ fields = append(fields, "flag_groups = "+flagGroups)
}
if len(flagSet.GetWithFeature()) > 0 {
withFeatures := "with_features = " +
parseWithFeatureSets(flagSet.GetWithFeature(), depth+1)
fields = append(fields, withFeatures)
}
- return createObject("flag_set", fields, depth)
+ return createObject("flag_set", fields, depth), nil
}
-func parseFlagGroups(flagGroups []*crosstoolpb.CToolchain_FlagGroup, depth int) string {
+func parseFlagGroups(flagGroups []*crosstoolpb.CToolchain_FlagGroup, depth int) (string, error) {
var res []string
for _, flagGroup := range flagGroups {
- res = append(res, parseFlagGroup(flagGroup, depth+1))
+ flagGroupString, err := parseFlagGroup(flagGroup, depth+1)
+ if err != nil {
+ return "", err
+ }
+ res = append(res, flagGroupString)
}
- return makeStringArr(res, depth /* isPlainString= */, false)
+ return makeStringArr(res, depth /* isPlainString= */, false), nil
}
-func parseFlagGroup(flagGroup *crosstoolpb.CToolchain_FlagGroup, depth int) string {
+func parseFlagGroup(flagGroup *crosstoolpb.CToolchain_FlagGroup, depth int) (string, error) {
var res []string
if len(flagGroup.GetFlag()) != 0 {
res = append(res, "flags = "+makeStringArr(flagGroup.GetFlag(), depth+1, true))
@@ -740,7 +762,14 @@
res = append(res, fmt.Sprintf("iterate_over = \"%s\"", flagGroup.GetIterateOver()))
}
if len(flagGroup.GetFlagGroup()) != 0 {
- res = append(res, "flag_groups = "+parseFlagGroups(flagGroup.GetFlagGroup(), depth+1))
+ flagGroupString, err := parseFlagGroups(flagGroup.GetFlagGroup(), depth+1)
+ if err != nil {
+ return "", err
+ }
+ res = append(res, "flag_groups = "+flagGroupString)
+ }
+ if len(flagGroup.GetExpandIfAllAvailable()) > 1 {
+ return "", errors.New("Flag group must not have more than one 'expand_if_all_available' field")
}
if len(flagGroup.GetExpandIfAllAvailable()) != 0 {
res = append(res,
@@ -748,6 +777,9 @@
"expand_if_available = \"%s\"",
flagGroup.GetExpandIfAllAvailable()[0]))
}
+ if len(flagGroup.GetExpandIfNoneAvailable()) > 1 {
+ return "", errors.New("Flag group must not have more than one 'expand_if_none_available' field")
+ }
if len(flagGroup.GetExpandIfNoneAvailable()) != 0 {
res = append(res,
fmt.Sprintf(
@@ -767,7 +799,7 @@
"expand_if_equal = "+parseVariableWithValue(
flagGroup.GetExpandIfEqual(), depth+1))
}
- return createObject("flag_group", res, depth)
+ return createObject("flag_group", res, depth), nil
}
func parseVariableWithValue(variable *crosstoolpb.CToolchain_VariableWithValue, depth int) string {
@@ -1242,9 +1274,15 @@
cToolchainIdentifiers := toolchainToCToolchainIdentifier(crosstool)
- toolchainToFeatures, featureNameToFeature := getFeatures(crosstool)
+ toolchainToFeatures, featureNameToFeature, err := getFeatures(crosstool)
+ if err != nil {
+ return "", err
+ }
- toolchainToActions, actionNameToAction := getActions(crosstool)
+ toolchainToActions, actionNameToAction, err := getActions(crosstool)
+ if err != nil {
+ return "", err
+ }
header := getCcToolchainConfigHeader()
if _, err := b.WriteString(header); err != nil {
diff --git a/tools/migration/crosstool_to_starlark_lib_test.go b/tools/migration/crosstool_to_starlark_lib_test.go
index ddb082b..f8c2c48 100644
--- a/tools/migration/crosstool_to_starlark_lib_test.go
+++ b/tools/migration/crosstool_to_starlark_lib_test.go
@@ -992,106 +992,90 @@
}
}
-func TestFeatureAssignment(t *testing.T) {
- toolchainEmpty1 := getCToolchain("1", "cpuA", "compilerA", []string{})
- toolchainEmpty2 := getCToolchain("2", "cpuB", "compilerA", []string{})
- toolchainA1 := getCToolchain("3", "cpuC", "compilerA",
- []string{
- getActionConfig([]string{"action_name: 'A'", "config_name: 'A'"}),
- },
+func TestAllAndNoneAvailableErrorsWhenMoreThanOneElement(t *testing.T) {
+ toolchainFeatureAllAvailable := getCToolchain("1", "cpu", "compiler",
+ []string{getFeature([]string{
+ "name: 'A'",
+ "flag_set {",
+ " action: 'A'",
+ " flag_group {",
+ " flag: 'f'",
+ " expand_if_all_available: 'e1'",
+ " expand_if_all_available: 'e2'",
+ " }",
+ "}",
+ })},
)
- toolchainA2 := getCToolchain("4", "cpuC", "compilerB",
- []string{
- getActionConfig([]string{"action_name: 'A'", "config_name: 'A'"}),
- },
+ toolchainFeatureNoneAvailable := getCToolchain("1", "cpu", "compiler",
+ []string{getFeature([]string{
+ "name: 'A'",
+ "flag_set {",
+ " action: 'A'",
+ " flag_group {",
+ " flag: 'f'",
+ " expand_if_none_available: 'e1'",
+ " expand_if_none_available: 'e2'",
+ " }",
+ "}",
+ })},
)
- toolchainAB := getCToolchain("5", "cpuC", "compilerC",
- []string{
- getActionConfig([]string{"action_name: 'A'", "config_name: 'A'"}),
- getActionConfig([]string{"action_name: 'B'", "config_name: 'B'"}),
- },
+ toolchainActionConfigAllAvailable := getCToolchain("1", "cpu", "compiler",
+ []string{getActionConfig([]string{
+ "config_name: 'A'",
+ "action_name: 'A'",
+ "flag_set {",
+ " action: 'A'",
+ " flag_group {",
+ " flag: 'f'",
+ " expand_if_all_available: 'e1'",
+ " expand_if_all_available: 'e2'",
+ " }",
+ "}",
+ })},
)
- toolchainBA := getCToolchain("6", "cpuD", "compilerA",
- []string{
- getActionConfig([]string{"action_name: 'B'", "config_name: 'B'"}),
- getActionConfig([]string{"action_name: 'A'", "config_name: 'A'"}),
- },
+ toolchainActionConfigNoneAvailable := getCToolchain("1", "cpu", "compiler",
+ []string{getActionConfig([]string{
+ "config_name: 'A'",
+ "action_name: 'A'",
+ "flag_set {",
+ " action: 'A'",
+ " flag_group {",
+ " flag: 'f'",
+ " expand_if_none_available: 'e1'",
+ " expand_if_none_available: 'e2'",
+ " }",
+ "}",
+ })},
)
- toolchainsEmpty := []string{toolchainEmpty1, toolchainEmpty2}
-
- toolchainsOneNonempty := []string{toolchainEmpty1, toolchainA1}
-
- toolchainsSameNonempty := []string{toolchainA1, toolchainA2}
-
- toolchainsDifferentOrder := []string{toolchainAB, toolchainBA}
-
- allToolchains := []string{
- toolchainEmpty1,
- toolchainEmpty2,
- toolchainA1,
- toolchainA2,
- toolchainAB,
- toolchainBA,
- }
testCases := []struct {
field string
- toolchains []string
+ toolchain string
expectedText string
}{
+ {field: "features",
+ toolchain: toolchainFeatureAllAvailable,
+ expectedText: "Error in feature 'A': Flag group must not have more " +
+ "than one 'expand_if_all_available' field"},
+ {field: "features",
+ toolchain: toolchainFeatureNoneAvailable,
+ expectedText: "Error in feature 'A': Flag group must not have more " +
+ "than one 'expand_if_none_available' field"},
{field: "action_configs",
- toolchains: toolchainsEmpty,
- expectedText: `
- action_configs = []`},
+ toolchain: toolchainActionConfigAllAvailable,
+ expectedText: "Error in action_config 'A': Flag group must not have more " +
+ "than one 'expand_if_all_available' field"},
{field: "action_configs",
- toolchains: toolchainsOneNonempty,
- expectedText: `
- if (ctx.attr.cpu == "cpuA"):
- action_configs = []
- elif (ctx.attr.cpu == "cpuC"):
- action_configs = [a_action]
- else:
- fail("Unreachable")`},
- {field: "action_configs",
- toolchains: toolchainsSameNonempty,
- expectedText: `
- action_configs = [a_action]`},
- {field: "action_configs",
- toolchains: toolchainsDifferentOrder,
- expectedText: `
- if (ctx.attr.cpu == "cpuC"):
- action_configs = [a_action, b_action]
- elif (ctx.attr.cpu == "cpuD"):
- action_configs = [b_action, a_action]
- else:
- fail("Unreachable")`},
- {field: "action_configs",
- toolchains: allToolchains,
- expectedText: `
- if (ctx.attr.cpu == "cpuA"
- or ctx.attr.cpu == "cpuB"):
- action_configs = []
- elif (ctx.attr.cpu == "cpuC" and ctx.attr.compiler == "compilerA"
- or ctx.attr.cpu == "cpuC" and ctx.attr.compiler == "compilerB"):
- action_configs = [a_action]
- elif (ctx.attr.cpu == "cpuC" and ctx.attr.compiler == "compilerC"):
- action_configs = [a_action, b_action]
- elif (ctx.attr.cpu == "cpuD"):
- action_configs = [b_action, a_action]
- else:
- fail("Unreachable")`}}
+ toolchain: toolchainActionConfigNoneAvailable,
+ expectedText: "Error in action_config 'A': Flag group must not have more " +
+ "than one 'expand_if_none_available' field"},
+ }
for _, tc := range testCases {
- crosstool := makeCrosstool(tc.toolchains)
- got, err := Transform(crosstool)
- if err != nil {
- t.Fatalf("CROSSTOOL conversion failed: %v", err)
- }
- if !strings.Contains(got, tc.expectedText) {
- t.Errorf("Failed to correctly convert '%s' field, expected to contain:\n%v\n",
- tc.field, tc.expectedText)
- t.Fatalf("Tested CROSSTOOL:\n%v\n\nGenerated rule:\n%v\n",
- strings.Join(tc.toolchains, "\n"), got)
+ crosstool := makeCrosstool([]string{tc.toolchain})
+ _, err := Transform(crosstool)
+ if err == nil || !strings.Contains(err.Error(), tc.expectedText) {
+ t.Errorf("Expected error: %s, got: %v", tc.expectedText, err)
}
}
}
