From 6b13232996f759a8a98d21e4b94d086c4b604d64 Mon Sep 17 00:00:00 2001 From: Tony Allevato Date: Mon, 29 Nov 2021 10:07:50 -0800 Subject: [PATCH] Correctly exclude both the actual and virtual header when using `swift_interop_hint.exclude_hdrs` on a `cc_library` that has `include_prefix` and/or `strip_include_prefix` set. PiperOrigin-RevId: 412917671 --- swift/internal/swift_clang_module_aspect.bzl | 66 +++++++++++-- test/BUILD | 4 + test/cc_library_tests.bzl | 53 +++++++++++ test/fixtures/cc_library/BUILD | 95 +++++++++++++++++++ .../ImportPrefixAndStripPrefix.swift | 3 + ...ortPrefixAndStripPrefixWithExclusion.swift | 3 + .../cc_library/ImportPrefixOnly.swift | 3 + .../cc_library/ImportStripPrefixOnly.swift | 3 + test/fixtures/cc_library/header.h | 8 ++ .../header_prefix_and_strip_prefix.h | 6 ++ test/fixtures/cc_library/header_prefix_only.h | 6 ++ .../cc_library/header_strip_prefix_only.h | 6 ++ test/fixtures/cc_library/header_with_error.h | 6 ++ 13 files changed, 253 insertions(+), 9 deletions(-) create mode 100644 test/cc_library_tests.bzl create mode 100644 test/fixtures/cc_library/BUILD create mode 100644 test/fixtures/cc_library/ImportPrefixAndStripPrefix.swift create mode 100644 test/fixtures/cc_library/ImportPrefixAndStripPrefixWithExclusion.swift create mode 100644 test/fixtures/cc_library/ImportPrefixOnly.swift create mode 100644 test/fixtures/cc_library/ImportStripPrefixOnly.swift create mode 100644 test/fixtures/cc_library/header.h create mode 100644 test/fixtures/cc_library/header_prefix_and_strip_prefix.h create mode 100644 test/fixtures/cc_library/header_prefix_only.h create mode 100644 test/fixtures/cc_library/header_strip_prefix_only.h create mode 100644 test/fixtures/cc_library/header_with_error.h diff --git a/swift/internal/swift_clang_module_aspect.bzl b/swift/internal/swift_clang_module_aspect.bzl index 871bfd569..f62a06fb7 100644 --- a/swift/internal/swift_clang_module_aspect.bzl +++ b/swift/internal/swift_clang_module_aspect.bzl @@ -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") @@ -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, @@ -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), @@ -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, diff --git a/test/BUILD b/test/BUILD index 4a8e807e9..8b2242be2 100644 --- a/test/BUILD +++ b/test/BUILD @@ -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") @@ -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() @@ -27,5 +30,6 @@ bzl_library( deps = [ "//test/fixtures:starlark_tests_bzls", "//test/rules:starlark_tests_bzls", + "@bazel_skylib//rules:build_test", ], ) diff --git a/test/cc_library_tests.bzl b/test/cc_library_tests.bzl new file mode 100644 index 000000000..7b58eae95 --- /dev/null +++ b/test/cc_library_tests.bzl @@ -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], + ) diff --git a/test/fixtures/cc_library/BUILD b/test/fixtures/cc_library/BUILD new file mode 100644 index 000000000..7e473a8e3 --- /dev/null +++ b/test/fixtures/cc_library/BUILD @@ -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"], +) diff --git a/test/fixtures/cc_library/ImportPrefixAndStripPrefix.swift b/test/fixtures/cc_library/ImportPrefixAndStripPrefix.swift new file mode 100644 index 000000000..fa63322ff --- /dev/null +++ b/test/fixtures/cc_library/ImportPrefixAndStripPrefix.swift @@ -0,0 +1,3 @@ +import test_fixtures_cc_library_prefix_and_strip_prefix + +func f(_ x: some_structure) {} diff --git a/test/fixtures/cc_library/ImportPrefixAndStripPrefixWithExclusion.swift b/test/fixtures/cc_library/ImportPrefixAndStripPrefixWithExclusion.swift new file mode 100644 index 000000000..83e47256d --- /dev/null +++ b/test/fixtures/cc_library/ImportPrefixAndStripPrefixWithExclusion.swift @@ -0,0 +1,3 @@ +import test_fixtures_cc_library_prefix_and_strip_prefix_with_exclusion + +func f(_ x: some_structure) {} diff --git a/test/fixtures/cc_library/ImportPrefixOnly.swift b/test/fixtures/cc_library/ImportPrefixOnly.swift new file mode 100644 index 000000000..75b9bf61e --- /dev/null +++ b/test/fixtures/cc_library/ImportPrefixOnly.swift @@ -0,0 +1,3 @@ +import test_fixtures_cc_library_prefix_only + +func f(_ x: some_structure) {} diff --git a/test/fixtures/cc_library/ImportStripPrefixOnly.swift b/test/fixtures/cc_library/ImportStripPrefixOnly.swift new file mode 100644 index 000000000..5a9910a7f --- /dev/null +++ b/test/fixtures/cc_library/ImportStripPrefixOnly.swift @@ -0,0 +1,3 @@ +import test_fixtures_cc_library_strip_prefix_only + +func f(_ x: some_structure) {} diff --git a/test/fixtures/cc_library/header.h b/test/fixtures/cc_library/header.h new file mode 100644 index 000000000..0ce38ca4e --- /dev/null +++ b/test/fixtures/cc_library/header.h @@ -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_ diff --git a/test/fixtures/cc_library/header_prefix_and_strip_prefix.h b/test/fixtures/cc_library/header_prefix_and_strip_prefix.h new file mode 100644 index 000000000..4b8f42c46 --- /dev/null +++ b/test/fixtures/cc_library/header_prefix_and_strip_prefix.h @@ -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_ diff --git a/test/fixtures/cc_library/header_prefix_only.h b/test/fixtures/cc_library/header_prefix_only.h new file mode 100644 index 000000000..137e8038a --- /dev/null +++ b/test/fixtures/cc_library/header_prefix_only.h @@ -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_ diff --git a/test/fixtures/cc_library/header_strip_prefix_only.h b/test/fixtures/cc_library/header_strip_prefix_only.h new file mode 100644 index 000000000..2f307994c --- /dev/null +++ b/test/fixtures/cc_library/header_strip_prefix_only.h @@ -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_ diff --git a/test/fixtures/cc_library/header_with_error.h b/test/fixtures/cc_library/header_with_error.h new file mode 100644 index 000000000..6a89e9677 --- /dev/null +++ b/test/fixtures/cc_library/header_with_error.h @@ -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_