Add rules for building SDK frameworks for explicit module compilation #562

dierksen · 2022-09-15T16:35:14Z

No description provided.

thiagohmcruz

Just a few questions/nits for now! Can take a second pass once it's ready for review

tools/toolchains/xcode_configure/xcode_configure.bzl

tools/toolchains/xcode_configure/xcode_sdk_frameworks.bzl

jerrymarino

@dierksen Nice - the comments I've got are mostly minor - if we want to iterate on this incrementally in smaller bits that seems fine 👍

.vscode/settings.json

tools/toolchains/xcode_configure/xcode_configure.bzl

tools/toolchains/xcode_configure/xcode_sdk_frameworks.bzl

* master: Add provisioning_profile to test rules (bazel-ios#561)

* master: Forward testonly attribute for all rules (bazel-ios#573) Run buildifier fix, clean up unused variables/functions (bazel-ios#572) Add discussion bullet points to README (bazel-ios#565) Fixing git_repository assumption in xcbuildkit dep (bazel-ios#566)

* framework_repo_rule: Forward testonly attribute for all rules (bazel-ios#573) Run buildifier fix, clean up unused variables/functions (bazel-ios#572) Add discussion bullet points to README (bazel-ios#565) Fixing git_repository assumption in xcbuildkit dep (bazel-ios#566)

dierksen · 2022-10-17T20:11:23Z

Lots of stuff still to revert here, but overall the repo rule is about finished.

The biggest issue I'm working through at the moment is XCTest, which doesn't work with swift_c_module, so I'll probably need to put together a simple objc_library-esque rule and/or create a separate new_external_repository to pull that in.

* xc13: Bump macos image, bazel, and carthage versions set index-import priority to lowest (bazel-ios#547)

* master: Enable versions of index-import that use runfiles (bazel-ios#583) Change macos-latest to macos-11 (bazel-ios#584) Remove duplication in xcode gen json file when using VFS (bazel-ios#578) Add ability to split a test bundle. (bazel-ios#545) static vendored lib missing include paths (bazel-ios#581) Fixes formatting for middleman error (bazel-ios#579)

* master: [Xcode 13.3.1] Flip on in Swift PCH fix (bazel-ios#586)

* master: Bump Xcode, macos image, bazel, and carthage versions (bazel-ios#580) Make the wrap_resources_in_filegroup rule name morre unique (bazel-ios#592) Add ability to configure build service (bazel-ios#577) Updating rules_apple to point to the latest on the bazel/5.x branch (bazel-ios#589)

* master: Revert "Bump xchammer and xcbuildkit (bazel-ios#597)" (bazel-ios#599) Bump xchammer and xcbuildkit (bazel-ios#597) Increase bazel server startup timeout (bazel-ios#596) Fix issue merging xcconfig values that are lists (bazel-ios#590) Remove INFOPLIST_FILE setting from the legacy xcodeproj rule (bazel-ios#593)

* master: Add SwiftInfo to hmap/vfs rules to suppress module map generation (bazel-ios#603) Bump rules_apple, rules_swift, and stardoc (bazel-ios#604) Update xcconfig merging to override keys (bazel-ios#602) Explicitly unset the plist value, so XcodeGen doesn't go searching for one (bazel-ios#601) Bump xchammer and xcbuildkit (2) (bazel-ios#600)

dierksen · 2022-11-18T00:20:12Z

Happy 2-month birthday to this PR 🎂

Remaining issues that I'm pondering:

still not completely sure on -import-underlying-module: I don't completely understand the behavior for this flag, and it seems weirdly inconsistent between mixed-module and swift-only libraries
need to mark sdk frameworks as extension-safe (assuming all of them are? I really don't know)
all of the xchammer--namespaced libraries need the frameworks added

jerrymarino · 2022-11-14T22:21:23Z

rules/library.bzl

@@ -975,7 +975,7 @@ def apple_library(name, library_tools = {}, export_private_headers = True, names
            deps = deps + private_deps + lib_names + select({
                "@build_bazel_rules_ios//:virtualize_frameworks": [framework_vfs_overlay_name_swift],
                "//conditions:default": [framework_vfs_overlay_name_swift] if enable_framework_vfs else [],
-            }),
+            }) + (["@xcode_sdk_frameworks//:XCTest"] if testonly else []),


Why does this need to link XCTest to all compile actions? I think if the user includes XCTest in the sdk_frameworks it'd only need it then.

sdk_frameworks only affects linking and it's only on objc_* AFAICT.

Yeah today it's used for linking. However, the users already give us the SDK frameworks via sdk_frameworks so I imaged we might need / want to leverage that API for explicit PCMs

Without such a switch it looks like the PR has to injects every SDK to the swift compile action? I imagined that wouldn't be ideal from a performance angle but will have to dig into it a bit more.

jerrymarino · 2022-11-14T22:21:34Z

rules/library.bzl

@@ -561,7 +561,7 @@ def apple_library(name, library_tools = {}, export_private_headers = True, names
    tags_manual = tags if "manual" in tags else tags + _MANUAL
    platforms = kwargs.pop("platforms", None)
    private_deps = [] + kwargs.pop("private_deps", [])
-    lib_names = []
+    lib_names = ["@xcode_sdk_frameworks"]


I'd suggest to make this a feature that flips it on and any of the other rules_swift features that it currently uses, so they can set apple.explicit_module_compilation and it adds the relevant deps

jerrymarino · 2022-11-14T22:23:23Z

.github/workflows/tests.yml

+          -//tests/macos/xcconfig:used_as_condition_xcconfig
+
+          # `deleted_packages` is needed below in order to override the value of the .bazelrc file
+          EXPLICIT_MODULES=1 bazelisk test --local_test_jobs=1 --apple_platform_type=ios --deleted_packages='' -- \


Does this need to have the arguments as it does above?

jerrymarino · 2022-11-14T22:28:55Z

tools/toolchains/xcode_configure/xcode_sdk_frameworks.bzl

+            overrides = overrides,
+        )
+
+    repository_ctx.symlink(


Would you add a comment about why this symlinks in XCTest here?

jerrymarino · 2022-11-14T22:34:02Z

tools/toolchains/xcode_configure/xcode_sdk_frameworks.bzl

+    }})
+)
+
+alias(


Similar to other bits on XCTest special casing

jerrymarino · 2022-11-14T22:43:58Z

.github/workflows/tests.yml

+        run: |
+          # Host config
+          EXPLICIT_MODULES=1 bazelisk test --local_test_jobs=1 \
+          --features=swift.use_c_modules \


Can rules_ios turn on the dependent rules_swift features when it sets expilict module compilation enabled?

### What It scans and configures all SDK frameworks which are required for explicit module builds. Specifically, it 1) uses the bazel_tools//tools/osx:xcode_configure.bzl to fetch all local Xcode infos, 2) finds all frameworks under the xcode SDK path, and 3) uses the swiftc -scan-dependencies mode to fetch the dependency graph among all Swift and Clang SDK modules. We use swift_module_alias to represent a Swift SDK module and sdk_clang_module for a Clang SDK module. This PR is based on the previous attempt to support SDK frameworks: #562 ### Test Runs `bazel build --config=explicit_modules --nobuild @xcode_sdk_frameworks//...` and inspects the generated repository at `$(bazel info output_base)/external/_main~xcode_sdk_frameworks~xcode_sdk_frameworks`. ### Next steps: The dependency among swift_module_alias and sdk_clang_module doesn't work yet. We need to fix that.

Add rules for building SDK frameworks for explicit module compilation

c27f34e

thiagohmcruz reviewed Sep 15, 2022

View reviewed changes

tools/toolchains/xcode_configure/xcode_configure.bzl Outdated Show resolved Hide resolved

tools/toolchains/xcode_configure/xcode_sdk_frameworks.bzl Outdated Show resolved Hide resolved

tools/toolchains/xcode_configure/xcode_sdk_frameworks.bzl Show resolved Hide resolved

jerrymarino approved these changes Sep 15, 2022

View reviewed changes

dierksen added 5 commits September 16, 2022 10:27

Merge branch 'master' into framework_repo_rule

e0f837c

* master: Add provisioning_profile to test rules (bazel-ios#561)

split out repo rule; bump bazel and rules_swift

567d839

revert arm64-to-sim tweak

183306e

testing some stuff

399edef

add umbrella dep under the hood

59fe2f4

dierksen mentioned this pull request Sep 30, 2022

Filter out non-standard header extensions from module maps bazelbuild/rules_swift#912

Closed

dierksen added 3 commits October 11, 2022 14:33

further repo rule cleanup

e905fe1

dierksen added 17 commits October 17, 2022 15:21

change .bazelrc for now

9792e61

symlink in xctest framework, pass to testonly swift libraries

0b47395

cleanup

1b7afe4

Bump macos image, bazel, and carthage versions

ebb38c2

Merge branch 'xc13' into framework_repo_rule

90aa093

* xc13: Bump macos image, bazel, and carthage versions set index-import priority to lowest (bazel-ios#547)

gradually trying to fix tests

0a92978

bump cocoapods to 1.11 to add ruby 3 support

c891e65

newest bazel release

55c9710

update gemfile.lock

e83f15d

bump checkout action, mark mixed framework test as manual for now

901234d

disable another one

efc8ff6

try newer carthage fix

7d01fbe

re-enable a couple of tests for now

a6d85c3

reverts

0d11ade

Merge branch 'master' into xc13

3eee080

* master: [Xcode 13.3.1] Flip on in Swift PCH fix (bazel-ios#586)

revert more formatting

865789c

dierksen added 3 commits November 7, 2022 16:30

fix deps array (ugh), fix watch simulator cpu

b921c35

add -k to tests so i can see progress at least

c17cbbe

dierksen mentioned this pull request Nov 10, 2022

Allow for deps with only CcInfo on framework imports bazelbuild/rules_apple#1751

Merged

dierksen added 20 commits November 14, 2022 10:10

reverts, test cleanup

a076ac1

bump rules versions

893bddb

trying to clean up while preserving existing tests

5422ec0

upgrade stardoc due to rules_proto changes :-/

e09da21

alias XCTest dummy module so it doesn't replace the real one

5951403

trying one more thing

fc1b9b3

more tweaks, getting closer

bb6bbf1

whitespace revert

82b3662

revert vscode change

6346d6d

attempting to generate for intel mac, might need to iterate a bit

d505eb0

this stupid diff test again

723efe9

add default mapping to mac to deal with darwin cpu weirdness

510bee3

one more tweak for intel mac

1504731

add frameworks dep to arm64-to-sim

9795596

make config flag, fix import underlying module flag

ebf77f8

tweak config setting

997b0f9

address lingering nit

55f13ec

add -k to tests

4e34aec

jerrymarino requested changes Nov 18, 2022

View reviewed changes

jerrymarino mentioned this pull request Nov 18, 2022

Add testonly to libraries that link XCTest bazel-ios/cocoapods-bazel#82

Merged

hopefully green test setup

0e4c235

congt mentioned this pull request Aug 21, 2024

Adds the repository rule xcode_sdk_frameworks #898

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add rules for building SDK frameworks for explicit module compilation #562

Add rules for building SDK frameworks for explicit module compilation #562

dierksen commented Sep 15, 2022

thiagohmcruz left a comment

jerrymarino left a comment

dierksen commented Oct 17, 2022

dierksen commented Nov 18, 2022

jerrymarino Nov 14, 2022

dierksen Nov 18, 2022

jerrymarino Nov 18, 2022

jerrymarino Nov 14, 2022

jerrymarino Nov 14, 2022

jerrymarino Nov 14, 2022

jerrymarino Nov 14, 2022

jerrymarino Nov 14, 2022

+                  }})
+              )
+              alias(

Add rules for building SDK frameworks for explicit module compilation #562

Are you sure you want to change the base?

Add rules for building SDK frameworks for explicit module compilation #562

Conversation

dierksen commented Sep 15, 2022

thiagohmcruz left a comment

Choose a reason for hiding this comment

jerrymarino left a comment

Choose a reason for hiding this comment

dierksen commented Oct 17, 2022

dierksen commented Nov 18, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment