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: use lipo -info instead of file for architecture detection #2478

Merged
merged 1 commit into from
Oct 9, 2024

Conversation

arietis
Copy link
Contributor

@arietis arietis commented Oct 8, 2024

Summary:

This PR updates the verifyApplicationPlatform function to use lipo -info instead of the file command for determining the architectures supported by the app’s executable. The change improves accuracy when verifying compatibility on both Intel and Apple Silicon devices by aligning with the consistent output format of lipo.

Reasoning:

Inconsistent file Output: The file command’s output can vary based on the system, leading to unreliable architecture detection.

Consistent lipo Output: lipo -info provides consistent architecture details, which makes it a better choice for ensuring the correct Simulator architecture is detected.

Simplified Logic: This change also simplifies string matching and error handling, reducing potential issues with cross-architecture setups.

Example of inconsistent file Output

# System version:
/usr/bin/file --version
file-5.41
/usr/bin/file test.app
test.app/test: Mach-O 64-bit executable arm64

# Custom version in PATH:
file --version
file-5.45
file test.app
test.app/test: Mach-O 64-bit arm64 executable, flags:<NOUNDEFS|DYLDLINK|TWOLEVEL|PIE>

Example of consistent lipo Output

lipo -info test.app
Non-fat file: test.app/test is architecture: arm64

Copy link

linux-foundation-easycla bot commented Oct 8, 2024

CLA Signed


The committers listed above are authorized under a signed CLA.

Copy link
Member

@KazuCocoa KazuCocoa left a comment

Choose a reason for hiding this comment

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

lgtm, thank you!

@KazuCocoa KazuCocoa merged commit 993aa3a into appium:master Oct 9, 2024
17 of 18 checks passed
github-actions bot pushed a commit that referenced this pull request Oct 9, 2024
## [7.27.1](v7.27.0...v7.27.1) (2024-10-09)

### Bug Fixes

* use lipo -info instead of file for architecture detection ([#2478](#2478)) ([993aa3a](993aa3a))
Copy link
Contributor

github-actions bot commented Oct 9, 2024

🎉 This PR is included in version 7.27.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@eglitise eglitise added the size:S contribution size: S label Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released size:S contribution size: S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants