Skip to content

Commit d09eca8

Browse files
authored
Fix dependency linker flags being de-duped (#779)
Wrapping these in a depset caused these flags to be de-duplicated, which meant if a single dependency added `-framework foo -framework bar` it would turn into the invalid `-framework foo bar`. This mode is also discouraged by the documentation: https://docs.bazel.build/versions/main/skylark/lib/cc_common.html#create_linker_input We now use a list instead.
1 parent f684f34 commit d09eca8

File tree

7 files changed

+81
-10
lines changed

7 files changed

+81
-10
lines changed

swift/internal/linking.bzl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ def register_link_binary_action(
247247
linker_inputs = depset([
248248
cc_common.create_linker_input(
249249
owner = owner,
250-
user_link_flags = depset(dep_link_flags),
250+
user_link_flags = dep_link_flags,
251251
additional_inputs = objc.static_framework_file,
252252
),
253253
]),

test/BUILD

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ load(":ast_file_tests.bzl", "ast_file_test_suite")
33
load(":coverage_settings_tests.bzl", "coverage_settings_test_suite")
44
load(":debug_settings_tests.bzl", "debug_settings_test_suite")
55
load(":generated_header_tests.bzl", "generated_header_test_suite")
6+
load(":linking_tests.bzl", "linking_test_suite")
67
load(":module_cache_settings_tests.bzl", "module_cache_settings_test_suite")
78
load(":output_file_map_tests.bzl", "output_file_map_test_suite")
89
load(":private_deps_tests.bzl", "private_deps_test_suite")
@@ -20,6 +21,8 @@ debug_settings_test_suite(name = "debug_settings")
2021

2122
generated_header_test_suite(name = "generated_header")
2223

24+
linking_test_suite(name = "linking")
25+
2326
module_cache_settings_test_suite(name = "module_cache_settings")
2427

2528
output_file_map_test_suite(name = "output_file_map")

test/fixtures/linking/BUILD

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
load("//swift:swift.bzl", "swift_binary")
2+
load(":fake_framework.bzl", "fake_framework")
3+
4+
package(
5+
default_visibility = ["//test:__subpackages__"],
6+
)
7+
8+
fake_framework(
9+
name = "framework",
10+
)
11+
12+
swift_binary(
13+
name = "bin",
14+
srcs = ["main.swift"],
15+
deps = [":framework"],
16+
)
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
"""Simple rule to emulate apple_static_framework_import"""
2+
3+
def _impl(ctx):
4+
binary1 = ctx.actions.declare_file("framework1.framework/framework")
5+
ctx.actions.write(binary1, "empty")
6+
binary2 = ctx.actions.declare_file("framework2.framework/framework")
7+
ctx.actions.write(binary2, "empty")
8+
return apple_common.new_objc_provider(
9+
static_framework_file = depset([binary1, binary2]),
10+
)
11+
12+
fake_framework = rule(
13+
implementation = _impl,
14+
)

test/fixtures/linking/main.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
print("hello")

test/linking_tests.bzl

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
"""Tests for validating linking behavior"""
2+
3+
load(
4+
"@build_bazel_rules_swift//test/rules:action_command_line_test.bzl",
5+
"make_action_command_line_test_rule",
6+
)
7+
8+
linking_test = make_action_command_line_test_rule()
9+
10+
def linking_test_suite(name):
11+
linking_test(
12+
name = "{}_duplicate_linking_args".format(name),
13+
expected_argv = [
14+
"-framework framework1",
15+
"-framework framework2",
16+
],
17+
mnemonic = "CppLink",
18+
tags = [name],
19+
target_under_test = "@build_bazel_rules_swift//test/fixtures/linking:bin",
20+
)
21+
22+
native.test_suite(
23+
name = name,
24+
tags = [name],
25+
)

test/rules/action_command_line_test.bzl

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -44,15 +44,27 @@ def _action_command_line_test_impl(ctx):
4444
)
4545
return analysistest.end(env)
4646
if len(matching_actions) != 1:
47-
unittest.fail(
48-
env,
49-
("Expected exactly one action with the mnemonic '{}', " +
50-
"but found {}.").format(
51-
mnemonic,
52-
len(matching_actions),
53-
),
54-
)
55-
return analysistest.end(env)
47+
# This is a hack to avoid CppLink meaning binary linking and static
48+
# library archiving https://github.com/bazelbuild/bazel/pull/15060
49+
if mnemonic == "CppLink" and len(matching_actions) == 2:
50+
matching_actions = [
51+
action
52+
for action in matching_actions
53+
if action.argv[0] not in [
54+
"/usr/bin/ar",
55+
"external/local_config_cc/libtool",
56+
]
57+
]
58+
if len(matching_actions) != 1:
59+
unittest.fail(
60+
env,
61+
("Expected exactly one action with the mnemonic '{}', " +
62+
"but found {}.").format(
63+
mnemonic,
64+
len(matching_actions),
65+
),
66+
)
67+
return analysistest.end(env)
5668

5769
action = matching_actions[0]
5870
message_prefix = "In {} action for target '{}', ".format(

0 commit comments

Comments
 (0)