From c0ac741cbed87b7e487cf640211c178cfe09ba91 Mon Sep 17 00:00:00 2001 From: hlopko Date: Fri, 11 Jan 2019 00:11:20 -0800 Subject: [PATCH] Improve legacy_fields_migrator to work properly for objc crosstools https://github.com/bazelbuild/bazel/issues/5883 https://github.com/bazelbuild/bazel/issues/6861 RELNOTES: None. PiperOrigin-RevId: 228839863 --- .../migration/legacy_fields_migration_lib.py | 51 +++++++++---- .../legacy_fields_migration_lib_test.py | 76 ++++++++++++++++--- 2 files changed, 102 insertions(+), 25 deletions(-) diff --git a/tools/migration/legacy_fields_migration_lib.py b/tools/migration/legacy_fields_migration_lib.py index 247b6b4..f761847 100644 --- a/tools/migration/legacy_fields_migration_lib.py +++ b/tools/migration/legacy_fields_migration_lib.py @@ -14,25 +14,50 @@ https://github.com/bazelbuild/bazel/issues/5380. from third_party.com.github.bazelbuild.bazel.src.main.protobuf import crosstool_config_pb2 -ALL_COMPILE_ACTIONS = [ +ALL_CC_COMPILE_ACTIONS = [ "assemble", "preprocess-assemble", "linkstamp-compile", "c-compile", "c++-compile", "c++-header-parsing", "c++-module-compile", "c++-module-codegen", "lto-backend", "clif-match" ] -ALL_CXX_COMPILE_ACTIONS = [ - action for action in ALL_COMPILE_ACTIONS if action != "c-compile" +ALL_OBJC_COMPILE_ACTIONS = [ + "objc-compile", "objc++-compile" ] -ALL_LINK_ACTIONS = [ +ALL_CXX_COMPILE_ACTIONS = [ + action for action in ALL_CC_COMPILE_ACTIONS if action != "c-compile" +] + +ALL_CC_LINK_ACTIONS = [ "c++-link-executable", "c++-link-dynamic-library", "c++-link-nodeps-dynamic-library" ] +ALL_OBJC_LINK_ACTIONS = [ + "objc-executable", "objc++-executable", +] + DYNAMIC_LIBRARY_LINK_ACTIONS = [ "c++-link-dynamic-library", "c++-link-nodeps-dynamic-library" ] +def compile_actions(toolchain): + """Returns compile actions for cc or objc rules.""" + if _is_objc_toolchain(toolchain): + return ALL_CC_COMPILE_ACTIONS + ALL_OBJC_COMPILE_ACTIONS + else: + return ALL_CC_COMPILE_ACTIONS + +def link_actions(toolchain): + """Returns link actions for cc or objc rules.""" + if _is_objc_toolchain(toolchain): + return ALL_CC_LINK_ACTIONS + ALL_OBJC_LINK_ACTIONS + else: + return ALL_CC_LINK_ACTIONS + +def _is_objc_toolchain(toolchain): + return any(ac.action_name == "objc-compile" for ac in toolchain.action_config) + # Map converting from LinkingMode to corresponding feature name LINKING_MODE_TO_FEATURE_NAME = { "FULLY_STATIC": "fully_static_link", @@ -182,7 +207,7 @@ def migrate_legacy_fields(crosstool): feature.name = "user_compile_flags" feature.enabled = True flag_set = feature.flag_set.add() - flag_set.action[:] = ALL_COMPILE_ACTIONS + flag_set.action[:] = compile_actions(toolchain) flag_group = flag_set.flag_group.add() flag_group.expand_if_all_available[:] = ["user_compile_flags"] flag_group.iterate_over = "user_compile_flags" @@ -193,7 +218,7 @@ def migrate_legacy_fields(crosstool): feature.name = "sysroot" feature.enabled = True flag_set = feature.flag_set.add() - flag_set.action[:] = ALL_COMPILE_ACTIONS + ALL_LINK_ACTIONS + flag_set.action[:] = compile_actions(toolchain) + link_actions(toolchain) flag_group = flag_set.flag_group.add() flag_group.expand_if_all_available[:] = ["sysroot"] flag_group.flag[:] = ["--sysroot=%{sysroot}"] @@ -202,7 +227,7 @@ def migrate_legacy_fields(crosstool): feature.name = "unfiltered_compile_flags" feature.enabled = True flag_set = feature.flag_set.add() - flag_set.action[:] = ALL_COMPILE_ACTIONS + flag_set.action[:] = compile_actions(toolchain) flag_group = flag_set.flag_group.add() flag_group.flag[:] = toolchain.unfiltered_cxx_flag @@ -267,7 +292,7 @@ def _extract_legacy_compile_flag_sets_for(toolchain): """Get flag sets for default_compile_flags feature.""" result = [] if toolchain.compiler_flag: - result.append([None, ALL_COMPILE_ACTIONS, toolchain.compiler_flag, []]) + result.append([None, compile_actions(toolchain), toolchain.compiler_flag, []]) if toolchain.cxx_flag: result.append([None, ALL_CXX_COMPILE_ACTIONS, toolchain.cxx_flag, []]) @@ -284,7 +309,7 @@ def _extract_legacy_compile_flag_sets_for(toolchain): feature.name = mode if cmf.compiler_flag: - result.append([mode, ALL_COMPILE_ACTIONS, cmf.compiler_flag, []]) + result.append([mode, compile_actions(toolchain), cmf.compiler_flag, []]) if cmf.cxx_flag: result.append([mode, ALL_CXX_COMPILE_ACTIONS, cmf.cxx_flag, []]) @@ -298,7 +323,7 @@ def _extract_legacy_link_flag_sets_for(toolchain): # Migrate linker_flag if toolchain.linker_flag: - result.append([None, ALL_LINK_ACTIONS, toolchain.linker_flag, []]) + result.append([None, link_actions(toolchain), toolchain.linker_flag, []]) # Migrate linker_flags from compilation_mode_flags for cmf in toolchain.compilation_mode_flags: @@ -312,7 +337,7 @@ def _extract_legacy_link_flag_sets_for(toolchain): feature.name = mode if cmf.linker_flag: - result.append([mode, ALL_LINK_ACTIONS, cmf.linker_flag, []]) + result.append([mode, link_actions(toolchain), cmf.linker_flag, []]) # Migrate linker_flags from linking_mode_flags for lmf in toolchain.linking_mode_flags: @@ -327,7 +352,7 @@ def _extract_legacy_link_flag_sets_for(toolchain): feature.name = mode if lmf.linker_flag: - result.append([mode, ALL_LINK_ACTIONS, lmf.linker_flag, []]) + result.append([mode, link_actions(toolchain), lmf.linker_flag, []]) if toolchain.dynamic_library_linker_flag: result.append([ @@ -337,7 +362,7 @@ def _extract_legacy_link_flag_sets_for(toolchain): if toolchain.test_only_linker_flag: result.append([ - None, ALL_LINK_ACTIONS, toolchain.test_only_linker_flag, ["is_cc_test"] + None, link_actions(toolchain), toolchain.test_only_linker_flag, ["is_cc_test"] ]) return result diff --git a/tools/migration/legacy_fields_migration_lib_test.py b/tools/migration/legacy_fields_migration_lib_test.py index 9357c32..eaa4317 100644 --- a/tools/migration/legacy_fields_migration_lib_test.py +++ b/tools/migration/legacy_fields_migration_lib_test.py @@ -1,9 +1,11 @@ import unittest from google.protobuf import text_format from third_party.com.github.bazelbuild.bazel.src.main.protobuf import crosstool_config_pb2 -from tools.migration.legacy_fields_migration_lib import ALL_COMPILE_ACTIONS +from tools.migration.legacy_fields_migration_lib import ALL_CC_COMPILE_ACTIONS +from tools.migration.legacy_fields_migration_lib import ALL_OBJC_COMPILE_ACTIONS from tools.migration.legacy_fields_migration_lib import ALL_CXX_COMPILE_ACTIONS -from tools.migration.legacy_fields_migration_lib import ALL_LINK_ACTIONS +from tools.migration.legacy_fields_migration_lib import ALL_CC_LINK_ACTIONS +from tools.migration.legacy_fields_migration_lib import ALL_OBJC_LINK_ACTIONS from tools.migration.legacy_fields_migration_lib import DYNAMIC_LIBRARY_LINK_ACTIONS from tools.migration.legacy_fields_migration_lib import migrate_legacy_fields @@ -93,7 +95,20 @@ class LegacyFieldsMigrationLibTest(unittest.TestCase): output = crosstool.toolchain[0] self.assertEqual(len(output.compiler_flag), 0) self.assertEqual(output.feature[0].name, "default_compile_flags") - self.assertEqual(output.feature[0].flag_set[0].action, ALL_COMPILE_ACTIONS) + self.assertEqual(output.feature[0].flag_set[0].action, ALL_CC_COMPILE_ACTIONS) + self.assertEqual(output.feature[0].flag_set[0].flag_group[0].flag, + ["clang-flag-1"]) + + def test_migrate_compiler_flags_for_objc(self): + crosstool = make_crosstool(""" + action_config { action_name: "objc-compile" } + compiler_flag: 'clang-flag-1' + """) + migrate_legacy_fields(crosstool) + output = crosstool.toolchain[0] + self.assertEqual(len(output.compiler_flag), 0) + self.assertEqual(output.feature[0].name, "default_compile_flags") + self.assertEqual(output.feature[0].flag_set[0].action, ALL_CC_COMPILE_ACTIONS + ALL_OBJC_COMPILE_ACTIONS) self.assertEqual(output.feature[0].flag_set[0].flag_group[0].flag, ["clang-flag-1"]) @@ -118,7 +133,7 @@ class LegacyFieldsMigrationLibTest(unittest.TestCase): migrate_legacy_fields(crosstool) output = crosstool.toolchain[0] self.assertEqual(output.feature[0].name, "default_compile_flags") - self.assertEqual(output.feature[0].flag_set[0].action, ALL_COMPILE_ACTIONS) + self.assertEqual(output.feature[0].flag_set[0].action, ALL_CC_COMPILE_ACTIONS) self.assertEqual(output.feature[0].flag_set[1].action, ALL_CXX_COMPILE_ACTIONS) self.assertEqual(output.feature[0].flag_set[0].flag_group[0].flag, @@ -134,7 +149,7 @@ class LegacyFieldsMigrationLibTest(unittest.TestCase): output = crosstool.toolchain[0] self.assertEqual(len(output.linker_flag), 0) self.assertEqual(output.feature[0].name, "default_link_flags") - self.assertEqual(output.feature[0].flag_set[0].action, ALL_LINK_ACTIONS) + self.assertEqual(output.feature[0].flag_set[0].action, ALL_CC_LINK_ACTIONS) self.assertEqual(output.feature[0].flag_set[0].flag_group[0].flag, ["linker-flag-1"]) @@ -309,15 +324,15 @@ class LegacyFieldsMigrationLibTest(unittest.TestCase): output = crosstool.toolchain[0] self.assertEqual(output.feature[0].name, "default_link_flags") self.assertEqual(output.feature[0].enabled, True) - self.assertEqual(output.feature[0].flag_set[0].action[:], ALL_LINK_ACTIONS) + self.assertEqual(output.feature[0].flag_set[0].action[:], ALL_CC_LINK_ACTIONS) self.assertEqual(output.feature[0].flag_set[0].flag_group[0].flag[:], ["linker-flag-1"]) - self.assertEqual(output.feature[0].flag_set[1].action[:], ALL_LINK_ACTIONS) + self.assertEqual(output.feature[0].flag_set[1].action[:], ALL_CC_LINK_ACTIONS) self.assertEqual(output.feature[0].flag_set[1].with_feature[0].feature[0], "opt") self.assertEqual(output.feature[0].flag_set[1].flag_group[0].flag[:], ["cmf-flag-2"]) - self.assertEqual(output.feature[0].flag_set[2].action[:], ALL_LINK_ACTIONS) + self.assertEqual(output.feature[0].flag_set[2].action[:], ALL_CC_LINK_ACTIONS) self.assertEqual(output.feature[0].flag_set[2].with_feature[0].feature[0], "static_linking_mode") self.assertEqual(output.feature[0].flag_set[2].flag_group[0].flag[:], @@ -326,13 +341,37 @@ class LegacyFieldsMigrationLibTest(unittest.TestCase): DYNAMIC_LIBRARY_LINK_ACTIONS) self.assertEqual(output.feature[0].flag_set[3].flag_group[0].flag[:], ["dl-flag-4"]) - self.assertEqual(output.feature[0].flag_set[4].action[:], ALL_LINK_ACTIONS) + self.assertEqual(output.feature[0].flag_set[4].action[:], ALL_CC_LINK_ACTIONS) self.assertEqual(output.feature[0].flag_set[4].flag_group[0].flag[:], ["to-flag-5"]) self.assertEqual( output.feature[0].flag_set[4].flag_group[0].expand_if_all_available[:], ["is_cc_test"]) + def test_all_linker_flag_objc_actions(self): + crosstool = make_crosstool(""" + action_config { action_name: "objc-compile" } + linker_flag: 'linker-flag-1' + compilation_mode_flags { + mode: OPT + linker_flag: 'cmf-flag-2' + } + linking_mode_flags { + mode: MOSTLY_STATIC + linker_flag: 'lmf-flag-3' + } + dynamic_library_linker_flag: 'dl-flag-4' + test_only_linker_flag: 'to-flag-5' + """) + migrate_legacy_fields(crosstool) + output = crosstool.toolchain[0] + self.assertEqual(output.feature[0].name, "default_link_flags") + self.assertEqual(output.feature[0].flag_set[0].action[:], ALL_CC_LINK_ACTIONS + ALL_OBJC_LINK_ACTIONS) + self.assertEqual(output.feature[0].flag_set[1].action[:], ALL_CC_LINK_ACTIONS + ALL_OBJC_LINK_ACTIONS) + self.assertEqual(output.feature[0].flag_set[2].action[:], ALL_CC_LINK_ACTIONS + ALL_OBJC_LINK_ACTIONS) + self.assertEqual(output.feature[0].flag_set[3].action[:], DYNAMIC_LIBRARY_LINK_ACTIONS) + self.assertEqual(output.feature[0].flag_set[4].action[:], ALL_CC_LINK_ACTIONS + ALL_OBJC_LINK_ACTIONS) + def test_linking_mode_features_are_not_added_when_present(self): crosstool = make_crosstool(""" linking_mode_flags { @@ -415,7 +454,7 @@ class LegacyFieldsMigrationLibTest(unittest.TestCase): output = crosstool.toolchain[0] self.assertEqual(output.feature[0].name, "user_compile_flags") self.assertEqual(output.feature[0].enabled, True) - self.assertEqual(output.feature[0].flag_set[0].action, ALL_COMPILE_ACTIONS) + self.assertEqual(output.feature[0].flag_set[0].action, ALL_CC_COMPILE_ACTIONS) self.assertEqual( output.feature[0].flag_set[0].flag_group[0].expand_if_all_available, ["user_compile_flags"]) @@ -433,7 +472,7 @@ class LegacyFieldsMigrationLibTest(unittest.TestCase): self.assertEqual(output.feature[1].name, "sysroot") self.assertEqual(output.feature[1].enabled, True) self.assertEqual(output.feature[1].flag_set[0].action, - ALL_COMPILE_ACTIONS + ALL_LINK_ACTIONS) + ALL_CC_COMPILE_ACTIONS + ALL_CC_LINK_ACTIONS) self.assertEqual( output.feature[1].flag_set[0].flag_group[0].expand_if_all_available, ["sysroot"]) @@ -536,7 +575,20 @@ class LegacyFieldsMigrationLibTest(unittest.TestCase): self.assertEqual(output.feature[1].name, "user_compile_flags") self.assertEqual(output.feature[2].name, "sysroot") self.assertEqual(output.feature[3].name, "unfiltered_compile_flags") - self.assertEqual(output.feature[3].flag_set[0].action, ALL_COMPILE_ACTIONS) + self.assertEqual(output.feature[3].flag_set[0].action, ALL_CC_COMPILE_ACTIONS) + self.assertEqual(output.feature[3].flag_set[0].flag_group[0].flag, + ["unfiltered-flag-1"]) + + def test_unfiltered_compile_flags_are_not_added_for_objc(self): + crosstool = make_crosstool(""" + action_config { action_name: "obc-compile" } + feature { name: 'something_else' } + unfiltered_cxx_flag: 'unfiltered-flag-1' + """) + migrate_legacy_fields(crosstool) + output = crosstool.toolchain[0] + self.assertEqual(output.feature[3].name, "unfiltered_compile_flags") + self.assertEqual(output.feature[3].flag_set[0].action, ALL_CC_COMPILE_ACTIONS) self.assertEqual(output.feature[3].flag_set[0].flag_group[0].flag, ["unfiltered-flag-1"])