Skip to content
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

Fix import_middleman broken with Bazel 7 + sandbox mode #910

Merged
merged 5 commits into from
Sep 5, 2024

Conversation

gyfelton
Copy link
Contributor

@gyfelton gyfelton commented Aug 30, 2024

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 Fix import_middleman in Bazel 7 #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

@gyfelton gyfelton changed the title Fix import_middleman broken with Bazel 7 Fix import_middleman broken with Bazel 7 (take 2) Aug 30, 2024
@@ -1,9 +1,12 @@
import Foundation
import SomeFramework
import Basic
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this still work with bazel 7 probably because swift compile does not depend on CcInfo

@gyfelton gyfelton changed the title Fix import_middleman broken with Bazel 7 (take 2) Fix import_middleman broken with Bazel 7 + sandbox mode (take 2) Aug 30, 2024
@gyfelton gyfelton force-pushed the yuanfeng/fix-bazel7-import-middleman branch from 3073bd1 to b767843 Compare August 31, 2024 03:21
@gyfelton gyfelton marked this pull request as ready for review September 3, 2024 02:51
@luispadron luispadron changed the title Fix import_middleman broken with Bazel 7 + sandbox mode (take 2) Fix import_middleman broken with Bazel 7 + sandbox mode Sep 3, 2024
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
@gyfelton gyfelton force-pushed the yuanfeng/fix-bazel7-import-middleman branch from b767843 to 0c95c65 Compare September 3, 2024 17:35
@gyfelton gyfelton enabled auto-merge (squash) September 3, 2024 19:19
@gyfelton gyfelton merged commit 9b3ba29 into master Sep 5, 2024
23 checks passed
@gyfelton gyfelton deleted the yuanfeng/fix-bazel7-import-middleman branch September 5, 2024 02:46
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.

3 participants