Skip to content

Commit

Permalink
Correctly exclude both the actual and virtual header when using `swif…
Browse files Browse the repository at this point in the history
…t_interop_hint.exclude_hdrs` on a `cc_library` that has `include_prefix` and/or `strip_include_prefix` set.

PiperOrigin-RevId: 412917671
  • Loading branch information
allevato authored and swiple-rules-gardener committed Nov 29, 2021
1 parent c8b58b5 commit 6b13232
Show file tree
Hide file tree
Showing 13 changed files with 253 additions and 9 deletions.
66 changes: 57 additions & 9 deletions swift/internal/swift_clang_module_aspect.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

"""Propagates unified `SwiftInfo` providers for C/Objective-C targets."""

load("@bazel_skylib//lib:sets.bzl", "sets")
load(":attrs.bzl", "swift_toolchain_attrs")
load(":compiling.bzl", "derive_module_name", "precompile_clang_module")
load(":derived_files.bzl", "derived_files")
Expand Down Expand Up @@ -188,7 +189,42 @@ def create_swift_interop_info(
unsupported_features = unsupported_features,
)

def _compute_all_excluded_headers(*, exclude_headers, target):
"""Returns the full set of headers to exclude for a target.
This function specifically handles the `cc_library` logic around the
`include_prefix` and `strip_include_prefix` attributes, which cause Bazel to
create a virtual header (symlink) for every public header in the target. For
the generated module map to be created, we must exclude both the actual
header file and the symlink.
Args:
exclude_headers: A list of `File`s representing headers that should be
excluded from the module.
target: The target to which the aspect is being applied.
Returns:
A list containing the complete set of headers that should be excluded,
including any virtual header symlinks that match a real header in the
excluded headers list passed into the function.
"""
exclude_headers_set = sets.make(exclude_headers)
virtual_exclude_headers = []

for action in target.actions:
if action.mnemonic != "Symlink":
continue

original_header = action.inputs.to_list()[0]
virtual_header = action.outputs.to_list()[0]

if sets.contains(exclude_headers_set, original_header):
virtual_exclude_headers.append(virtual_header)

return exclude_headers + virtual_exclude_headers

