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

Move mysql-ext to using mariadb connector #2086

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

idk1703
Copy link

@idk1703 idk1703 commented Nov 30, 2023

Currently mysql 5.7 end of life in 31 Oct 2023.
And new archives with mysqlclient 8.x so huge (i known on AM waterfall this not problem but i think better use connector)
connector - v3.3.8
openssl - v1.1.1
openssl configure options - no-shared no-srp no-ct no-tests no-unit-test no-comp no-pic no-asm no-dynamic-engine no-engine no-static-engine no-dso

@dvander
Copy link
Member

dvander commented Dec 1, 2023

Thanks for doing this. I'm especially delighted that you dropped the old prebuilt system. It's unfortunate you had to roll out AMBuild scripts for 3rd-party code, but "recursively calling out to make" is a can of worms and custom prebuilts are horrible.

I have no particular opinion on MySQL vs MariaDB as I'm way behind on the technical difference between them, but I'm guessing it's a drop-in replacement at the protocol/API level? If so, this seems fine to take for 1.12, though I think one other person should sign off.

@idk1703
Copy link
Author

idk1703 commented Dec 1, 2023

I think it doesn't have any major changes (some features for MariaDB).
Why I choose MariaDB:
MySQL Connector/C - Not supported, last updated: July 13, 2017
MySQL says use the latest MySQL server
Add MySQL-Server submodule and find all required files in CMakeLists.txt for libmysqlclient.a, bad way.
Yes, MySQL has Connector/C++, but some changes to ext are required to use it.
Also this already tested on MariaDB 11.1.2 and MySQL 8.2 but only on x86 (Linux, Windows) (not have any x64 srcds for test)

@peace-maker
Copy link
Member

Last time we updated the libmysqlclient version in #786, we chose the last version which didn't bump the libc requirements. Compiling this ourselves leaves control on our side I guess? Do you have an opinion @psychonic? Those AMBuilder scripts for submodules seem annoying to maintain and we can't really verify we did it right and included all the steps the native build system would do, can we?

@psychonic
Copy link
Member

Those AMBuilder scripts for submodules seem annoying to maintain and we can't really verify we did it right and included all the steps the native build system would do

I have the same concern, except I would have worded it as, "seem to be a nightmare to maintain". However, maybe that doesn't matter? At least for now, it looks like most of the tedious parts are done. It's always possible that things could dramatically change in the future, but we not forced to upgrade either. I'm not terribly concerned about missing things in the build steps, because it just needs to function. I think it would be unlikely for a resulting bug with meaningful impact to go unnoticed for too long.

We also realistically have the option to switch back to a precompiled binary (built by ourselves or upstream) at any time, at least as we don't go back down on version after raising it, in case people are relying on that functionality.

I don't like any of our options, including staying on an unsupported version.

If we go with this approach, I would like to see the --no-mysql flag added back to configure.py/AMBuildScript. As that extension's build is now even more different/complex than the others than it was before, being able to easily skip it on a whim is really handy for local development.

@peace-maker
Copy link
Member

Yes, I'm open to giving this a try too. Does this link openssl and the connector statically into the extension shared object or would we end up with multiple files in the extensions folder for the mysql extension btw?

Looking at #1856 this would differ from the pattern of forking the library, here mariadb-connector-c and openssl, into the alliedmodders organisation and maintaining the build changes there. I don't remember the details of that other PR and if it needs to change files in the submodule for the build to work? We should stick to one pattern for both though.

@idk1703
Copy link
Author

idk1703 commented Dec 13, 2023

Looking at #1856 this would differ from the pattern of forking the library, here mariadb-connector-c and openssl, into the alliedmodders organisation and maintaining the build changes there. I don't remember the details of that other PR and if it needs to change files in the submodule for the build to work? We should stick to one pattern for both though.

In the last commits I brought previous PR to the same pattern, changes in submodules not needs, all custom files (generated in configure or on build) contains in specific folder

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.

4 participants