From dfb180b48691315a8c67245bd42e072632083ea8 Mon Sep 17 00:00:00 2001 From: hlopko Date: Fri, 8 Feb 2019 04:53:38 -0800 Subject: [PATCH] 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 --- .../migration/legacy_fields_migration_lib.py | 43 +++++- .../legacy_fields_migration_lib_test.py | 140 +++++++++++++++++- 2 files changed, 177 insertions(+), 6 deletions(-) diff --git a/tools/migration/legacy_fields_migration_lib.py b/tools/migration/legacy_fields_migration_lib.py index a7f64be..a1da3cf 100644 --- a/tools/migration/legacy_fields_migration_lib.py +++ b/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) diff --git a/tools/migration/legacy_fields_migration_lib_test.py b/tools/migration/legacy_fields_migration_lib_test.py index 298b72e..d6603a3 100644 --- a/tools/migration/legacy_fields_migration_lib_test.py +++ b/tools/migration/legacy_fields_migration_lib_test.py @@ -1023,8 +1023,7 @@ class LegacyFieldsMigrationLibTest(unittest.TestCase): flag_group { flag: '%{foo}' } - flag_group { - expand_if_all_available: 'bar' + flag_group { 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__":