Skip to content

Commit

Permalink
Fix import_middleman broken with Bazel 7 + sandbox mode (#910)
Browse files Browse the repository at this point in the history
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 #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 #903
some checks should still fail but the ones using
`arm64_simulator_use_device_deps` should be green
  • Loading branch information
gyfelton authored Sep 5, 2024
1 parent a6e6e77 commit 9b3ba29
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 16 deletions.
7 changes: 6 additions & 1 deletion .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand All @@ -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 \
Expand Down
20 changes: 5 additions & 15 deletions rules/import_middleman.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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,
Expand Down
3 changes: 3 additions & 0 deletions tests/ios/unit-test/test-imports-app/empty.swift
Original file line number Diff line number Diff line change
@@ -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 ""
}

Expand Down
2 changes: 2 additions & 0 deletions tests/ios/unit-test/test-imports-app/main.m
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#import "Header.h"
#import <TestImports-App/Header.h>
#import <TestImports-App/TestImports_App-Swift.h>
#import <Basic/Basic.h>

#ifdef __IPHONE_OS_VERSION_MIN_REQUIRED
@import UIKit;
Expand All @@ -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];
Expand Down

0 comments on commit 9b3ba29

Please sign in to comment.