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

Switch build system to dune and dune-configurator. #52

Merged
merged 3 commits into from
Oct 15, 2024

Conversation

paurkedal
Copy link
Collaborator

I suggest we switch the build system to Dune to easy future contributions to the project. This PR is my proposal of how to do it as conservatively as possible, but I had to rename the mariadb_bindings library to mariadb.bindings.

There are some warnings which can easily be resolved with a subsequent commit, after which we can remove the dune-workspace file. There are also deprecations from async, which I think we can resolve now since they are from 2021.

@paurkedal
Copy link
Collaborator Author

I can see I've duplicated efforts from #51 with this PR (though with the differences that it uses dune-configurator to replace the logic of the previous configure script and that it uses the ctypes dune stanza).

This was referenced Aug 9, 2024
@paurkedal
Copy link
Collaborator Author

I can see I promised in a comment in the dune-workspace file to fix some warnings enabled by default by dune. I'm on it.

@paurkedal
Copy link
Collaborator Author

Done, except I left some warnings which may need to be addressed. Esp. I wonder whether this could explain the remaining leek reported in #29:

File "lib/nonblocking.ml", line 621, characters 8-12:
621 |     let free res =
              ^^^^
Warning 32 [unused-value-declaration]: unused value free.

@paurkedal paurkedal requested a review from ygrek as a code owner August 16, 2024 12:10
@paurkedal
Copy link
Collaborator Author

I missed some updates to the opam file (and .gitignore). Pinning the package to this branch works now. (It might be worth squashing the 3 last commits, or maybe all, before merging.)

This conversion tries to replicate the current distribution, but the
mariadb_bindings library was renamed to mariadb.bindings to comply with
the Dune policy.

Warnings were not resolved in this commit; the dune-workspace file
should be removed after addressing those.
Two kinds of warnings are left:

  - An unused free which may be related to ocaml-community#29.
  - Deprecations from Async.
@paurkedal
Copy link
Collaborator Author

Force pushed due to rebase to master and squashing two fixup commits. I'll use this as the base for CI.

@paurkedal
Copy link
Collaborator Author

@ygrek Do you want to have a look at this before I merge? I think I have reasonable OS coverage in my testing now:

  • The libmariadb and MariaDB C connectors have been tested in Add GitHub Actions workflow for testing #56, which was based on the current commit of this PR.
  • I did a manual test in an Ubuntu 18.04 container to test the libmariadbclient case, since I couldn't make it work under GitLab Actions.

@paurkedal paurkedal self-assigned this Sep 24, 2024
@paurkedal paurkedal merged commit c02767f into ocaml-community:master Oct 15, 2024
paurkedal added a commit to paurkedal/opam-repository that referenced this pull request Nov 28, 2024
CHANGES:

  - Added `Stmt.start_txn` (ocaml-community/ocaml-mariadb#59 by Corentin Leruth).
  - Added `Res.insert_id` as binding for `mysql_stmt_insert_id` (ocaml-community/ocaml-mariadb#58 by
    Corentin Leruth).
  - Updated to support recent OCaml versions (ocaml-community/ocaml-mariadb#45 by @kit-ty-kate).
  - Fixed too-early retrieval of statement metadata (ocaml-community/ocaml-mariadb#41 by Albert Peschar).
  - Fixed decoding bug for the integer type (ocaml-community/ocaml-mariadb#54 by Raman Varabets, tested
    by ocaml-community/ocaml-mariadb#61 by Corentin Leruth).
  - Fixed a memory leaks related to result metadata (ocaml-community/ocaml-mariadb#39 by Albert Peschar).
  - The build system is now dune and dune-configurator (ocaml-community/ocaml-mariadb#52 by Petter A.
    Urkedal) and some of the examples have been converted to a test suite
    (ocaml-community/ocaml-mariadb#60 by Petter A. Urkedal).
  - The project has been transferred to ocaml-community with Petter A.
    Urkedal as the new maintainer.
paurkedal added a commit to paurkedal/opam-repository that referenced this pull request Nov 28, 2024
CHANGES:

  - Added `Stmt.start_txn` (ocaml-community/ocaml-mariadb#59 by Corentin Leruth).
  - Added `Res.insert_id` as binding for `mysql_stmt_insert_id` (ocaml-community/ocaml-mariadb#58 by
    Corentin Leruth).
  - Updated to support recent OCaml versions (ocaml-community/ocaml-mariadb#45 by @kit-ty-kate).
  - Fixed too-early retrieval of statement metadata (ocaml-community/ocaml-mariadb#41 by Albert Peschar).
  - Fixed decoding bug for the integer type (ocaml-community/ocaml-mariadb#54 by Raman Varabets, tested
    by ocaml-community/ocaml-mariadb#61 by Corentin Leruth).
  - Fixed a memory leaks related to result metadata (ocaml-community/ocaml-mariadb#39 by Albert Peschar).
  - The build system is now dune and dune-configurator (ocaml-community/ocaml-mariadb#52 by Petter A.
    Urkedal) and some of the examples have been converted to a test suite
    (ocaml-community/ocaml-mariadb#60 by Petter A. Urkedal).
  - The project has been transferred to ocaml-community with Petter A.
    Urkedal as the new maintainer.
paurkedal added a commit to paurkedal/opam-repository that referenced this pull request Nov 29, 2024
CHANGES:

  - Added `Stmt.start_txn` (ocaml-community/ocaml-mariadb#59 by Corentin Leruth).
  - Added `Res.insert_id` as binding for `mysql_stmt_insert_id` (ocaml-community/ocaml-mariadb#58 by
    Corentin Leruth).
  - Updated to support recent OCaml versions (ocaml-community/ocaml-mariadb#45 by @kit-ty-kate).
  - Fixed too-early retrieval of statement metadata (ocaml-community/ocaml-mariadb#41 by Albert Peschar).
  - Fixed decoding bug for the integer type (ocaml-community/ocaml-mariadb#54 by Raman Varabets, tested
    by ocaml-community/ocaml-mariadb#61 by Corentin Leruth).
  - Fixed a memory leaks related to result metadata (ocaml-community/ocaml-mariadb#39 by Albert Peschar).
  - The build system is now dune and dune-configurator (ocaml-community/ocaml-mariadb#52 by Petter A.
    Urkedal) and some of the examples have been converted to a test suite
    (ocaml-community/ocaml-mariadb#60 by Petter A. Urkedal).
  - The project has been transferred to ocaml-community with Petter A.
    Urkedal as the new maintainer.
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.

1 participant