MySQL 8.0 onward changed a few details in the client API, and embedded support has apparently been removed completely. This PR makes minimally invasive changes that fix the related compile errors in a (hopefully) backward compatible manner.
Details
compile against MySQL <= 5, MySQL >= 8, and various versions of MariaDB (though it seems they didn't touch that part of the API yet)
Diff Detail
- Repository
- R181 Amarok
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
This breaks compilation here on openSUSE with MariaDB:
In file included from /home/abuild/rpmbuild/BUILD/amarok-2.9.70git.20200314T055024~d790424e56/src/core-impl/storage/sql/mysql-shared/MySqlStorage.cpp:31: /usr/include/mysql/mysql.h:39:14: error: conflicting declaration 'typedef char my_bool' 39 | typedef char my_bool; | ^~~~~~~ In file included from /home/abuild/rpmbuild/BUILD/amarok-2.9.70git.20200314T055024~d790424e56/src/core-impl/storage/sql/mysql-shared/MySqlStorage.cpp:20: /home/abuild/rpmbuild/BUILD/amarok-2.9.70git.20200314T055024~d790424e56/src/core-impl/storage/sql/mysql-shared/MySqlStorage.h:37:7: note: previous declaration as 'using my_bool = bool' 37 | using my_bool = bool; | ^~~~~~~ In file included from /home/abuild/rpmbuild/BUILD/amarok-2.9.70git.20200314T055024~d790424e56/src/core-impl/storage/sql/mysql-shared/MySqlStorage.cpp:31: /usr/include/mysql/mysql.h:362:3: error: conflicting declaration 'typedef struct st_mysql MYSQL' 362 | } MYSQL; | ^~~~~ In file included from /home/abuild/rpmbuild/BUILD/amarok-2.9.70git.20200314T055024~d790424e56/src/core-impl/storage/sql/mysql-shared/MySqlStorage.cpp:20: /home/abuild/rpmbuild/BUILD/amarok-2.9.70git.20200314T055024~d790424e56/src/core-impl/storage/sql/mysql-shared/MySqlStorage.h:36:8: note: previous declaration as 'struct MYSQL' 36 | struct MYSQL; | ^~~~~
The content of my /usr/include/mysql/mysql_version.h (which is just a symlink to mariadb_version.h):
/* Copyright Abandoned 1996, 1999, 2001 MySQL AB This file is public domain and comes with NO WARRANTY of any kind */ /* Version numbers for protocol & mysqld */ #ifndef _mariadb_version_h_ #define _mariadb_version_h_ #ifdef _CUSTOMCONFIG_ #include <custom_conf.h> #else #define PROTOCOL_VERSION 10 #define MARIADB_CLIENT_VERSION_STR "10.4.3" #define MARIADB_BASE_VERSION "mariadb-10.4" #define MARIADB_VERSION_ID 100403 #define MARIADB_PORT 3306 #define MARIADB_UNIX_ADDR "/run/mysql/mysql.sock" #define MYSQL_CONFIG_NAME "my" #define MYSQL_VERSION_ID 100403 #define MYSQL_SERVER_VERSION "10.4.3-MariaDB" #define MARIADB_PACKAGE_VERSION "3.1.7" #define MARIADB_PACKAGE_VERSION_ID 30107 #define MARIADB_SYSTEM_TYPE "Linux" #define MARIADB_MACHINE_TYPE "x86_64" #define MARIADB_PLUGINDIR "/usr//usr/lib64/mysql/plugin/" /* mysqld compile time options */ #ifndef MYSQL_CHARSET #define MYSQL_CHARSET "" #endif #endif /* Source information */ #define CC_SOURCE_REVISION "" #endif /* _mariadb_version_h_ */
Maybe you should also check if e.g. MARIADB_VERSION_ID is undefined or something.
storage: distinguish MariaDB and MySQL for API quirks
MariaDB doesn't seem to be updating the API the way MySQL does, so exclude it from the version hackery.
Great, thanks for testing! I think I updated the diff, but I'm confused by this whole Phabricator thingy. Not sure if I amended or replaced the patch now. This whole GUI makes absolutely no sense to me :-/ For me these are 3 separate commits, because they really deal with 3 separate issues (removed embedded MySQL, MySQL API change, and special treatment for MariaDB). Let me know if I should resubmit the whole thing as one patch.
I know, the question was whether that supersedes or amends the previous diff. But seems like it's the former. I'll update, sorry.
Thanks for the very nice patch! Compiles and works for me on Gentoo (MySQL Connector/C 8.0.19).
I just went back and installed mariadb-connector-c in Gentoo in mysqlcompat-mode (a soon to be removed option) and interestingly, it does not provide that symlink. Just something to take into account when using that header, I don't know how consistent other distros are doing this.
Locally though I have already been toying around with separate MySQL/MariaDB support so I could then probably amend that with another ifdef.
tests/core-impl/collections/CMakeLists.txt | ||
---|---|---|
3 | amarok cmake files seem to be using a tab size of 4 spaces fairly consistently, please keep it that way |
Another attempt at a simple solution that works with all versions of MariaDB and MySQL. This one comes with the added cost of including the full mysql.h because that seems to be the only consistently supplied drop-in header by MariaDB. Other solutions would involve additional CMake hackery to distinguish between MariaDB and MySQL, but for the sake of sanity, let's sacrifice compilation time for simplicity here.
Please test again, and thanks a lot for the review so far.
Successfully tested with:
mariadb-connector-c-3.0.6 (mysqlcompat mode)
mysql-connector-c-8.0.19
mariadb-connector-c-3.1.7 (with my local modifications)
Waiting for @wbauer to confirm it works as well - btw, does openSUSE still provide mysqle to test -DWITH_MYSQL_EMBEDDED=on?
Builds fine here with mariadb and WITH_MYSQL_EMBEDDED=TRUE.
I assume you don't have commit access and you'll need somebody to push this patch for you, right?
Yes, the latest version builds fine here (the previous did as well), and the embedded collection still works as well.
btw, does openSUSE still provide mysqle to test -DWITH_MYSQL_EMBEDDED=on?
Yes, as part of mariadb. (the "real" mysql is not included in the distribution anymore since a while)
I assume you don't have commit access and you'll need somebody to push this patch for you, right?
Yes, please go ahead if you think this has been tested well enough.
Indeed, the symlink is added in openSUSE's packaging. From mariadb-connector-c.spec:
# add a compatibility symlink ln -s mariadb_config %{buildroot}%{_bindir}/mysql_config ln -s mariadb_version.h %{buildroot}%{_includedir}/mysql/mysql_version.h