From 9b3ba291752730025035131c3fde8d2069eaa267 Mon Sep 17 00:00:00 2001 From: Elton Gao Date: Wed, 4 Sep 2024 22:46:38 -0400 Subject: [PATCH] Fix import_middleman broken with Bazel 7 + sandbox mode (#910) What changed and why: 1. added test case that broke with main, under sandbox mode and bazel 7 and `arm64_simulator_use_device_deps` feature turned on : To repro locally, run `bazel build //tests/ios/unit-test/test-imports-app:TestImports-App --config=ios --features apple.arm64_simulator_use_device_deps` and it fails. But success if change `.bazelversion` to 6.4.0 The error is "unable to find header `basic.h`" which is the same issue with what our own repo has. Also this only break Objc side not swift side (probably because CcInfo is more used by objc_library?) 2. To fix above: use the compilation_context generated originally. The original fix https://github.com/bazel-ios/rules_ios/pull/873 is missing fields inside `compilation_context` such as `headers`. So might as well use the original CcInfo collected, and only recreate the linking context. BTW i believe the original PR aims to fix this kind of error in bazel 7: ``` ld: building for 'iOS-simulator', but linking in object file (/path/to/someframework.framework[arm64][2] built for 'iOS' ``` Which is the error we got if trying to just use the original CcInfo. 3. Update the test matrix to have sandbox mode for the tests for `arm64_simulator_use_device_deps` feature Tests done: Without the change from https://github.com/bazel-ios/rules_ios/pull/903 some checks should still fail but the ones using `arm64_simulator_use_device_deps` should be green --- .github/workflows/tests.yml | 7 ++++++- rules/import_middleman.bzl | 20 +++++-------------- .../unit-test/test-imports-app/empty.swift | 3 +++ tests/ios/unit-test/test-imports-app/main.m | 2 ++ 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 4be3a0a19..6fa86386e 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -80,12 +80,13 @@ jobs: path: bazel-testlogs build_arm64_simulator: - name: arm64 Simulator (Bazel ${{ matrix.bazel_version }} / Xcode ${{ matrix.xcode_version }}) + name: arm64 Simulator (Bazel ${{ matrix.bazel_version }} / Xcode ${{ matrix.xcode_version }} / Sandbox ${{ matrix.sandbox }}) runs-on: macos-14 strategy: fail-fast: false matrix: bazel_version: [6.5.0, 7.1.0] + sandbox: [true, false] xcode_version: [15.2] env: XCODE_VERSION: ${{ matrix.xcode_version }} @@ -94,6 +95,10 @@ jobs: - uses: actions/checkout@v4 - name: Preflight Env run: .github/workflows/preflight_env.sh + - if: matrix.sandbox + name: Enable sandbox mode + run: | + echo "build --config=sandboxed" >> user.bazelrcc - name: Build and Test run: | bazelisk build \ diff --git a/rules/import_middleman.bzl b/rules/import_middleman.bzl index 8f75adabb..80ea3974b 100644 --- a/rules/import_middleman.bzl +++ b/rules/import_middleman.bzl @@ -265,19 +265,13 @@ def _file_collector_rule_impl(ctx): **objc_provider_fields ) - # Create the CcInfo provider, linking information from this is used in Bazel 7+. - cc_info = None + dep_cc_infos = [dep[CcInfo] for dep in ctx.attr.deps if CcInfo in dep] + cc_info = cc_common.merge_cc_infos(cc_infos = dep_cc_infos) if is_bazel_7: + # Need to recreate linking_context for Bazel 7 or later + # because of https://github.com/bazelbuild/bazel/issues/16939 cc_info = CcInfo( - compilation_context = cc_common.create_compilation_context( - framework_includes = depset( - transitive = [ - dep[CcInfo].compilation_context.framework_includes - for dep in ctx.attr.deps - if CcInfo in dep - ], - ), - ), + compilation_context = cc_info.compilation_context, linking_context = cc_common.create_linking_context( linker_inputs = depset([ cc_common.create_linker_input( @@ -297,10 +291,6 @@ def _file_collector_rule_impl(ctx): ]), ), ) - else: - dep_cc_infos = [dep[CcInfo] for dep in ctx.attr.deps if CcInfo in dep] - cc_info = cc_common.merge_cc_infos(cc_infos = dep_cc_infos) - return [ DefaultInfo(files = depset(dynamic_framework_dirs + replaced_frameworks)), objc, diff --git a/tests/ios/unit-test/test-imports-app/empty.swift b/tests/ios/unit-test/test-imports-app/empty.swift index 4671d0dc6..2250c5626 100644 --- a/tests/ios/unit-test/test-imports-app/empty.swift +++ b/tests/ios/unit-test/test-imports-app/empty.swift @@ -1,9 +1,12 @@ import Foundation import SomeFramework +import Basic @objc public class EmptyClass: NSObject { @objc public static func emptyDescription() -> String { + print(BasicString) + print(EmptyClass.emptyDescription) return "" } diff --git a/tests/ios/unit-test/test-imports-app/main.m b/tests/ios/unit-test/test-imports-app/main.m index 4bf3410f1..add6ee3db 100644 --- a/tests/ios/unit-test/test-imports-app/main.m +++ b/tests/ios/unit-test/test-imports-app/main.m @@ -1,6 +1,7 @@ #import "Header.h" #import #import +#import #ifdef __IPHONE_OS_VERSION_MIN_REQUIRED @import UIKit; @@ -18,6 +19,7 @@ - (BOOL)application:(UIApplication *)__unused application didFinishLaunchingWith self.window = [[UIWindow alloc] initWithFrame:[[UIScreen mainScreen] bounds]]; self.window.rootViewController = [UIViewController new]; self.window.rootViewController.view.backgroundColor = UIColor.whiteColor; + NSLog([NSString stringWithFormat:@"%@ %ld", BasicString, BasicVal_DownloadTheApp]); NSAssert([EmptyClass emptyDescription] != nil, @"Empty class description exists"); NSAssert([[EmptyClass new] emptyDescription] != nil, @"Empty instance description exists"); [self.window makeKeyAndVisible];