Skip to content

Commit

Permalink
Fix legacy_fields_migrator
Browse files Browse the repository at this point in the history 
Another round of fixes:

* if the toolchain contains legacy_compile_flags or legacy_link_flags, replace the feature with default_compile_flags or default_link_flags. This is to ensure the location of the flags stays intact.
* it fixes the order of compilation flags, the correct order is:
  1) compiler_flag
  2) compilation_mode_flags.compiler_flag
  3) cxx_flag
  4) compilation_mode_flags.cxx_flag
* We don't add cxx_flags to assemble and preprocess-assemble actions
* We don't add sysroot to assemble action

bazelbuild/bazel#6861
bazelbuild/bazel#5883

RELNOTES: None.
PiperOrigin-RevId: 229336027
  • Loading branch information
@hlopko @copybara-github
hlopko authored and copybara-github committed Jan 15, 2019
1 parent 5a13c61 commit 04195ad
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 21 deletions.
47 changes: 36 additions & 11 deletions tools/migration/legacy_fields_migration_lib.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@
]

ALL_CXX_COMPILE_ACTIONS = [
action for action in ALL_CC_COMPILE_ACTIONS if action != "c-compile"
action for action in ALL_CC_COMPILE_ACTIONS
if action not in ["c-compile", "preprocess-assemble", "assemble"]
]

ALL_CC_LINK_ACTIONS = [
Expand All @@ -44,16 +45,16 @@
def compile_actions(toolchain):
"""Returns compile actions for cc or objc rules."""
if _is_objc_toolchain(toolchain):
return ALL_CC_COMPILE_ACTIONS + ALL_OBJC_COMPILE_ACTIONS
return ALL_CC_COMPILE_ACTIONS + ALL_OBJC_COMPILE_ACTIONS
else:
return ALL_CC_COMPILE_ACTIONS
return ALL_CC_COMPILE_ACTIONS

def link_actions(toolchain):
"""Returns link actions for cc or objc rules."""
if _is_objc_toolchain(toolchain):
return ALL_CC_LINK_ACTIONS + ALL_OBJC_LINK_ACTIONS
return ALL_CC_LINK_ACTIONS + ALL_OBJC_LINK_ACTIONS
else:
return ALL_CC_LINK_ACTIONS
return ALL_CC_LINK_ACTIONS

def _is_objc_toolchain(toolchain):
return any(ac.action_name == "objc-compile" for ac in toolchain.action_config)
Expand Down Expand Up @@ -172,15 +173,29 @@ def migrate_legacy_fields(crosstool):
if flag_sets:
if _contains_feature(toolchain, "default_link_flags"):
continue
feature = _prepend_feature(toolchain)
if _contains_feature(toolchain, "legacy_link_flags"):
for f in toolchain.feature:
if f.name == "legacy_link_flags":
f.ClearField("flag_set")
feature = f
break
else:
feature = _prepend_feature(toolchain)
feature.name = "default_link_flags"
feature.enabled = True
_add_flag_sets(feature, flag_sets)

# Create default_compile_flags feature for compiler_flag, cxx_flag
flag_sets = _extract_legacy_compile_flag_sets_for(toolchain)
if flag_sets and not _contains_feature(toolchain, "default_compile_flags"):
feature = _prepend_feature(toolchain)
if _contains_feature(toolchain, "legacy_compile_flags"):
for f in toolchain.feature:
if f.name == "legacy_compile_flags":
f.ClearField("flag_set")
feature = f
break
else:
feature = _prepend_feature(toolchain)
feature.enabled = True
feature.name = "default_compile_flags"
_add_flag_sets(feature, flag_sets)
Expand Down Expand Up @@ -214,11 +229,13 @@ def migrate_legacy_fields(crosstool):
flag_group.flag[:] = ["%{user_compile_flags}"]

if not _contains_feature(toolchain, "sysroot"):
sysroot_actions = compile_actions(toolchain) + link_actions(toolchain)
sysroot_actions.remove("assemble")
feature = toolchain.feature.add()
feature.name = "sysroot"
feature.enabled = True
flag_set = feature.flag_set.add()
flag_set.action[:] = compile_actions(toolchain) + link_actions(toolchain)
flag_set.action[:] = sysroot_actions
flag_group = flag_set.flag_group.add()
flag_group.expand_if_all_available[:] = ["sysroot"]
flag_group.flag[:] = ["--sysroot=%{sysroot}"]
Expand Down Expand Up @@ -293,10 +310,8 @@ def _extract_legacy_compile_flag_sets_for(toolchain):
result = []
if toolchain.compiler_flag:
result.append([None, compile_actions(toolchain), toolchain.compiler_flag, []])
if toolchain.cxx_flag:
result.append([None, ALL_CXX_COMPILE_ACTIONS, toolchain.cxx_flag, []])

# Migrate compiler_flag/cxx_flag from compilation_mode_flags
# Migrate compiler_flag from compilation_mode_flags
for cmf in toolchain.compilation_mode_flags:
mode = crosstool_config_pb2.CompilationMode.Name(cmf.mode).lower()
# coverage mode has been a noop since a while
Expand All @@ -311,6 +326,16 @@ def _extract_legacy_compile_flag_sets_for(toolchain):
if cmf.compiler_flag:
result.append([mode, compile_actions(toolchain), cmf.compiler_flag, []])

if toolchain.cxx_flag:
result.append([None, ALL_CXX_COMPILE_ACTIONS, toolchain.cxx_flag, []])

# Migrate compiler_flag/cxx_flag from compilation_mode_flags
for cmf in toolchain.compilation_mode_flags:
mode = crosstool_config_pb2.CompilationMode.Name(cmf.mode).lower()
# coverage mode has been a noop since a while
if mode == "coverage":
continue

if cmf.cxx_flag:
result.append([mode, ALL_CXX_COMPILE_ACTIONS, cmf.cxx_flag, []])

Expand Down
53 changes: 43 additions & 10 deletions tools/migration/legacy_fields_migration_lib_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,37 @@ def test_deletes_default_toolchains(self):
migrate_legacy_fields(crosstool)
self.assertEqual(len(crosstool.default_toolchain), 0)

def test_replace_legacy_compile_flags(self):
crosstool = make_crosstool("""
feature { name: 'foo' }
feature { name: 'legacy_compile_flags' }
compiler_flag: 'clang-flag-1'
""")
migrate_legacy_fields(crosstool)
output = crosstool.toolchain[0]
self.assertEqual(len(output.compiler_flag), 0)
self.assertEqual(output.feature[0].name, "foo")
self.assertEqual(output.feature[1].name, "default_compile_flags")
self.assertEqual(output.feature[1].flag_set[0].action,
ALL_CC_COMPILE_ACTIONS)
self.assertEqual(output.feature[1].flag_set[0].flag_group[0].flag,
["clang-flag-1"])

def test_replace_legacy_link_flags(self):
crosstool = make_crosstool("""
feature { name: 'foo' }
feature { name: 'legacy_link_flags' }
linker_flag: 'ld-flag-1'
""")
migrate_legacy_fields(crosstool)
output = crosstool.toolchain[0]
self.assertEqual(len(output.compiler_flag), 0)
self.assertEqual(output.feature[0].name, "foo")
self.assertEqual(output.feature[1].name, "default_link_flags")
self.assertEqual(output.feature[1].flag_set[0].action, ALL_CC_LINK_ACTIONS)
self.assertEqual(output.feature[1].flag_set[0].flag_group[0].flag,
["ld-flag-1"])

def test_migrate_compiler_flags(self):
crosstool = make_crosstool("""
compiler_flag: 'clang-flag-1'
Expand Down Expand Up @@ -191,18 +222,18 @@ def test_compilation_mode_flags(self):
self.assertEqual(output.feature[0].flag_set[0].flag_group[0].flag,
["compile-flag-1"])

# flag set for cxx_flag fields
self.assertEqual(len(output.feature[0].flag_set[1].with_feature), 0)
self.assertEqual(output.feature[0].flag_set[1].flag_group[0].flag,
["cxx-flag-1"])

# flag set for compiler_flag from compilation_mode_flags
self.assertEqual(len(output.feature[0].flag_set[2].with_feature), 1)
self.assertEqual(output.feature[0].flag_set[2].with_feature[0].feature[0],
self.assertEqual(len(output.feature[0].flag_set[1].with_feature), 1)
self.assertEqual(output.feature[0].flag_set[1].with_feature[0].feature[0],
"opt")
self.assertEqual(output.feature[0].flag_set[2].flag_group[0].flag,
self.assertEqual(output.feature[0].flag_set[1].flag_group[0].flag,
["opt-flag-1"])

# flag set for cxx_flag fields
self.assertEqual(len(output.feature[0].flag_set[2].with_feature), 0)
self.assertEqual(output.feature[0].flag_set[2].flag_group[0].flag,
["cxx-flag-1"])

# flag set for cxx_flag from compilation_mode_flags
self.assertEqual(len(output.feature[0].flag_set[3].with_feature), 1)
self.assertEqual(output.feature[0].flag_set[3].with_feature[0].feature[0],
Expand Down Expand Up @@ -464,15 +495,17 @@ def test_user_compile_flags(self):
["%{user_compile_flags}"])

def test_sysroot(self):
sysroot_actions = ALL_CC_COMPILE_ACTIONS + ALL_CC_LINK_ACTIONS
sysroot_actions.remove("assemble")
self.assertTrue("assemble" not in sysroot_actions)
crosstool = make_crosstool("""
unfiltered_cxx_flag: 'unfiltered-flag-1'
""")
migrate_legacy_fields(crosstool)
output = crosstool.toolchain[0]
self.assertEqual(output.feature[1].name, "sysroot")
self.assertEqual(output.feature[1].enabled, True)
self.assertEqual(output.feature[1].flag_set[0].action,
ALL_CC_COMPILE_ACTIONS + ALL_CC_LINK_ACTIONS)
self.assertEqual(output.feature[1].flag_set[0].action, sysroot_actions)
self.assertEqual(
output.feature[1].flag_set[0].flag_group[0].expand_if_all_available,
["sysroot"])
Expand Down

0 comments on commit 04195ad

Please sign in to comment.