Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
c27f34e
Add rules for building SDK frameworks for explicit module compilation
dierksen Sep 15, 2022
e0f837c
Merge branch 'master' into framework_repo_rule
dierksen Sep 16, 2022
567d839
split out repo rule; bump bazel and rules_swift
dierksen Sep 28, 2022
183306e
revert arm64-to-sim tweak
dierksen Sep 28, 2022
399edef
testing some stuff
dierksen Sep 29, 2022
59fe2f4
add umbrella dep under the hood
dierksen Sep 30, 2022
b02c8ac
Merge branch 'master' into framework_repo_rule
dierksen Oct 11, 2022
87de123
Merge branch 'framework_repo_rule' into framework_tinkering
dierksen Oct 11, 2022
e905fe1
further repo rule cleanup
dierksen Oct 17, 2022
9792e61
change .bazelrc for now
dierksen Oct 17, 2022
0b47395
symlink in xctest framework, pass to testonly swift libraries
dierksen Oct 18, 2022
1b7afe4
cleanup
dierksen Oct 20, 2022
ebb38c2
Bump macos image, bazel, and carthage versions
dierksen Oct 20, 2022
90aa093
Merge branch 'xc13' into framework_repo_rule
dierksen Oct 20, 2022
0a92978
gradually trying to fix tests
dierksen Oct 21, 2022
c891e65
bump cocoapods to 1.11 to add ruby 3 support
dierksen Oct 21, 2022
55c9710
newest bazel release
dierksen Oct 21, 2022
e83f15d
update gemfile.lock
dierksen Oct 21, 2022
901234d
bump checkout action, mark mixed framework test as manual for now
dierksen Oct 21, 2022
efc8ff6
disable another one
dierksen Oct 21, 2022
fd3db05
Merge branch 'master' into xc13
dierksen Nov 1, 2022
7d01fbe
try newer carthage fix
dierksen Nov 2, 2022
a6d85c3
re-enable a couple of tests for now
dierksen Nov 2, 2022
0d11ade
reverts
dierksen Nov 2, 2022
3eee080
Merge branch 'master' into xc13
dierksen Nov 3, 2022
865789c
revert more formatting
dierksen Nov 4, 2022
de887dd
update test simulators
dierksen Nov 4, 2022
29dc203
just change the diff
dierksen Nov 7, 2022
8d06a2a
tweak diff
dierksen Nov 7, 2022
5f3bfd0
Merge branch 'xc13' into framework_repo_rule
dierksen Nov 7, 2022
ed943d9
tweak repos for testing on github
dierksen Nov 7, 2022
9dc858f
Merge branch 'master' into framework_repo_rule
dierksen Nov 7, 2022
b921c35
fix deps array (ugh), fix watch simulator cpu
dierksen Nov 8, 2022
c17cbbe
add -k to tests so i can see progress at least
dierksen Nov 8, 2022
8a6de35
Merge branch 'master' into framework_repo_rule
dierksen Nov 14, 2022
a076ac1
reverts, test cleanup
dierksen Nov 14, 2022
893bddb
bump rules versions
dierksen Nov 14, 2022
5422ec0
trying to clean up while preserving existing tests
dierksen Nov 14, 2022
e09da21
upgrade stardoc due to rules_proto changes :-/
dierksen Nov 14, 2022
5951403
alias XCTest dummy module so it doesn't replace the real one
dierksen Nov 16, 2022
fc1b9b3
trying one more thing
dierksen Nov 16, 2022
bb6bbf1
more tweaks, getting closer
dierksen Nov 17, 2022
a8278b0
Merge branch 'master' into framework_repo_rule
dierksen Nov 17, 2022
82b3662
whitespace revert
dierksen Nov 17, 2022
6346d6d
revert vscode change
dierksen Nov 17, 2022
d505eb0
attempting to generate for intel mac, might need to iterate a bit
dierksen Nov 17, 2022
723efe9
this stupid diff test again
dierksen Nov 17, 2022
510bee3
add default mapping to mac to deal with darwin cpu weirdness
dierksen Nov 17, 2022
1504731
one more tweak for intel mac
dierksen Nov 17, 2022
9795596
add frameworks dep to arm64-to-sim
dierksen Nov 17, 2022
ebf77f8
make config flag, fix import underlying module flag
dierksen Nov 17, 2022
997b0f9
tweak config setting
dierksen Nov 17, 2022
55f13ec
address lingering nit
dierksen Nov 18, 2022
4e34aec
add -k to tests
dierksen Nov 18, 2022
0e4c235
hopefully green test setup
dierksen Nov 18, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@ build --features apple.arm64_simulator_use_device_deps
# Note - tree artifacts needs to be on
build:lldb_ios_test --spawn_strategy=standalone --apple_platform_type=ios --define=apple.experimental.tree_artifact_outputs=1

