Bug 1470642

Summary: mariadb library upgrade to 10.2 causes FTBFS
Product: [Fedora] Fedora Reporter: Honza Horak <hhorak>
Component: net-snmpAssignee: Josef Ridky <jridky>
Status: CLOSED RAWHIDE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: rawhideCC: awilliam, jridky, jsafrane, ppisar, thozza
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: net-snmp-5.7.3-18.fc27 Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2017-07-19 19:36:03 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 1467297    
Attachments:
Description Flags
Proposed patch (untested)
none
Proposed patch (untested)
none
patch to check for existence of my_init and only use it if available
none
patch none

Description Honza Horak 2017-07-13 11:14:47 UTC
Created attachment 1297524 [details]
Proposed patch (untested)

Description of problem:
With new version of MariaDB 10.2 we see basically this build issue:

libtool: link: DIE_RPATH_DIE="/usr/lib64:" gcc -DNETSNMP_ENABLE_IPV6 -fno-strict-aliasing -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 -mtune=generic -D_RPM_4_4_COMPAT -Ulinux -Dlinux=linux -D_REENTRANT -D_GNU_SOURCE -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 -mtune=generic -fwrapv -fno-strict-aliasing -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -I/usr/lib64/perl5/CORE -o .libs/snmptrapd -pie .libs/snmptrapd.o -Wl,-z -Wl,relro -Wl,-z -Wl,now -Wl,--enable-new-dtags -Wl,-z -Wl,relro -specs=/usr/lib/rpm/redhat/redhat-hardened-ld  ./.libs/libnetsnmptrapd.so -L/usr/local/lib -L/usr/lib64/perl5/CORE /builddir/build/BUILD/net-snmp-5.7.3/agent/.libs/libnetsnmpmibs.so ../agent/.libs/libnetsnmpmibs.so /builddir/build/BUILD/net-snmp-5.7.3/agent/.libs/libnetsnmpagent.so -lsensors -lrpm -lrpmio ../agent/.libs/libnetsnmpagent.so /builddir/build/BUILD/net-snmp-5.7.3/snmplib/.libs/libnetsnmp.so -lwrap -lperl -lresolv -lnsl -lcrypt -lutil -lc ../snmplib/.libs/libnetsnmp.so -L/usr/lib64/mysql/ -lmariadb -lpthread -ldl -lssl -lcrypto -lm
./.libs/libnetsnmptrapd.so: undefined reference to `snmp'
./.libs/libnetsnmptrapd.so: undefined reference to `load_defaults'
./.libs/libnetsnmptrapd.so: undefined reference to `my_init'

The missing references to `load_defaults' and `my_init' are expected, these symbols are not exported by the mariadb client library any more, and I believe the attached patch should implement the same functionality.

However, the missing reference to `snmp' is still present, so I'm not able to build the package.

Nor I was able to test the patch, so take the patch as suggestion only, it's not tested at all.

Comment 1 Josef Ridky 2017-07-13 11:16:37 UTC
working on it

Comment 2 Honza Horak 2017-07-13 11:51:23 UTC
Upstream discussion about the missing symbols: https://jira.mariadb.org/browse/MDEV-13314

Comment 3 Honza Horak 2017-07-13 13:38:02 UTC
Created attachment 1297618 [details]
Proposed patch (untested)

A new variant of the patch, which removes the missing smtp() symbol (typo in the block which shouldn't be used, but actually was used because of missing MYSQL_VERSION_ID symbol.

The patch is still not tested, but at least fixes the FTBFS.

Comment 4 Josef Ridky 2017-07-13 15:27:33 UTC
Thanks for the patch. I will have to check it on f27 system.

Comment 5 Adam Williamson 2017-07-14 18:58:29 UTC
I'm not quite convinced that just using the existing HAVE_BROKEN_LIBMYSQLCLIENT definition is the right thing to do, here. All that does is check whether a function called `MY_INIT` works; the upstream code assumes that if it doesn't, `my_init` will. Your patch rather changes this to assume that if `MY_INIT` doesn't work, then there is *both* no need to explicitly initialize via `my_init` *either* (which doesn't seem like a safe assumption at least without research) and *also* that your MYSQL_READ_DEFAULT_GROUP change will be appropriate. Is it really the case that 'MY_INIT doesn't work' is the correct 'test' for both those changes?

I came up with an alternate patch for the my_init() issue, which I'll attach - it explicitly tests if `my_init()` is available, and skips using it if it isn't (following the MySQL documentation that states it is called by mysql_init() so the code doesn't need to call it explicitly). I wasn't able to come up with anything for the load_defaults() problem, though.

Comment 6 Adam Williamson 2017-07-14 18:59:24 UTC
Created attachment 1298586 [details]
patch to check for existence of my_init and only use it if available

Comment 7 Adam Williamson 2017-07-14 19:14:33 UTC
Oh, btw, upstream bug for the my_init and load_defaults problems is: https://sourceforge.net/p/net-snmp/bugs/2782/

Comment 8 Josef Ridky 2017-07-17 10:45:43 UTC
Hi Adam,

thank you for your interest an help with solving this issue.

(In reply to Adam Williamson from comment #5)
> I'm not quite convinced that just using the existing
> HAVE_BROKEN_LIBMYSQLCLIENT definition is the right thing to do, here. 

Agree, this have to be changed, so the check will be done against more relevant function.
 
> I came up with an alternate patch for the my_init() issue, which I'll attach
> - it explicitly tests if `my_init()` is available, and skips using it if it
> isn't 

The last version of MariaDB/MySQL library, which is available in fedora rawhide do not contain my_init() function any more, so I hope it isn't necessary to check it there, due older version of this library will not be officially provided there. I will setup discussion with MariaDB maintainer to solve this.

Comment 9 Adam Williamson 2017-07-17 16:03:09 UTC
josef: I generally try to write patches that are upstreamable; it's considered bad practice in Fedora to patch things downstream in ways that would not be appropriate to be sent upstream, because it can lead to unexpected differences in behaviour between upstream and downstream, and gives us the burden of maintaining the patch forevermore. It's considered better to come up with a change that upstream will accept and submit it upstream, only carrying it downstream until it's merged and released upstream.

Comment 10 Josef Ridky 2017-07-19 11:21:40 UTC
Created attachment 1300981 [details]
patch

After discussion with MariaDB/MySQL maintainer, this patch should retain functionality of snmptrapd_sql.c in f27 (with MariaDB 10.2) and is applicable even for f24-f26.

I will propose it to upstream.

Comment 11 Adam Williamson 2017-07-19 18:04:32 UTC
As explained at https://sourceforge.net/p/net-snmp/bugs/2782/#f326 , the config_os_libs2 portion of that patch seems unnecessary and incorrect to me. But the main part looks reasonable and I tried a build with just that part and it worked, so I'm gonna go ahead and commit that and do a build :)

Comment 12 Adam Williamson 2017-07-19 19:36:03 UTC
Build is now done: https://koji.fedoraproject.org/koji/taskinfo?taskID=20616265