Skip to content

fix bazel7 sim_arm64 dynamic framework link error, pass dynamic framework info - #915

Open
kuperxu wants to merge 9 commits into
bazel-ios:masterfrom
kuperxu:kuper/fix_dynamic_lib_link
Open

kuperxu wants to merge 9 commits into
bazel-ios:masterfrom
kuperxu:kuper/fix_dynamic_lib_link

Conversation

@kuperxu

@kuperxu kuperxu commented Sep 12, 2024

Copy link
Copy Markdown

When the "apple.arm64_simulator_use_device_deps" feature is enabled, dynamic framework information is not passed to Bazel. As a result, the linker encounters errors when attempting to link dynamic frameworks.

…bazel,with feature apple.arm64_simulator_use_device_deps enable
@kuperxu
kuperxu marked this pull request as draft September 12, 2024 12:35
@kuperxu
kuperxu marked this pull request as ready for review September 12, 2024 12:35
@kuperxu

kuperxu commented Sep 12, 2024

Copy link
Copy Markdown
Author

hi @luispadron, Help review the code. plz

@luispadron

Copy link
Copy Markdown
Collaborator

I don't use this rule, but @thiagohmcruz @gyfelton can help here

@kuperxu

kuperxu commented Sep 14, 2024

Copy link
Copy Markdown
Author

@thiagohmcruz @gyfelton please help me with the review, I didn't add the reviewer permissions.

@gyfelton

Copy link
Copy Markdown
Contributor

kicking CI to build.
Am I right that the added @import statement was a red to green test case with your fix? 🙏

Comment thread rules/import_middleman.bzl Outdated
@gyfelton

Copy link
Copy Markdown
Contributor

seems that CI are failing with

# Execution platform: @local_config_platform//:host
tests/ios/in-tree-vendor-prebuilt-deps/dynamic/app/App/main.m:3:9: fatal error: module 'MobileFlow' not found
@import MobileFlow;
 ~~~~~~~^~~~~~~~~~

xujiguang.kuper and others added 2 commits September 18, 2024 10:59
@kuperxu

kuperxu commented Sep 18, 2024

Copy link
Copy Markdown
Author

seems that CI are failing with

# Execution platform: @local_config_platform//:host
tests/ios/in-tree-vendor-prebuilt-deps/dynamic/app/App/main.m:3:9: fatal error: module 'MobileFlow' not found
@import MobileFlow;
 ~~~~~~~^~~~~~~~~~

The reason for this was a missing MobileFlow dependency, which I fixed in the latest commit

@gyfelton

Copy link
Copy Markdown
Contributor

@kuperxu thx sooo much for the contribution, may I ask about some basic info of the downstream repo that you are fixing for:
which bazel version?
is VFS feature also enabled? (beside arm64_simulator_use_device_deps feature)
which rules_swift/apple version you are using with?
do you have sandbox mode enabled build?
does it use latest HEAD of rules_ios?

This info is useful on planning our work and contribution to the repo.

@thiagohmcruz I don't find any usage of dynamic_framework_file in our downstream repo (otherwise we would also suffer from this issue) so I think this is a good change, WDYT?

@kuperxu

kuperxu commented Sep 19, 2024

Copy link
Copy Markdown
Author

downstream

hi @gyfelton , for your question:
I am using bazel 7.1.1
VFS feature is disable
In my project,i am using rules_swift 1.17.0 & rules_apple 3.3.0(The issue can also be reproduced in the rules_ios master branch)
sandbox mode is disabled in my project.
yeah, latest HEAD of rules_ios

@gyfelton

Copy link
Copy Markdown
Contributor

Thank you! I merged lastest master on your branch and rekicking the build so that we can land it when ready.

I am also testing my downstream repo with your change and I think it's a good change. Need some input from others though.

@kuperxu

kuperxu commented Sep 20, 2024

Copy link
Copy Markdown
Author

Thank you! I merged lastest master on your branch and rekicking the build so that we can land it when ready.

I am also testing my downstream repo with your change and I think it's a good change. Need some input from others though.

The MobileFlow framework had a dependency on MiSnapCore, which I fixed in a new commit. Please help review.

otool -L tests/ios/in-tree-vendor-prebuilt-deps/Pods/MobileFlow/Frameworks/MobileFlow.framework/MobileFlow | grep MiSnapCore                      
        @rpath/MiSnapCore.framework/MiSnapCore (compatibility version 1.0.0, current version 1.0.0)
        @rpath/MiSnapCore.framework/MiSnapCore (compatibility version 1.0.0, current version 1.0.0)

@kuperxu
kuperxu requested a review from gyfelton October 8, 2024 08:05
@kuperxu

kuperxu commented Oct 8, 2024

Copy link
Copy Markdown
Author

hi @thiagohmcruz @gyfelton ,Can you help me review the code?

@gyfelton

gyfelton commented Oct 8, 2024

Copy link
Copy Markdown
Contributor

thank you for your contribution! Since you are on bazel 7 also I feel we should decide when to remove support for bazel 6 so we can save room for next bazel version bump (esp. since apple_common.objc provider is being gone in some minor version of bazel 7) @thiagohmcruz thoughts?

@kuperxu

kuperxu commented Oct 9, 2024

Copy link
Copy Markdown
Author

could you help me run the workflow to merge the code? @gyfelton
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants