-
Notifications
You must be signed in to change notification settings - Fork 90
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
Add support for Bazel 7 #795
Comments
Are you working on this actively, @luispadron? |
I was going to pick it up next week (if I wasn't too busy) but if you want to take it, I wouldn't mind at all! |
I find it pretty bizarre that in rules_apple on the
With these changes: diff --git a/.bazelrc b/.bazelrc
index 44e77dbe..9872ad12 100644
--- a/.bazelrc
+++ b/.bazelrc
@@ -1,3 +1,5 @@
+common --experimental_enable_bzlmod
+
# NOTE: These are mainly just for the BazelCI setup so they don't have
# to be repeated multiple times in .bazelci/presubmit.yml.
diff --git a/examples/ios/HelloWorld/BUILD b/examples/ios/HelloWorld/BUILD
index 10edbf0b..ce41f4a3 100644
--- a/examples/ios/HelloWorld/BUILD
+++ b/examples/ios/HelloWorld/BUILD
@@ -39,6 +39,7 @@ ios_application(
minimum_os_version = "11.0",
version = ":HelloWorldVersion",
deps = [":Sources"],
+ output_discriminator = "",
) But maybe I'm missing something. Haven't been able to determine where that attribute is coming from WRT rules_ios on non-Bazel 7. |
Hm yah that is confusing, if it's failing in rules_apple with Bazel 6 I'd expect the same here since output_discriminator was added in the rules_apple 3 pr |
@mattrobmattrob fwiw Should we just remove setting this to |
That was my initial (and current) thought, @luispadron. I was just trying to explain what had changed because it didn't really make all that much sense to me. |
Yeah I searched a lil yesterday and couldn't find where that attr was removed |
Ah, okay. Now I've got it! It seems like it's Bazel validating the input attributes better when they come directly from macros. With these changes: diff --git a/apple/versioning.bzl b/apple/versioning.bzl
index c65ae432..7d309f79 100644
--- a/apple/versioning.bzl
+++ b/apple/versioning.bzl
@@ -23,11 +23,15 @@ load(
"AppleXPlatToolsToolchainInfo",
"apple_toolchain_utils",
)
+load("@build_bazel_rules_apple//apple:ios.bzl", "ios_application")
load(
"@bazel_skylib//lib:dicts.bzl",
"dicts",
)
+def fake_ios_application(**kwargs):
+ ios_application(output_discriminator = None, **kwargs)
+
def _collect_group_names(s):
"""Returns the list of placeholder names found in the given string.
diff --git a/examples/ios/HelloWorld/BUILD b/examples/ios/HelloWorld/BUILD
index 10edbf0b..dc05455d 100644
--- a/examples/ios/HelloWorld/BUILD
+++ b/examples/ios/HelloWorld/BUILD
@@ -4,6 +4,7 @@ load("//apple:xcarchive.bzl", "xcarchive")
load(
"//apple:versioning.bzl",
"apple_bundle_version",
+ "fake_ios_application",
)
licenses(["notice"])
@@ -26,7 +27,7 @@ apple_bundle_version(
build_version = "1.0",
)
-ios_application(
+fake_ios_application(
name = "HelloWorld",
app_icons = ["//examples/resources:PhoneAppIcon.xcassets"],
bundle_id = "com.example.hello-world", Works on 6.3.2 and not on
|
Fixes #795 Signed-off-by: Matt Robinson <mattrob@hey.com>
Updated to include the two issues we've found |
@mattrobmattrob Have you made any more progress here? Thanks again for working on this! |
Nope. I haven’t had time to investigate the oddity in platform SDK selection yet. All PRs should link to this issue though if you want to pick it up. |
The |
|
#797 is an example of the CI moved over to Bazel 7.0.0, @karim-alweheshy. It has some issues that'll need fixed as well if you want to look at the errors on CI. |
The current issue seems to be: ld: Undefined symbols:
_main, referenced from:
<initial-undefines> This is failing when running the following: bazel build \
--config=ios \
--config=vfs \
--ios_multi_cpus=sim_arm64 \
-- \
//tests/ios/... \
-//tests/ios/unit-test/test-imports-app/... This might be related to We'll likely need to audit and update our usage of ObjcProvider for Bazel 7 specifically. |
@luispadron confirming your findings More details here |
Add support for apple platform command line options. Works towards #795
Updates GitHub CI to make it easier to add more Bazel/Xcode Versions. Changes: - Update `checkout` and `upload-artifacts` actions to `v4`, this fixes warnings we get on every PR for deprecated Node versions - Add `matrix` strategy for all jobs, this adds a `bazel_version` and `xcode_version` which each job will be configured to run against. We can add Bazel 7 once #795 is done. - Merge the `--config=vfs` and non-vfs job, this can be set with a custom matrix without duplicating the job code. - Remove `--remote-cache` flag, this was set by every job already.
Fixes bazel-ios/rules_ios#795. Bazel 7 seems to validate the usage of non-defined attributes better than Bazel 6 and lower. ~Testing Bazel 7 in CI using bazel-ios/rules_ios#797 This gets us one step closer to being prepared for Bazel 7. Signed-off-by: Matt Robinson <mattrob@hey.com>
Bazel 7 is coming out soon (November 2023), building with
USE_BAZEL_VERSION=last_rc
we get the following error:Tasks
output_discriminator
is not a valid attr: fixed in Remove missingoutput_discriminator
attribute usage #796apple_grte_top
usage: bazelbuild/bazel@fb4106b/Users/runner/work/rules_ios/rules_ios/rules/transition_support.bzl:125:36: transition inputs [//command_line_option:apple_compiler, //command_line_option:apple_grte_top] do not correspond to valid settings
apple_compiler
usage: bazelbuild/bazel@1acdfc4/Users/runner/work/rules_ios/rules_ios/rules/transition_support.bzl:125:36: transition inputs [//command_line_option:apple_compiler, //command_line_option:apple_grte_top] do not correspond to valid settings
toolchains
argument torule
sError in fail: In order to use find_cpp_toolchain, you must include the '@bazel_tools//tools/cpp:toolchain_type' in the toolchains argument to your rule.
sysroot
issues on CIclang: error: using sysroot for 'iPhoneSimulator' but targeting 'MacOSX' [-Werror,-Wincompatible-sysroot]
manual
tags toobjc_library
: https://bazelbuild.slack.com/archives/CD3QY5C2X/p1701890904270059ld: Undefined symbols
, we'll need to auditObjcProvider
for use in Bazel 7 given: incompatible_objc_linking_info_migration bazelbuild/bazel#17377The text was updated successfully, but these errors were encountered: