Migrate repeated expand_if_(all|none)_available into nested flag_groups

Crosstool in Starlark assumes these fields as singular, not collections. This cl updates the migration script to prepare crosstool in proto for this.

https://github.com/bazelbuild/bazel/issues/6861
https://github.com/bazelbuild/bazel/issues/5883

RELNOTES: None.
PiperOrigin-RevId: 233041028

tools/migration/legacy_fields_migration_lib.py

@@ -87,6 +87,8 @@ def migrate_legacy_fields(crosstool):
   for toolchain in crosstool.toolchain:
     _ = [_migrate_expand_if_all_available(f) for f in toolchain.feature]
     _ = [_migrate_expand_if_all_available(ac) for ac in toolchain.action_config]
+    _ = [_migrate_repeated_expands(f) for f in toolchain.feature]
+    _ = [_migrate_repeated_expands(ac) for ac in toolchain.action_config]
 
     if (toolchain.dynamic_library_linker_flag or
         _contains_dynamic_flags(toolchain)) and not _contains_feature(
@@ -437,7 +439,7 @@ def _contains_feature(toolchain, name):
 
 
 def _migrate_expand_if_all_available(message):
-  """Move expand_if_all_available fields to flag_groups."""
+  """Move expand_if_all_available field to flag_groups."""
   for flag_set in message.flag_set:
     if flag_set.expand_if_all_available:
       for flag_group in flag_set.flag_group:
@@ -448,6 +450,45 @@ def _migrate_expand_if_all_available(message):
       flag_set.ClearField("expand_if_all_available")
 
 
+def _migrate_repeated_expands(message):
+  """Replace repeated legacy fields with nesting."""
+  todo_queue = []
+  for flag_set in message.flag_set:
+    todo_queue.extend(flag_set.flag_group)
+  while todo_queue:
+    flag_group = todo_queue.pop()
+    todo_queue.extend(flag_group.flag_group)
+    if len(flag_group.expand_if_all_available) <= 1 and len(
+        flag_group.expand_if_none_available) <= 1:
+      continue
+
+    current_children = flag_group.flag_group
+    current_flags = flag_group.flag
+    flag_group.ClearField("flag_group")
+    flag_group.ClearField("flag")
+
+    new_flag_group = flag_group.flag_group.add()
+    new_flag_group.flag_group.extend(current_children)
+    new_flag_group.flag.extend(current_flags)
+
+    if len(flag_group.expand_if_all_available) > 1:
+      expands_to_move = flag_group.expand_if_all_available[1:]
+      flag_group.expand_if_all_available[:] = [
+          flag_group.expand_if_all_available[0]
+      ]
+      new_flag_group.expand_if_all_available.extend(expands_to_move)
+
+    if len(flag_group.expand_if_none_available) > 1:
+      expands_to_move = flag_group.expand_if_none_available[1:]
+      flag_group.expand_if_none_available[:] = [
+          flag_group.expand_if_none_available[0]
+      ]
+      new_flag_group.expand_if_none_available.extend(expands_to_move)
+
+    todo_queue.append(new_flag_group)
+    todo_queue.append(flag_group)
+
+
 def _contains_dynamic_flags(toolchain):
   for lmf in toolchain.linking_mode_flags:
     mode = crosstool_config_pb2.LinkingMode.Name(lmf.mode)

tools/migration/legacy_fields_migration_lib_test.py

@@ -1023,8 +1023,7 @@ class LegacyFieldsMigrationLibTest(unittest.TestCase):
             flag_group {
               flag: '%{foo}'
             }
-            flag_group {
+            flag_group {              
-              expand_if_all_available: 'bar'
               flag: 'bar'
             }
           }
@@ -1038,7 +1037,6 @@ class LegacyFieldsMigrationLibTest(unittest.TestCase):
               flag: '%{foo}'
             }
             flag_group {
-              expand_if_all_available: 'bar'
               flag: 'bar'
             }
           }
@@ -1056,7 +1054,7 @@ class LegacyFieldsMigrationLibTest(unittest.TestCase):
         .expand_if_all_available, ["foo"])
     self.assertEqual(
         output.action_config[0].flag_set[0].flag_group[1]
-        .expand_if_all_available, ["bar", "foo"])
+        .expand_if_all_available, ["foo"])
 
     self.assertEqual(output.feature[0].name, "something_else")
     self.assertEqual(len(output.feature[0].flag_set), 1)
