capstone-engine / capstone

Capstone disassembly/disassembler framework for ARM, ARM64 (ARMv8), Alpha, BPF, Ethereum VM, HPPA, LoongArch, M68K, M680X, Mips, MOS65XX, PPC, RISC-V(rv32G/rv64G), SH, Sparc, SystemZ, TMS320C64X, TriCore, Webassembly, XCore and X86.
http://www.capstone-engine.org
7.51k stars 1.54k forks source link

Port OCaml bindings to use Dune and Opam #2319

Open XVilka opened 5 months ago

XVilka commented 5 months ago

Your checklist for this pull request

Detailed description

Test plan

Closing issues

Closes https://github.com/capstone-engine/capstone/issues/985

XVilka commented 5 months ago

There is still missing proper testing, I think I will just write a proper tests, that will compare the output with the expected, and will add the testing step to the CI.

Rot127 commented 5 months ago

There is still missing proper testing, I think I will just write a proper tests, that will compare the output with the expected, and will add the testing step to the CI.

Yes, for now this is enough. But at minimum the build should added in the CI.

strub commented 4 months ago

Hi. Against which version of capstone this binding should be compiled? The OCaml stub does not seem in sync with the Capstone API as found in the branch the OCaml stub belongs to.

strub commented 4 months ago

The bindings build correctly with Capstone 5.0.1.

XVilka commented 4 months ago

Hi, it's still in progress. I am targeting 6.0. I still need to implement missing architectures support that were added since the last time OCaml bindings were touched. I also plan to add proper testing with alcotest. Moreover, the auto-sync scripts should be extended to auto-regenerate constants so that they are in sync with the C version. Once everything is done, I also plan to package it for opam and send PR to the opam-repository.

aquynh commented 3 months ago

if this is working for 5.0.1, please make PR for branch v5, then we can merge it there.

thanks for taking care of this binding!

XVilka commented 3 months ago

if this is working for 5.0.1, please make PR for branch v5, then we can merge it there.

thanks for taking care of this binding!

I don't really want to spend time on 5.0 for these, since I am working on changing the Python code generators to regenerate constants for newly updated architectures - currently there is a mismatch between those.

aquynh commented 3 months ago

Ok, I thought you wanted that since someone mentioned v5.

On Thu, Jun 13, 2024, 09:52 Anton Kochkov @.***> wrote:

if this is working for 5.0.1, please make PR for branch v5, then we can merge it there.

thanks for taking care of this binding!

I don't really want to spend time on 5.0 for these, since I am working on changing the Python code generators to regenerate constants for newly updated architectures - currently there is a mismatch between those.

— Reply to this email directly, view it on GitHub https://github.com/capstone-engine/capstone/pull/2319#issuecomment-2164203817, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABNQNYGQQ57RZNNVMHYB2XDZHD3MPAVCNFSM6AAAAABF6A5U42VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRUGIYDGOBRG4 . You are receiving this because you commented.Message ID: @.***>

Rot127 commented 1 month ago

@XVilka When you continue to work on this one, please note the minimal integration_tests every binding must pass. See the suite/cstest/test/integration_tests.py in https://github.com/capstone-engine/capstone/pull/2384.

You just have to make sure the logging messages are the same as the tests expect and they are passed to stdout and stderr.

The tests should be run like the for Python:

cd suite/cstest/test/
python3 ./integration_tests.py "python3 ../../../bindings/python/py_cstest/cstest.py"