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

By default compile as universal2 for macOS #2221

Merged
merged 3 commits into from
Jan 12, 2024

Conversation

rickmark
Copy link
Contributor

Currently the python bindings for universal2 in fact ship only the x64 version of libcapstone.dylib causing it to fail to load in arm64 python environments. This fix makes building of libcapstone.a and libcapstone.dylib universal2 (fat) by default with an option to disable if need be. This should fix the issue with python bindings as well as generally make capstone more pleasant for M1/2/3 users

Copy link
Collaborator

@Rot127 Rot127 left a comment

Choose a reason for hiding this comment

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

@kabeor Could you please trigger the workflows.

@rickmark
Copy link
Contributor Author

Realized that for the bindings it was still relying on makefile build, so it wasn't taking the universal version. Used the same code path as windows to invoke cmake (is there value in keeping the makefile based version anymore?)

@Rot127
Copy link
Collaborator

Rot127 commented Dec 21, 2023

is there value in keeping the makefile based version anymore?

Not really. It's considered deprecated and its easy to forget about it. The Python bindings need to be reworked anyways to get rid of the old stuff. But there is a lot to modernize and module updates + testing is currently higher on the list (at least for us Rizin folks).
But if you find the time, feel free to join the "renovate Capstone" struggle :)

@XVilka
Copy link
Contributor

XVilka commented Dec 21, 2023

Note, once it's green, it would be good to cherry-pick to the 5.0.x series: #2081

Copy link
Contributor

@XVilka XVilka left a comment

Choose a reason for hiding this comment

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

Building C extensions
    PWD: /Users/runner/work/capstone/capstone/build
    CMake Error: CMake was unable to find a build program corresponding to "Ninja".  CMAKE_MAKE_PROGRAM is not set.  You probably need to select a different build tool.
    CMake Error: CMAKE_C_COMPILER not set, after EnableLanguage
    CMake Error: CMAKE_CXX_COMPILER not set, after EnableLanguage
    -- Configuring incomplete, errors occurred!
    No such file or directory
    CMake Error: Generator: execution of make failed. Make command was:
    error: [Errno 2] No such file or directory: 'libcapstone.5.dylib'
    ----------------------------------------
    ERROR: Failed building wheel for capstone

bindings/python/setup.py Outdated Show resolved Hide resolved
@rickmark
Copy link
Contributor Author

I amended to resolve some of the issues (format strings are python3, and Ninja wasn't strictly needed as unix makefiles is fine), guess it needs a re-trigger?

@rickmark
Copy link
Contributor Author

Building C extensions
    PWD: /Users/runner/work/capstone/capstone/build
    CMake Error: CMake was unable to find a build program corresponding to "Ninja".  CMAKE_MAKE_PROGRAM is not set.  You probably need to select a different build tool.
    CMake Error: CMAKE_C_COMPILER not set, after EnableLanguage
    CMake Error: CMAKE_CXX_COMPILER not set, after EnableLanguage
    -- Configuring incomplete, errors occurred!
    No such file or directory
    CMake Error: Generator: execution of make failed. Make command was:
    error: [Errno 2] No such file or directory: 'libcapstone.5.dylib'
    ----------------------------------------
    ERROR: Failed building wheel for capstone

Options were adding Ninja to build host or switching cmake to unix makefiles, so using unix makefiles

@XVilka
Copy link
Contributor

XVilka commented Dec 27, 2023

@kabeor please trigger workflows.

@XVilka
Copy link
Contributor

XVilka commented Dec 28, 2023

Red CI looks like a network problem. It probably makes sense to restart.

@XVilka
Copy link
Contributor

XVilka commented Jan 1, 2024

@kabeor please restart failed jobs one more time - I think it will be green.

@kabeor
Copy link
Member

kabeor commented Jan 3, 2024

LGTM, but plz resolve the conflicts:)

@XVilka
Copy link
Contributor

XVilka commented Jan 7, 2024

@kabeor I wonder if you can solve conflicts yourself, this is major problem for many macOS users, would be nice to get this merged sooner than later.

@rickmark
Copy link
Contributor Author

rickmark commented Jan 9, 2024

Looks like PyPI is again failing for network reasons, merge of conflicts is complete

@XVilka
Copy link
Contributor

XVilka commented Jan 11, 2024

@kabeor please merge this one

@kabeor
Copy link
Member

kabeor commented Jan 12, 2024

Thank you!

@kabeor kabeor merged commit a554a1d into capstone-engine:next Jan 12, 2024
11 checks passed
@rickmark rickmark deleted the universal2-fix branch January 16, 2024 01:27
@rickmark
Copy link
Contributor Author

Any chance you're gonna release a pre / 5.X with this? I'd love to be able to use capstone on M1's soon

@Rot127
Copy link
Collaborator

Rot127 commented Jan 16, 2024

Actually a early pre-release was planned. @kabeor Is there something missing for it?

@kabeor
Copy link
Member

kabeor commented Jan 18, 2024

@Rot127 should be ready, will release after changing some version things

@rickmark
Copy link
Contributor Author

Will this be a 5.0.2 or a 6.0pre?

@Rot127
Copy link
Collaborator

Rot127 commented Jan 20, 2024

The pre-release will be 6.0pre. Because so much stuff changed, we want some kind of beta version.

@XVilka XVilka mentioned this pull request Jan 21, 2024
24 tasks
@roblabla
Copy link

A couple months have passed. Is a pre-release still planned?

@Rot127
Copy link
Collaborator

Rot127 commented Apr 16, 2024

Pre-release is still planned. Also the 5.0.2 release. Please be patient, I was recently added as maintainer to support, but it will take a little longer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build & packaging Build system and packaging related python bindings
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants