-
Notifications
You must be signed in to change notification settings - Fork 497
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
dependencies: remove capstone #1670
base: develop
Are you sure you want to change the base?
Conversation
the implementation already supports that capstone may not be installed. capstone is somewhat slow to install on some platforms (e.g. raspberry pi), and not always needed.
f09da41
to
9844089
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello, Thank you for the contribution.
As I understand this, this is a matter of weighing the benefit for people using this step command and relying on it printing the asm versus people installing pyocd from pip on "slow" systems.
I don't personally have an opinion on that matter and don't feel legitimate to make the call on this.
Not indicating if this should be removed or kept in Of course, it's understandable not wanting to do the extra work without first knowing if a PR is going to be accepted, I just wanted to point this out to have it in consideration as well. |
@carlosperate Thank you! I am not familiar enough with this aspect of python to do this myself but I’m happy to review/approve/merge such PR (provided there’s some explanations for me to understand what I’m approving). |
Yeah, I'm happy to help! The main idea is to create a new group for Ah, and we should update this error message as well: pyOCD/pyocd/commands/commands.py Lines 1732 to 1734 in 90ee63a
The way it would then work is that by default As the Line 53 in 90ee63a
Alternatively we could increase the max @jsiverskog once we know the next steps would you like to update this PR yourself? I'd like to make sure your contribution is recognised and credited, but I also understand it's been a year and maybe you've moved onto other things. |
I am FreeBSD user and Capstone always caused troubles here as system package is different from required by pyOCD, when used in virtual environment, had to built from sources, had to patch the upstream, etc etc :D Also on rPI building this package every time in a new environment adds to already time consuming bootstrap. I know this is really useful feature but not always required. Having this as runtime not build time option would be of a great benefit :-) |
the implementation already supports that capstone may not be installed. capstone is somewhat slow to install on some platforms (e.g. raspberry pi), and not always needed.