← Back to team overview

maria-developers team mailing list archive

Re: Latest libmysqld.so patch

 

Hi, Kristian!

On Sep 30, Kristian Nielsen wrote:
> Sergei Golubchik <serg@xxxxxxxxxxxx> writes:
> 
> Thanks for review!
> 
> What I would now like to do is to discuss how to proceeed (where to
> push this basically).
> 
> On the one hand, the patch got rather more intrusive than I first
> wanted. The main issue is that now the mysys/mystrings/dbug code is
> linked dynamically by default, so these additional libmysys.so etc.
> libraries must be installed by packagers etc. for things to work;
> similarly extra -lmysys etc. is needed when using libmysqld.a.

I've fixed that - now libmysys/mystrings/dbug/vio are built statically
again. And they're included in libmysqld. See two patches attached.

But they're still built with -fPIC, otherwise libmysqld.so cannot take
them. Unless --disable-shared was used, that is.

I had to hack around automake - as it didn't want to install static
convenience libraries. I hope it'll go away eventually - I don't see why
we should install them at all.
 
> On the other hand, I think this is generally an improvement, linking
> code dynamically rather than statically. Saves duplicating code and so
> on, and dynamic linking is where the world is moving (has moved,
> really).

I agree. But perhaps we'd better postpone this (making them dynamic) to
5.3. Besides, let's see what cmake based builds bring in 5.5.
 
> And btw, I would like your help/hints on how to extend mysql_config to
> provide the right information for how to link.

Looks fine the way it is. We may want to document that for libtool
enabled builds it's better to link with

  /usr/lib/mysql/libmysqld.la

instead of

  `mysql_config --libmysqld-libs`

because libtool knows all the dependencies and required linker options.

> >> The plugin may optionally specify @plugin_static_if_no_embedded@ in
> >> CFLAGS/CXXFLAGS for the static target; this will avoid needlessly
> >> compiling with -fPIC if no libmysqld is being built.
> >
> With this patch, every source file for a plugin is compiled twice,
> once with -fPIC and once without.
> 
> However, if we are not building libmysqld, then the -fPIC versions
> will never be used.
> 
> In this case, @plugin_static_if_no_embedded@ will add -static to
> CFLAGS, avoiding the needless compilations with -fPIC. Otherwise it
> will be empty.

I'm not a big fan of providing @plugin_static_if_no_embedded@,
@plugin_embedded_defs@ etc - and allowing plugins to use them.

I'd rather prefer us to append the flags automatically, where needed.

But let's not spend too much time on the perfect solution as we're
moving to cmake based build system anyway.
 
> > I'd rather not build libmysql at all.  Or, at least, not by default.
> 
> Right, let's add this on top of this patch once we find out where we
> want to push it?

ok
 
> If we link libmysql_r with libmysys, it will also solve another issue
> I have (with conflicts between C and assembler versions of bmovXXX); I
> postponed this issue until we decided about linking libmysql_r with
> mysys etc (it seems you agree we should do this).

ok
 
> >> -AC_DEFUN([MYSQL_PLUGIN_DEPENDS_ON_MYSQL_INTERNALS],[
> >> - MYSQL_REQUIRE_PLUGIN([$1])
> >> - m4_define([MYSQL_PLUGIN_DEPENDS_ON_MYSQL_INTERNALS_]AS_TR_CPP([$1]), [$2])
> >> -])
> >
> > I thought we agreed on IRC to keep it.
> > Do you mean, you tried it and it didn't work?
> 
> Yes.
> 
> I tried quite hard to find a way to keep compatibility. But failed :-(
> 
> So in the end I gave up on this compatibility. Sad, however a small comfort is
> that
> 
>  - This only hits plugins that say MYSQL_PLUGIN_DEPENDS_ON_MYSQL_INTERNALS(),
>    which ideally they shouldn't anyway.
> 
>  - This only hits static plugins, which in any case need an incompatible
>    maria_declare_plugin() added in the source (though this can be handled with
>    #ifdef; I wonder if there is m4/automake equivalent ifdef that could help
>    here?)

okay. because

1. I don't want us to spend much time on this, as cmake based build
   system is coming (yup, I've said it for the third time :)

2. I only care that pbxt is affected, but pbxt is in our tree so we can
   apply our changes - not in the Paul's tree - and merge his changed
   periodically while keeping ours (bzr can handle that).
   I would prefer us to use vanilla pbxt, but as these are only changes
   in Makefile.am and plug.in, and taking 1 into account (didn't want to
   repeat it again :), I'm think it's fine to keep modified Makefile.am
   and plug.in in storage/pbxt in our tree.
   Assuming Paul doesn't mind.

Regards,
Sergei
# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: sergii@xxxxxxxxx-20101005200032-13ojibjzz352v273
# target_branch: bzr+ssh://bazaar.launchpad.net/~knielsen/maria\
#   /mariadb-5.1-mwl74-libmysqld.so/
# testament_sha1: 6bb79093d3b4c6bc7ffde024797a1a456712aafb
# timestamp: 2010-10-05 22:11:16 +0200
# base_revision_id: knielsen@xxxxxxxxxxxxxxx-20100930115302-\
#   16jdmwyy7fbgsknl
# 
# Begin patch
=== modified file 'configure.in'
--- configure.in	2010-09-16 12:49:35 +0000
+++ configure.in	2010-10-05 20:00:32 +0000
@@ -250,7 +250,7 @@
 
 # Ensure that we have --preserve-dup-deps defines, otherwise we get link
 # problems of 'mysql' with CXX=g++
-LIBTOOL="$LIBTOOL --preserve-dup-deps"
+#LIBTOOL="$LIBTOOL --preserve-dup-deps"
 AC_SUBST(LIBTOOL)dnl
 
 AC_SUBST(NM)dnl

=== modified file 'dbug/Makefile.am'
--- dbug/Makefile.am	2010-09-30 11:53:02 +0000
+++ dbug/Makefile.am	2010-10-05 19:27:24 +0000
@@ -17,7 +17,8 @@
 
 INCLUDES =              -I$(top_builddir)/include -I$(top_srcdir)/include
 LDADD =                 libdbug.la $(top_builddir)/mysys/libmysys.la $(top_builddir)/strings/libmystrings.la
-pkglib_LTLIBRARIES =	libdbug.la
+pkglib_LIBRARIES =	libdbug.a
+noinst_LTLIBRARIES =	libdbug.la
 noinst_HEADERS =        dbug_long.h
 libdbug_la_SOURCES =	dbug.c sanity.c
 EXTRA_DIST =            CMakeLists.txt example1.c example2.c example3.c \
@@ -65,3 +66,7 @@
 # a hack to have executable in builddir, not in srcdir
 tests-t:	tests-t.pl
 		cp -f $(srcdir)/tests-t.pl ./tests-t
+
+libdbug.a: libdbug.la .libs/libdbug.a
+	$(CP) .libs/libdbug.a $@
+

=== modified file 'extra/Makefile.am'
--- extra/Makefile.am	2010-09-30 11:53:02 +0000
+++ extra/Makefile.am	2010-10-05 19:27:24 +0000
@@ -17,8 +17,8 @@
 			-I$(top_srcdir)/sql
 LDADD =			@CLIENT_EXTRA_LDFLAGS@ \
 			$(top_builddir)/mysys/libmysys.la \
+			$(top_builddir)/strings/libmystrings.la \
 			$(top_builddir)/dbug/libdbug.la \
-			$(top_builddir)/strings/libmystrings.la \
 			$(ZLIB_LIBS)
 BUILT_SOURCES=		$(top_builddir)/include/mysqld_error.h \
                         $(top_builddir)/include/sql_state.h \

=== modified file 'libmysqld/examples/Makefile.am'
--- libmysqld/examples/Makefile.am	2010-09-30 11:53:02 +0000
+++ libmysqld/examples/Makefile.am	2010-10-05 19:27:24 +0000
@@ -39,10 +39,7 @@
 LIBS =		@LIBS@ @WRAPLIBS@ @CLIENT_LIBS@ $(yassl_libs)
 LDADD =		@CLIENT_EXTRA_LDFLAGS@ \
 		$(top_builddir)/libmysqld/libmysqld.la @LIBDL@ $(CXXLDFLAGS) \
-                @ndbcluster_libs@ @NDB_SCI_LIBS@ \
-		$(top_builddir)/mysys/libmysys.la \
-		$(top_builddir)/strings/libmystrings.la \
-		$(top_builddir)/dbug/libdbug.la
+                @ndbcluster_libs@ @NDB_SCI_LIBS@
 
 mysqltest_embedded_LINK = $(CXXLINK)
 nodist_mysqltest_embedded_SOURCES =	mysqltest.cc

=== modified file 'mysys/Makefile.am'
--- mysys/Makefile.am	2010-09-28 13:07:42 +0000
+++ mysys/Makefile.am	2010-10-05 19:27:24 +0000
@@ -18,7 +18,8 @@
 MYSQLBASEdir=		$(prefix)
 INCLUDES =		@ZLIB_INCLUDES@ -I$(top_builddir)/include \
 			-I$(top_srcdir)/include -I$(srcdir)
-pkglib_LTLIBRARIES =	libmysys.la
+pkglib_LIBRARIES =	libmysys.a
+noinst_LTLIBRARIES =	libmysys.la
 LDADD =			libmysys.la $(top_builddir)/strings/libmystrings.la $(top_builddir)/dbug/libdbug.la
 noinst_HEADERS =	mysys_priv.h my_static.h my_handler_errors.h \
 			my_safehash.h
@@ -58,7 +59,7 @@
 			my_windac.c my_access.c base64.c my_libwrap.c \
 		        wqueue.c
 libmysys_la_LDFLAGS =	$(AM_LDFLAGS) @WRAPLIBS@
-libmysys_la_LIBADD =	$(top_builddir)/strings/libmystrings.la $(ZLIB_LIBS)
+libmysys_la_LIBADD =	$(ZLIB_LIBS)
 
 if NEED_THREAD
 # mf_keycache is used only in the server, so it is safe to leave the file
@@ -96,6 +97,9 @@
 			testhash$(EXEEXT) test_gethwaddr$(EXEEXT) \
 			test_base64$(EXEEXT) test_thr_mutex$(EXEEXT)
 
+libmysys.a: libmysys.la .libs/libmysys.a
+	$(CP) .libs/libmysys.a $@
+
 #
 # The CP .. RM stuff is to avoid problems with some compilers (like alpha ccc)
 # which automaticly removes the object files you use to compile a final program

=== modified file 'strings/Makefile.am'
--- strings/Makefile.am	2010-09-16 12:49:35 +0000
+++ strings/Makefile.am	2010-10-05 19:27:24 +0000
@@ -16,7 +16,9 @@
 # This file is public domain and comes with NO WARRANTY of any kind
 
 INCLUDES =		-I$(top_builddir)/include -I$(top_srcdir)/include
-pkglib_LTLIBRARIES =	libmystrings.la
+
+pkglib_LIBRARIES =	libmystrings.a
+noinst_LTLIBRARIES =	libmystrings.la
 
 # Exact one of ASSEMBLER_X
 if ASSEMBLER_x86
@@ -67,6 +69,8 @@
 # This is because the dependency tracking misses @FOO@ vars in sources.
 #strtoull.o:		@CHARSET_OBJS@
 
+libmystrings.a: libmystrings.la .libs/libmystrings.a
+	$(CP) .libs/libmystrings.a $@
 
 FLAGS=$(DEFS) $(INCLUDES) $(CPPFLAGS) $(CFLAGS) @NOINST_LDFLAGS@
 

=== modified file 'unittest/strings/Makefile.am'
--- unittest/strings/Makefile.am	2010-09-28 13:07:42 +0000
+++ unittest/strings/Makefile.am	2010-10-05 19:27:24 +0000
@@ -17,9 +17,9 @@
 AM_CPPFLAGS     += -I$(top_srcdir)/include -I$(top_srcdir)/unittest/mytap
 
 LDADD 		= $(top_builddir)/unittest/mytap/libmytap.a \
-		  $(top_builddir)/mysys/libmysys.la \
+		  $(top_builddir)/strings/libmystrings.la \
 		  $(top_builddir)/dbug/libdbug.la \
-		  $(top_builddir)/strings/libmystrings.la
+		  $(top_builddir)/mysys/libmysys.la
 
 noinst_PROGRAMS  = strings-t
 

=== modified file 'vio/Makefile.am'
--- vio/Makefile.am	2010-09-28 13:07:42 +0000
+++ vio/Makefile.am	2010-10-05 19:27:24 +0000
@@ -16,7 +16,9 @@
 INCLUDES =		-I$(top_builddir)/include -I$(top_srcdir)/include \
 			$(openssl_includes)
 LDADD =			@CLIENT_EXTRA_LDFLAGS@ $(openssl_libs) $(yassl_libs)
-pkglib_LTLIBRARIES =	libvio.la
+
+pkglib_LIBRARIES =	libvio.a
+noinst_LTLIBRARIES =	libvio.la
 
 noinst_HEADERS =	vio_priv.h
 
@@ -24,3 +26,7 @@
 libvio_la_LIBADD =	$(openssl_libs) $(yassl_libs)
 
 EXTRA_DIST=		CMakeLists.txt
+
+libvio.a: libvio.la .libs/libvio.a
+	$(CP) .libs/libvio.a $@
+

# Begin bundle
IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWayqjiEABuLfgEA0XO//936n
3FS////wYAvLgz5ioAAAECbYAUAFCgiADIlTMkbU8npEeyJhQaPU0DQyaGQAAGok9Rpowho9Q0xB
oaBoAaNDRoMIHGTJkxGJgBMmCZADRhGAIYBIkIaKMqfieiaZTamnip6T1NHqP1IBkyNlNqbUHGTJ
kxGJgBMmCZADRhGAIYBJQCAAAiYEAjQk0ANNADTJIwsKAUKgAaeoCo3BuTOogc+iojNfCIZWGKZV
Tb81hLN8fWh8eSoYca6Fch1AcRlBBarKJUKJrYOayYEASKQBWAPCTdS+u2iraRJvAyKdt7it18lr
CcN237gl6DTItD415fGehttvQB8ANADQ0NNobBsXj0pKQ/KC80xjEZ3q06ZBawqnY0ScDid0iwnd
KB2pwWOyJk7nSd0RKJqCLIo+o9vvA3+GBoLPkZ8QJeZxp6j26mWvVZ+JrApv3SA5qwIwYBYYL/Re
zO4BjnfTsR4U4uOWu26DiT++MlU75eRorMqAYBhgkh4V5yTDCoBtQEHLlIYiEWfS9ZFxVNtPoBZj
Nj2ZFvDLwXIDmMHHFpJg5IckLihHwLmonD8j2G0riTcVWhUlJ4E1QJw2vqOP3nsQcXNx+G/a248s
K5ysQRDghkaEIq1KU4GyGaQNKEvokWkgJpG/n05sEoiNVBUMmUy3gDGJNg2hd2oihNKYJjIFMQiQ
1cNIUIESBQCZZMT0Ie8rIjvrETKR0mApCdjOS5kHRGpCkepZymoWFihkhjQUgkCgqCkjUc5aes1l
SwYrk9glMwLKxo2A5QZHhdaehTMgQ9MSQfalzgXMcpxww9Qu1HwCitT806As5icchDbims/BqhoR
dTrtZQx4+3NUvvMxULYFBA3uXk+Y8V5G0aFbtBuA0tPqWCcbkKjfzalXQhF5OCcDQGIQ4zGGAq9B
xeVmU8kkxCNqZGtambUwyiymYTRRTZIhMJhxGCSWdBAVCooa5STODpeJ+Y7VP45GfBXVCRToMgbu
LFcRvKxkoOQj9BSGjrWIaoFIwMDdjRK051E5fka1kjQvIEe5MWCrpLiynBz4kmjExxeJ4oxkmTAp
TNFCWNxIhIy6TzEYuPqKcmKjhokiy3YzG1F5Re94xrLZqLmnEvL7mtYYp2ud46JMKPicjckoLtRE
nuKC9BO1CK3Y0Ao6mA2dePDLnTOg2kTPK1UOi6BgTRJsFgaDricbQGgi81K775GhGBQZDiYpTSoO
B93msi2NF2ZTVxazeRgMjjIdAnNkIuHk5NfAW1ZETxoojRo4ZOZsHBqkPKCWVzzAhJ4kxJGZQY3a
1llxTSNJxxOzsrCagiWyY1lbC0YyoZgxOG2jjKMxcRmHkxQTFnpLKgzkPKLcKBj4jjMsmSGCGGkw
05muhxUoaTUmm/3WC3O8ObLyngVMpxvzdwY4FWQJsDyNVOJGs40kxfwoKDeiHChUDjwtGlRuNgx5
Jpec5OpLzDssioFMZ3HbtAeGRk4qNhYkuPmdNLh9pIYZwKitxs7Q0REloSLy+TNjVs0u/WeRZwrL
StpqOKNyLiUi1F/AlCGIUPMDAnG1YWXVqLXukMTJSBRJyFg0YTrEehDx6CaI2JAaY+jQVuOMQVY8
5YW3GQTlBLAcXsGmBsUBBnvQiAzyKhkGqDU8pjJjodA7RpFRiJkAmWFERQRahStGd4jOkfp7D6kX
pjYDTebYk2wD/dZ/IFpyXJ/bzVSPd2oS6/IDLmQ2DAbB+Il7vDn/oOafz8K3AUNQlgB4+R5n0bbb
Zj8/OQdSX+WlpcGIGUDoAgu7zIEj0W45j194WdHWEfkuzp3FEoSbQvT+r98wDkH4IYDBQ+hAFWMA
b6oR7n1Oe33kSw+LTOYeWH30UOKS4Gy/UfhWUDRx+sv1gVsmYYldxkhRPw/NJgl9tr2kEOADB3O2
R0t8RG8zDESRBXTJriaRHdi14Nqxg06ZzbvJz6zsz/tMh3BKRIlMgxqdAcDA/c2msZUg7n8yucTJ
juQKjteKQv7aNBFIa9BwjzMLAH/RQF00G+KSTLje0Ggw9StxTxLhp24+Bw9sLqC0gG1R21OBqMxK
yw5GLu3PpCo2GEi7+uBqYm3oYkjynmdsb0mv4nobliaQtKTTx/HPzE57Dmczt8yMkcDibHX32GyL
Uhv6FIK0vImUi2u47VCDD3ozYT1fV/IWOZSdSsm8iteOhSUZBOItXkNOGwuKpyMz5VHodYgZzVv8
2EEI5pgGZ28M1a2m2T+ulPGHRksnOGtv6WllNbfA5FRykOJEfUDUCMWC9BiaWgYoVB0KhpzkVGhY
XWjyq6uQ5zTQnOnx0dT7G9LTqTbkToX9EIyN9jxPc1t0Mxg2rT8h4PEjc0Ql30gSJ9k0hiYLQZXu
3L4FwkuIyt7KlccQizFCVEQDyPjJ4g9JzlHkku408VuHgmnkfP45+hb49ORSVuKQaIaVnzHDif0E
TDB3ER4nYmPY4r8wJIR8AfQDqfBUek5PFanNVmDjo9d/+8vZeSQ1KCtro9TPyOCSnLztkZD0JfJH
tzJIQ8Yhewua0+yNz4wH6+JyKDxHeAI6N6X7nSkrXh7zFvuhETuYnVIhEnIcTseheAnjUlQSGONk
JcH4CLhMBBtLEjqTXHyEDCoIwSRGR6iLlwQl0qMAehIiG59Vl1QmUoWHAI/b2ORI6gdAyGCLDFNc
hJ4lh6xPQiBY8wPkL1OBlNiL0N+hxPQW9nQgQc9xYXlgkQyDohYyyv/kkbpEBdyY6DuB7wQOEqhw
4DqkQLyzWjKYGL6MQgzQwDCNNmbt6Q+Xk+yOBuU5OcTkocoiUnoAzTo6RQ2TnKA7YhqJF5meC7S3
o5TDHoQ1CVE14K344ouh8TgN5nEJzuesD5WTEwhjfmYnPIX5NDZgDK2pYstKGKhIrFWULuOAekMK
AKOmEKCjRrULynOiEmJzH5IQ5C4FJSJFHiQcKvmHmnoigGwWECwJ38JgbTiX7aB3pf8Xd3bwMADg
QuL6iyZQacMDYgkt0jFHs4UPM9yRYUZeBOUKg7FZE2QjkYHW5cPQYBU0FLj8iO5SI6jjIqORv9Tu
hLuGhVTdgds9hpnmdzCOSEYpHLc9beATA/hsMCc1btXc3knFSEMHHOBSP5NQqltgspJI1PA0OBAR
OkUpJRGfOapJcyKIug0hEhcNGivbtsx1ubhtsY2xtmcGMhm86VslPJrNZUnfzIS3iR2ncA0tQMgc
ewdy6I9LG0tIFRLfAGrEXZnPUxD4mVLK222222+fOjKtiGbd1NjBClNi0CghCRKzoZTJD6nsBKsc
NNgcB4gDQi0wHOuANjd9LxpQdGeZcGJIsssKL/oqSpCiJpFyE0dSwFOczr1PNSJsDiyR3NwLxgeZ
5nWiU5Rn1VVpt6lQxgpzIpg0wySsKyn43twFiJQ+ZlmeYyXz0/YqZ6h5s/3OthQ1CMSmjUoVYuK6
GzedRo01UO5I4nkTCBoyHkci1gU1eQpknHEctx0Dk4wekV4IFRwwQR8DHgLCc5ndBdaTs6ONDttn
MTFgv/i7kinChIVlVHEI

References