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

Port OCaml bindings to use Dune and Opam #2319

Open
wants to merge 7 commits into
base: next
Choose a base branch
from

Conversation

XVilka
Copy link
Contributor

@XVilka XVilka commented Apr 9, 2024

Your checklist for this pull request

  • I've documented or updated the documentation of every function and struct this PR changes. If not so I've explained why.
  • I've added tests that prove my fix is effective or that my feature works (if possible)

Detailed description

Test plan

  • CI is green
  • opam install .
  • dune test

Closing issues

Closes #985

@XVilka
Copy link
Contributor Author

XVilka commented Apr 9, 2024

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
Copy link
Collaborator

Rot127 commented Apr 10, 2024

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.

@Rot127 Rot127 added AArch64 Arch ocaml bindings build & packaging Build system and packaging related labels Apr 10, 2024
@Rot127 Rot127 added this to the v6 milestone Apr 10, 2024
@strub
Copy link

strub commented May 7, 2024

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
Copy link

strub commented May 7, 2024

The bindings build correctly with Capstone 5.0.1.

@XVilka
Copy link
Contributor Author

XVilka commented May 7, 2024

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.

@XVilka XVilka mentioned this pull request Jun 1, 2024
2 tasks
@aquynh
Copy link
Collaborator

aquynh commented Jun 13, 2024

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
Copy link
Contributor Author

XVilka commented Jun 13, 2024

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
Copy link
Collaborator

aquynh commented Jun 13, 2024 via email

@Rot127
Copy link
Collaborator

Rot127 commented Aug 16, 2024

@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 #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"

@Rot127 Rot127 modified the milestones: v6 - Alpha, v6 - Beta Aug 26, 2024
@github-actions github-actions bot removed the AArch64 Arch label Nov 10, 2024
@XVilka XVilka force-pushed the ocaml-dune branch 2 times, most recently from 4f87a5e to f790d23 Compare December 20, 2024 06:42
@XVilka
Copy link
Contributor Author

XVilka commented Dec 20, 2024

@Rot127 I did some updates in the generation script, and also added missing architectures. Now everything compiles just fine. The only missing part now is alcotest and fixes along the way.

@github-actions github-actions bot added the Github-files Github related files label Dec 20, 2024
@XVilka XVilka force-pushed the ocaml-dune branch 3 times, most recently from 36c1684 to 1497a91 Compare December 20, 2024 07:55
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 Github-files Github related files ocaml bindings
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

Add capstone to OPAM
4 participants