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

adds the dune build system #68

Merged
merged 3 commits into from
Aug 2, 2022
Merged

adds the dune build system #68

merged 3 commits into from
Aug 2, 2022

Conversation

ivg
Copy link
Contributor

@ivg ivg commented Jul 29, 2022

The existing build system based on OCamlMake files is left untouched. We build only essentials with dune, so right now tests, booting, and code generation is not implemented. We can port the rest, but I would suggest to do it in a separate PR.

Note, that we're currently moving BAP to dune and to the dune plugin system and piqi is a road blocker for us, since it doesn't provide the cmxs file that enables dynamic loading. I was thinking about fixing the OCamlMakefile... but decided that porting to dune is a much easier task) Also, moving piqi to dune will enable vendoring piqi into duniverse, which is our ultimate goal.

I will soon push a PR to ocaml-piqi and if you will accept both, I can work on make the opam-repository PR, if you want.

Fixes #66

The existing build system based on OCamlMake files left untouched. We
build only essentials with dune, so right now tests, booting, and code
generation is not implemented.
ivg and others added 2 commits July 29, 2022 19:54
Remove unnecessary piqi package. This also makes naming consistent with
opam naming as piqi package is provided by `piqi-ocaml`, which is hosted
in a separate repo (https://github.com/alavrik/piqi-ocaml).
@alavrik
Copy link
Owner

alavrik commented Aug 1, 2022

@ivg thank you for contributing this. This dune thing look pretty neat and seems to work. I'll be honest I am not up to date with the state of OCaml ecosystem.

I've made one small change to simplify and make this cleaner in terms of naming: 7632b07

Hope this is ok? Let me know what you think and I'll have this merged in.

On a related topic, do you need to turn this into an official opam release to use? Or you are happy with the dev version? I am not at all familiar with duniverse.

@ivg
Copy link
Contributor Author

ivg commented Aug 1, 2022

Yes, I was pretty confused with the package naming. Ideally, we should have a one-to-one correspondence between the packages defined in dune and the opam packages that we release via the opam-repository. At least it is how it was designed.

On a related topic, do you need to turn this into an official opam release to use?

Yes, eventually I will need this to be released to opam-repository so that people can do opam install bap and it will pull the piqi dependency that is built with dune (which includes the properly built cmxs files). But this will just require updating the opam files.

@alavrik alavrik merged commit eee553b into alavrik:master Aug 2, 2022
@alavrik
Copy link
Owner

alavrik commented Aug 2, 2022

Ok this is merged now. Would you be able to help with opem-repository? Let me know when you want me to cut a new release for it.

@alavrik
Copy link
Owner

alavrik commented Aug 2, 2022

@ivg by the way, that's the best CI for open-source ocaml projects these days? Travis-ci seems to be no longer working, at least by default.

@ivg
Copy link
Contributor Author

ivg commented Aug 2, 2022

Ok this is merged now. Would you be able to help with opem-repository? Let me know when you want me to cut a new release for it.

Sure, I will prepare the PR. I will cc-you when I it will be ready. I will need the release eventually, but let me check first that everything works fine with my own opam-repository.

@ivg by the way, that's the best CI for open-source ocaml projects these days? Travis-ci seems to be no longer working, at least by default.

Github action is what we're using in BAP and what is recommended. If you're interested I can make a PR that adds CI, at least building for starters. Don't recall if you need to enable something in the repository settings, but we will figure it out)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Port to Dune build system
2 participants