Skip to content

Commit

Permalink
Migrate repeated expand_if_(all|none)_available into nested flag_groups
Browse files Browse the repository at this point in the history 
Crosstool in Starlark assumes these fields as singular, not collections. This cl updates the migration script to prepare crosstool in proto for this.

bazelbuild/bazel#6861
bazelbuild/bazel#5883

RELNOTES: None.
PiperOrigin-RevId: 233041028
  • Loading branch information
@hlopko @copybara-github
hlopko authored and copybara-github committed Feb 8, 2019
1 parent 903ad72 commit dfb180b
Show file tree
Hide file tree
Showing 2 changed files with 177 additions and 6 deletions.
43 changes: 42 additions & 1 deletion tools/migration/legacy_fields_migration_lib.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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:
Expand All @@ -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)
Expand Down
140 changes: 135 additions & 5 deletions tools/migration/legacy_fields_migration_lib_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1023,8 +1023,7 @@ def test_migrate_expand_if_all_available_from_flag_sets(self):
flag_group {
flag: '%{foo}'
}
flag_group {
expand_if_all_available: 'bar'
flag_group {
flag: 'bar'
}
}
Expand All @@ -1038,7 +1037,6 @@ def test_migrate_expand_if_all_available_from_flag_sets(self):
flag: '%{foo}'
}
flag_group {
expand_if_all_available: 'bar'
flag: 'bar'
}
}
Expand All @@ -1056,7 +1054,7 @@ def test_migrate_expand_if_all_available_from_flag_sets(self):
.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)
Expand All @@ -1068,7 +1066,139 @@ def test_migrate_expand_if_all_available_from_flag_sets(self):
["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__":
Expand Down

0 comments on commit dfb180b

Please sign in to comment.