Fix static linking of mysql library to root4star#343
Conversation
According to cons scripts, the entire `mysqlclient` library is requested to be linked statically to `root4star` using the following linker flags: ``` ... -Wl,--whole-archive -Wl,-Bstatic -Wl,-z -Wl,muldefs `mysql_config --libs` -Wl,--no-whole-archive -Wl,-Bdynamic ... ``` The linker is instructed to look for static versions of the libraries enclosed between the `-Wl,--whole-archive` and `-Wl,--no-whole-archive` options, however when such libs cannot be found it rightfully fails. Now, normally, `mysql_config --libs` is supposed to return something similar to this: ``` -L$pkglibdir -lmysqlclient -lpthread -lz -lm -lssl -lcrypto -ldl ``` But the `mysql_config` script on the farm was apparently modified to match the specific linker command in cons, so it returns additional flags: ```diff $ diff /opt/star/sl73_gcc485/bin/mysql_config /opt/star/sl73_gcc485/bin/mysql_config~ 110c110 < libs=" $ldflags -L$pkglibdir -lmysqlclient -L/usr/lib -Wl,--no-whole-archive -Wl,-Bdynamic -lpthread -lz -lm -lssl -lcrypto -ldl " --- > libs=" $ldflags -L$pkglibdir -lmysqlclient -lpthread -lz -lm -lssl -lcrypto -ldl " 112c112 < libs_r=" $ldflags -L$pkglibdir -lmysqlclient -L/usr/lib -Wl,--no-whole-archive -Wl,-Bdynamic -lpthread -lz -lm -lssl -lcrypto -ldl " --- > libs_r=" $ldflags -L$pkglibdir -lmysqlclient -lpthread -lz -lm -lssl -lcrypto -ldl " ``` ```shell $ ls -la /opt/star/sl73_gcc485/bin/mysql_config /opt/star/sl73_gcc485/bin/mysql_config~ -rwxr-xr-x 1 jeromel rhstar 6842 Oct 10 2017 /opt/star/sl73_gcc485/bin/mysql_config -rwxr-xr-x 1 jeromel rhstar 6746 Oct 10 2017 /opt/star/sl73_gcc485/bin/mysql_config~ ``` The point of this manual intervention was obviously to exclude the non-existing static libraries following the `-Wl,-Bdynamic` option. Since modifying generated package configurations is never a good idea, this fix will allow to rely on standard mysql installations without any further patching of cons scripts.
The default behavior of linking mysqlclient to root4star statically does not seem to work with the unpatched library. Perhaps the requirement to include all object files is not really necessary `-Wl,--whole-archive -Wl,-Bstatic` and can be relaxed. Nevertheless, we can give the users an option to link the library dynamically by setting the environment variable `STAR_MYSQL_LINKSO`
plexoos
changed the title
plexoos
changed the title
plexoos
requested review from
genevb,
klendathu2k,
nigmatkulov and
perevbnlgov
as code owners
|
Actually in root.exe it is working with shared library. but if you preferes to load statically, it is not a big deal |
|
Last time I did dive into this rabbit hole and root4star was broken with shared linking. |
|
Shared linking is how we were doing it in the containers from the beginning. Static linking did not work in the container for the reason described above. The existing tests work with mysqlclient.so and now after the fix. What's broken then? |
|
My tests were with modern binutils and gcc. I would need to revisit it to say exactly, but I think the shared libraries loaded as part of the chains would fail to find symbols linked into the executable. |
| // A dummy global definition to satisfy the linker when linking mysqlclient library statically with --whole-archive | ||
| // /opt/software/linux-scientific7-x86_64/gcc-4.8.5/mysql-5.7.27-pfyt3fwtkubcc5eazmoqfick3lgp67mf/lib/libmysqlclient.a(posix_timers.c.o): In function `my_timer_initialize': | ||
| // (.text+0x140): undefined reference to `key_thread_timer_notifier' | ||
| unsigned int key_thread_timer_notifier = 0; |
There was a problem hiding this comment.
Would be nice to find this symbol. Maybe try:
find /opt -name "*.a" -exec nm {} \; | grep key_thread_timer_notifier
There was a problem hiding this comment.
I think I saw it defined in libmysqld.a
There was a problem hiding this comment.
But you only see this issue after we strip the -lpthread -lz -lm -lssl -lcrypto -ldl?
There was a problem hiding this comment.
... or when we strip -L$pkglibdir?
What you say makes perfect sense, but is it really relevant to this specific fix dealing with libmysqlclient only? With this change we will bring the build process in the container even closer to how it is done in the SDCC environment. |
Yeah, seems like my issue is not relevant to this specific PR. I'm looking forward to having this reviewed and merged. Enabling shared linking, if it somehow works, would be even nicer. |
|
I fully support switching to shared linking as it would be much cleaner. It is also trivial since all it takes is just applying the patch we have hard coded in the Dockerfiles. My only worry is that someone may have a script not ready for the switch... |
An alternative solution to the issue described in PR #343
An alternative solution to the issue described in PR #343
|
Superseded by #346 |
3 participants
According to cons scripts, the entire
mysqlclientlibrary is requested to be linked statically toroot4starusing the following linker flags:The linker is instructed to look for static versions of the libraries enclosed between the
-Wl,--whole-archiveand-Wl,--no-whole-archiveoptions, however when such libs cannot be found it rightfully fails. Now, normally,mysql_config --libsis supposed to return something similar to this:But the
mysql_configscript on the SDCC machines was apparently modified to match the specific linker command in cons, so it returns additional flags:The point of that manual intervention was obviously to exclude the non-existing static libraries following the
-Wl,-Bdynamicoption. Since modifying generated package configurations is never a good idea, this fix will allow to rely on standard mysql installations without any further patching of cons scripts.