[Mac] Add support for butler.py to generate Mac ARM64 deployment zip - #5442
[Mac] Add support for butler.py to generate Mac ARM64 deployment zip#5442JuanMBriones wants to merge 2 commits into
Conversation
|
Looks like All-Tests and Run basic tests are failing. Can you fix those? |
g-ortuno
left a comment
g-ortuno
left a comment
There was a problem hiding this comment.
Please add a description to the PR! It should summarize all the changes we are making and why.
| 'Darwin': 'macos' | ||
| } | ||
| platform_name = platform_mappings[platform_name] | ||
| if platform_name == 'macos' and environment.get_cpu_arch() == 'arm64': |
There was a problem hiding this comment.
Why do we only do this for macOS? Can you add a comment to give some context?
There was a problem hiding this comment.
I added a comment before this line
# macOS bots run on both x86_64 and arm64 (Apple Silicon), requiring distinct
# deployment bundles. Other desktop platforms currently share a single bundle.
3e406ac#diff-80a5a8c67ccee4970fdf4eea7407768f75d7a29e0374c882eaf49a60d03ad5a9
| return 'x86_64' | ||
| if machine in ('i386', 'i686', 'x86'): | ||
| return 'x86' | ||
| return machine or None |
There was a problem hiding this comment.
Are we changing the return value for all architectures? Is that not going to affect all platforms? I did the analysis and this is what I found:
It seems there's three callers of this function:
- commands.is_supported_cpu_arch_for_job()
- update_task.get_source_url()
- environment.get_default_tool_path(tool_name)
1. is_supported_cpu_arch_for_job()
Before this PR, since get_cpu_arch() returned None for desktop, we would hit the first if and we would return True.
With this PR, we don't hit that if so we go into the next if and check the value of CPU_ARCH. None of the desktop jobs set it, so we return True.
It seems like our change doesn't affect this method except if the job sets CPU_ARCH.
2. update_task.get_source_url()
Here is where the trouble starts. It looks like get_cpu_arch() returns the target CPU architecture which doesn't necessarily match the host CPU architecture. For example, for Android, the host CPU architecture is amd64 since we run on linux bots, but the target architecture is arm64, the architecture of the Android device.
We should rename this method to get_target_cpu_arch() to clarify what this is.
3. environment.get_default_tool_path()
This one is also problematic because of the target/host issue. I left a comment there.
There was a problem hiding this comment.
Thanks for the help with this!! I solved this by separating host vs target architecture:
- In
update_task.get_source_url(), we switched toplatform.machine().lower() == 'arm64'to check the host machine architecture directly when fetching the deployment bundle. - Reverted
get_default_tool_path()to preserve directory invariants without fallback logic. - Removed
BOT_CPU_ARCHand keptget_cpu_arch()clean.
9a3f9b8 to
873518f
Compare
Thanks for the review:D I just modified the commit message. Sorry I forgot about this! |
### Problem ClusterFuzz lacked support for generating macOS ARM64 deployment zip packages and running bots on Apple Silicon. Additionally, packaging failed due to mismatched macOS wheel SDK tags in pip download, and is_supported_cpu_arch_for_job incorrectly parsed string CPU_ARCH values. ### Changes * butler.py & constants.py: Add macos_arm64 platform target, fallback SDK tags, and Python 3.7–3.11 ABIs. * common.py: Download platform pip requirements individually to resolve differing wheel tags, and support ARM64 ChromeDriver. * platform_requirements.txt: Add native C-extension packages (bcrypt, cffi, cryptography, google-crc32c, PyYAML, wrapt). * environment.py: Add get_cpu_arch() and support arch-specific tool paths in get_default_tool_path(). * update_task.py: Map macOS ARM64 bots to macos_arm64 deployment packages. * commands.py: Fix CPU_ARCH parsing in is_supported_cpu_arch_for_job(). * Tests: Add unit tests for environment arch detection, tool path resolution, deployment URLs, and job arch validation. Signed-off-by: Manuel Briones <manuelbriones@google.com>
- Rename PLATFORMS to DEPLOYMENT_TARGETS with backward-compatible alias. - Add macOS version tag comment in constants.py. - Use tempfile.TemporaryDirectory and add explanatory comments in common.py. - Remove default platform argument in butler.py package command. - Add documentation headers to Pipfile and platform_requirements.txt. - Use platform.machine() for host architecture detection in update_task.py. - Update CPU_ARCH format comment in commands.py. - Simplify get_default_tool_path and remove BOT_CPU_ARCH in environment.py. - Update and add unit tests in environment_test.py and update_task_test.py. ### Tests Executed * environment_test: 40/40 passed (GetCpuArchTest, GetDefaultToolPathTest, etc.) * update_task_test: 18/18 passed (GetSourceUrlTest, GetNewerSourceRevisionTest, etc.) * commands_test: 10/10 passed (IsSupportedCpuArchForJobTest, SetTaskPayloadTest, UpdateEnvironmentForJobTest) * testcase_manager_test: 5/5 passed (UploadTestcaseOutputTest) * Linters: pylint passed with 0 errors, yapf passed with 0 diffs.
873518f to
3e406ac
Compare
I already fixed failing tests. I saw an error on https://github.com/google/clusterfuzz/pull/5442/checks?check_run_id=99666204114 but it seems theres no correlation with the changes |
2 participants
No description provided.