Skip to content

Commit

Permalink
Always put linker_flags from linking_mode_flags.DYNAMIC to nodeps-dyn…
Browse files Browse the repository at this point in the history 
…amic-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.

bazelbuild/bazel#6861
bazelbuild/bazel#5883

RELNOTES: None.
PiperOrigin-RevId: 229692958
  • Loading branch information
@hlopko @copybara-github
hlopko authored and copybara-github committed Jan 17, 2019
1 parent 0fd5bb9 commit f9cdb36
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 30 deletions.
41 changes: 33 additions & 8 deletions tools/migration/legacy_fields_migration_lib.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@
"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):
"""Returns compile actions for cc or objc rules."""
if _is_objc_toolchain(toolchain):
Expand All @@ -56,6 +61,15 @@ def link_actions(toolchain):
else:
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):
return any(ac.action_name == "objc-compile" for ac in toolchain.action_config)

Expand Down Expand Up @@ -312,7 +326,8 @@ def _extract_legacy_compile_flag_sets_for(toolchain):
"""Get flag sets for default_compile_flags feature."""
result = []
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
for cmf in toolchain.compilation_mode_flags:
Expand Down Expand Up @@ -370,17 +385,25 @@ def _extract_legacy_link_flag_sets_for(toolchain):
# Migrate linker_flags from linking_mode_flags
for lmf in toolchain.linking_mode_flags:
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 _contains_feature(toolchain, mode):
if _contains_feature(toolchain, feature_name):
continue

if lmf.linker_flag:
feature = toolchain.feature.add()
feature.name = mode

if lmf.linker_flag:
result.append([mode, link_actions(toolchain), lmf.linker_flag, []])
feature.name = feature_name
if mode == "DYNAMIC":
result.append(
[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:
result.append([
Expand All @@ -390,7 +413,9 @@ def _extract_legacy_link_flag_sets_for(toolchain):

if toolchain.test_only_linker_flag:
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
Expand Down
75 changes: 53 additions & 22 deletions tools/migration/legacy_fields_migration_lib_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
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 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


Expand Down Expand Up @@ -473,8 +475,12 @@ def test_all_linker_flag_ordering(self):
mode: MOSTLY_STATIC
linker_flag: 'lmf-flag-3'
}
dynamic_library_linker_flag: 'dl-flag-4'
test_only_linker_flag: 'to-flag-5'
linking_mode_flags {
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)
output = crosstool.toolchain[0]
Expand All @@ -483,25 +489,42 @@ def test_all_linker_flag_ordering(self):
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[:],
["linker-flag-1"])

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],
"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"])
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],
"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"])
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)
self.assertEqual(output.feature[0].flag_set[3].flag_group[0].flag[:],
["dl-flag-4"])
self.assertEqual(output.feature[0].flag_set[4].action[:], ALL_CC_LINK_ACTIONS)
self.assertEqual(output.feature[0].flag_set[4].flag_group[0].flag[:],
["to-flag-5"])

self.assertEqual(output.feature[0].flag_set[6].flag_group[0].flag,
["to-flag-6"])
self.assertEqual(output.feature[0].flag_set[6].action, ALL_CC_LINK_ACTIONS)
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"])

def test_all_linker_flag_objc_actions(self):
Expand All @@ -516,17 +539,22 @@ def test_all_linker_flag_objc_actions(self):
mode: MOSTLY_STATIC
linker_flag: 'lmf-flag-3'
}
dynamic_library_linker_flag: 'dl-flag-4'
test_only_linker_flag: 'to-flag-5'
dynamic_library_linker_flag: 'dl-flag-5'
test_only_linker_flag: 'to-flag-6'
""")
migrate_legacy_fields(crosstool)
output = crosstool.toolchain[0]
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[1].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)
self.assertEqual(output.feature[0].flag_set[0].action[:],
ALL_CC_LINK_ACTIONS + ALL_OBJC_LINK_ACTIONS)
self.assertEqual(output.feature[0].flag_set[1].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):
crosstool = make_crosstool("""
Expand Down Expand Up @@ -610,7 +638,8 @@ def test_user_compile_flags(self):
output = crosstool.toolchain[0]
self.assertEqual(output.feature[0].name, "user_compile_flags")
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(
output.feature[0].flag_set[0].flag_group[0].expand_if_all_available,
["user_compile_flags"])
Expand Down Expand Up @@ -733,7 +762,8 @@ def test_unfiltered_compile_flags_is_added_at_the_end(self):
self.assertEqual(output.feature[1].name, "user_compile_flags")
self.assertEqual(output.feature[2].name, "sysroot")
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,
["unfiltered-flag-1"])

Expand All @@ -746,7 +776,8 @@ def test_unfiltered_compile_flags_are_not_added_for_objc(self):
migrate_legacy_fields(crosstool)
output = crosstool.toolchain[0]
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,
["unfiltered-flag-1"])

Expand Down

0 comments on commit f9cdb36

Please sign in to comment.