Remove fail("Unreachable") from features' and action_configs' declaration statements
If toolchain A and B need feature f, but toolchain C doesn't, the generated declaration statement for feature f would be:
if ctx.attr.cpu == 'A.cpu':
f = feature(...)
elif ctx.attr.cpu == "B.cpu" and ctx.attr.compiler == "B.compiler":
f = feature(...)
else:
fail("Unreachable")
This will break the rule implementation in the case of toolchain C because it will reach the fail("Unreachable") although it doesn't need to declare feature f
This cl fixes that
Issue #5380
PiperOrigin-RevId: 232683639
diff --git a/tools/migration/crosstool_to_starlark_lib.go b/tools/migration/crosstool_to_starlark_lib.go
index e19b42d..c05e99c 100644
--- a/tools/migration/crosstool_to_starlark_lib.go
+++ b/tools/migration/crosstool_to_starlark_lib.go
@@ -347,7 +347,7 @@
mappedValuesToIds := getMappedStringValuesToIdentifiers(identifiers, fieldValues)
return getAssignmentStatement(field, mappedValuesToIds, crosstool,
- cToolchainIdentifiers, depth /* isPlainString= */, true)
+ cToolchainIdentifiers, depth /* isPlainString= */, true, /* shouldFail= */ true)
}
func getFeatures(crosstool *crosstoolpb.CrosstoolRelease) (
@@ -392,7 +392,8 @@
crosstool,
cToolchainIdentifiers,
depth,
- /* isPlainString= */ false))
+ /* isPlainString= */ false,
+ /* shouldFail= */ false))
}
return strings.Join(res, "")
}
@@ -467,7 +468,8 @@
crosstool,
cToolchainIdentifiers,
depth,
- /* isPlainString= */ false))
+ /* isPlainString= */ false,
+ /* shouldFail= */ false))
}
return strings.Join(res, "")
}
@@ -650,7 +652,8 @@
crosstool,
cToolchainIdentifiers,
depth,
- /* isPlainString= */ false))
+ /* isPlainString= */ false,
+ /* shouldFail= */ true))
return strings.Join(res, "\n")
}
@@ -826,7 +829,8 @@
crosstool,
cToolchainIdentifiers,
depth,
- /* isPlainString= */ false))
+ /* isPlainString= */ false,
+ /* shouldFail= */ true))
return strings.Join(res, "\n")
}
@@ -865,7 +869,8 @@
crosstool,
cToolchainIdentifiers,
depth,
- /* isPlainString= */ false))
+ /* isPlainString= */ false,
+ /* shouldFail= */ true))
return strings.Join(res, "\n")
}
@@ -1034,7 +1039,7 @@
func getAssignmentStatement(field string, valToIds map[string][]string,
crosstool *crosstoolpb.CrosstoolRelease,
toCToolchainIdentifier map[string]CToolchainIdentifier,
- depth int, isPlainString bool) string {
+ depth int, isPlainString, shouldFail bool) string {
var b bytes.Buffer
if len(valToIds) <= 1 {
// if there is only one possible value for this field, we don't need if statements
@@ -1063,10 +1068,12 @@
toCToolchainIdentifier, depth, isPlainString))
first = false
}
- b.WriteString(
- fmt.Sprintf(
- "%selse:\n%sfail(\"Unreachable\")\n",
- getTabs(depth), getTabs(depth+1)))
+ if shouldFail {
+ b.WriteString(
+ fmt.Sprintf(
+ "%selse:\n%sfail(\"Unreachable\")\n",
+ getTabs(depth), getTabs(depth+1)))
+ }
}
b.WriteString("\n")
return b.String()
diff --git a/tools/migration/crosstool_to_starlark_lib_test.go b/tools/migration/crosstool_to_starlark_lib_test.go
index 56f208f..0090ce8 100644
--- a/tools/migration/crosstool_to_starlark_lib_test.go
+++ b/tools/migration/crosstool_to_starlark_lib_test.go
@@ -1080,6 +1080,73 @@
}
}
+func TestNoFailUnreachableInFeaturesAndActionConfigsDeclaration(t *testing.T) {
+ toolchainFeatureAEnabled := getCToolchain("1", "cpuA", "compilerA",
+ []string{getFeature([]string{"name: 'A'", "enabled: true"})},
+ )
+ toolchainFeatureADisabled := getCToolchain("2", "cpuA", "compilerB",
+ []string{getFeature([]string{"name: 'A'", "enabled: false"})},
+ )
+
+ toolchainWithoutFeatureA := getCToolchain("3", "cpuA", "compilerC", []string{})
+
+ toolchainActionConfigAEnabled := getCToolchain("4", "cpuA", "compilerD",
+ []string{getActionConfig([]string{
+ "config_name: 'A'",
+ "action_name: 'A'",
+ "enabled: true",
+ })})
+
+ toolchainActionConfigADisabled := getCToolchain("5", "cpuA", "compilerE",
+ []string{getActionConfig([]string{
+ "config_name: 'A'",
+ "action_name: 'A'",
+ })})
+
+ toolchainWithoutActionConfigA := getCToolchain("6", "cpuA", "compilerF", []string{})
+
+ testCases := []struct {
+ field string
+ toolchains []string
+ expectedText string
+ }{
+ {field: "features",
+ toolchains: []string{
+ toolchainFeatureAEnabled, toolchainFeatureADisabled, toolchainWithoutFeatureA},
+ expectedText: `
+ if (ctx.attr.cpu == "cpuA" and ctx.attr.compiler == "compilerB"):
+ a_feature = feature(name = "A")
+ elif (ctx.attr.cpu == "cpuA" and ctx.attr.compiler == "compilerA"):
+ a_feature = feature(name = "A", enabled = True)
+
+` /* empty line after the elif means there's no else statement */},
+ {field: "action_config",
+ toolchains: []string{
+ toolchainActionConfigAEnabled, toolchainActionConfigADisabled, toolchainWithoutActionConfigA},
+ expectedText: `
+ if (ctx.attr.cpu == "cpuA" and ctx.attr.compiler == "compilerE"):
+ a_action = action_config(action_name = "A")
+ elif (ctx.attr.cpu == "cpuA" and ctx.attr.compiler == "compilerD"):
+ a_action = action_config(action_name = "A", enabled = True)
+
+` /* empty line after the elif means there's no else statement */ },
+ }
+
+ 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)
+ }
+ }
+}
+
func TestActionConfigDeclaration(t *testing.T) {
toolchainEmpty1 := getCToolchain("1", "cpuA", "compilerA", []string{})
toolchainEmpty2 := getCToolchain("2", "cpuB", "compilerA", []string{})
