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.
working on it
Upstream discussion about the missing symbols: https://jira.mariadb.org/browse/MDEV-13314
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.
Thanks for the patch. I will have to check it on f27 system.
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.
Created attachment 1298586 [details]
patch to check for existence of my_init and only use it if available
Oh, btw, upstream bug for the my_init and load_defaults problems is: https://sourceforge.net/p/net-snmp/bugs/2782/
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
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.
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.
Created attachment 1300981 [details]
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.
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 :)
Build is now done: https://koji.fedoraproject.org/koji/taskinfo?taskID=20616265