Uploaded image for project: 'MariaDB Server'
  1. MariaDB Server
  2. MDEV-13773

client packages need my_global.h and/or my_config.h

Details

    • Bug
    • Status: Closed (View Workflow)
    • Critical
    • Resolution: Fixed
    • 10.2.8
    • 10.2.9
    • Packaging
    • None

    Description

      after MDEV-13370 server includes got installed under /usr/include/mysql/server. But some client packages need them, e.g. mysql-python needs my_config.h.

      They shouldn't need them and mysql-python doesn't really use the included file, but some might use them and they need time to be fixed.

      Possible solution, install headers, like

      /usr/include/mysql/my_global.h

      #include "server/my_global.h"
      #warning Clients should not include this file, it's a server header. Report a bug to project maintainers!
      

      Attachments

        Issue Links

          Activity

            serg Sergei Golubchik created issue -
            serg Sergei Golubchik made changes -
            Field Original Value New Value
            serg Sergei Golubchik made changes -
            grknight Brian Evans added a comment -

            Suggest symlinking compatible components to their libmysqlclient equivalents

            The following exist in popular software (like Ruby's mysql2 connector)

            mysql_version.h -> mariadb_version.h
            mysql_com.h -> mariadb_com.h

            grknight Brian Evans added a comment - Suggest symlinking compatible components to their libmysqlclient equivalents The following exist in popular software (like Ruby's mysql2 connector) mysql_version.h -> mariadb_version.h mysql_com.h -> mariadb_com.h

            Failing client ebuilds in gentoo (incomplete list as of now):

            • dev-python/mysqlclient
            • sci-libs/gdal
            • dev-ruby/mysql2
            • net-analyzer/net-snmp
            • dev-db/mysql++
            serg Sergei Golubchik added a comment - Failing client ebuilds in gentoo (incomplete list as of now): dev-python/mysqlclient sci-libs/gdal dev-ruby/mysql2 net-analyzer/net-snmp dev-db/mysql++
            serg Sergei Golubchik made changes -
            Status Open [ 1 ] In Progress [ 3 ]

            Btw, mysqlclient-1.3.12 seems to be fixed

            serg Sergei Golubchik added a comment - Btw, mysqlclient-1.3.12 seems to be fixed
            serg Sergei Golubchik made changes -
            Status In Progress [ 3 ] Stalled [ 10000 ]
            kaamos Alexey Kopytov added a comment - - edited

            sysbench also includes mysql_version.h. And the only reason for that was compatibility with MariaDB 10.2. Starting from MariaDB 10.2 it is insufficient to include mysql.h to have MYSQL_VERSION_ID defined, that's why sysbench also includes mysql_version.h, which breaks MariaDB compatibility again in 10.2.8.

            As I understand, 10.2.9 will include a compatibility header throwing a compile-time warning. Which leaves me wondering what's the correct AND portable way to have MYSQL_VERSION_ID defined?

            kaamos Alexey Kopytov added a comment - - edited sysbench also includes mysql_version.h. And the only reason for that was compatibility with MariaDB 10.2. Starting from MariaDB 10.2 it is insufficient to include mysql.h to have MYSQL_VERSION_ID defined, that's why sysbench also includes mysql_version.h, which breaks MariaDB compatibility again in 10.2.8. As I understand, 10.2.9 will include a compatibility header throwing a compile-time warning. Which leaves me wondering what's the correct AND portable way to have MYSQL_VERSION_ID defined?
            serg Sergei Golubchik added a comment - - edited

            It's a warning saying "Don't include this file directly, include <mysql.h>"
            mysql.h is (as we've understood) the only documented C API header, and it's always sufficient, client programs don't need to include mysql_version.h, mysql_com.h, or anything else from /usr/include/mysql. This is the case with MariaDB Connector-C too.

            serg Sergei Golubchik added a comment - - edited It's a warning saying "Don't include this file directly, include <mysql.h>" mysql.h is (as we've understood) the only documented C API header, and it's always sufficient, client programs don't need to include mysql_version.h , mysql_com.h , or anything else from /usr/include/mysql . This is the case with MariaDB Connector-C too.
            kaamos Alexey Kopytov added a comment - - edited

            OK, after looking more into it, here's the code in sysbench that required including mysql_version.h:

            #if !defined(MARIADB_BASE_VERSION) && MYSQL_VERSION_ID >= 80001 && \
              MYSQL_VERSION_ID != 80002 /* see https://bugs.mysql.com/?id=87337 */
            typedef bool my_bool;
            #endif
            

            I.e. due to MySQL-specific issues, I must be able to tell between MariaDB and MySQL. So I thought using MARIADB_BASE_VERSION for that purpose was a good idea. And it was until MariaDB 10.2, which requires including mysql_version.h, including mysql.h is not enough. Is there a better way to do it?

            kaamos Alexey Kopytov added a comment - - edited OK, after looking more into it, here's the code in sysbench that required including mysql_version.h : #if !defined(MARIADB_BASE_VERSION) && MYSQL_VERSION_ID >= 80001 && \ MYSQL_VERSION_ID != 80002 /* see https://bugs.mysql.com/?id=87337 */ typedef bool my_bool; #endif I.e. due to MySQL-specific issues, I must be able to tell between MariaDB and MySQL. So I thought using MARIADB_BASE_VERSION for that purpose was a good idea. And it was until MariaDB 10.2, which requires including mysql_version.h , including mysql.h is not enough. Is there a better way to do it?
            serg Sergei Golubchik added a comment - - edited

            I see. First, your mysql_version.h include is redundant. In

            #include <mysql.h> 
            #include <mysql_version.h> 
            

            you can safely drop the second line, because mysql.h includes mysql_version.h anyway. That's what I meant by "it's always sufficient". In libmysqlclient, it includes mysql_version.h, in libmariadb mysql_version.h is renamed and mysql.h includes the renamed file.

            Now, the problem is that libmariadb headers don't define MARIADB_BASE_VERSION. I will fix that. And you'll be able to include only mysql.h — which will work in any MariaDB or MySQL version, and test for MARIADB_BASE_VERSION as before.

            I've tested that with these changes sysbench (latest from github) builds fine.

            serg Sergei Golubchik added a comment - - edited I see. First, your mysql_version.h include is redundant. In #include <mysql.h> #include <mysql_version.h> you can safely drop the second line, because mysql.h includes mysql_version.h anyway. That's what I meant by "it's always sufficient". In libmysqlclient, it includes mysql_version.h , in libmariadb mysql_version.h is renamed and mysql.h includes the renamed file. Now, the problem is that libmariadb headers don't define MARIADB_BASE_VERSION . I will fix that. And you'll be able to include only mysql.h — which will work in any MariaDB or MySQL version, and test for MARIADB_BASE_VERSION as before. I've tested that with these changes sysbench (latest from github) builds fine.

            Thanks! So I have the following choices:

            1. Leave everything as is. This will work for MariaDB < 10.2.8, not work for MariaDB = 10.2.8, will work with a warning for MariaDB > 10.2.8

            2. Remove the redundant #include <mysql_version.h> line. This will work for MariaDB 10.2.8, will break 10.2.x, where x < 8

            3. Do some configure-time pushups to test for server/mysql_version.h availability, and if available, include that one instead of just mysql_version.h. Which will apparently work everywhere.

            It looks like #3, as ugly as it is, provides better compatibility.

            kaamos Alexey Kopytov added a comment - Thanks! So I have the following choices: 1. Leave everything as is. This will work for MariaDB < 10.2.8, not work for MariaDB = 10.2.8, will work with a warning for MariaDB > 10.2.8 2. Remove the redundant #include <mysql_version.h> line. This will work for MariaDB 10.2.8, will break 10.2.x, where x < 8 3. Do some configure-time pushups to test for server/mysql_version.h availability, and if available, include that one instead of just mysql_version.h . Which will apparently work everywhere. It looks like #3, as ugly as it is, provides better compatibility.

            Oh, right. Then the easiest fix would be this:

            --- drv_mysql.c.old     2017-09-15 10:14:09.000000000 +0200
            +++ drv_mysql.c 2017-09-15 10:13:40.000000000 +0200
            @@ -35,7 +35,6 @@
             #include <stdio.h>
             
             #include <mysql.h>
            -#include <mysql_version.h>
             #include <mysqld_error.h>
             #include <errmsg.h>
             
            @@ -50,8 +49,8 @@
             
             #define SAFESTR(s) ((s != NULL) ? (s) : "(null)")
             
            -#if !defined(MARIADB_BASE_VERSION) && MYSQL_VERSION_ID >= 80001 && \
            -  MYSQL_VERSION_ID != 80002 /* see https://bugs.mysql.com/?id=87337 */
            +#if !defined(MARIADB_BASE_VERSION) && !defined(MARIADB_VERSION_ID) && \
            +  MYSQL_VERSION_ID >= 80001 && MYSQL_VERSION_ID != 80002 /* see https://bugs.mysql.com/?id=87337 */
             typedef bool my_bool;
             #endif
             

            As far as I understand it should work for all MariaDB and MySQL versions.

            serg Sergei Golubchik added a comment - Oh, right. Then the easiest fix would be this: --- drv_mysql.c.old 2017-09-15 10:14:09.000000000 +0200 +++ drv_mysql.c 2017-09-15 10:13:40.000000000 +0200 @@ -35,7 +35,6 @@ #include <stdio.h> #include <mysql.h> -#include <mysql_version.h> #include <mysqld_error.h> #include <errmsg.h> @@ -50,8 +49,8 @@ #define SAFESTR(s) ((s != NULL) ? (s) : "(null)") -#if !defined(MARIADB_BASE_VERSION) && MYSQL_VERSION_ID >= 80001 && \ - MYSQL_VERSION_ID != 80002 /* see https://bugs.mysql.com/?id=87337 */ +#if !defined(MARIADB_BASE_VERSION) && !defined(MARIADB_VERSION_ID) && \ + MYSQL_VERSION_ID >= 80001 && MYSQL_VERSION_ID != 80002 /* see https://bugs.mysql.com/?id=87337 */ typedef bool my_bool; #endif As far as I understand it should work for all MariaDB and MySQL versions.

            kaamos, could you, please, confirm that the patch above worked for you? If not — I'll try to come up with something that does.

            serg Sergei Golubchik added a comment - kaamos , could you, please, confirm that the patch above worked for you? If not — I'll try to come up with something that does.

            @serg, sorry, I missed the last 2 comments (no emails, not even in the spam folder), so I implemented #3 from my list.

            I tested your patch just now and it works with MySQL, MariaDB 10.1 and MariaDB 10.2.8. I'm going to replace my (ugly) fix with yours. Thanks a lot!

            kaamos Alexey Kopytov added a comment - @serg, sorry, I missed the last 2 comments (no emails, not even in the spam folder), so I implemented #3 from my list. I tested your patch just now and it works with MySQL, MariaDB 10.1 and MariaDB 10.2.8. I'm going to replace my (ugly) fix with yours. Thanks a lot!
            serg Sergei Golubchik made changes -
            Fix Version/s 10.2.9 [ 22611 ]
            Fix Version/s 10.2 [ 14601 ]
            Resolution Fixed [ 1 ]
            Status Stalled [ 10000 ] Closed [ 6 ]

            Thanks! Pushed.

            serg Sergei Golubchik added a comment - Thanks! Pushed.
            serg Sergei Golubchik made changes -
            Workflow MariaDB v3 [ 82513 ] MariaDB v4 [ 152798 ]

            People

              serg Sergei Golubchik
              serg Sergei Golubchik
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Git Integration

                  Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.