# Configure for explicit module compilation
# also requires setting EXPLICIT_MODULE env variable to generate the frameworks repo.
build:explicit_modules --features=swift.use_c_modules
build:explicit_modules --features=swift.emit_c_module
build:explicit_modules --action_env=EXPLICIT_MODULES=1

# Delete the VM test suite for github
build --deleted_packages tests/ios/vmd

Expand Down
54 changes: 54 additions & 0 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -131,3 +131,57 @@ jobs:
run: .github/workflows/xcode_select.sh
- name: Build App
run: bazelisk build -s tests/ios/app/App --apple_platform_type=ios --ios_minimum_os=10.2 --ios_multi_cpus=i386,x86_64
explicit_module_tests:
name: Build and Test w/ explicit modules
runs-on: macos-12
steps:
- uses: actions/checkout@v3
- name: Select Xcode
run: .github/workflows/xcode_select.sh
- name: Build and Test
run: |
# Host config
bazelisk test --build_tests_only --local_test_jobs=1 \
--config=explicit_modules -- \
//... -//tests/ios/...

bazelisk build --config=explicit_modules -- \
//... -//tests/ios/... \
-//tests/macos/xcconfig:all

# `deleted_packages` is needed below in order to override the value of the .bazelrc file
bazelisk test --build_tests_only -k --local_test_jobs=1 --apple_platform_type=ios --deleted_packages='' \
--config=explicit_modules -- \
//tests/ios/... \
-//tests/ios/frameworks/dynamic:AppWithExtension_output_test \
-//tests/ios/frameworks/dynamic:App_output_test \
-//tests/ios/frameworks/dynamic:TestAppWithDylibs \
-//tests/ios/frameworks/dynamic:TestAppWithDylibs_output_test \
-//tests/ios/frameworks/mixed-source/custom-module-map:CustomModuleMapTests \
-//tests/ios/frameworks/mixed-source/only-source:MixedSourceTest \
-//tests/ios/frameworks/testonly:MixedSourceTest \
-//tests/ios/unit-test/test-imports-app:TestImports-Unit-Tests \
-//tests/ios/frameworks/sources-with-prebuilt-binaries:MixedSourceFrameworkTests

bazelisk build --apple_platform_type=ios --deleted_packages='' \
--config=explicit_modules -- \
`bazel query 'kind("ios_application rule", //...)'` \
-//tests/ios/app:AppTestOnly \
-//tests/ios/app:AppTestOnlyWithExtension \
-//tests/ios/app:AppWithDefines \
-//tests/ios/app:AppWithSelectableCopts \
-//tests/ios/frameworks/dynamic:App \
-//tests/ios/frameworks/dynamic:AppWithExtension \
-//tests/ios/in-tree-vendor-prebuilt-deps/dynamic/app:App \
-//tests/ios/in-tree-vendor-prebuilt-deps/dynamic/app:AppWithSelectableCopts \
-//tests/ios/in-tree-vendor-prebuilt-deps/static/app:App \
-//tests/ios/in-tree-vendor-prebuilt-deps/static/app:AppWithSelectableCopts \
-//tests/ios/unit-test/test-imports-app:TestImports-App \
-//tests/ios/xcconfig:App \
-//tests/ios/xcframeworks/Basic:XCFrameworksApp \
-//tests/ios/xcframeworks/StaticLib:StaticLibApp
- uses: actions/upload-artifact@v2
if: failure()
with:
name: bazel-testlogs
path: bazel-testlogs
9 changes: 8 additions & 1 deletion BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Pull buildifer.mac as an http_file, then depend on the file group to make an
# executable
load("@build_bazel_rules_swift//swift/internal:feature_names.bzl", "SWIFT_FEATURE_USE_GLOBAL_INDEX_STORE")
load("@build_bazel_rules_swift//swift/internal:feature_names.bzl", "SWIFT_FEATURE_USE_C_MODULES", "SWIFT_FEATURE_USE_GLOBAL_INDEX_STORE")
load("//rules:features.bzl", "feature_names")
load("@bazel_skylib//lib:selects.bzl", "selects")

Expand All @@ -19,6 +19,13 @@ config_setting(
},
)

config_setting(
name = "explicit_modules",
values = {
"features": SWIFT_FEATURE_USE_C_MODULES,
},
)

