Skip to content

Support rules_apple 3.x.x #773

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.

Sign up for GitHub

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

Merged
jerrymarino merged 4 commits into from
Oct 19, 2023
Merged

Support rules_apple 3.x.x #773

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
847a217
Support rules_apple 3.x.x
jerrymarino Sep 29, 2023
5c4f8eb
Bump projects
jerrymarino Oct 2, 2023
6b53b4b
Fix linker warning when `minimum_os_version` is not set on a test's i…
jszumski Oct 4, 2023
7efdb03
Adding for bzlmod
jerrymarino Oct 17, 2023
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
40 changes: 36 additions & 4 deletions .github/workflows/tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,8 @@ jobs:
name: bazel-testlogs
path: bazel-testlogs

lts_ios_integration_tests:
name: Build and Test ( Virtual Frameworks + LTS )
bazel_5_ios_integration_tests:
name: Build and Test ( Virtual Frameworks + Bazel 5 )
runs-on: macos-12
env:
USE_BAZEL_VERSION: 5.3.2
Expand All @@ -109,6 +109,31 @@ jobs:
name: bazel-testlogs
path: bazel-testlogs

rules_apple_2_ios_integration_tests:
name: Build and Test ( Virtual Frameworks + rules_apple 2.x )
runs-on: macos-12
env:
USE_BAZEL_VERSION: 6.1.2
steps:
- uses: actions/checkout@v3
- name: Preflight Env
run: .github/workflows/preflight_env.sh --no-bzlmod
- name: Build and Test
run: |
# iOS tests
bazelisk build \
--config=ci_with_caches \
--config=ios \
--config=vfs \
-- \
//tests/ios/...

- uses: actions/upload-artifact@v2
if: failure()
with:
name: bazel-testlogs
path: bazel-testlogs

build_arm64_simulator:
name: Build arm64 Simulator
runs-on: macos-12
Expand Down Expand Up @@ -149,7 +174,10 @@ jobs:
steps:
- uses: actions/checkout@v3
- name: Preflight Env
run: .github/workflows/preflight_env.sh
# stardoc latest version and bzlmod is broken with this in a few ways
# Please revert this to the version which worked reasonablly:
# https://github.com/bazelbuild/stardoc/issues/192
run: .github/workflows/preflight_env.sh --no-bzlmod
# Note: we need to pass the absolute to the Bazel run
- name: buildifier
run: |
Expand Down Expand Up @@ -200,12 +228,16 @@ jobs:
path: bazel-testlogs

multi_arch_support:
# i386 was removed on rules_apple 3.x.x - this test case needs reworking
# to exemplify fat binaries on the latest version
name: Build iOS App for Multiple Architecture
runs-on: macos-12
env:
USE_BAZEL_VERSION: 6.1.2
steps:
- uses: actions/checkout@v3
- name: Preflight Env
run: .github/workflows/preflight_env.sh
run: .github/workflows/preflight_env.sh --no-bzlmod
Copy link
Collaborator

@luispadron luispadron Oct 17, 2023

Choose a reason for hiding this comment

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

Bzlmod support was merged with 6.1.2, why should this be disabled here?

Copy link
Contributor Author

@jerrymarino jerrymarino Oct 17, 2023

Choose a reason for hiding this comment

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

Please find in in the proceeding comment that it uses 6.1.2 and rules_apple 2 - where bzlmod is hardcoded to use 3.0.0.

Copy link
Collaborator

@luispadron luispadron Oct 17, 2023

Choose a reason for hiding this comment

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

Gotcha so it doesnt work yet with rules_apple 3. I guess I got confused because we're essentially using Bazel 6.1.2 to mean rules_apple 2.

Not for this PR, but in the future I wonder if specifying these Bazel version + rules_apple versions wouldn't be better in a specific WORKSPACE for that specific test. Some repos do this with their integration tests and it allows them to test the different bazel version / rule versions they support without setting one version/feature for all their tests

Copy link
Contributor

@thiagohmcruz thiagohmcruz Oct 17, 2023

Choose a reason for hiding this comment

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

Nit: I'd still explicitly say in that comment why that implies --no-bzlmod is needed. The same way it's confusing during review it will be once we're changing this in the future.

Saw what @luispadron is talking about in other repos too and also like the idea. Wondering if we should create "Feature Request" templates for issues in rules_ios to track things like this 🤔

Copy link
Contributor Author

@jerrymarino jerrymarino Oct 18, 2023

Choose a reason for hiding this comment

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

Yeah it basically exists because people aren't using bzlmod on 5 and 6 so it's good to have some coverage of both ways to test this. Though, we could bump to a later version but needs more consideration.

Also, the way it's configured now get's us a larger matrix of bzlmod vs non bzlmod across a few versions. Though bzlmod support works on all the versions in the repo 5.4.1, 6.x.x we'd still like like the coverage to we don't break it - but probably not testing 100% of permutations

- name: Build App
run: |
bazelisk build \
Expand Down
8 changes: 7 additions & 1 deletion MODULE.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ bazel_dep(
)
bazel_dep(
name = "rules_apple",
version = "2.3.0",
version = "3.0.0",
repo_name = "build_bazel_rules_apple",
)
bazel_dep(
Expand Down Expand Up @@ -60,6 +60,12 @@ bazel_dep(
repo_name = "io_bazel_stardoc",
)

rules_apple_api_deps = use_extension("//rules:module_extensions.bzl", "rules_apple_api_deps")
use_repo(
rules_apple_api_deps,
"rules_apple_api",
)

# Load non-bzlmod dependencies from rules_ios
non_module_deps = use_extension("//rules:module_extensions.bzl", "non_module_deps")
use_repo(
Expand Down
1 change: 1 addition & 0 deletions data/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,6 @@ bzl_library(
"xcspec_evals.bzl",
"xcspecs.bzl",
],
tags = ["manual"],
visibility = ["//visibility:public"],
)
3 changes: 2 additions & 1 deletion docs/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ _DOC_SRCS = [
"apple_patched",
"extension",
"features",
"force_load_direct_deps",
"framework",
"hmap",
"import_middleman",
"library",
"plists",
"force_load_direct_deps",
"precompiled_apple_resource_bundle",
"providers",
"substitute_build_settings",
Expand All @@ -33,6 +33,7 @@ _DOC_SRCS = [
out = file + ".gen.md",
input = "//rules:%s.bzl" % file,
tags = [
"manual",
"no-cache",
"no-sandbox", # https://github.com/bazelbuild/stardoc/issues/112
],
Expand Down
6 changes: 4 additions & 2 deletions docs/force_load_direct_deps_doc.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
## force_load_direct_deps

<pre>
force_load_direct_deps(<a href="#force_load_direct_deps-name">name</a>, <a href="#force_load_direct_deps-deps">deps</a>, <a href="#force_load_direct_deps-should_force_load">should_force_load</a>)
force_load_direct_deps(<a href="#force_load_direct_deps-name">name</a>, <a href="#force_load_direct_deps-deps">deps</a>, <a href="#force_load_direct_deps-minimum_os_version">minimum_os_version</a>, <a href="#force_load_direct_deps-platform_type">platform_type</a>, <a href="#force_load_direct_deps-should_force_load">should_force_load</a>)
</pre>

A rule to link with `-force_load` for direct`deps`
Expand Down Expand Up @@ -40,7 +40,9 @@ perspective so it isn't used.
| Name | Description | Type | Mandatory | Default |
| :------------- | :------------- | :------------- | :------------- | :------------- |
| <a id="force_load_direct_deps-name"></a>name | A unique name for this target. | <a href="https://bazel.build/concepts/labels#target-names">Name</a> | required | |
| <a id="force_load_direct_deps-deps"></a>deps | - | <a href="https://bazel.build/concepts/labels">List of labels</a> | optional | `[]` |
| <a id="force_load_direct_deps-deps"></a>deps | Deps | <a href="https://bazel.build/concepts/labels">List of labels</a> | required | |
| <a id="force_load_direct_deps-minimum_os_version"></a>minimum_os_version | Internal - currently rules_ios the dict `platforms` | String | optional | `""` |
| <a id="force_load_direct_deps-platform_type"></a>platform_type | Internal - currently rules_ios uses the dict `platforms` | String | optional | `""` |
| <a id="force_load_direct_deps-should_force_load"></a>should_force_load | Allows parametrically enabling the functionality in this rule. | Boolean | optional | `True` |


33 changes: 32 additions & 1 deletion rules/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,51 +14,67 @@ genrule(
bzl_library(
name = "providers",
srcs = ["providers.bzl"],
tags = ["manual"],
visibility = ["//visibility:public"],
)

bzl_library(
name = "transition_support",
srcs = ["transition_support.bzl"],
tags = ["manual"],
visibility = ["//visibility:public"],
deps = [
"@build_bazel_apple_support//lib:apple_support",
"@rules_apple_api//:api",
],
)

bzl_library(
name = "hmap",
srcs = ["hmap.bzl"],
tags = ["manual"],
visibility = ["//visibility:public"],
deps = ["@build_bazel_rules_swift//swift"],
)

bzl_library(
name = "features",
srcs = ["features.bzl"],
tags = ["manual"],
visibility = ["//visibility:public"],
)

bzl_library(
name = "substitute_build_settings",
srcs = ["substitute_build_settings.bzl"],
tags = ["manual"],
visibility = ["//visibility:public"],
)

bzl_library(
name = "force_load_direct_deps",
srcs = ["force_load_direct_deps.bzl"],
tags = ["manual"],
visibility = ["//visibility:public"],
deps = [":providers"],
deps = [
":providers",
":transition_support",
"@rules_apple_api//:api",
],
)

bzl_library(
name = "xcconfig.doc",
srcs = ["xcconfig.doc.bzl"],
tags = ["manual"],
visibility = ["//visibility:public"],
deps = ["//rules/library:xcconfig"],
)

bzl_library(
name = "plists",
srcs = ["plists.bzl"],
tags = ["manual"],
visibility = ["//visibility:public"],
deps = [
":substitute_build_settings",
Expand All @@ -72,13 +88,15 @@ bzl_library(
bzl_library(
name = "vfs_overlay.doc",
srcs = ["vfs_overlay.doc.bzl"],
tags = ["manual"],
visibility = ["//visibility:public"],
deps = ["//rules/framework:vfs_overlay"],
)

bzl_library(
name = "precompiled_apple_resource_bundle",
srcs = ["precompiled_apple_resource_bundle.bzl"],
tags = ["manual"],
visibility = ["//visibility:public"],
deps = [
":transition_support",
Expand All @@ -93,17 +111,20 @@ bzl_library(
bzl_library(
name = "import_middleman",
srcs = ["import_middleman.bzl"],
tags = ["manual"],
visibility = ["//visibility:public"],
deps = [
":features",
"//rules/internal:objc_provider_utils",
"@build_bazel_rules_apple//apple",
"@rules_apple_api//:api",
],
)

bzl_library(
name = "library",
srcs = ["library.bzl"],
tags = ["manual"],
visibility = ["//visibility:public"],
deps = [
":hmap",
Expand All @@ -121,12 +142,14 @@ bzl_library(
"@build_bazel_rules_apple//apple:apple_static_library",
"@build_bazel_rules_apple//apple:resources",
"@build_bazel_rules_swift//swift",
"@rules_apple_api//:api",
],
)

bzl_library(
name = "framework",
srcs = ["framework.bzl"],
tags = ["manual"],
visibility = ["//visibility:public"],
deps = [
":features",
Expand All @@ -142,12 +165,14 @@ bzl_library(
"@bazel_tools//tools/cpp:toolchain_utils.bzl",
"@build_bazel_rules_apple//apple",
"@build_bazel_rules_swift//swift",
"@rules_apple_api//:api",
],
)

bzl_library(
name = "apple_patched",
srcs = ["apple_patched.bzl"],
tags = ["manual"],
visibility = ["//visibility:public"],
deps = [
":features",
Expand All @@ -161,6 +186,7 @@ bzl_library(
bzl_library(
name = "app_clip",
srcs = ["app_clip.bzl"],
tags = ["manual"],
visibility = ["//visibility:public"],
deps = [
":force_load_direct_deps",
Expand All @@ -173,6 +199,7 @@ bzl_library(
bzl_library(
name = "app",
srcs = ["app.bzl"],
tags = ["manual"],
visibility = ["//visibility:public"],
deps = [
":force_load_direct_deps",
Expand All @@ -186,18 +213,22 @@ bzl_library(
bzl_library(
name = "extension",
srcs = ["extension.bzl"],
tags = ["manual"],
visibility = ["//visibility:public"],
deps = [
":force_load_direct_deps",
":plists",
":transition_support",
"//rules/internal:framework_middleman",
"@build_bazel_rules_apple//apple:ios",
"@rules_apple_api//:api",
],
)

bzl_library(
name = "test",
srcs = ["test.bzl"],
tags = ["manual"],
visibility = ["//visibility:public"],
deps = [
":library",
Expand Down
6 changes: 6 additions & 0 deletions rules/app.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ def ios_application(
deps = kwargs.get("deps", []) + library.lib_names,
tags = ["manual"],
testonly = testonly,
platform_type = "ios",
minimum_os_version = application_kwargs.get("minimum_os_version"),
)

# Setup framework middlemen - need to process deps and libs
Expand All @@ -105,6 +107,8 @@ def ios_application(
framework_deps = kwargs.get("deps", []) + library.lib_names,
tags = ["manual"],
testonly = testonly,
platform_type = "ios",
minimum_os_version = application_kwargs.get("minimum_os_version"),
)
frameworks = [fw_name] + kwargs.pop("frameworks", [])

Expand All @@ -114,6 +118,8 @@ def ios_application(
deps = kwargs.get("deps", []) + library.lib_names,
tags = ["manual"],
testonly = testonly,
platform_type = "ios",
minimum_os_version = application_kwargs.get("minimum_os_version"),
)
deps = [dep_name] + [force_load_name]

Expand Down
25 changes: 22 additions & 3 deletions rules/force_load_direct_deps.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
load("//rules:providers.bzl", "AvoidDepsInfo")
load("//rules:transition_support.bzl", "split_transition_rule_attrs", "transition_support")
load("@bazel_skylib//lib:dicts.bzl", "dicts")

def _impl(ctx):
if not ctx.attr.should_force_load:
Expand Down Expand Up @@ -30,13 +32,30 @@ def _impl(ctx):

force_load_direct_deps = rule(
implementation = _impl,
attrs = {
"deps": attr.label_list(),
attrs = dicts.add(split_transition_rule_attrs, {
"deps": attr.label_list(
cfg = transition_support.split_transition,
mandatory = True,
doc =
"Deps",
),
"should_force_load": attr.bool(
default = True,
doc = "Allows parametrically enabling the functionality in this rule.",
),
},
"platform_type": attr.string(
mandatory = False,
doc =
"""Internal - currently rules_ios uses the dict `platforms`
""",
),
"minimum_os_version": attr.string(
mandatory = False,
doc =
"""Internal - currently rules_ios the dict `platforms`
""",
),
}),
doc = """
A rule to link with `-force_load` for direct`deps`

Expand Down
Loading