Always put linker_flags from linking_mode_flags.DYNAMIC to nodeps-dynamic-library

This cl fixes a bug in the migrator where it didn't pass mentioned flags to c++-nodeps-dynamic-library unconditionally (it only passed them as with feature { feature: 'dynamic_linking_mode' }, which is incorrect, the feature doesn't not apply to nodeps-dynamic-library, only to transitive linking actions.

https://github.com/bazelbuild/bazel/issues/6861
https://github.com/bazelbuild/bazel/issues/5883

RELNOTES: None.
PiperOrigin-RevId: 229692958
This commit is contained in:
hlopko 2019-01-17 00:07:12 -08:00 committed by Copybara-Service
parent 0fd5bb9b75
commit f9cdb36627
2 changed files with 86 additions and 30 deletions

View File

@ -42,6 +42,11 @@ DYNAMIC_LIBRARY_LINK_ACTIONS = [
"c++-link-dynamic-library", "c++-link-nodeps-dynamic-library" "c++-link-dynamic-library", "c++-link-nodeps-dynamic-library"
] ]
NODEPS_DYNAMIC_LIBRARY_LINK_ACTIONS = ["c++-link-nodeps-dynamic-library"]
TRANSITIVE_LINK_ACTIONS = ["c++-link-executable", "c++-link-dynamic-library"]
def compile_actions(toolchain): def compile_actions(toolchain):
"""Returns compile actions for cc or objc rules.""" """Returns compile actions for cc or objc rules."""
if _is_objc_toolchain(toolchain): if _is_objc_toolchain(toolchain):
@ -56,6 +61,15 @@ def link_actions(toolchain):
else: else:
return ALL_CC_LINK_ACTIONS return ALL_CC_LINK_ACTIONS
def transitive_link_actions(toolchain):
"""Returns transitive link actions for cc or objc rules."""
if _is_objc_toolchain(toolchain):
return TRANSITIVE_LINK_ACTIONS + ALL_OBJC_LINK_ACTIONS
else:
return TRANSITIVE_LINK_ACTIONS
def _is_objc_toolchain(toolchain): def _is_objc_toolchain(toolchain):
return any(ac.action_name == "objc-compile" for ac in toolchain.action_config) return any(ac.action_name == "objc-compile" for ac in toolchain.action_config)
@ -312,7 +326,8 @@ def _extract_legacy_compile_flag_sets_for(toolchain):
"""Get flag sets for default_compile_flags feature.""" """Get flag sets for default_compile_flags feature."""
result = [] result = []
if toolchain.compiler_flag: if toolchain.compiler_flag:
result.append([None, compile_actions(toolchain), toolchain.compiler_flag, []]) result.append(
[None, compile_actions(toolchain), toolchain.compiler_flag, []])
# Migrate compiler_flag from compilation_mode_flags # Migrate compiler_flag from compilation_mode_flags
for cmf in toolchain.compilation_mode_flags: for cmf in toolchain.compilation_mode_flags:
@ -370,17 +385,25 @@ def _extract_legacy_link_flag_sets_for(toolchain):
# Migrate linker_flags from linking_mode_flags # Migrate linker_flags from linking_mode_flags
for lmf in toolchain.linking_mode_flags: for lmf in toolchain.linking_mode_flags:
mode = crosstool_config_pb2.LinkingMode.Name(lmf.mode) mode = crosstool_config_pb2.LinkingMode.Name(lmf.mode)
mode = LINKING_MODE_TO_FEATURE_NAME.get(mode) feature_name = LINKING_MODE_TO_FEATURE_NAME.get(mode)
# if the feature is already there, we don't migrate, lmf is not used # if the feature is already there, we don't migrate, lmf is not used
if _contains_feature(toolchain, mode): if _contains_feature(toolchain, feature_name):
continue continue
if lmf.linker_flag: if lmf.linker_flag:
feature = toolchain.feature.add() feature = toolchain.feature.add()
feature.name = mode feature.name = feature_name
if mode == "DYNAMIC":
if lmf.linker_flag: result.append(
result.append([mode, link_actions(toolchain), lmf.linker_flag, []]) [None, NODEPS_DYNAMIC_LIBRARY_LINK_ACTIONS, lmf.linker_flag, []])
result.append([
feature_name,
transitive_link_actions(toolchain), lmf.linker_flag, []
])
else:
result.append(
[feature_name,
link_actions(toolchain), lmf.linker_flag, []])
if toolchain.dynamic_library_linker_flag: if toolchain.dynamic_library_linker_flag:
result.append([ result.append([
@ -390,7 +413,9 @@ def _extract_legacy_link_flag_sets_for(toolchain):
if toolchain.test_only_linker_flag: if toolchain.test_only_linker_flag:
result.append([ result.append([
None, link_actions(toolchain), toolchain.test_only_linker_flag, ["is_cc_test"] None,
link_actions(toolchain), toolchain.test_only_linker_flag,
["is_cc_test"]
]) ])
return result return result

View File

@ -7,6 +7,8 @@ from tools.migration.legacy_fields_migration_lib import ALL_CXX_COMPILE_ACTIONS
from tools.migration.legacy_fields_migration_lib import ALL_CC_LINK_ACTIONS from tools.migration.legacy_fields_migration_lib import ALL_CC_LINK_ACTIONS
from tools.migration.legacy_fields_migration_lib import ALL_OBJC_LINK_ACTIONS from tools.migration.legacy_fields_migration_lib import ALL_OBJC_LINK_ACTIONS
from tools.migration.legacy_fields_migration_lib import DYNAMIC_LIBRARY_LINK_ACTIONS from tools.migration.legacy_fields_migration_lib import DYNAMIC_LIBRARY_LINK_ACTIONS
from tools.migration.legacy_fields_migration_lib import NODEPS_DYNAMIC_LIBRARY_LINK_ACTIONS
from tools.migration.legacy_fields_migration_lib import TRANSITIVE_LINK_ACTIONS
from tools.migration.legacy_fields_migration_lib import migrate_legacy_fields from tools.migration.legacy_fields_migration_lib import migrate_legacy_fields
@ -473,8 +475,12 @@ class LegacyFieldsMigrationLibTest(unittest.TestCase):
mode: MOSTLY_STATIC mode: MOSTLY_STATIC
linker_flag: 'lmf-flag-3' linker_flag: 'lmf-flag-3'
} }
dynamic_library_linker_flag: 'dl-flag-4' linking_mode_flags {
test_only_linker_flag: 'to-flag-5' mode: DYNAMIC
linker_flag: 'lmf-dynamic-flag-4'
}
dynamic_library_linker_flag: 'dl-flag-5'
test_only_linker_flag: 'to-flag-6'
""") """)
migrate_legacy_fields(crosstool) migrate_legacy_fields(crosstool)
output = crosstool.toolchain[0] output = crosstool.toolchain[0]
@ -483,25 +489,42 @@ class LegacyFieldsMigrationLibTest(unittest.TestCase):
self.assertEqual(output.feature[0].flag_set[0].action[:], ALL_CC_LINK_ACTIONS) self.assertEqual(output.feature[0].flag_set[0].action[:], ALL_CC_LINK_ACTIONS)
self.assertEqual(output.feature[0].flag_set[0].flag_group[0].flag[:], self.assertEqual(output.feature[0].flag_set[0].flag_group[0].flag[:],
["linker-flag-1"]) ["linker-flag-1"])
self.assertEqual(output.feature[0].flag_set[1].action[:], ALL_CC_LINK_ACTIONS) self.assertEqual(output.feature[0].flag_set[1].action[:], ALL_CC_LINK_ACTIONS)
self.assertEqual(output.feature[0].flag_set[1].with_feature[0].feature[0], self.assertEqual(output.feature[0].flag_set[1].with_feature[0].feature[0],
"opt") "opt")
self.assertEqual(output.feature[0].flag_set[1].flag_group[0].flag[:], self.assertEqual(output.feature[0].flag_set[1].flag_group[0].flag,
["cmf-flag-2"]) ["cmf-flag-2"])
self.assertEqual(output.feature[0].flag_set[2].action[:], ALL_CC_LINK_ACTIONS)
self.assertEqual(output.feature[0].flag_set[2].action, ALL_CC_LINK_ACTIONS)
self.assertEqual(output.feature[0].flag_set[2].with_feature[0].feature[0], self.assertEqual(output.feature[0].flag_set[2].with_feature[0].feature[0],
"static_linking_mode") "static_linking_mode")
self.assertEqual(output.feature[0].flag_set[2].flag_group[0].flag[:], self.assertEqual(output.feature[0].flag_set[2].flag_group[0].flag,
["lmf-flag-3"]) ["lmf-flag-3"])
self.assertEqual(output.feature[0].flag_set[3].action[:],
self.assertEqual(len(output.feature[0].flag_set[3].with_feature), 0)
self.assertEqual(output.feature[0].flag_set[3].flag_group[0].flag,
["lmf-dynamic-flag-4"])
self.assertEqual(output.feature[0].flag_set[3].action,
NODEPS_DYNAMIC_LIBRARY_LINK_ACTIONS)
self.assertEqual(output.feature[0].flag_set[4].with_feature[0].feature[0],
"dynamic_linking_mode")
self.assertEqual(output.feature[0].flag_set[4].flag_group[0].flag,
["lmf-dynamic-flag-4"])
self.assertEqual(output.feature[0].flag_set[4].action,
TRANSITIVE_LINK_ACTIONS)
self.assertEqual(output.feature[0].flag_set[5].flag_group[0].flag,
["dl-flag-5"])
self.assertEqual(output.feature[0].flag_set[5].action,
DYNAMIC_LIBRARY_LINK_ACTIONS) DYNAMIC_LIBRARY_LINK_ACTIONS)
self.assertEqual(output.feature[0].flag_set[3].flag_group[0].flag[:],
["dl-flag-4"]) self.assertEqual(output.feature[0].flag_set[6].flag_group[0].flag,
self.assertEqual(output.feature[0].flag_set[4].action[:], ALL_CC_LINK_ACTIONS) ["to-flag-6"])
self.assertEqual(output.feature[0].flag_set[4].flag_group[0].flag[:], self.assertEqual(output.feature[0].flag_set[6].action, ALL_CC_LINK_ACTIONS)
["to-flag-5"])
self.assertEqual( self.assertEqual(
output.feature[0].flag_set[4].flag_group[0].expand_if_all_available[:], output.feature[0].flag_set[6].flag_group[0].expand_if_all_available,
["is_cc_test"]) ["is_cc_test"])
def test_all_linker_flag_objc_actions(self): def test_all_linker_flag_objc_actions(self):
@ -516,17 +539,22 @@ class LegacyFieldsMigrationLibTest(unittest.TestCase):
mode: MOSTLY_STATIC mode: MOSTLY_STATIC
linker_flag: 'lmf-flag-3' linker_flag: 'lmf-flag-3'
} }
dynamic_library_linker_flag: 'dl-flag-4' dynamic_library_linker_flag: 'dl-flag-5'
test_only_linker_flag: 'to-flag-5' test_only_linker_flag: 'to-flag-6'
""") """)
migrate_legacy_fields(crosstool) migrate_legacy_fields(crosstool)
output = crosstool.toolchain[0] output = crosstool.toolchain[0]
self.assertEqual(output.feature[0].name, "default_link_flags") self.assertEqual(output.feature[0].name, "default_link_flags")
self.assertEqual(output.feature[0].flag_set[0].action[:], ALL_CC_LINK_ACTIONS + ALL_OBJC_LINK_ACTIONS) self.assertEqual(output.feature[0].flag_set[0].action[:],
self.assertEqual(output.feature[0].flag_set[1].action[:], ALL_CC_LINK_ACTIONS + ALL_OBJC_LINK_ACTIONS) ALL_CC_LINK_ACTIONS + ALL_OBJC_LINK_ACTIONS)
self.assertEqual(output.feature[0].flag_set[2].action[:], ALL_CC_LINK_ACTIONS + ALL_OBJC_LINK_ACTIONS) self.assertEqual(output.feature[0].flag_set[1].action[:],
self.assertEqual(output.feature[0].flag_set[3].action[:], DYNAMIC_LIBRARY_LINK_ACTIONS) ALL_CC_LINK_ACTIONS + ALL_OBJC_LINK_ACTIONS)
self.assertEqual(output.feature[0].flag_set[4].action[:], ALL_CC_LINK_ACTIONS + ALL_OBJC_LINK_ACTIONS) self.assertEqual(output.feature[0].flag_set[2].action[:],
ALL_CC_LINK_ACTIONS + ALL_OBJC_LINK_ACTIONS)
self.assertEqual(output.feature[0].flag_set[3].action[:],
DYNAMIC_LIBRARY_LINK_ACTIONS)
self.assertEqual(output.feature[0].flag_set[4].action[:],
ALL_CC_LINK_ACTIONS + ALL_OBJC_LINK_ACTIONS)
def test_linking_mode_features_are_not_added_when_present(self): def test_linking_mode_features_are_not_added_when_present(self):
crosstool = make_crosstool(""" crosstool = make_crosstool("""
@ -610,7 +638,8 @@ class LegacyFieldsMigrationLibTest(unittest.TestCase):
output = crosstool.toolchain[0] output = crosstool.toolchain[0]
self.assertEqual(output.feature[0].name, "user_compile_flags") self.assertEqual(output.feature[0].name, "user_compile_flags")
self.assertEqual(output.feature[0].enabled, True) self.assertEqual(output.feature[0].enabled, True)
self.assertEqual(output.feature[0].flag_set[0].action, ALL_CC_COMPILE_ACTIONS) self.assertEqual(output.feature[0].flag_set[0].action,
ALL_CC_COMPILE_ACTIONS)
self.assertEqual( self.assertEqual(
output.feature[0].flag_set[0].flag_group[0].expand_if_all_available, output.feature[0].flag_set[0].flag_group[0].expand_if_all_available,
["user_compile_flags"]) ["user_compile_flags"])
@ -733,7 +762,8 @@ class LegacyFieldsMigrationLibTest(unittest.TestCase):
self.assertEqual(output.feature[1].name, "user_compile_flags") self.assertEqual(output.feature[1].name, "user_compile_flags")
self.assertEqual(output.feature[2].name, "sysroot") self.assertEqual(output.feature[2].name, "sysroot")
self.assertEqual(output.feature[3].name, "unfiltered_compile_flags") self.assertEqual(output.feature[3].name, "unfiltered_compile_flags")
self.assertEqual(output.feature[3].flag_set[0].action, ALL_CC_COMPILE_ACTIONS) self.assertEqual(output.feature[3].flag_set[0].action,
ALL_CC_COMPILE_ACTIONS)
self.assertEqual(output.feature[3].flag_set[0].flag_group[0].flag, self.assertEqual(output.feature[3].flag_set[0].flag_group[0].flag,
["unfiltered-flag-1"]) ["unfiltered-flag-1"])
@ -746,7 +776,8 @@ class LegacyFieldsMigrationLibTest(unittest.TestCase):
migrate_legacy_fields(crosstool) migrate_legacy_fields(crosstool)
output = crosstool.toolchain[0] output = crosstool.toolchain[0]
self.assertEqual(output.feature[3].name, "unfiltered_compile_flags") self.assertEqual(output.feature[3].name, "unfiltered_compile_flags")
self.assertEqual(output.feature[3].flag_set[0].action, ALL_CC_COMPILE_ACTIONS) self.assertEqual(output.feature[3].flag_set[0].action,
ALL_CC_COMPILE_ACTIONS)
self.assertEqual(output.feature[3].flag_set[0].flag_group[0].flag, self.assertEqual(output.feature[3].flag_set[0].flag_group[0].flag,
["unfiltered-flag-1"]) ["unfiltered-flag-1"])