config_setting(
name = "virtualize_frameworks",
values = {
Expand Down
1 change: 0 additions & 1 deletion WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@ load_framework_dependencies()
load("//tools/toolchains/xcode_configure:xcode_configure.bzl", "xcode_configure")

xcode_configure(
remote_xcode_label = "",
xcode_locator_label = "//tools/toolchains/xcode_configure:xcode_locator.m",
)

Expand Down
2 changes: 1 addition & 1 deletion rules/legacy_xcodeproj.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ def _xcodeproj_aspect_impl(target, ctx):
swift_info = target[SwiftInfo]
swift_defines.append(depset(_collect_swift_defines(swift_info.direct_modules)))
swift_defines.append(depset(_collect_swift_defines(swift_info.transitive_modules.to_list())))
swift_module_paths = [m.swift.swiftmodule.path for m in target[SwiftInfo].direct_modules]
swift_module_paths = [m.swift.swiftmodule.path for m in target[SwiftInfo].direct_modules if m.swift]
providers.append(
_SrcsInfo(
srcs = depset(srcs, transitive = _get_attr_values_for_name(deps, _SrcsInfo, "srcs")),
Expand Down
18 changes: 9 additions & 9 deletions rules/library.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Copy link
Contributor

Choose a reason for hiding this comment

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

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

fetch_default_xcconfig = library_tools["fetch_default_xcconfig"](name, default_xcconfig_name, **kwargs) if default_xcconfig_name else {}
copts_by_build_setting = copts_by_build_setting_with_defaults(xcconfig, fetch_default_xcconfig, xcconfig_by_build_setting)
enable_framework_vfs = kwargs.pop("enable_framework_vfs", False) or namespace_is_module_name
Expand Down Expand Up @@ -915,13 +915,10 @@ def apple_library(name, library_tools = {}, export_private_headers = True, names

if swift_sources:
additional_swift_copts.extend(("-Xcc", "-I."))
if module_map:
# Frameworks find the modulemap file via the framework vfs overlay
if not namespace_is_module_name:
additional_swift_copts += ["-Xcc", "-fmodule-map-file=" + "$(execpath " + module_map + ")"]
additional_swift_copts.append(
"-import-underlying-module",
)

# Frameworks find the modulemap file via the framework vfs overlay
if module_map and not namespace_is_module_name:
additional_swift_copts += ["-Xcc", "-fmodule-map-file=" + "$(execpath " + module_map + ")"]
swiftc_inputs = other_inputs + objc_hdrs + objc_private_hdrs
if module_map:
swiftc_inputs.append(module_map)
Expand Down Expand Up @@ -971,11 +968,14 @@ def apple_library(name, library_tools = {}, export_private_headers = True, names
copts = copts_by_build_setting.swift_copts + swift_copts + select({
"@build_bazel_rules_ios//:virtualize_frameworks": framework_vfs_swift_copts,
"//conditions:default": framework_vfs_swift_copts if enable_framework_vfs else [],
}) + select({
"@build_bazel_rules_ios//:explicit_modules": [],
"//conditions:default": ["-import-underlying-module"] if module_map else [],
}) + additional_swift_copts,
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 []),
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

swiftc_inputs = swiftc_inputs,
features = ["swift.no_generated_module_map", "swift.use_pch_output_dir"] + select({
"@build_bazel_rules_ios//:virtualize_frameworks": ["swift.vfsoverlay"],
Expand Down
1 change: 1 addition & 0 deletions rules/repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ swift_binary(
name = "arm64-to-sim",
srcs = glob(["Sources/arm64-to-sim/*.swift"]),
visibility = ["//visibility:public"],
deps = ["@xcode_sdk_frameworks"],
)
""",
)
Expand Down
2 changes: 1 addition & 1 deletion rules/third_party/xcbuildkit_repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ swift_library(
name = name,
srcs = ",\n".join(['"%s"' % x for x in srcs]),
defines = ",\n".join(['"%s"' % x for x in defines]),
deps = ",\n".join(['"%s"' % namespaced_dep_name(x) for x in deps]),
deps = ",\n".join(['"%s"' % namespaced_dep_name(x) for x in deps] + ['"@xcode_sdk_frameworks"']),
copts = ",\n".join(['"%s"' % x for x in copts]),
))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ swift_library(
srcs = ["empty.swift"],
deps = [
"//tests/ios/frameworks/objc:ObjcFramework",
"@xcode_sdk_frameworks",
],
)

Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
diff -r ./tests/ios/xcodeproj/Test-LLDB-Logs-Project.xcodeproj/project.pbxproj ./tests/ios/xcodeproj/custom_output_path/Test-LLDB-Logs-Project.xcodeproj/project.pbxproj
45c45
47c47
< path = ../../..;
---
> path = ../../../..;
211c211
213c213
< BAZEL_WORKSPACE_ROOT = $SRCROOT/../../..;
---
> BAZEL_WORKSPACE_ROOT = $SRCROOT/../../../..;
268c268
270c270
< BAZEL_WORKSPACE_ROOT = $SRCROOT/../../..;
---
> BAZEL_WORKSPACE_ROOT = $SRCROOT/../../../..;
Expand Down
Loading