Bug 1470133 - mariadb library upgrade to 10.2 causes libnss-mysql FTBFS
Summary: mariadb library upgrade to 10.2 causes libnss-mysql FTBFS
Keywords:
Status: CLOSED WORKSFORME
Alias: None
Product: Fedora
Classification: Fedora
Component: libnss-mysql
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Jan ONDREJ
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 1467297
TreeView+ depends on / blocked
 
Reported: 2017-07-12 12:32 UTC by Augusto Caringi
Modified: 2021-04-11 07:07 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2017-07-21 08:16:40 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)
Patch to allow compilation of libnss-mysql with MariaDB 10.2 (889 bytes, patch)
2017-07-12 12:32 UTC, Augusto Caringi
no flags Details | Diff

Description Augusto Caringi 2017-07-12 12:32:35 UTC
Created attachment 1296944 [details]
Patch to allow compilation of libnss-mysql with MariaDB 10.2

Trying to build libnss-mysql with the new version of MariaDB 10.2 causes a build error:

mysql.c:241:14: error: 'MYSQL {aka struct st_mysql}' has no member named 'reconnect'
       ci.link.reconnect = 0; /* Safety: We can't let MySQL assume socket is
              ^

In this new version of the library the reconnect field was moved to another struct. I think that the best approach is to set it through API. I attached a patch proposal to fix the problem.

We are tracking all the possible problems regarding this MariaDB upgrade in the bug 1467297.

Copr build: https://copr.fedorainfracloud.org/coprs/g/db-sig/mariadb-10.2/package/libnss-mysql/

Comment 1 Jan ONDREJ 2017-07-15 15:43:27 UTC
Thank you for a patch. Planning to apply it and rebuild. Just currently I see, that mysql_options is deprecated. There is a replacement mysql_optionsv:

https://mariadb.com/kb/en/mariadb/mysql_optionsv/

According to documentation, it's possible to set value also as constant, not needed to define my_bool variable. I am not a good C programmer, so can you confirm, that I can do it in one line patch?

mysql_optionsv(&ci.link, MYSQL_OPT_RECONNECT, 1);

or may be:

mysql_optionsv(&ci.link, MYSQL_OPT_RECONNECT, (my_bool *)1);

Comment 2 Augusto Caringi 2017-07-17 10:36:35 UTC
The best approach is to submit the patch to upstream, to not have to keep applying a downstream patch forever.

So, the ideal patch must work with both MariaDB and MySQL, and as far as I know, the function mysql_options is not deprecated in MySQL. For this reason I think that's a good idea to talk to upstream to find out what's the best solution regarding this question.

About the another question (one line patch), I don't think so. The example in the link is:

my_bool reconnect= 1; /* enable reconnect */
mysql_optionsv(mysql, MYSQL_OPT_RECONNECT, (void *)&reconnect);

Because it's not possible to pass a literal to a function that expects a pointer.

Well, actually maybe it's possible using some C99 construct like is showed here:

https://stackoverflow.com/questions/1564701/pointer-to-literal-value

But I think that it's not worth using this.

Comment 3 Jan ONDREJ 2017-07-20 17:25:30 UTC
Upstream is more than 10 years without response.

Building with your original patch.

Thank you for help.

Comment 4 papasj 2021-04-10 13:16:43 UTC
Hi all,

I think this patch is wrong, the original source sets the reconnect flag to 0:

----
ci.link.reconnect = 0;
----

but the patch sets the flag to 1:

----
my_bool reconnect = 1;
----

Shouldn't reconnect be equal to 0?

Comment 5 Jan ONDREJ 2021-04-10 18:17:13 UTC
Do you have some special problems with this recoonect settings?

According to mariadb documentation:
https://mariadb.com/kb/en/mysql_optionsv/

my_bool reconnect= 1; /* enable reconnect */
mysql_optionsv(mysql, MYSQL_OPT_RECONNECT, (void *)&reconnect);

Looks like 1 is a good choice.
May be mysql and mariadb has different options, but currently we are linking agains mariadb, so 1 is good.

We should change to mysql_optionsv() in future.

Comment 6 papasj 2021-04-11 06:41:42 UTC
I'm sorry, I failed to explain it correctly, there is no problem with how the patch uses the reconnect flag and the mysql_options() or mysql_optionsv().

The problem is the value that is set, the patch sets the reconnect value to 1 while the upstream (libnss-mysql) sets the value to 0. 

The upstream in line 241 does this:
---
ci.link.reconnect = 0;
---

and the patch replaces the above and sets the reconnect to 1.

There is no syntax error with the patch, the problem is that it changes the value of the original source from 0 to 1.

Comment 7 Jan ONDREJ 2021-04-11 06:53:27 UTC
I still don't understand, why this is a problem.

If libnss-mysql is working well, also if this setting is suggested by mariadb developers, why it's a problem?

Possibly original sources doesn't support mariadb or newer mysql, what can cause this change.

If you let me know, why this setting is a problem, I can do more investigations, but if everything works fine, I am not interested.

Comment 8 papasj 2021-04-11 07:06:12 UTC
The original author sets the reconnect to 0 and comments it with this:

/* Safety: We can't let MySQL assume socket is still valid; see _nss_mysql_validate_socket */

It seems to me that the original author intentionally set the reconnect to 0 to prevent MySQL from reconnecting. It's not a compatibility issue, the reconnect flag was there and was used by the author to set it to 0.

Let me be clear on this, I haven't seen any problem, the source compiles and works with either reconnect value but that doesn't mean that the

Comment 9 papasj 2021-04-11 07:07:40 UTC
sorry, I hit the submit button earlier, I wanted to end the sentence by saying that even if it compiles it changes the author's intention to prevent the mysql from reconnecting.


Note You need to log in before you can comment on or make changes to this bug.