From 0fd5bb9b752e8b9c191530cf6047524d68b9f9ad Mon Sep 17 00:00:00 2001 From: hlopko Date: Wed, 16 Jan 2019 01:41:08 -0800 Subject: [PATCH] Rename/remove fields referencing legacy features in legacy_fields_migrator When renaming legacy_*_flags to default_*_flags, also rename other fields such as requires. We're intentionally not replacing the implies line, just removing it, because default_*_features are enabled by default, so they don't need to be implied (Implied field is older than enabled field, so there are uses of it where enabled would work just fine. The only semantic difference is that implied features cannot be disabled, whereas enabled can. But since the standard style of writing crosstools seems to prefer enabled, I'm doing the same here.) https://github.com/bazelbuild/bazel/issues/6861 https://github.com/bazelbuild/bazel/issues/5883 RELNOTES: None. PiperOrigin-RevId: 229518029 --- .../migration/legacy_fields_migration_lib.py | 37 +++++- .../legacy_fields_migration_lib_test.py | 125 ++++++++++++++++++ 2 files changed, 161 insertions(+), 1 deletion(-) diff --git a/tools/migration/legacy_fields_migration_lib.py b/tools/migration/legacy_fields_migration_lib.py index 9b42d143..34d2a941 100644 --- a/tools/migration/legacy_fields_migration_lib.py +++ b/tools/migration/legacy_fields_migration_lib.py @@ -67,7 +67,6 @@ def _is_objc_toolchain(toolchain): "MOSTLY_STATIC_LIBRARIES": "static_linking_mode_nodeps_library", } - def migrate_legacy_fields(crosstool): """Migrates parsed crosstool (inplace) to not use legacy fields.""" crosstool.ClearField("default_toolchain") @@ -178,6 +177,8 @@ def migrate_legacy_fields(crosstool): if f.name == "legacy_link_flags": f.ClearField("flag_set") feature = f + _rename_feature_in_toolchain(toolchain, "legacy_link_flags", + "default_link_flags") break else: feature = _prepend_feature(toolchain) @@ -193,6 +194,8 @@ def migrate_legacy_fields(crosstool): if f.name == "legacy_compile_flags": f.ClearField("flag_set") feature = f + _rename_feature_in_toolchain(toolchain, "legacy_compile_flags", + "default_compile_flags") break else: feature = _prepend_feature(toolchain) @@ -425,3 +428,35 @@ def _contains_dynamic_flags(toolchain): if mode == "DYNAMIC": return True return False + + +def _rename_feature_in_toolchain(toolchain, from_name, to_name): + for f in toolchain.feature: + _rename_feature_in(f, from_name, to_name) + for a in toolchain.action_config: + _rename_feature_in(a, from_name, to_name) + + +def _rename_feature_in(msg, from_name, to_name): + if from_name in msg.implies: + msg.implies.remove(from_name) + for requires in msg.requires: + if from_name in requires.feature: + requires.feature.remove(from_name) + requires.feature.extend([to_name]) + for flag_set in msg.flag_set: + for with_feature in flag_set.with_feature: + if from_name in with_feature.feature: + with_feature.feature.remove(from_name) + with_feature.feature.extend([to_name]) + if from_name in with_feature.not_feature: + with_feature.not_feature.remove(from_name) + with_feature.not_feature.extend([to_name]) + for env_set in msg.env_set: + for with_feature in env_set.with_feature: + if from_name in with_feature.feature: + with_feature.feature.remove(from_name) + with_feature.feature.extend([to_name]) + if from_name in with_feature.not_feature: + with_feature.not_feature.remove(from_name) + with_feature.not_feature.extend([to_name]) diff --git a/tools/migration/legacy_fields_migration_lib_test.py b/tools/migration/legacy_fields_migration_lib_test.py index ee127fab..3117c3ee 100644 --- a/tools/migration/legacy_fields_migration_lib_test.py +++ b/tools/migration/legacy_fields_migration_lib_test.py @@ -103,6 +103,68 @@ def test_replace_legacy_compile_flags(self): self.assertEqual(output.feature[1].flag_set[0].flag_group[0].flag, ["clang-flag-1"]) + def test_replace_legacy_compile_flags_in_action_configs(self): + crosstool = make_crosstool(""" + feature { + name: 'foo' + implies: 'legacy_compile_flags' + requires: { feature: 'legacy_compile_flags' } + flag_set { + with_feature { feature: 'legacy_compile_flags' } + with_feature { not_feature: 'legacy_compile_flags' } + } + env_set { + with_feature { feature: 'legacy_compile_flags' } + with_feature { not_feature: 'legacy_compile_flags' } + } + } + feature { name: 'legacy_compile_flags' } + action_config { + action_name: 'foo' + config_name: 'foo' + implies: 'legacy_compile_flags' + requires: { feature: 'legacy_compile_flags' } + flag_set { + with_feature { feature: 'legacy_compile_flags' } + with_feature { not_feature: 'legacy_compile_flags' } + } + env_set { + with_feature { feature: 'legacy_compile_flags' } + with_feature { not_feature: 'legacy_compile_flags' } + } + } + compiler_flag: 'clang-flag-1' + """) + migrate_legacy_fields(crosstool) + output = crosstool.toolchain[0] + self.assertEqual(output.action_config[0].action_name, "foo") + self.assertEqual(output.action_config[0].implies, []) + self.assertEqual(output.action_config[0].requires[0].feature, + ["default_compile_flags"]) + self.assertEqual( + output.action_config[0].flag_set[0].with_feature[0].feature, + ["default_compile_flags"]) + self.assertEqual( + output.action_config[0].flag_set[0].with_feature[1].not_feature, + ["default_compile_flags"]) + self.assertEqual(output.action_config[0].env_set[0].with_feature[0].feature, + ["default_compile_flags"]) + self.assertEqual( + output.action_config[0].env_set[0].with_feature[1].not_feature, + ["default_compile_flags"]) + self.assertEqual(output.feature[0].name, "foo") + self.assertEqual(output.feature[0].implies, []) + self.assertEqual(output.feature[0].requires[0].feature, + ["default_compile_flags"]) + self.assertEqual(output.feature[0].flag_set[0].with_feature[0].feature, + ["default_compile_flags"]) + self.assertEqual(output.feature[0].flag_set[0].with_feature[1].not_feature, + ["default_compile_flags"]) + self.assertEqual(output.feature[0].env_set[0].with_feature[0].feature, + ["default_compile_flags"]) + self.assertEqual(output.feature[0].env_set[0].with_feature[1].not_feature, + ["default_compile_flags"]) + def test_replace_legacy_link_flags(self): crosstool = make_crosstool(""" feature { name: 'foo' } @@ -118,6 +180,69 @@ def test_replace_legacy_link_flags(self): self.assertEqual(output.feature[1].flag_set[0].flag_group[0].flag, ["ld-flag-1"]) + def test_replace_legacy_link_flags_in_action_configs(self): + crosstool = make_crosstool(""" + feature { + name: 'foo' + implies: 'legacy_link_flags' + requires: { feature: 'legacy_link_flags' } + flag_set { + with_feature { feature: 'legacy_link_flags' } + with_feature { not_feature: 'legacy_link_flags' } + } + env_set { + with_feature { feature: 'legacy_link_flags' } + with_feature { not_feature: 'legacy_link_flags' } + } + } + feature { name: 'legacy_link_flags' } + action_config { + action_name: 'foo' + config_name: 'foo' + implies: 'legacy_link_flags' + requires: { feature: 'legacy_link_flags' } + flag_set { + with_feature { feature: 'legacy_link_flags' } + with_feature { not_feature: 'legacy_link_flags' } + } + env_set { + with_feature { feature: 'legacy_link_flags' } + with_feature { not_feature: 'legacy_link_flags' } + } + } + linker_flag: 'clang-flag-1' + """) + migrate_legacy_fields(crosstool) + output = crosstool.toolchain[0] + self.assertEqual(output.action_config[0].action_name, "foo") + self.assertEqual(output.action_config[0].implies, []) + self.assertEqual(output.action_config[0].requires[0].feature, + ["default_link_flags"]) + self.assertEqual( + output.action_config[0].flag_set[0].with_feature[0].feature, + ["default_link_flags"]) + self.assertEqual( + output.action_config[0].flag_set[0].with_feature[1].not_feature, + ["default_link_flags"]) + self.assertEqual(output.action_config[0].env_set[0].with_feature[0].feature, + ["default_link_flags"]) + self.assertEqual( + output.action_config[0].env_set[0].with_feature[1].not_feature, + ["default_link_flags"]) + self.assertEqual(output.feature[0].name, "foo") + self.assertEqual(output.feature[0].implies, []) + self.assertEqual(output.feature[0].requires[0].feature, + ["default_link_flags"]) + self.assertEqual(output.feature[0].flag_set[0].with_feature[0].feature, + ["default_link_flags"]) + self.assertEqual(output.feature[0].flag_set[0].with_feature[1].not_feature, + ["default_link_flags"]) + self.assertEqual(output.feature[0].env_set[0].with_feature[0].feature, + ["default_link_flags"]) + self.assertEqual(output.feature[0].env_set[0].with_feature[1].not_feature, + ["default_link_flags"]) + + def test_migrate_compiler_flags(self): crosstool = make_crosstool(""" compiler_flag: 'clang-flag-1'