@@ -1068,7 +1066,139 @@ class LegacyFieldsMigrationLibTest(unittest.TestCase):
         ["foo"])
     self.assertEqual(
         output.feature[0].flag_set[0].flag_group[1].expand_if_all_available,
-        ["bar", "foo"])
+        ["foo"])
+
+  def test_migrate_repeated_expand_if_all_available_from_flag_groups(self):
+    crosstool = make_crosstool("""
+          action_config {
+            action_name: 'something'
+            config_name: 'something'
+            flag_set {
+              flag_group {
+                expand_if_all_available: 'foo'
+                expand_if_all_available: 'bar'
+                flag: '%{foo}'
+              }
+              flag_group {
+                expand_if_none_available: 'foo'
+                expand_if_none_available: 'bar'
+                flag: 'bar'
+              }
+            }
+          }
+          feature {
+            name: 'something_else'
+            flag_set {
+              action: 'c-compile'
+              flag_group {
+                expand_if_all_available: 'foo'
+                expand_if_all_available: 'bar'
+                flag: '%{foo}'
+              }
+              flag_group {
+                expand_if_none_available: 'foo'
+                expand_if_none_available: 'bar'
+                flag: 'bar'
+              }
+            }
+          }
+          """)
+    migrate_legacy_fields(crosstool)
+    output = crosstool.toolchain[0]
+    self.assertEqual(output.action_config[0].action_name, "something")
+    self.assertEqual(len(output.action_config[0].flag_set), 1)
+    self.assertEqual(
+        len(output.action_config[0].flag_set[0].expand_if_all_available), 0)
+    self.assertEqual(len(output.action_config[0].flag_set[0].flag_group), 2)
+    self.assertEqual(
+        output.action_config[0].flag_set[0].flag_group[0]
+        .expand_if_all_available, ["foo"])
+    self.assertEqual(
+        output.action_config[0].flag_set[0].flag_group[0].flag_group[0]
+        .expand_if_all_available, ["bar"])
+    self.assertEqual(
+        output.action_config[0].flag_set[0].flag_group[1]
+        .expand_if_none_available, ["foo"])
+    self.assertEqual(
+        output.action_config[0].flag_set[0].flag_group[1].flag_group[0]
+        .expand_if_none_available, ["bar"])
+
+    self.assertEqual(output.feature[0].name, "something_else")
+    self.assertEqual(len(output.feature[0].flag_set), 1)
+    self.assertEqual(
+        len(output.feature[0].flag_set[0].expand_if_all_available), 0)
+    self.assertEqual(len(output.feature[0].flag_set[0].flag_group), 2)
+    self.assertEqual(
+        output.feature[0].flag_set[0].flag_group[0].expand_if_all_available,
+        ["foo"])
+    self.assertEqual(
+        output.feature[0].flag_set[0].flag_group[0].flag_group[0]
+        .expand_if_all_available, ["bar"])
+    self.assertEqual(
+        output.feature[0].flag_set[0].flag_group[1].expand_if_none_available,
+        ["foo"])
+    self.assertEqual(
+        output.feature[0].flag_set[0].flag_group[1].flag_group[0]
+        .expand_if_none_available, ["bar"])
+
+  def test_migrate_repeated_expands_from_nested_flag_groups(self):
+    crosstool = make_crosstool("""
+          feature {
+            name: 'something'
+            flag_set {
+              action: 'c-compile'
+              flag_group {
+                flag_group {
+                  expand_if_all_available: 'foo'
+                  expand_if_all_available: 'bar'
+                  flag: '%{foo}'
+                }
+              }
+              flag_group {
+                flag_group {
+                  expand_if_all_available: 'foo'
+                  expand_if_all_available: 'bar'
+                  expand_if_none_available: 'foo'
+                  expand_if_none_available: 'bar'
+                  flag: '%{foo}'
+                }
+              }
+            }
+          }
+          """)
+    migrate_legacy_fields(crosstool)
+    output = crosstool.toolchain[0]
+
+    self.assertEqual(output.feature[0].name, "something")
+    self.assertEqual(len(output.feature[0].flag_set[0].flag_group), 2)
+    self.assertEqual(
+        len(output.feature[0].flag_set[0].flag_group[0].expand_if_all_available
+           ), 0)
+    self.assertEqual(
+        output.feature[0].flag_set[0].flag_group[0].flag_group[0]
+        .expand_if_all_available, ["foo"])
+    self.assertEqual(
+        output.feature[0].flag_set[0].flag_group[0].flag_group[0].flag_group[0]
+        .expand_if_all_available, ["bar"])
+    self.assertEqual(
+        output.feature[0].flag_set[0].flag_group[0].flag_group[0].flag_group[0]
+        .flag, ["%{foo}"])
+
+    self.assertEqual(
+        output.feature[0].flag_set[0].flag_group[1].flag_group[0]
+        .expand_if_all_available, ["foo"])
+    self.assertEqual(
+        output.feature[0].flag_set[0].flag_group[1].flag_group[0]
+        .expand_if_none_available, ["foo"])
+    self.assertEqual(
+        output.feature[0].flag_set[0].flag_group[1].flag_group[0].flag_group[0]
+        .expand_if_none_available, ["bar"])
+    self.assertEqual(
+        output.feature[0].flag_set[0].flag_group[1].flag_group[0].flag_group[0]
+        .expand_if_all_available, ["bar"])
+    self.assertEqual(
+        output.feature[0].flag_set[0].flag_group[1].flag_group[0].flag_group[0]
+        .flag, ["%{foo}"])
 
 
 if __name__ == "__main__":