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 in Bazel 7 #873

Merged
merged 7 commits into from
Jun 5, 2024

Conversation

thiagohmcruz
Copy link
Contributor

@thiagohmcruz thiagohmcruz commented Jun 5, 2024

Can be considered a follow up to #850 to fix import_middleman in Bazel 7 while keeping it backwards compatible with Bazel 6.

@thiagohmcruz thiagohmcruz force-pushed the thiago/import_middleman-bazel-7 branch from 0edeacc to d9d5a87 Compare June 5, 2024 15:11
@thiagohmcruz thiagohmcruz force-pushed the thiago/import_middleman-bazel-7 branch from 59b0f7e to ab630a9 Compare June 5, 2024 16:01
@@ -8,7 +8,7 @@ set -e
# - XCODE_VERSION: The version of Xcode to use.

# GitHub runners are hitting 'module not found pkg_resources' required by prepare_sim.py
pip3 install setuptools --break-system-packages
pip3 install setuptools==69.5.1 --break-system-packages
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unrelated to the other changes in this PR. Fixes this error happening on CI (see repro):

/Users/runner/work/rules_ios/rules_ios/tools/tests/prepare_sim.py:9: DeprecationWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html
  from pkg_resources import packaging
Traceback (most recent call last):
  File "/Users/runner/work/rules_ios/rules_ios/tools/tests/prepare_sim.py", line 9, in <module>
    from pkg_resources import packaging
ImportError: cannot import name 'packaging' from 'pkg_resources' (/opt/homebrew/lib/python3.12/site-packages/pkg_resources/__init__.py)

(found a thread online where someone was suggesting stick to this version to fix it)

@thiagohmcruz thiagohmcruz marked this pull request as ready for review June 5, 2024 17:43
@thiagohmcruz thiagohmcruz merged commit 839cf60 into master Jun 5, 2024
12 checks passed
@thiagohmcruz thiagohmcruz deleted the thiago/import_middleman-bazel-7 branch June 5, 2024 17:50
gyfelton added a commit that referenced this pull request Sep 5, 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 #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
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.

2 participants