Improve legacy_fields_migrator to work properly for objc crosstools

bazelbuild/bazel#5883
bazelbuild/bazel#6861

RELNOTES: None.
PiperOrigin-RevId: 228839863

tools/migration/legacy_fields_migration_lib.py

@@ -14,25 +14,50 @@
 
 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_OBJC_COMPILE_ACTIONS = [
+    "objc-compile", "objc++-compile"
+]
+
 ALL_CXX_COMPILE_ACTIONS = [
-    action for action in ALL_COMPILE_ACTIONS if action != "c-compile"
+    action for action in ALL_CC_COMPILE_ACTIONS if action != "c-compile"
 ]
 
-ALL_LINK_ACTIONS = [
+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

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 @@ def test_migrate_compiler_flags(self):
     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 @@ def test_compiler_flag_come_before_cxx_flags(self):
     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 @@ def test_migrate_linker_flags(self):
     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 @@ def test_all_linker_flag_ordering(self):
     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 @@ def test_all_linker_flag_ordering(self):
                      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 @@ def test_user_compile_flags(self):
     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 @@ def test_sysroot(self):
     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 @@ def test_unfiltered_compile_flags_is_added_at_the_end(self):
     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"])