Skip to content

Commit

Permalink
Fixes for using -embed-bitcode on Xcode13 (#684)
Browse files Browse the repository at this point in the history
Adds the file path of `llvm-bc` files to the output file maps created by rules_swift, when `-embed-bitcode` is requested.

Without this file path, on Xcode13, we get compile errors due to object files not created in the correct paths - the paths specified as `object` in the output file maps are ignored.

Fixes #682.
  • Loading branch information
rsahara authored Sep 17, 2021
1 parent 15e8c3d commit fa254e1
Show file tree
Hide file tree
Showing 4 changed files with 257 additions and 3 deletions.
18 changes: 15 additions & 3 deletions swift/internal/compiling.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ load(":debugging.bzl", "should_embed_swiftmodule_for_debugging")
load(":derived_files.bzl", "derived_files")
load(
":feature_names.bzl",
"SWIFT_FEATURE_BITCODE_EMBEDDED",
"SWIFT_FEATURE_CACHEABLE_SWIFTMODULES",
"SWIFT_FEATURE_COVERAGE",
"SWIFT_FEATURE_COVERAGE_PREFIX_MAP",
Expand Down Expand Up @@ -2266,6 +2267,12 @@ def _declare_compile_outputs(
feature_name = SWIFT_FEATURE_EMIT_BC,
)

# If enabled the compiler will embed LLVM BC in the object files.
embeds_bc = is_feature_enabled(
feature_configuration = feature_configuration,
feature_name = SWIFT_FEATURE_BITCODE_EMBEDDED,
)

if not output_nature.emits_multiple_objects:
# If we're emitting a single object, we don't use an object map; we just
# declare the output file that the compiler will generate and there are
Expand All @@ -2286,6 +2293,7 @@ def _declare_compile_outputs(
# object files so that we can pass them all to the archive action.
output_info = _declare_multiple_outputs_and_write_output_file_map(
actions = actions,
embeds_bc = embeds_bc,
emits_bc = emits_bc,
emits_partial_modules = output_nature.emits_partial_modules,
srcs = srcs,
Expand Down Expand Up @@ -2329,6 +2337,7 @@ def _declare_compile_outputs(

def _declare_multiple_outputs_and_write_output_file_map(
actions,
embeds_bc,
emits_bc,
emits_partial_modules,
srcs,
Expand All @@ -2337,6 +2346,8 @@ def _declare_multiple_outputs_and_write_output_file_map(
Args:
actions: The object used to register actions.
embeds_bc: If `True` the compiler will embed LLVM BC in the object
files.
emits_bc: If `True` the compiler will generate LLVM BC files instead of
object files.
emits_partial_modules: `True` if the compilation action is expected to
Expand Down Expand Up @@ -2381,16 +2392,17 @@ def _declare_multiple_outputs_and_write_output_file_map(
for src in srcs:
src_output_map = {}

if emits_bc:
if embeds_bc or emits_bc:
# Declare the llvm bc file (there is one per source file).
obj = derived_files.intermediate_bc_file(
actions = actions,
target_name = target_name,
src = src,
)
output_objs.append(obj)
(output_objs if emits_bc else other_outputs).append(obj)
src_output_map["llvm-bc"] = obj.path
else:

if not emits_bc:
# Declare the object file (there is one per source file).
obj = derived_files.intermediate_object_file(
actions = actions,
Expand Down
3 changes: 3 additions & 0 deletions test/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ load(":coverage_settings_tests.bzl", "coverage_settings_test_suite")
load(":debug_settings_tests.bzl", "debug_settings_test_suite")
load(":generated_header_tests.bzl", "generated_header_test_suite")
load(":module_cache_settings_tests.bzl", "module_cache_settings_test_suite")
load(":output_file_map_tests.bzl", "output_file_map_test_suite")
load(":private_deps_tests.bzl", "private_deps_test_suite")
load(":swift_through_non_swift_tests.bzl", "swift_through_non_swift_test_suite")
load(":split_derived_files_tests.bzl", "split_derived_files_test_suite")
Expand All @@ -20,6 +21,8 @@ generated_header_test_suite()

module_cache_settings_test_suite()

output_file_map_test_suite()

private_deps_test_suite()

swift_through_non_swift_test_suite()
Expand Down
89 changes: 89 additions & 0 deletions test/output_file_map_tests.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
# Copyright 2021 The Bazel Authors. All rights reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

"""Tests for `output_file_map`."""

load(
"@build_bazel_rules_swift//test/rules:output_file_map_test.bzl",
"make_output_file_map_test_rule",
"output_file_map_test",
)

output_file_map_embed_bitcode_test = make_output_file_map_test_rule(
config_settings = {
"//command_line_option:features": [
"swift.bitcode_embedded",
],
},
)

output_file_map_embed_bitcode_wmo_test = make_output_file_map_test_rule(
config_settings = {
"//command_line_option:swiftcopt": [
"-whole-module-optimization",
],
"//command_line_option:features": [
"swift.bitcode_embedded",
],
},
)

def output_file_map_test_suite(name = "output_file_map"):
"""Test suite for `swift_library` generating output file maps.
Args:
name: The name prefix for all the nested tests
"""

output_file_map_test(
name = "{}_without_bitcode".format(name),
expected_mapping = {
"object": "test/fixtures/debug_settings/simple_objs/Empty.swift.o",
},
file_entry = "test/fixtures/debug_settings/Empty.swift",
output_file_map = "test/fixtures/debug_settings/simple.output_file_map.json",
tags = [name],
target_under_test = "@build_bazel_rules_swift//test/fixtures/debug_settings:simple",
)

# In Xcode13, the bitcode file needs to be in the output file map
# (https://github.com/bazelbuild/rules_swift/issues/682).
output_file_map_embed_bitcode_test(
name = "{}_embed_bitcode".format(name),
expected_mapping = {
"llvm-bc": "test/fixtures/debug_settings/simple_objs/Empty.swift.bc",
"object": "test/fixtures/debug_settings/simple_objs/Empty.swift.o",
},
file_entry = "test/fixtures/debug_settings/Empty.swift",
output_file_map = "test/fixtures/debug_settings/simple.output_file_map.json",
tags = [name],
target_under_test = "@build_bazel_rules_swift//test/fixtures/debug_settings:simple",
)

output_file_map_embed_bitcode_wmo_test(
name = "{}_embed_bitcode_wmo".format(name),
expected_mapping = {
"llvm-bc": "test/fixtures/debug_settings/simple_objs/Empty.swift.bc",
"object": "test/fixtures/debug_settings/simple_objs/Empty.swift.o",
},
file_entry = "test/fixtures/debug_settings/Empty.swift",
output_file_map = "test/fixtures/debug_settings/simple.output_file_map.json",
tags = [name],
target_under_test = "@build_bazel_rules_swift//test/fixtures/debug_settings:simple",
)

native.test_suite(
name = name,
tags = [name],
)
150 changes: 150 additions & 0 deletions test/rules/output_file_map_test.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
# Copyright 2021 The Bazel Authors. All rights reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

"""Rules for testing the output file maps."""

load("@bazel_skylib//lib:collections.bzl", "collections")
load("@bazel_skylib//lib:unittest.bzl", "analysistest", "unittest")

def _output_file_map_test_impl(ctx):
env = analysistest.begin(ctx)
target_under_test = analysistest.target_under_test(env)

# Find the WriteFile action that outputs the file map.
actions = analysistest.target_actions(env)
output_file_map = ctx.attr.output_file_map
action_outputs = [
(action, [file.short_path for file in action.outputs.to_list()])
for action in actions
]
matching_actions = [
action
for action, outputs in action_outputs
if output_file_map in outputs
]
if not matching_actions:
actual_outputs = collections.uniq([
output.short_path
for action in actions
for output in action.outputs.to_list()
if output.path.endswith(".output_file_map.json")
])
unittest.fail(
env,
("Target '{}' registered no actions that outputs '{}' " +
"(it had {}).").format(
str(target_under_test.label),
output_file_map,
actual_outputs,
),
)
return analysistest.end(env)
if len(matching_actions) != 1:
unittest.fail(
env,
("Expected exactly one action that outputs '{}', " +
"but found {}.").format(
output_file_map,
len(matching_actions),
),
)
return analysistest.end(env)

action = matching_actions[0]
message_prefix = "In {} action for output '{}' for target '{}', ".format(
action.mnemonic,
output_file_map,
str(target_under_test.label),
)

content = json.decode(action.content)
file_entry = ctx.attr.file_entry
if file_entry not in content:
unittest.fail(
env,
("Output file map '{}' doesn't contain file entry '{}' " +
"(it had {}).").format(
output_file_map,
file_entry,
content.keys(),
),
)
return analysistest.end(env)

file_map = content[file_entry]
for expected_key, expected_value in ctx.attr.expected_mapping.items():
if expected_key not in file_map:
unittest.fail(
env,
("{}expected file map to contain '{}', " +
"but it did not: {}.").format(
message_prefix,
expected_key,
file_map.keys(),
),
)
elif not file_map[expected_key].endswith(expected_value):
unittest.fail(
env,
("{}expected file map to have '{}' as '{}', " +
"but it did not: {}.").format(
message_prefix,
expected_value,
expected_key,
file_map[expected_key],
),
)

return analysistest.end(env)

def make_output_file_map_test_rule(config_settings = {}):
"""Returns a new `output_file_map_test`-like rule with custom configs.
Args:
config_settings: A dictionary of configuration settings and their values
that should be applied during tests.
Returns:
A rule returned by `analysistest.make` that has the
`output_file_map_test` interface and the given config settings.
"""
return analysistest.make(
_output_file_map_test_impl,
attrs = {
"expected_mapping": attr.string_dict(
mandatory = False,
doc = """\
A dict with the keys and the values expected to be set in the output file map
for the given file entry.
""",
),
"file_entry": attr.string(
mandatory = True,
doc = """\
The file entry in the output file map that will be inspected.
""",
),
"output_file_map": attr.string(
mandatory = True,
doc = """\
The path of the output file map that is expected to be generated.
The WriteFile action that outputs this file will be inspected.
""",
),
},
config_settings = config_settings,
)

# A default instantiation of the rule when no custom config settings are needed.
output_file_map_test = make_output_file_map_test_rule()

0 comments on commit fa254e1

Please sign in to comment.