Skip to content

Commit

Permalink
Rename/remove fields referencing legacy features in legacy_fields_mig…
Browse files Browse the repository at this point in the history 
…rator

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.)

bazelbuild/bazel#6861
bazelbuild/bazel#5883

RELNOTES: None.
PiperOrigin-RevId: 229518029
  • Loading branch information
@hlopko @copybara-github
hlopko authored and copybara-github committed Jan 16, 2019
1 parent f79b930 commit 0fd5bb9
Show file tree
Hide file tree
Showing 2 changed files with 161 additions and 1 deletion.
37 changes: 36 additions & 1 deletion tools/migration/legacy_fields_migration_lib.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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])
125 changes: 125 additions & 0 deletions tools/migration/legacy_fields_migration_lib_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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' }
Expand All @@ -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'
Expand Down

0 comments on commit 0fd5bb9

Please sign in to comment.