def _generate_module_map(
*,
actions,
compilation_context,
dependent_module_names,
Expand Down Expand Up @@ -234,17 +270,32 @@ def _generate_module_map(
else:
private_headers = compilation_context.direct_private_headers

module_map_file = derived_files.module_map(
actions = actions,
target_name = target.label.name,
)

# Sort dependent module names and the headers to ensure a deterministic
# order in the output file, in the event the compilation context would ever
# change this on us. For files, use the execution path as the sorting key.
def _path_sorting_key(file):
return file.path

public_headers = sorted(
compilation_context.direct_public_headers,
key = _path_sorting_key,
)

module_map_file = derived_files.module_map(
actions = actions,
target_name = target.label.name,
)

if exclude_headers:
# If we're excluding headers from the module map, make sure to pick up
# any virtual header symlinks that might be created, for example, by a
# `cc_library` using the `include_prefix` and/or `strip_include_prefix`
# attributes.
exclude_headers = _compute_all_excluded_headers(
exclude_headers = exclude_headers,
target = target,
)

write_module_map(
actions = actions,
dependent_module_names = sorted(dependent_module_names),
Expand All @@ -253,10 +304,7 @@ def _generate_module_map(
module_map_file = module_map_file,
module_name = module_name,
private_headers = sorted(private_headers, key = _path_sorting_key),
public_headers = sorted(
compilation_context.direct_public_headers,
key = _path_sorting_key,
),
public_headers = public_headers,
public_textual_headers = sorted(
compilation_context.direct_textual_headers,
key = _path_sorting_key,
Expand Down
4 changes: 4 additions & 0 deletions test/BUILD
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
load("@bazel_skylib//:bzl_library.bzl", "bzl_library")
load(":cc_library_tests.bzl", "cc_library_test_suite")
load(":debug_settings_tests.bzl", "debug_settings_test_suite")
load(":generated_header_tests.bzl", "generated_header_test_suite")
load(":interop_hints_tests.bzl", "interop_hints_test_suite")
Expand All @@ -7,6 +8,8 @@ load(":swift_through_non_swift_tests.bzl", "swift_through_non_swift_test_suite")

licenses(["notice"])

cc_library_test_suite()

debug_settings_test_suite()

generated_header_test_suite()
Expand All @@ -27,5 +30,6 @@ bzl_library(
deps = [
"//test/fixtures:starlark_tests_bzls",
"//test/rules:starlark_tests_bzls",
"@bazel_skylib//rules:build_test",
],
)
53 changes: 53 additions & 0 deletions test/cc_library_tests.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
# 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 interoperability with `cc_library`-specific features."""

load(
"@bazel_skylib//rules:build_test.bzl",
"build_test",
)

def cc_library_test_suite():
"""Test suite for interoperability with `cc_library`-specific features."""
name = "cc_library"

# Verify that Swift can import a `cc_library` that uses `include_prefix`,
# `strip_include_prefix`, or both.
build_test(
name = "{}_swift_imports_cc_library_with_include_prefix_manipulation".format(name),
targets = [
"@build_bazel_rules_swift//test/fixtures/cc_library:import_prefix_and_strip_prefix",
"@build_bazel_rules_swift//test/fixtures/cc_library:import_prefix_only",
"@build_bazel_rules_swift//test/fixtures/cc_library:import_strip_prefix_only",
],
tags = [name],
)

# Verify that `swift_interop_hint.exclude_hdrs` correctly excludes headers
# from a `cc_library` that uses `include_prefix` and/or
# `strip_include_prefix` (i.e., both the real header and the virtual header
# are excluded).
build_test(
name = "{}_swift_interop_hint_excludes_headers_with_include_prefix_manipulation".format(name),
targets = [
"@build_bazel_rules_swift//test/fixtures/cc_library:import_prefix_and_strip_prefix_with_exclusion",
],
tags = [name],
)

native.test_suite(
name = name,
tags = [name],
)
95 changes: 95 additions & 0 deletions test/fixtures/cc_library/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
load(
"//swift:swift.bzl",
"swift_interop_hint",
"swift_library",
)
load("//test/fixtures:common.bzl", "FIXTURE_TAGS")

package(
default_visibility = ["//test:__subpackages__"],
)

licenses(["notice"])

###############################################################################
# Fixtures for testing complex `cc_library` interop cases.

cc_library(
name = "prefix_only",
hdrs = [
"header.h",
"header_prefix_only.h",
],
aspect_hints = ["//swift:auto_module"],
include_prefix = "some/prefix",
tags = FIXTURE_TAGS,
)

cc_library(
name = "strip_prefix_only",
hdrs = [
"header.h",
"header_strip_prefix_only.h",
],
aspect_hints = ["//swift:auto_module"],
strip_include_prefix = "//test",
tags = FIXTURE_TAGS,
)

cc_library(
name = "prefix_and_strip_prefix",
hdrs = [
"header.h",
"header_prefix_and_strip_prefix.h",
],
aspect_hints = ["//swift:auto_module"],
include_prefix = "some/prefix",
strip_include_prefix = "//test",
tags = FIXTURE_TAGS,
)

cc_library(
name = "prefix_and_strip_prefix_with_exclusion",
hdrs = [
"header.h",
"header_with_error.h",
],
aspect_hints = [":prefix_and_strip_prefix_with_exclusion_swift_hint"],
include_prefix = "some/prefix",
strip_include_prefix = "//test",
tags = FIXTURE_TAGS,
)

swift_interop_hint(
name = "prefix_and_strip_prefix_with_exclusion_swift_hint",
exclude_hdrs = ["header_with_error.h"],
tags = FIXTURE_TAGS,
)

swift_library(
name = "import_prefix_only",
srcs = ["ImportPrefixOnly.swift"],
tags = FIXTURE_TAGS,
deps = [":prefix_only"],
)

swift_library(
name = "import_strip_prefix_only",
srcs = ["ImportStripPrefixOnly.swift"],
tags = FIXTURE_TAGS,
deps = [":strip_prefix_only"],
)

swift_library(
name = "import_prefix_and_strip_prefix",
srcs = ["ImportPrefixAndStripPrefix.swift"],
tags = FIXTURE_TAGS,
deps = [":prefix_and_strip_prefix"],
)

swift_library(
name = "import_prefix_and_strip_prefix_with_exclusion",
srcs = ["ImportPrefixAndStripPrefixWithExclusion.swift"],
tags = FIXTURE_TAGS,
deps = [":prefix_and_strip_prefix_with_exclusion"],
)
3 changes: 3 additions & 0 deletions test/fixtures/cc_library/ImportPrefixAndStripPrefix.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import test_fixtures_cc_library_prefix_and_strip_prefix

func f(_ x: some_structure) {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import test_fixtures_cc_library_prefix_and_strip_prefix_with_exclusion

func f(_ x: some_structure) {}
3 changes: 3 additions & 0 deletions test/fixtures/cc_library/ImportPrefixOnly.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import test_fixtures_cc_library_prefix_only

func f(_ x: some_structure) {}
3 changes: 3 additions & 0 deletions test/fixtures/cc_library/ImportStripPrefixOnly.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import test_fixtures_cc_library_strip_prefix_only

func f(_ x: some_structure) {}
8 changes: 8 additions & 0 deletions test/fixtures/cc_library/header.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#ifndef RULES_SWIFT_TEST_FIXTURES_CC_LIBRARY_HEADER_H_
#define RULES_SWIFT_TEST_FIXTURES_CC_LIBRARY_HEADER_H_

typedef struct {
int field;
} some_structure;

#endif // RULES_SWIFT_TEST_FIXTURES_CC_LIBRARY_HEADER_H_
6 changes: 6 additions & 0 deletions test/fixtures/cc_library/header_prefix_and_strip_prefix.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#ifndef RULES_SWIFT_TEST_FIXTURES_CC_LIBRARY_HEADER_PREFIX_AND_STRIP_PREFIX_H_
#define RULES_SWIFT_TEST_FIXTURES_CC_LIBRARY_HEADER_PREFIX_AND_STRIP_PREFIX_H_

#include "some/prefix/fixtures/cc_library/header.h"

#endif // RULES_SWIFT_TEST_FIXTURES_CC_LIBRARY_HEADER_PREFIX_AND_STRIP_PREFIX_H_
6 changes: 6 additions & 0 deletions test/fixtures/cc_library/header_prefix_only.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#ifndef RULES_SWIFT_TEST_FIXTURES_CC_LIBRARY_HEADER_PREFIX_ONLY_H_
#define RULES_SWIFT_TEST_FIXTURES_CC_LIBRARY_HEADER_PREFIX_ONLY_H_

#include "some/prefix/header.h"

#endif // RULES_SWIFT_TEST_FIXTURES_CC_LIBRARY_HEADER_PREFIX_ONLY_H_
6 changes: 6 additions & 0 deletions test/fixtures/cc_library/header_strip_prefix_only.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#ifndef RULES_SWIFT_TEST_FIXTURES_CC_LIBRARY_HEADER_STRIP_PREFIX_ONLY_H_
#define RULES_SWIFT_TEST_FIXTURES_CC_LIBRARY_HEADER_STRIP_PREFIX_ONLY_H_

#include "fixtures/cc_library/header.h"

#endif // RULES_SWIFT_TEST_FIXTURES_CC_LIBRARY_HEADER_STRIP_PREFIX_ONLY_H_
6 changes: 6 additions & 0 deletions test/fixtures/cc_library/header_with_error.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#ifndef RULES_SWIFT_TEST_FIXTURES_CC_LIBRARY_HEADER_WITH_ERROR_H_
#define RULES_SWIFT_TEST_FIXTURES_CC_LIBRARY_HEADER_WITH_ERROR_H_

#error This should never get processed.

#endif // RULES_SWIFT_TEST_FIXTURES_CC_LIBRARY_HEADER_WITH_ERROR_H_

2 comments on commit 6b13232

@keith
Copy link
Member

@keith keith commented on 6b13232 Jan 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brentleyjones
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.