Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow generated headers with path separators #439

Merged
merged 3 commits into from
Aug 13, 2020

Conversation

michaeleisel
Copy link
Contributor

E.g., I work on a large project which has always used #import <SomeModule/SomeModule-Swift.h> for swift-to-objc, but using SomeModule/SomeModule-Swift.h as the generated header for our bazel migration causes it to fail

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@michaeleisel
Copy link
Contributor Author

@googlebot I signed it!

@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@michaeleisel
Copy link
Contributor Author

@googlebot I fixed it.

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: yes and removed cla: no labels Jun 9, 2020
@michaeleisel
Copy link
Contributor Author

@keith @kastiglione are you guys the ones to reach out to about discussing this? if not, who might be?

@keith
Copy link
Member

keith commented Jun 11, 2020

@allevato is the person to talk to

@keith
Copy link
Member

keith commented Aug 10, 2020

do you have more context on why this was disabled in the first place? I'm assuming there was some reason so it would be nice to be clear on the trade-offs

@segiddins
Copy link
Contributor

The original conversation on the restriction came from #350 (comment)

@keith
Copy link
Member

keith commented Aug 10, 2020

Ah nice, thanks for that context. I definitely get the restriction imposed there

@michaeleisel
Copy link
Contributor Author

for us, we're dealing with an existing xcodebuild setup that relies on generated swift headers to be at certain include paths, and so not having this means we have to work around that which gets hairy iirc

@segiddins
Copy link
Contributor

Looks like ctx.actions.declare_file already does validation: (from another project) the output artifact 'tests/framework/foo' is not under package directory 'tests/framework/platforms' for target '//tests/framework/platforms:framework_macos_101000_ios_100000_tvos_100000_watchos_30200' when passing in ../foo as the path. on the other hand, it looks like you can declare_file into a subpackage...

Overall, I think it's probably OK to remove the restriction, especially since it's a thing you can do in Xcode.

@thii
Copy link
Member

thii commented Aug 11, 2020

There're some ways to work around this now:

  • Replace #import <SomeModule/SomeModule-Swift.h> by #import "SomeModule-Swift.h" in your code-base. Xcode works with this too.
  • Generate and use a header map
  • Re-organize your code-base so that the SomeModule target is at the //SomeModule package. Because BINDIR is included by default, you can use #import <SomeModule/SomeModule-Swift.h>.

@michaeleisel
Copy link
Contributor Author

it's tough in this case because bazel support for this library is experimental, so we don't want to make sweeping changes for the sake of bazel yet. BUILD.bazel files are generated dynamically based on xcodegen, the standard there. header maps are a road i don't want to go down yet. my disposition is still to merge. who can we ping that has merge permission?

@michaeleisel
Copy link
Contributor Author

@keith thoughts on merging, based on @segiddins comment?

@googlebot
Copy link

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@keith
Copy link
Member

keith commented Aug 13, 2020

