From d4fef611196fd572c017f0a3006e8db78f369951 Mon Sep 17 00:00:00 2001 From: rosica Date: Wed, 6 Feb 2019 23:50:32 -0800 Subject: [PATCH] Add feature declaration tests And fix an error where we added previously unseen action_names to the dictionary of familiar ACTION_NAMES, so later when we would encounter them, we would treat them as variables, not as string literals eg env_set { action = "a" } would translate to env_set( actions = [a], ) which is, obviously wrong. Issue #5380 PiperOrigin-RevId: 232817515 --- tools/migration/crosstool_to_starlark_lib.go | 1 - .../crosstool_to_starlark_lib_test.go | 215 +++++++++++++++++- 2 files changed, 209 insertions(+), 7 deletions(-) diff --git a/tools/migration/crosstool_to_starlark_lib.go b/tools/migration/crosstool_to_starlark_lib.go index c05e99c..bd373f1 100644 --- a/tools/migration/crosstool_to_starlark_lib.go +++ b/tools/migration/crosstool_to_starlark_lib.go @@ -434,7 +434,6 @@ func getActions(crosstool *crosstoolpb.CrosstoolRelease) ( actionName = strings.Replace(actionName, "+", "p", -1) actionName = strings.Replace(actionName, ".", "_", -1) actionName = strings.Replace(actionName, "-", "_", -1) - actionNames[actionName] = actionName } stringAction, err := parseAction(action, 1) if err != nil { diff --git a/tools/migration/crosstool_to_starlark_lib_test.go b/tools/migration/crosstool_to_starlark_lib_test.go index 0090ce8..bcbc997 100644 --- a/tools/migration/crosstool_to_starlark_lib_test.go +++ b/tools/migration/crosstool_to_starlark_lib_test.go @@ -1158,14 +1158,14 @@ func TestActionConfigDeclaration(t *testing.T) { ) toolchainNameInDictA := getCToolchain("4", "cpuC", "compilerA", []string{ - getActionConfig([]string{"action_name: 'c++-executable'", "config_name: 'c++-executable'"}), + getActionConfig([]string{"action_name: 'c++-compile'", "config_name: 'c++-compile'"}), }, ) toolchainNameInDictB := getCToolchain("5", "cpuC", "compilerB", []string{ getActionConfig([]string{ - "action_name: 'c++-executable'", - "config_name: 'c++-executable'", + "action_name: 'c++-compile'", + "config_name: 'c++-compile'", "tool {", " tool_path: '/a/b/c'", "}", @@ -1258,12 +1258,12 @@ func TestActionConfigDeclaration(t *testing.T) { toolchains: []string{toolchainNameInDictA, toolchainNameInDictB}, expectedText: ` if (ctx.attr.cpu == "cpuC" and ctx.attr.compiler == "compilerB"): - cpp_executable_action = action_config( - action_name = "c++-executable", + cpp_compile_action = action_config( + action_name = ACTION_NAMES.cpp_compile, tools = [tool(path = "/a/b/c")], ) elif (ctx.attr.cpu == "cpuC" and ctx.attr.compiler == "compilerA"): - cpp_executable_action = action_config(action_name = "c++-executable")`}, + cpp_compile_action = action_config(action_name = ACTION_NAMES.cpp_compile)`}, { toolchains: []string{toolchainComplexActionConfig}, expectedText: ` @@ -1325,3 +1325,206 @@ func TestActionConfigDeclaration(t *testing.T) { } } } + +func TestFeatureDeclaration(t *testing.T) { + toolchainEmpty1 := getCToolchain("1", "cpuA", "compilerA", []string{}) + toolchainEmpty2 := getCToolchain("2", "cpuB", "compilerA", []string{}) + + toolchainSimpleFeatureA1 := getCToolchain("3", "cpuB", "compilerB", + []string{ + getFeature([]string{"name: 'Feature-c++.a'", "enabled: true"}), + }, + ) + toolchainSimpleFeatureA2 := getCToolchain("4", "cpuC", "compilerA", + []string{ + getFeature([]string{"name: 'Feature-c++.a'"}), + }, + ) + toolchainComplexFeature := getCToolchain("5", "cpuC", "compilerC", + []string{ + getFeature([]string{ + "name: 'complex-feature'", + "enabled: true", + "flag_set {", + " action: 'c++-compile'", // in ACTION_NAMES + " action: 'something-else'", // not in ACTION_NAMES + " flag_group {", + " flag: 'a'", + " flag: '%b'", + " iterate_over: 'c'", + " expand_if_all_available: 'd'", + " expand_if_none_available: 'e'", + " expand_if_true: 'f'", + " expand_if_false: 'g'", + " expand_if_equal {", + " variable: 'var'", + " value: 'val'", + " }", + " }", + " flag_group {", + " flag_group {", + " flag: 'a'", + " }", + " }", + "}", + "flag_set {", // all_compile_actions + " action: 'c-compile'", + " action: 'c++-compile'", + " action: 'linkstamp-compile'", + " action: 'assemble'", + " action: 'preprocess-assemble'", + " action: 'c++-header-parsing'", + " action: 'c++-module-compile'", + " action: 'c++-module-codegen'", + " action: 'clif-match'", + " action: 'lto-backend'", + "}", + "flag_set {", // all_cpp_compile_actions + " action: 'c++-compile'", + " action: 'linkstamp-compile'", + " action: 'c++-header-parsing'", + " action: 'c++-module-compile'", + " action: 'c++-module-codegen'", + " action: 'clif-match'", + "}", + "flag_set {", // all_link_actions + " action: 'c++-link-executable'", + " action: 'c++-link-dynamic-library'", + " action: 'c++-link-nodeps-dynamic-library'", + "}", + "flag_set {", // all_cpp_compile_actions + all_link_actions + " action: 'c++-compile'", + " action: 'linkstamp-compile'", + " action: 'c++-header-parsing'", + " action: 'c++-module-compile'", + " action: 'c++-module-codegen'", + " action: 'clif-match'", + " action: 'c++-link-executable'", + " action: 'c++-link-dynamic-library'", + " action: 'c++-link-nodeps-dynamic-library'", + "}", + "flag_set {", // all_link_actions + something else + " action: 'c++-link-executable'", + " action: 'c++-link-dynamic-library'", + " action: 'c++-link-nodeps-dynamic-library'", + " action: 'some.unknown-c++.action'", + "}", + "env_set {", + " action: 'a'", + " env_entry {", + " key: 'k'", + " value: 'v'", + " }", + " with_feature {", + " feature: 'a'", + " }", + "}", + "env_set {", + " action: 'c-compile'", + "}", + "env_set {", // all_compile_actions + " action: 'c-compile'", + " action: 'c++-compile'", + " action: 'linkstamp-compile'", + " action: 'assemble'", + " action: 'preprocess-assemble'", + " action: 'c++-header-parsing'", + " action: 'c++-module-compile'", + " action: 'c++-module-codegen'", + " action: 'clif-match'", + " action: 'lto-backend'", + "}", + "requires {", + " feature: 'a'", + " feature: 'b'", + "}", + "implies: 'a'", + "implies: 'b'", + "provides: 'c'", + "provides: 'd'", + }), + }, + ) + + testCases := []struct { + toolchains []string + expectedText string + }{ + { + toolchains: []string{toolchainEmpty1, toolchainEmpty2}, + expectedText: ` + features = [] +`}, + { + toolchains: []string{toolchainEmpty1, toolchainSimpleFeatureA1}, + expectedText: ` + feature_cpp_a_feature = feature(name = "Feature-c++.a", enabled = True)`}, + { + toolchains: []string{toolchainSimpleFeatureA1, toolchainSimpleFeatureA2}, + expectedText: ` + if (ctx.attr.cpu == "cpuC"): + feature_cpp_a_feature = feature(name = "Feature-c++.a") + elif (ctx.attr.cpu == "cpuB"): + feature_cpp_a_feature = feature(name = "Feature-c++.a", enabled = True)`}, + { + toolchains: []string{toolchainComplexFeature}, + expectedText: ` + complex_feature_feature = feature( + name = "complex-feature", + enabled = True, + flag_sets = [ + flag_set( + actions = [ACTION_NAMES.cpp_compile, "something-else"], + flag_groups = [ + flag_group( + flags = ["a", "%b"], + iterate_over = "c", + expand_if_available = "d", + expand_if_not_available = "e", + expand_if_true = "f", + expand_if_false = "g", + expand_if_equal = variable_with_value(name = "var", value = "val"), + ), + flag_group(flag_groups = [flag_group(flags = ["a"])]), + ], + ), + flag_set(actions = all_compile_actions), + flag_set(actions = all_cpp_compile_actions), + flag_set(actions = all_link_actions), + flag_set( + actions = all_cpp_compile_actions + + all_link_actions, + ), + flag_set( + actions = all_link_actions + + ["some.unknown-c++.action"], + ), + ], + env_sets = [ + env_set( + actions = ["a"], + env_entries = [env_entry(key = "k", value = "v")], + with_features = [with_feature_set(features = ["a"])], + ), + env_set(actions = [ACTION_NAMES.c_compile]), + env_set(actions = all_compile_actions), + ], + requires = [feature_set(features = ["a", "b"])], + implies = ["a", "b"], + provides = ["c", "d"], + )`}} + + 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 declare a feature, expected to contain:\n%v\n", + tc.expectedText) + t.Fatalf("Tested CROSSTOOL:\n%v\n\nGenerated rule:\n%v\n", + strings.Join(tc.toolchains, "\n"), got) + } + } +}