sgrif / mysqlclient-sys

Rust bindings for libmysqlclient
Apache License 2.0
37 stars 31 forks source link

Mention MariaDB in README? #47

Closed robertsilen closed 4 months ago

robertsilen commented 4 months ago

There appears to be a check for MariaDB in the code, however MariaDB is not mentioned in the README.md - would it make sense to add a mention of MariaDB? Maybe add MariaDB to CI tests too?

weiznich commented 4 months ago

I'm open to accept a PR for this as long as it comes with a sufficient set of CI tests to ensure that everything works as described in the README.

grooverdan commented 4 months ago

@weiznich , @robertsilen asked for a bit of help and I got this going: https://github.com/grooverdan/mysqlclient-sys/actions/runs/9381010934/job/25829294150

MySQL macos 13,14 default - this is a lack of bindings. While it looks like these tests where there before I changed them to actually MySQL rather than the brew install of MariaDB. Is generating those bindings easy or should I just skip the test? (note I don't have a macos to do it).

MariaDB macos 14, default, buildtime_bindgen - this is a bumped 11.2 version - this is failing to link to ssl. Do you know if this is usually determined by pkgconfig for something else?

MariaDB windows 2022 - its failing to link to mysqlclient.lib as the Windows MariaDB client library is mariadbclient.lib. Do you know how to do this properly in rust build.rs as my attempts have failed.

I think I need to drop MariaDB bundled tests as they aren't tested anything additional to what the MYSQL bundled where.

The last two tests where testing different client libraries against different MariaDB container versions. I could go from a MySQL client libraries of Ubuntu to a MariaDB server as a test. I think this has more value.

Suggestions welcome.

The current diff is: https://github.com/sgrif/mysqlclient-sys/compare/master...grooverdan:mysqlclient-sys:master?expand=1

weiznich commented 4 months ago

Thanks for looking into this. This looks already not bad.

MySQL macos 13,14 default - this is a lack of bindings. While it looks like these tests where there before I changed them to actually MySQL rather than the brew install of MariaDB. Is generating those bindings easy or should I just skip the test? (note I don't have a macos to do it).

I think we already have bindings for these version, we "just" need to fix the version detection here:

https://github.com/sgrif/mysqlclient-sys/blob/4026e260399d236b9adbbde9b82d16ab3edafdab/build.rs#L139-L155

It seems like the reported package version is literally 21 or 23 in these cases and we expect something that starts with 21. or 23. there. Might be as simple as adding a version == "21" (or 23) there to match the right version. It still would be helpful to check that the generated bindings match what's in the bindings directory. You can do that by locally executing the bindgen command from here on a macos system with that version installed: https://github.com/sgrif/mysqlclient-sys/blob/master/DEVELOPMENT.md

MariaDB macos 14, default, buildtime_bindgen - this is a bumped 11.2 version - this is failing to link to ssl. Do you know if this is usually determined by pkgconfig for something else?

I think I had a similar failure somewhere in the CI before and it went away by setting up pkgconfig correctly. Not sure what's wrong there. It should pickup the correct dependencies.

MariaDB windows 2022 - its failing to link to mysqlclient.lib as the Windows MariaDB client library is mariadbclient.lib. Do you know how to do this properly in rust build.rs as my attempts have failed.

That's coming from here: https://github.com/sgrif/mysqlclient-sys/blob/4026e260399d236b9adbbde9b82d16ab3edafdab/build.rs#L49-L50

I think there is no real good solution here, other than introducing a MARIADB_LIB_DIR environment branch that just sets the right lib name.

I think I need to drop MariaDB bundled tests as they aren't tested anything additional to what the MYSQL bundled where.

That's fine

The last two tests where testing different client libraries against different MariaDB container versions. I could go from a MySQL client libraries of Ubuntu to a MariaDB server as a test. I think this has more value.

If possible I would rather not use docker containers there, just use whatever dbms (mysql/mariadb) is easiest to install for each OS. Should be fine to just test against that. After all we are more interested in the client libraries here.

The current diff is: https://github.com/sgrif/mysqlclient-sys/compare/master...grooverdan:mysqlclient-sys:master?expand=1

I think I would prefer if the actual test jobs were the same for all variants, as that makes it easier to update the these checks.

weiznich commented 4 months ago

Fixed by #49