@googlebot I consent.

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@keith keith merged commit fc74b43 into bazelbuild:master Aug 13, 2020
severnt added a commit to Enflick/rules_swift that referenced this pull request Sep 21, 2021
* Add .clang-format (bazelbuild#467)

Fixes bazelbuild#305

* Add error for no Linux swiftc path (bazelbuild#466)

Fixes bazelbuild#16

* Update dump_toolchains to support home dir (bazelbuild#471)

Before:

```
swift-DEVELOPMENT-SNAPSHOT-2020-01-29-a.xctoolchain -> org.swift.50202001291a
swift-TEST-SNAPSHOT-2020-01-23-a.xctoolchain -> org.swift.50202001231a
swift-latest.xctoolchain -> org.swift.50202001291a
```

After:

```
/Library/Developer/Toolchains/swift-DEVELOPMENT-SNAPSHOT-2020-01-29-a.xctoolchain -> org.swift.50202001291a
/Library/Developer/Toolchains/swift-TEST-SNAPSHOT-2020-01-23-a.xctoolchain -> org.swift.50202001231a
/Library/Developer/Toolchains/swift-latest.xctoolchain -> org.swift.50202001291a
/Users/ksmiley/Library/Developer/Toolchains/swift-5.2-DEVELOPMENT-SNAPSHOT-2020-04-28-a.xctoolchain -> org.swift.52202004281a
/Users/ksmiley/Library/Developer/Toolchains/swift-latest.xctoolchain -> org.swift.52202004281a
```

* Run clang-format on everything (bazelbuild#470)

* Update README instructions (bazelbuild#472)

* Ensure that `swift_module_alias` propagates instrumented file info from its dependencies, otherwise coverage data might get dropped.

PiperOrigin-RevId: 325306099

* Allow generated headers with path separators (bazelbuild#439)

Co-authored-by: Michael Eisel <meisel@spotify.com>
Co-authored-by: Keith Smiley <keithbsmiley@gmail.com>

* Delete CONTRIBUTORS (bazelbuild#475)

This is outdated, best to look at the github insights tab

* Various changes related to explicit module compilation: (bazelbuild#477)

*   Add a feature to support compiling system modules with reduced
    diagnostic output.
*   Support system modules passed as path names (strings) instead of
    `File` artifacts.
*   Add the necessary tool config to support compiling explicit
    modules with Xcode 12 and higher (when the features are also
    enabled).
*   When compiling explicit modules, generate module maps for
    `objc_library` targets in the Swift rules instead of using
    the one propagated by `ObjcProvider` to ensure that `use`
    declarations for dependencies unknown to the native Bazel
    module map generation code are present.

PiperOrigin-RevId: 327813526

Co-authored-by: Tony Allevato <allevato@google.com>

* Use a parameter file when linking Swift libraries when the host machine is macOS (bazelbuild#473)

This is to prevent the linking failures if a `swift_library` target has
too many files that can cause its linking args to become longer than the
system's command length limits. This does not change the behavior when
building on Linux yet, since `ar` does not support passing arguments
using a parameter file.

* Read the `custom_malloc` configuration field in `swift_{binary,test}` to use an allocator provided by the `--custom_malloc` flag, if any. (bazelbuild#478)

PiperOrigin-RevId: 329317960

Co-authored-by: Tony Allevato <allevato@google.com>

* No need to check if we're on macOS for non-Darwin Swift toolchain (bazelbuild#479)

* Update protobuf deps (bazelbuild#480)

* Run //test/... on Linux CI (bazelbuild#463)

* Ensure that intermediate outputs created for Swift source files (such as object files) do not have spaces in their names.

This was already handled for basenames, but not for directory names between the target's package and the file; for example, if one had `swift_library(name = "ArgumentParser", ...)` in a `BUILD` file that also contained `Sources/ArgumentParser/Parsable Types/ParsableCommand.swift`.

PiperOrigin-RevId: 329799243

* Update bzl_library targets (bazelbuild#482)

* Pin rules_proto to a rev that contains bzl_library definitions

* Update bzl_library targets

Mostly so depending on them from a stardoc rule works now we load rules_proto

Mostly autogenerated & then tweaked based on the gazelle in bazelbuild/bazel-skylib#271

* Revert removal of for_bazel_tests filegroup target

* Update internal bzl_libraries to only be visible to //swift:__subpackages__

* Also limit visibility of //test/fixtures:common

* Add a feature flag to enable generation from raw proto sources.

The DescriptorSets ProtoInfo exposes don't have source info, so the generated
source don't have the comment. bazelbuild/bazel#9337
is open to attempt to get that, but this provides a way to opt into forcing it.

This does come with a minor risk for cross repository and/or generated proto
files where the protoc command line might not be crafted correctly, so it
remains opt in.

RELNOTES: None
PiperOrigin-RevId: 330538313

* Honor the feature flag to enable generation from raw proto sources.

The DescriptorSets ProtoInfo exposes don't have source info, so the generated
source don't have the comment. bazelbuild/bazel#9337
is open to attempt to get that, but this provides a way to opt into forcing it.

This does come with a minor risk for cross repository and/or generated proto
files where the protoc command line might not be crafted correctly, so it
remains opt in.

This is followup to the previous change that did this for swift_proto_library.

NOTE: The current version of swift grpc depended on doesn't include comments,
but the NIO version appears to support it for methods, so landing the work
in advance of things updating to that version.

RELNOTES: None
PiperOrigin-RevId: 330729710

* Remove unused value.

RELNOTES: None
PiperOrigin-RevId: 330801521

* Don't propagate an empty module map for `{cc,objc}_library` targets that don't have any headers (and that don't specify an explicit module map using the `module_map` attribute).

This is typically the case for libraries that group other libraries via their `deps`. Users mistakenly import these thinking they're required to use the underlying declarations, but they do nothing (except make more work for the compiler).

PiperOrigin-RevId: 332246632
(cherry picked from commit 639ecfa)

* Added helper macro for declaring transitive dependencies

* Update README shas

* Fix buildifier warning

* Update rules_swift to use the toolchain transition.

This is phase 2 of of the switch to toolchain transitions. See bazelbuild/bazel#11584 for details.

This shouldn't be merged into rules_swift's master branch until https://cs.opensource.google/bazel/bazel/+/58fdf63d81cc71f0315918b111fc56f4c039f1a5 is in a Bazel release (which should be 3.7, bazelbuild/bazel#12188).

PiperOrigin-RevId: 334610489

* Update apple_support

* Implement -coverage-prefix-map feature (bazelbuild#489)

* Update worker docstring with example (bazelbuild#505)

This comment made it sound like this shouldn't ever happen, but if you
pass `-rpath @loader_path/something` you will always hit this case.

* Introduce feature to register a separate action to generate swift derived files (bazelbuild#504)

Co-authored-by: Keith Smiley <keithbsmiley@gmail.com>

* Fix index while building with derived files (bazelbuild#506)

The swiftmodule generation should not attempt to perform indexing.

* Fix derive files with bitcode (bazelbuild#509)

* Output command lines of failed action command line tests (bazelbuild#508)

This is helpful when debugging what does or doesn't exist

* Add swift.remap_xcode_path feature (bazelbuild#511)

* Only read output_file_map if it's going to be written

The output_file_map is only written to disk in a transformed state if an incremental build is happening. This change causes the output_file_map to only be read and processed in that case as well, leading to less work in the WMO case.

* Create a build setting to allow forcing a Swift target to Apple with bazel transitions. (bazelbuild#515)

PiperOrigin-RevId: 339779917
(cherry picked from commit e4ad1cc)

Co-authored-by: Googler <noreply@google.com>

* Update apple_support (bazelbuild#517)

* Update README with new URLs (bazelbuild#518)

* Migrate to the modern Starlark linker input API. (bazelbuild#512)

See bazelbuild/bazel#10860.

* Update README with new URLs (bazelbuild#520)

* Update protobuf (bazelbuild#524)

* Clean up some old TODOs.

- Remove some deprecated fields.
- Assert that atleast one module is provided.

RELNOTES: None
PiperOrigin-RevId: 344261262
(cherry picked from commit eefede5)

* Remove the `swift.implicit_modules` feature.

This feature permitted an odd three-state situation around the way the implicit module cache is handled:

1. The so-called "global module cache", which is placed in `bazel-out`
2. The ephemeral module cache, which is created and destroyed around each compile action
3. A state where no flags related to the implicit module cache are passed to the compiler, meaning "do whatever the compiler does by default".

This third state is the one we want to get rid of, since the default is to place the module cache in a temp location outside the sandbox.

PiperOrigin-RevId: 344942165
(cherry picked from commit 575b819)

* Stop computing transitive_generated_headers.

RELNOTES: None
PiperOrigin-RevId: 344857866
(cherry picked from commit 5167ae9)

* Standardize on FIXTURE_TAGS, ensure all targets tagged.

RELNOTES: None
PiperOrigin-RevId: 345024048
(cherry picked from commit d5fa2f5)

* Deduplicate module map flags in Swift compilation

Module map files being propagated through different providers seem to
ended up being treated like different modules, even though they point to
the same .modulemap files. Swift compilation in targets with a lot of
objc_library dependencies were getting a lot of duplicate flags. This
fixes that by deduplicate the module map list before passing them to the
compiler.

* Revert "Migrate to the modern Starlark linker input API. (bazelbuild#512)"

This reverts commit aff1754.

* Migrate Swift build rules to the new C++ `LinkerInput` APIs.

Based partially on bazelbuild#512 by @benjaminp.

PiperOrigin-RevId: 341831304
(cherry picked from commit bf9560d)

* Fix tests

* Make the `srcs` attribute of `swift_library` mandatory and non-empty.

PiperOrigin-RevId: 345120894
(cherry picked from commit c210804)

* Update README with new URLs

* Migration off SwiftInfo.module_name.

- Make module alias use the direct modules instead with legacy fall back for the
  existing support.
- Validate during creation of SwiftInfo that any provided module_name matches the
  first module (if any modules are provided).

RELNOTES: None
PiperOrigin-RevId: 345237478
(cherry picked from commit 46613b2)

* Add the SDK's `Developer/Library/Frameworks` directory to the compiler and linker search paths on platforms that have it.

Added in Xcode 12, `StoreKitTest.framework` is in a new Developer directory hierarchy under the SDK root (not to be confused with the one under the `.platform` root), except on macOS where it *does* live in the `.platform`'s Developer directory.

A small cleanup here means we also no longer pass a non-existent developer frameworks path when targeting watchOS.

PiperOrigin-RevId: 346784495
(cherry picked from commit 0ec7a57)

* Require `name` to be a named argument.

RELNOTES: None
PiperOrigin-RevId: 347033042
(cherry picked from commit 3355e61)

* Retire `module_name` from create_swift_info.

RELNOTES: None
PiperOrigin-RevId: 347035229
(cherry picked from commit 6e4e3ca)

* Allow `optional_implicit_deps` and `required_implicit_deps` to be set on an `xcode_swift_toolchain` target.

PiperOrigin-RevId: 347841955
(cherry picked from commit 875de9f)

* Fix XCTest -I for derived files

In 768bfe3 this was moved and this
wasn't added (since it isn't used inside Google)

* Remove SwiftInfo.module_name, it was deprecated a while ago.

RELNOTES: None
PiperOrigin-RevId: 348027802
(cherry picked from commit 789b20b)

* Ignore a feature when a `SwiftToolchainInfo` says it does not support it instead of making it a build failure. This is closer to the C++ crosstool behavior.

This change also cleans up the internal representation of the feature configuration, by no longer tracking unsupported features (which were never used elsewhere) and by preceding the `struct` fields with underscores to indicate that they should not be read by clients (the value should only be queried by passing it to other `swift_common` functions).

PiperOrigin-RevId: 348055621
(cherry picked from commit 3c85e63)

* Uniquify `-fmodule-map-file=` flags passed to `swiftc`.

In some cases (e.g., if a module map defines multiple modules and both or more were present in the dependency graph), the `depset` won't deduplicate the module structures because their values are distinct (they contain different module names). That's the behavior we want, but it also resulted in the `-fmodule-map-file=` for that module map appearing multiple times on the command line.

PiperOrigin-RevId: 348139990
(cherry picked from commit df1198b)

* Work around bazelbuild/stardoc#78.

Markdown bullet lists work fine in the general descriptions block of macros/rules/providers, but for rule attributes and provider fields the the markdown is a table, and `*` has no means (and stardoc transforms the content to remove newlines). The above issue is to support catch these and marking html bullet lists out of them, but until that is done, adding a blank line between each bullet gets slightly more readable generated/render documents.

PiperOrigin-RevId: 348682502
(cherry picked from commit 5383052)

* Update README with new URLs

* Allow modules propagated by `SwiftInfo` to indicate whether they are system modules.

System modules differ from non-system modules in that we don't pass the module map to the compiler via `-fmodule-map-file` for system modules in implicit module builds, only in explicit module builds. This is because module maps passed via `-fmodule-map-file` are always treated as non-system modules, which affects how diagnostics are emitted when their headers are parsed, and some Swift modules like `SwiftOverlayShims` don't declare themselves as `[system]` in their module map file but contain small nullability issues. Such headers *must* be found through a standard header search via a system path like `-isystem` so that they don't cause builds to fail if they treat warnings more severely.

PiperOrigin-RevId: 351896080
(cherry picked from commit 2140619)

* Stop computing/tracking defines.

They are already exposed in the modules, so use them directly from there instead.

This also means they union doesn't have to keep being computed going up the
build graph, which will save some cycles.

RELNOTES: None
PiperOrigin-RevId: 351588940
(cherry picked from commit 52b9ffa)

* Remove swift_version.

It was deprecated a while ago, and it avoids the collection in swift_library now.

RELNOTES: None
PiperOrigin-RevId: 351595739
(cherry picked from commit 5b1aabd)

* Add a `swift_explicit_module` output group to the `swift_clang_module_aspect` to provide a way to invoke the aspect from the command line and retrieve the `.pcm` file.

This provides an easier way to manually verify that a `{cc,objc}_library` builds correctly as an explicit module.

PiperOrigin-RevId: 353103538
(cherry picked from commit 1b63232)

* Lessen restrictions on when implicit modules can be used:

- Allow Swift compiles to use implicit modules even if `swift.use_c_modules` is enabled (a necessary fallback if some dependencies don't emit explicit modules).
- Continue to disallow usage of implicit modules when compiling explicit C/Objective-C modules because those implicit module paths would become embedded in the `.pcm` files, making them immovable.

PiperOrigin-RevId: 353665654
(cherry picked from commit 2711a2b)

* Fix header selection when falling back to implicit modules.

If we're compiling a `swift_library` with explicit modules enabled but some dependency subtree doesn't propagate explicit modules, we weren't collecting the right headers; we need the full transitive set for that subtree, not just the direct headers.

Also cleaned up the comments and control flow in this function, which had gotten out-of-date and confusing, which no doubt contributed to the problem.

PiperOrigin-RevId: 354142943
(cherry picked from commit 0ad70ff)

* Simplify the paths to module maps generated by the Swift build rules.

Since we pass these module maps directly to the compiler with `-fmodule-map-file`, we don't need them to be named exactly `module.modulemap`, so the extra subdirectory is also unnecessary.

PiperOrigin-RevId: 354151400
(cherry picked from commit 71998c8)

* Automated rollback of commit 71998c8.

PiperOrigin-RevId: 354172938
(cherry picked from commit 6dc76fe)

* Simplify the paths to module maps generated by the Swift build rules.

Since we pass these module maps directly to the compiler with `-fmodule-map-file`, we don't need them to be named exactly `module.modulemap`, so the extra subdirectory is also unnecessary.

This change also migrates generation of the modulemap file content for the generated header's module to the same code (`module_maps.bzl`) already used by `swift_clang_module_aspect`, instead of having its own separate code path.

PiperOrigin-RevId: 354324121
(cherry picked from commit 5f51ca9)

* Use released protoc binaries where possible (bazelbuild#555)

This updates `rules_proto`, which has been configured to use released
protoc binaries where it can.

This can be a breaking change if users have a call to `protobuf_deps()`
_before_ `swift_rules_extra_dependencies()` in their WORKSPACE. To
fix, remove `load("@com_google_protobuf//:protobuf_deps.bzl", "protobuf_deps")`
and `protobuf_deps()` from the WORKSPACE file, and use the following
instead:

```
load(
    "@build_bazel_rules_swift//swift:extras.bzl",
    "swift_rules_extra_dependencies",
)

swift_rules_extra_dependencies()
```

* Add disable system index feature (bazelbuild#563)

* Don't re-export the modules imported by a Swift generated header.

This was an unintentional change in behavior from bazelbuild@5f51ca9; this puts us back to the original behavior, but leaves an API in place for finer-grained control over re-exporting modules in the future. (But the BUILD rules today don't really have the flexibility to support it yet.)

PiperOrigin-RevId: 356338982
(cherry picked from commit f45eea8)

* Explicitly set --macos_minimum_os=10.15 to make `swift_{binary,test}` runnable on macOS Catalina (which is the macOS version used by Bazel CI) when building with Xcode 12 or above (bazelbuild#566)

Since there's no way to set the deployment version for
`*_{binary,test}`, this makes rules_swift on CI green again without
having to go back to using Xcode 11.7.

* Move toolchain- and command-line-provided compiler flags into action configurators.

This change removes of the `SwiftToolchainInfo.command_line_copts` field, no longer treating those command line flags as an odd special case. It also ensures that flags from the toolchain are passed in a unified manner regardless of which rule registers the action.

For example, Bazel-legacy Objective-C flags determined by compilation mode, like `-fstack-protector`, affect the compatibility of Clang modules and need to be passed both when compiling an explicit module and when compiling the Swift code that consumes those modules.

This required a bit more shuffling around of the way we need to handle differing outptus for WMO, since the `--swiftcopt` flags would no longer be able to be scanned directly after the toolchain configuration.

PiperOrigin-RevId: 355451642
(cherry picked from commit 1bea35e)

* Fix incorrect param name

* Fix non-threaded mode WMO

The non-threaded mode WMO, which produces a single object file for the
entire compilation unit (.o), is determined by the non-presence of the
`-num-threads` flag, or `-num-threads 0` flags are passed.

* No longer scan for `-num-threads=X` (specifically the form with the equal sign).

We've always supported this, but `swiftc` today doesn't accept the version with the equal sign and briefly looking through the history of Options.td I can't find any evidence that it ever did. We continue to support space-separated `-num-threads X`.

PiperOrigin-RevId: 355647561
(cherry picked from commit d26868f)

* Unconditionally add `ObjcProvider.umbrella_header` files to compiler inputs, even when preferring explicit modules.

A small number of rules use `umbrella_header` to propagate an umbrella header for their module map that they don't propagate in any other header field. Since there is no `direct_*` version of this field, `swift_clang_module_aspect` can't easily extract the one umbrella header from the `depset` in the provider that may also include transitive umbrella headers.

PiperOrigin-RevId: 355667496
(cherry picked from commit bd8c7d4)

* Compile an explicit module for the generated header of a `swift_library`.

The notion of "implicit compile-time dependencies for generated header modules only" has been added to the toolchain because generated Obj-C headers always import Foundation and Darwin, but we don't necessarily want to force those as dependencies of every Swift compilation. Likewise, the private (implementation-only) dependencies of a module shouldn't be included when generating the module map `use` decls for the Obj-C header module, because by definition, they can't be exported.

PiperOrigin-RevId: 358406043
(cherry picked from commit ab1c5cd)

* Pass framework search paths directly to clang for pcms

This is needed since swiftc doesn't pass the framework search paths to clang, causing issues with framework style imports in headers.

* Pass `-no-clang-module-breadcrumbs` to frontend jobs (bazelbuild#574)

By default, absolute paths to the dependent pcm files are embedded
into the debug info, which was used as a fallback path for LLDB to
reconstruct Clang types from DWARF, when it wasn't able to import
a dependent Clang module from source. 

Since those paths aren't shareable across machines, disabling this
behavior makes the compilation outputs more reproducible.

* Add remotely debuggable Swift document (bazelbuild#576)

* Update apple_support (bazelbuild#577)

* Update README for new URL (bazelbuild#578)

* Support "Make" variable expansion in `copts` and `linkopts`

Resolves bazelbuild#314.

* Add a `generates_header` attribute to `swift_library` to control Objective-C header generation.

Today, `swift_library` unconditionally generates a header and module map for all targets (unless rarely-used features are applied), even if the library has no APIs exported to Objective-C. This creates a small amount of unnecessary work, but that work grows significantly when explicit modules are used in the build, because we also need to compile the explicit modules for those generated headers _just in case_ some `objc_library` further up in the build graph needs to import the header.

This is the first of a few changes that will eventually require the `generates_header` attribute to be set on targets to have the generated header emitted and propagated.

This change also removes the `swift.no_generated_header` feature, since the attribute's behavior supplants it.

PiperOrigin-RevId: 362562962
(cherry picked from commit e621e5e)

* Internal change.

PiperOrigin-RevId: 366495458
(cherry picked from commit d27e5a6)

* Switch to XcodeVersionConfig.execution_info().

RELNOTES: None
PiperOrigin-RevId: 364797902
(cherry picked from commit aeb56dc)

* Distinguish between direct and transitive information when creating the `CcInfo` for a `swift_library`.

This ensures that, for a `swift_library` target `t`, reading from either the direct fields of `t[CcInfo].compilation_context` or `t[SwiftInfo].direct_modules[...].clang.compilation_context` produces consistent results.

PiperOrigin-RevId: 363418185
(cherry picked from commit 027e180)

* Delete support for compiler performance stats collection.

This wasn't used frequently enough to rationalize being so deeply woven into the build rules. A better approach might be to use an aspect that runs the same actions in parallel but with the necessary stats flags and collects the data files; while this would double the actions executed, it's better code-health wise. But I don't have any plans to do that in the near-term.

PiperOrigin-RevId: 363887607
(cherry picked from commit 4234e38)

* Simplify and cleanup how implicit toolchain dependencies are managed.

-   When propagating implicit deps in the toolchain, merge and propagate the providers instead of the `Target` object itself. The `Target` object is fairly heavyweight and not intended to be propagated around the build graph.
-   Remove the `swift.minimal_deps` feature and the distinction between "required" and "optional" implicit deps.

PiperOrigin-RevId: 367044294
(cherry picked from commit 665ea75)

* Update examples/tests with `generates_header = True` where necessary before flipping the attribute.

PiperOrigin-RevId: 362932692
(cherry picked from commit 1bcdd57)

* Make `swift_library` not generate an Objective-C header by default.

To generate a header, `generates_header` must be set to `True` on the target.

PiperOrigin-RevId: 367012879
(cherry picked from commit 413cc02)

* Update apple_support to new release

* Update README with new URLs

* Add -Werror for C++

* Fix skylib branch name

Fixes bazelbuild/rules_apple#1130

* Add first stardoc API docgen target (bazelbuild#598)

* Generate providers.md from sources (bazelbuild#599)

* Add `swift_common.create_swift_interop_info`.

This API allows custom rules to opt-in to "lightweight Swift interop". If they are already propagating a `CcInfo` with a compilation context, then `swift_clang_module_aspect` will detect this provider and generate a module name/module map (if necessary), and also compile the explicit module (if enabled). This frees the custom rule from having to understand details of the Swift compilation APIs, or even having to have a dependency on the Swift toolchain at all.

PiperOrigin-RevId: 367251939
(cherry picked from commit 9be992e)

* Move swift_clang_module_aspect out of swift_common and directly expose it.

RELNOTES: swift_clang_module_aspect is directly exported instead of being in swift_common.
PiperOrigin-RevId: 367673524
(cherry picked from commit bf8243d)

* Update the module name derivation algorithm to handle any sequence of non-identifier characters safely.

PiperOrigin-RevId: 368004995
(cherry picked from commit ec94196)

* API changes to `create_swift_interop_info`.

- Require all dependencies' `SwiftInfo`s to be passed instead of implicitly (and unconditionally) traversing `deps`.
- Add `requested_features` and `unsupported_features` parameters so rule implementations can provide these values (to be automatically added to the values on the target) as if they were supplying the feature configuration themselves.
- Correctly derive a module name if it is not provided.

PiperOrigin-RevId: 368653094
(cherry picked from commit 24a0449)

* Factor out Bazel placeholder substitution functionality into a separate reusable class.

PiperOrigin-RevId: 366461152
(cherry picked from commit 89c6ab2)

* Support older C++ versions

Google's internal C++ version is higher than bazel's default.
Theoretically we could mess with the `-std` flag that's passed, and I
tried that a bit, but the public crosstool differs between macOS and
Linux so it makes it a bit of a pain. For this case this is easy enough.

* Sort headers in the module maps generated by `swift_clang_module_aspect`, and rewrite generation to use a multi-line `Args` object as a file writer instead of building up an analysis time string.

With this approach, each "arg" (or each entry in a list/depset of args) is treated as its own line in the output file; we use the `format_each` and `map_each` parameters to provide any additional text that should surround that value on the line.

More importantly, this adds support for expanding tree artifacts (directories) if one is provided in a compilation context.

PiperOrigin-RevId: 367221120
(cherry picked from commit 5f7af69)

* Remove top level functions for now

* Use cpp fragment, instead of objc fragment, to determine whether to generate dsym

The information is identical, and we would like to migrate all uses to
cpp fragment so that we can delete the info in objc fragment.

PiperOrigin-RevId: 370899517
(cherry picked from commit 3e53aa2)

* Add support for older bazel versions for now

* Add Xcode toolchain support for an optional binary that can rewrite the generated header of a Swift module after compilation.

PiperOrigin-RevId: 369323385
(cherry picked from commit e4912a3)

* Disable generated_header_rewriter for now

This doesn't work without nested functions. I think this use case is
pretty rare but if someone wants to enable this before bazel supports
nested functions that's fine.

* Make the strict include paths from `objc_proto_library` targets available so headers can be found when `swift_clang_module_aspect` compiles a module.

`swift_clang_module_aspect` assumes that CcInfo contains all of the necessary compiler flags to be able to find the headers that are used when compiling an explicit module. This is true for all flags except the header search path in the Objc provider's `strict_include` field. This means that the `strict_include` field needs to be explicitly translated to an equivalent CcInfo when compiling the module.

PiperOrigin-RevId: 370991285
(cherry picked from commit 1aca023)

* Rename `swift.strict_modules` to `swift.layering_check` to match the C++ feature name.

PiperOrigin-RevId: 371693623
(cherry picked from commit 06322f1)

* Add a user settable build setting to specify additional swiftcopts for a swift_libray target.

The flag accepts a comma separated list of "target=copts" pairs, where "target" is a fully qualified target label and "copts" is a colon separated list of Swift copts.

PiperOrigin-RevId: 371728552
(cherry picked from commit de9c4e1)

* Disable allow_multiple for per_module_swiftcopt_flag

Bazel doesn't support this yet

* Update README with new URLs

* Use target_compatible_with for macOS only tests (bazelbuild#514)

* Add `swift.global_module_cache_uses_tmpdir` feature (bazelbuild#581)

* Add `swift.global_module_cache_uses_tmpdir` feature

This adds a new feature, that can be enabled by passing `--features=swift.use_global_module_cache` and `--features=swift.global_module_cache_uses_tmpdir` to the build command.

This feature, when enabled, will makes the Swift compilation actions use the shared Clang module cache path written to `/tmp/__build_bazel_rules_swift/swift_module_cache/REPOSITORY_NAME`. This makes the embedded Clang module breadcrumbs deterministic between Bazel instances, because they are always embedded as absolute paths. Note that the use of this cache is non-hermetic--the cached modules are not wiped between builds, and won't be cleaned when invoking bazel clean; the user is responsible for manually cleaning them.

Additionally, this can be used as a workaround for a bug in the Swift compiler that causes the module breadcrumbs to be embedded even though the `-no-clang-module-breadcrumbs` flag is passed (https://bugs.swift.org/browse/SR-13275).

Since the source path of modulemaps might be different for the same module, (i.e. multiple checkouts of the same repository, or remote execution), multiple modules with different hashes can end up in the cache. This can result in build failures. Don't use this feature with sandboxing (or probably remote execution as well).

* Stop passing `uses_swift` to `new_objc_provider`.

This flag hasn't been used by Bazel for a very very very very very long time.

PiperOrigin-RevId: 373845026
(cherry picked from commit 05f18b8)

* Enable CI testing for docgen (bazelbuild#617)

* Generate api.md from sources (bazelbuild#618)

* Generate setup.md from sources (bazelbuild#619)

This is the last doc that should be generated from sources, but wasn't, from what I can tell.

* Update README with new URLs (bazelbuild#621)

* Update bazel_skylib (bazelbuild#620)

* Update the worker protocol proto (bazelbuild#622)

* Add ubsan support (bazelbuild#544)

* Remove unused command_line_copts (bazelbuild#625)

* Include the headers from direct dependencies of a Swift target when compiling the explicit module for its generated header.

PiperOrigin-RevId: 372240650
(cherry picked from commit cc15b6e)

* Use `-fsystem-module` if the compiler is new enough to support it (Xcode 12.5/Swift 5.4 or higher).

PiperOrigin-RevId: 373046121
(cherry picked from commit 4cd9b2c)

* Add the `swift_feature_allowlist` rule that lets toolchains control which packages are allowed to enable/disable specific features. (bazelbuild#627)

Co-authored-by: Tony Allevato <allevato@google.com>

* Disable layering checks when compiling the explicit module for a Swift generated header.

The Swift compiler determines which module to import a symbol from based on the module that defines that symbol. Due to modular re-exports (which are particularly common among system frameworks), this may not be the same framework that the user imported from Swift and added to their dependencies.

(For example, most users will import and depend on `Foundation` to use `NSObject`, but Swift will import it from the module it is actually defined in, `ObjectiveC`.)

PiperOrigin-RevId: 371919747
(cherry picked from commit d8a381c)

* Disable layering checks for now

This depends on lambda support which bazel 4.1 does not have

* Revert "Disable layering checks for now"

This reverts commit 021c11b.

* Get rid of the non-strict use of `-fmodules-decluse`.

For layering checks with explicit modules, we only want
`-fmodules-strict-decluse`; it's not useful to let headers
without a known module map sneak through.

PiperOrigin-RevId: 375825780
(cherry picked from commit 214f725)

* Support implicit C/Objective-C dependencies that are passed when compiling any explicit Clang module.

The toolchain's `clang_implicit_deps` are not passed to Swift compilations;
only to actions that compile an explicit C/Objective-C module using
`swift_common.precompile_clang_module`.

PiperOrigin-RevId: 375816204
(cherry picked from commit 9afbc30)

* Update docs

* Get rid of spurious diagnostics when compiling system modules with Xcode 12.5:

```
<unknown>:0: error: invalid argument '-fsystem-module' only allowed with '-emit-module'
```

This happens because ClangImporter changes the invocation's action to `GenerateModule` *after* the invocation is created by parsing the `-Xcc` flags from the Swift command line, and Clang's argument parser emits a diagnostic if `-fsystem-module` is used with any other action.

PiperOrigin-RevId: 375942286
(cherry picked from commit c497e6e)

* Add buildifier pre-commit hook (bazelbuild#632)

Matches the one just landed in rules_apple: bazelbuild/rules_apple@61bc7c0

* Reuse swiftmodule for incremental builds (bazelbuild#633)

* Add returns (bazelbuild#634)

Missed with 84286ad.

Also reverts the hard error on not creating directories. It could try to create the same directory multiple times, and the previous behavior silently errored on those. This can get fixed up in a later change to only try to create each directory once, and to only error if the directory doesn't already exist.

* Implement global index store cache (bazelbuild#567)

swift and clang write index data based on the status of what resides in
`index-store-path`. When using a transient per-swift-library index, it
writes O( M imports * N libs) indexer data and slows down compilation
significantly: to the tune of 6GB+ index data in my testing. Bazel likes
to use per `swift_library` data in order to remote cache them, which
needs extra consideration to work with the design of swift and clang and
preserve the performance

This PR does 2 things to make index while building use the original
model of the index-store so we don't have to patch clang and swift for
this yet. When the feature, `swift.use_global_index_store`, is set:

1. it writes to a global index cache, `bazel-out/global_index_store`
2. it copies indexstore data (records and units) to where Bazel
   expected them for caching: e.g. the original location with
   `swift.index_while_building`

I added a detailed description of this feature in the RFC to add it
there MobileNativeFoundation/index-import#53. In practice,
transitive imports will be indexed by deps and if they aren't cached
then the fallback is Xcode wil index transitive files if we set those as
deps on the unit files

* Modernize `xcode_swift_toolchain` features and linkopts.

This change adds the search path for StoreKitTest/XCTest now available on watchOS in Xcode 12.5, and retires a number of workarounds and/or conditional code paths that are for older versions of Xcode that we don't care about anymore. For example, statically linking the Swift runtime is no longer supported on Darwin with modern versions of Xcode.

PiperOrigin-RevId: 377920556
(cherry picked from commit bf5fccc)

* Remove the `-rpath` for the Xcode's developer frameworks directory from the Swift toolchain.

It may have been needed in the past with older versions of Swift, but it doesn't appear to be anymore (and it causes problems if the version targeted during the build is mismatched with the simulator/OS version under which the test is running).

PiperOrigin-RevId: 378400910
(cherry picked from commit 92b727e)

* Propagate Swift-specific linker flags as an implicit dependency of the toolchain.

Apple binaries/bundles will automatically get these linker flags if they have any Swift target in their dependencies; this eliminates the need to pass them separately at the top level.

PiperOrigin-RevId: 378024438
(cherry picked from commit 96f0a67)

# Conflicts:
#	swift/internal/providers.bzl
#	swift/internal/swift_toolchain.bzl
#	swift/internal/xcode_swift_toolchain.bzl

* Fix worker based error reporting for WMO (bazelbuild#642)

* Fix race condition in MakeDirs (bazelbuild#644)

If there are 2 concurrent processes creating the same directories, then it will fail

I noticed this on the CI errors of bazelbuild#567, and a co-worker was hitting it
today. I suspect invalidating the worker or other updates triggered big
rebuilds.

I don't know of a perfect repro but, at a 10% rate: invoking Bazel to
build 2 swift libraries in the same BUILD file.

* Error out on failed `MakeDir` (bazelbuild#645)

Now that we only report failures from `MakeDir` if there is actually an error, we can error out earlier for a better error message.

* Delete the `swift_common.swift_runtime_linkopts` function.

This should have been deleted in the previous change that refactored the linker flags. It no longer works because it tries to access a provide field that no longer exists, but nothing broke because nothing calls it anymore. Yay weak typing.

PiperOrigin-RevId: 378673728
(cherry picked from commit 503c9cc)

* Refer to build_bazel_rules_swift_index_import via a file rather than inline string (bazelbuild#647)

For consistency with other repos that do not define their own build files

* Add files in third_party to :for_bazel_tests (bazelbuild#648)

So they are copied over in integration tests in rules_apple

* Fix index_import label

Not sure why this works sometimes and not others but we need to
terminate this where the BUILD file lives

* Update apple_support to 0.11.0

Co-authored-by: Keith Smiley <keithbsmiley@gmail.com>
Co-authored-by: Tony Allevato <allevato@google.com>
Co-authored-by: Michael Eisel <michael.eisel@gmail.com>
Co-authored-by: Michael Eisel <meisel@spotify.com>
Co-authored-by: Thi Doãn <t@thi.im>
Co-authored-by: Samuel Giddins <segiddins@segiddins.me>
Co-authored-by: Thomas Van Lenten <thomasvl@google.com>
Co-authored-by: Andre Brisco <andre.brisco@gmail.com>
Co-authored-by: Googler <noreply@google.com>
Co-authored-by: Kanglei Fang <ghvg1313@hotmail.com>
Co-authored-by: Brentley Jones <brentley.jones@target.com>
Co-authored-by: Benjamin Peterson <benjamin@python.org>
Co-authored-by: Brentley Jones <brentleyjones@lyft.com>
Co-authored-by: Omar Zúñiga <omarzl@hotmail.es>
Co-authored-by: Alex Eagle <eagle@post.harvard.edu>
Co-authored-by: Walter Lee <waltl@google.com>
Co-authored-by: Brentley Jones <github@brentleyjones.com>
Co-authored-by: Samuel Giddins <segiddins@squareup.com>
Co-authored-by: Jerry Marino <i@jerrymarino.com>
tymurmustafaiev pushed a commit to tymurmustafaiev/rules_swift that referenced this pull request Jul 19, 2023
Co-authored-by: Michael Eisel <meisel@spotify.com>
Co-authored-by: Keith Smiley <keithbsmiley@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants