Skip to content

Allow Bazel 6 users to explicitly opt-in to rules_apple 2.x #793

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
thiagohmcruz merged 5 commits into from
Oct 27, 2023
Merged

Allow Bazel 6 users to explicitly opt-in to rules_apple 2.x #793

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
95c1581
Allow Bazel 6 users to opt-in for rules_apple 2.x
thiagohmcruz Oct 26, 2023
99a8a75
Make it rules_apple 2.x specific opt-in instead (also back to old SHAs)
thiagohmcruz Oct 26, 2023
b041251
Add LOAD_RULES_APPLE_2_DEPS to WORKSPACE
thiagohmcruz Oct 26, 2023
9787b88
Mutate LOAD_RULES_APPLE_2_DEPS on CI
thiagohmcruz Oct 26, 2023
5278bc2
Update comments
thiagohmcruz Oct 27, 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
2 changes: 2 additions & 0 deletions .github/workflows/tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ jobs:
- name: Build and Test
run: |
# iOS tests
sed -i '' 's/LOAD_RULES_APPLE_2_DEPS = False/LOAD_RULES_APPLE_2_DEPS = True/' WORKSPACE && \
bazelisk build \
--config=ios \
--config=vfs \
Expand Down Expand Up @@ -227,6 +228,7 @@ jobs:
run: .github/workflows/preflight_env.sh --no-bzlmod --use-remote-cache
- name: Build App
run: |
sed -i '' 's/LOAD_RULES_APPLE_2_DEPS = False/LOAD_RULES_APPLE_2_DEPS = True/' WORKSPACE && \
bazelisk build \
--config=ios_multi_arch_test \
-- \
Expand Down
6 changes: 5 additions & 1 deletion WORKSPACE
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,11 @@ load(
"rules_ios_dev_dependencies",
)

rules_ios_dependencies()
# Storing in a variable so `.github/workflows/tests.yml` can easily
# mutate this before running `rules_ios`s CI tests
LOAD_RULES_APPLE_2_DEPS = False

rules_ios_dependencies(load_rules_apple_2_dependencies = LOAD_RULES_APPLE_2_DEPS)

rules_ios_dev_dependencies()

Expand Down
35 changes: 18 additions & 17 deletions rules/repositories.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,15 +59,17 @@ def _get_bazel_version():
return struct(major = 0, minor = 0, patch = 0)

def rules_ios_dependencies(
load_bzlmod_dependencies = True):
load_bzlmod_dependencies = True,
load_rules_apple_2_dependencies = False):
"""Fetches repositories that are public dependencies of `rules_ios`.

Args:
load_bzlmod_dependencies: if `True` loads dependencies that are available via bzlmod (set to True when using WORKSPACE, and False when using bzlmod)
load_rules_apple_2_dependencies: if `True` loads rules_apple 2.x dependencies instead of the latest
"""

if load_bzlmod_dependencies:
_rules_ios_bzlmod_dependencies()
_rules_ios_bzlmod_dependencies(load_rules_apple_2_dependencies = load_rules_apple_2_dependencies)

# Non-bzlmod tool dependencies that are used in the rule APIs
_rules_ios_tool_dependencies()
Expand Down Expand Up @@ -108,7 +110,7 @@ rules_apple_api = repository_rule(
local = True,
)

def _rules_ios_bzlmod_dependencies():
def _rules_ios_bzlmod_dependencies(load_rules_apple_2_dependencies = False):
"""Fetches repositories that are dependencies of `rules_ios`

These are only included when using WORKSPACE, when using bzlmod they're loaded in MODULE.bazel
Expand Down Expand Up @@ -149,22 +151,22 @@ def _rules_ios_bzlmod_dependencies():
],
sha256 = "c6966ec828da198c5d9adbaa94c05e3a1c7f21bd012a0b29ba8ddbccb2c93b0d",
)
elif bazel_version.major == "6" and bazel_version.minor == "1":
Copy link
Collaborator

@justinseanmartin justinseanmartin Oct 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@thiagohmcruz and I discussed offline - This feels like a weird special behavior to have for handling the CI use-case (per this comment). I like that the proposed change makes the rules_apple 2.x support a bit more explicit in the API contract. Before this change, someone using 6.1 and accidentally getting some weird default dependencies would probably be confusing.

I think we're going to omit the Bazel version check here and have this be defaults that we know work for rules_apple 2.x based on an explicit use_rules_apple_2 = True param. We're also going to reset this back to the values used prior to #773 (rules_apple v2.5.0 and rules_swift v1.10.0) as the defaults. For CI, Thiago is going to mutate the WORKSPACE file in the CI script to pass use_rules_apple_2 = True (default in the WORKSPACE will be False). Separately to this, we could bump everything to run on Bazel 6.4 as well and things should continue to work.

Copy link
Collaborator

@luispadron luispadron Oct 26, 2023

Choose a reason for hiding this comment

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

Thank you both, that's easier to reason about

elif load_rules_apple_2_dependencies:
# This `load_rules_apple_2_dependencies` flag is simply a convenience for consumers on
# any supported Bazel version wanting to use the default versions of the deps below (e.g. `rules_apple` 2.x.x).
#
# For context see discussion in this PR: https://github.com/bazel-ios/rules_ios/pull/793
_maybe(
github_repo,
http_archive,
name = "build_bazel_rules_swift",
project = "bazelbuild",
ref = "17e20f7edf27e647f1b45f11ed75d51c17820c3b",
repo = "rules_swift",
sha256 = "d50c2cb6f1c2c30cf44a8ea60469cd399f7458061169bde76a177b63d6b74330",
sha256 = "3a595a64afdcaf65b74b794661556318041466d727e175fa8ce20bdf1bb84ba0",
url = "https://github.com/bazelbuild/rules_swift/releases/download/1.10.0/rules_swift.1.10.0.tar.gz",
)
_maybe(
github_repo,
http_archive,
name = "build_bazel_rules_apple",
ref = "915ac30a9fa1fd3809599a5ab90fa1c6640fe8dc",
project = "bazelbuild",
repo = "rules_apple",
sha256 = "0204016496a39d5c70247650e098905d129f25347c7e1f019f838ca74252ce2d",
sha256 = "8ac4c7997d863f3c4347ba996e831b5ec8f7af885ee8d4fe36f1c3c8f0092b2c",
url = "https://github.com/bazelbuild/rules_apple/releases/download/2.5.0/rules_apple.2.5.0.tar.gz",
)
_maybe(
rules_apple_api,
Expand All @@ -182,9 +184,8 @@ def _rules_ios_bzlmod_dependencies():
)

else:
# Bazel 6+ codepath - you can use the 2_x version of rules_apple here if
# you'd like to set it. If you want to use a different rules_apple than
# we have here - define rules_apple_api - to the value you'd like
# (Bazel 6.x.x, rules_apple 3.x.x) codepath - you can use the 2.x.x version of rules_apple if
# you'd like either by setting `load_rules_apple_2_dependencies = True` or defining `rules_apple_api` in your WORKSPACE.
_maybe(
http_archive,
name = "build_bazel_rules_swift",
Expand Down