Merge lp:~flameeyes/gearmand/gentoo-fixes into lp:gearmand/1.0

[Diego Elio Pettenò]

Fix building of the libpq-based queue manager for gearmand, which has been disabled in Gentoo since start of packaging.

In our system, libpq-fe.h is available in /usr/include, but pg_config_manual is only available in, e.g. /usr/include/postgresql-8.4 (we may have multiple postgresql installs available at the same time), so we have to use pg_config to discover the right place for it.

HTH!
Diego

[Brian Aker (brianaker)]

Monty, is this something you want for Pandora?

On Apr 7, 2010, at 6:16 PM, Diego E. Flameeyes Pettenò wrote:

> Diego E. "Flameeyes" Pettenò has proposed merging lp:~flameeyes/
> gearmand/gentoo-fixes into lp:gearmand.
>
> Requested reviews:
> Gearman-developers (gearman-developers)
>
>
> Fix building of the libpq-based queue manager for gearmand, which
> has been disabled in Gentoo since start of packaging.
>
> In our system, libpq-fe.h is available in /usr/include, but
> pg_config_manual is only available in, e.g. /usr/include/
> postgresql-8.4 (we may have multiple postgresql installs available
> at the same time), so we have to use pg_config to discover the right
> place for it.
>
> HTH!
> Diego
> --
> https://code.launchpad.net/~flameeyes/gearmand/gentoo-fixes/+merge/22991
> Your team Gearman-developers is subscribed to branch lp:gearmand.
> === modified file 'libgearman-server/include.am'
> --- libgearman-server/include.am 2010-04-02 02:04:04 +0000
> +++ libgearman-server/include.am 2010-04-08 01:16:15 +0000
> @@ -88,6 +88,7 @@
>
> libgearman_server_libgearman_server_la_CFLAGS= \
> ${AM_CFLAGS} \
> + $(POSTGRESQL_CFLAGS) \
> -DBUILDING_LIBGEARMAN
>
> libgearman_server_libgearman_server_la_LIBADD= \
> @@ -97,5 +98,6 @@
> $(LTLIBPQ) \
> $(LTLIBSQLITE3) \
> $(LTLIBTOKYOCABINET) \
> + $(POSTGRESQL_LIBS) \
> libgearman/libgearman.la \
> libgearman/libgearmancore.la
>
> === modified file 'libgearman-server/queue_libpq.c'
> --- libgearman-server/queue_libpq.c 2010-04-02 02:04:04 +0000
> +++ libgearman-server/queue_libpq.c 2010-04-08 01:16:15 +0000
> @@ -11,18 +11,20 @@
> * @brief libpq Queue Storage Definitions
> */
>
> +#include <libpq-fe.h>
> +#include <pg_config.h>
> +#include <pg_config_manual.h>
> +
> +#undef PACKAGE_BUGREPORT
> +#undef PACKAGE_NAME
> +#undef PACKAGE_STRING
> +#undef PACKAGE_TARNAME
> +#undef PACKAGE_VERSION
> +
> #include "common.h"
>
> #include <libgearman-server/queue_libpq.h>
>
> -#if defined(HAVE_LIBPQ_FE_H)
> -# include <libpq-fe.h>
> -# include <pg_config_manual.h>
> -#else
> -# include <postgresql/libpq-fe.h>
> -# include <postgresql/pg_config_manual.h>
> -#endif
> -
> /**
> * @addtogroup gearman_queue_libpq_static Static libpq Queue Storage
> Definitions
> * @ingroup gearman_queue_libpq
>
> === modified file 'm4/pandora_have_libpq.m4'
> --- m4/pandora_have_libpq.m4 2009-07-29 18:13:26 +0000
> +++ m4/pandora_have_libpq.m4 2010-04-08 01:16:15 +0000
> @@ -17,21 +17,28 @@
> [ac_enable_libpq="yes"])
>
> AS_IF([test "x$ac_enable_libpq" = "xyes"],[
> - AC_CHECK_HEADERS([libpq-fe.h])
> - AC_LIB_HAVE_LINKFLAGS(pq,,[
> - #ifdef HAVE_LIBPQ_FE_H
> - # include <libpq-fe.h>
> - #else
> - # include <postgresql/libpq-fe.h>
> - #endif
> + AS_IF([test -z "$PG_CONFIG"], [
> + AC_PATH_PROG([PG_CONFIG], [pg_config], [])
> + ])
> +
> + AS_IF([test ! -x "$PG_CONFIG"], [
> + AC_MSG_WARN([$PG_CONFIG does not exist or it is not an
> exectuable file])
> + PG_CONFIG="no"
> + ac_cv_libpq="no"
> ], [
> - PGconn *conn;
> - conn = PQconnectd...

[Brian Aker (brianaker)]

I am marking this as Needs Information, since Monty will need to be the person to fix this.

review: Needs Information

[Monty Taylor (mordred)]

Yeah - so I see what you're doing, but I'm going to need to poke at this some on the other platforms. I _really_ don't like trusting things like pg_config.

Also- why did you add and include of the pg_config.h include? That's going to have values from their run of configure which we do not care about?

[Diego Elio Pettenò (flameeyes)]

Il giorno lun, 05/07/2010 alle 16.44 +0000, Monty Taylor ha scritto:
>
> Yeah - so I see what you're doing, but I'm going to need to poke at
> this some on the other platforms. I _really_ don't like trusting
> things like pg_config.

Not sure on Ubuntu but Gentoo tries to keep these scripts valid because
they _are_ the only way upstream has to signal them..
>
> Also- why did you add and include of the pg_config.h include? That's
> going to have values from their run of configure which we do not care
> about?

That's how most other projects get the correct build parameters, without
this it fails here ... I sincerely forgot the error message right now.

--
Diego Elio Pettenò — “Flameeyes”
http://blog.flameeyes.eu/

If you found a .asc file in this mail and know not what it is,
it's a GnuPG digital signature: http://www.gnupg.org/

[Monty Taylor (mordred)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/05/2010 09:55 AM, Diego E. "Flameeyes" Pettenò wrote:
> Il giorno lun, 05/07/2010 alle 16.44 +0000, Monty Taylor ha scritto:
>>
>> Yeah - so I see what you're doing, but I'm going to need to poke at
>> this some on the other platforms. I _really_ don't like trusting
>> things like pg_config.
>
> Not sure on Ubuntu but Gentoo tries to keep these scripts valid because
> they _are_ the only way upstream has to signal them..

Sure - the problem is that using autoconf at all is about making sure
that I can also build on broken platforms like OSX and Solaris... so
trusting that a script or a system config has been properly maintained
is just asking for failure. (This is the reason I hate pkg-config,
too... did they learn nothing from imake?)

It works _great_ when it works, but I _still_ need to test that it's not
lying or broken, which is why I'd prefer to just actually test the files
in the first place.

I'll see if I can merge the approaches though...

>> Also- why did you add and include of the pg_config.h include? That's
>> going to have values from their run of configure which we do not care
>> about?
>
> That's how most other projects get the correct build parameters, without
> this it fails here ... I sincerely forgot the error message right now.

I really need to blog more about this. It's so fundamentally wrong to do
that. Every project that does it is completely breaking everything.
MySQL does it - it was the first thing I fixed in Drizzle.

I guess it's time to go make a patch to postgres. :)
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkwyECAACgkQ2Jv7/VK1RgErngCgvcFuXpV9ymns8Ya4fPphgTY7
uLsAn3BRXSCCwiu6Vg+ak2VbhRZxYFZM
=YhS+
-----END PGP SIGNATURE-----

[Diego Elio Pettenò (flameeyes)]

Il giorno lun, 05/07/2010 alle 17.04 +0000, Monty Taylor ha scritto:
>
> I really need to blog more about this. It's so fundamentally wrong to
> do
> that. Every project that does it is completely breaking everything.
> MySQL does it - it was the first thing I fixed in Drizzle.

You have no idea how much I know it's wrong... sigh.

[Olaf van der Spek (olafvdspek)]

> (we may have multiple postgresql installs available at the same time), so we have to use pg_config to discover the right place for it.

Can't the pkg system use the build dependencies to ensure the compiler paths include all necessary includes and libs?

Unmerged revisions

352. By Diego Elio Pettenò on 2010-04-08

Fix PostgreSQL queue manager building on Gentoo and other platforms.

When the includes are installed in non-standard positions, and especially
when libpq-fe.h is available without -I directives, but pg_config_manual.h
isn't, you cannot rely on just finding one to get the other.

What you have to do in this case is use the pg_config script shipped with
PostgreSQL to get the right include directory, and use the simple basename
for the include headers.

Also, on modern (glibc-2.10) systems, using simply pg_config_manual.h
without pg_config.h or without adding further checks in configure, the
build will fail with this error:

In file included from libgearman-server/queue_libpq.c:19:
/usr/include/postgresql-8.4/pg_config_manual.h:128:5: error: "HAVE_DECL_POSIX_FADVISE" is not defined
make[1]: *** [libgearman-server/libgearman_server_libgearman_server_la-queue_libpq.lo] Error 1

So include the PostgreSQL headers first thing, undefine package-specific
entries, and proceed.

=== modified file 'libgearman-server/include.am'
--- libgearman-server/include.am	2010-04-02 02:04:04 +0000
+++ libgearman-server/include.am	2010-04-08 01:16:15 +0000
@@ -88,6 +88,7 @@
 
 libgearman_server_libgearman_server_la_CFLAGS= \
 					       ${AM_CFLAGS} \
+					       $(POSTGRESQL_CFLAGS) \
 					       -DBUILDING_LIBGEARMAN
 
 libgearman_server_libgearman_server_la_LIBADD= \
@@ -97,5 +98,6 @@
 					       $(LTLIBPQ) \
 					       $(LTLIBSQLITE3) \
 					       $(LTLIBTOKYOCABINET) \
+					       $(POSTGRESQL_LIBS) \
 					       libgearman/libgearman.la \
 					       libgearman/libgearmancore.la
 
=== modified file 'libgearman-server/queue_libpq.c'
--- libgearman-server/queue_libpq.c	2010-04-02 02:04:04 +0000
+++ libgearman-server/queue_libpq.c	2010-04-08 01:16:15 +0000
@@ -11,18 +11,20 @@
  * @brief libpq Queue Storage Definitions
  */
 
+#include <libpq-fe.h>
+#include <pg_config.h>
+#include <pg_config_manual.h>
+
+#undef PACKAGE_BUGREPORT
+#undef PACKAGE_NAME
+#undef PACKAGE_STRING
+#undef PACKAGE_TARNAME
+#undef PACKAGE_VERSION
+
 #include "common.h"
 
 #include <libgearman-server/queue_libpq.h>
 
-#if defined(HAVE_LIBPQ_FE_H)
-# include <libpq-fe.h>
-# include <pg_config_manual.h>
-#else
-# include <postgresql/libpq-fe.h>
-# include <postgresql/pg_config_manual.h>
-#endif
-
 /**
  * @addtogroup gearman_queue_libpq_static Static libpq Queue Storage Definitions
  * @ingroup gearman_queue_libpq
 
=== modified file 'm4/pandora_have_libpq.m4'
--- m4/pandora_have_libpq.m4	2009-07-29 18:13:26 +0000
+++ m4/pandora_have_libpq.m4	2010-04-08 01:16:15 +0000
@@ -17,21 +17,28 @@
     [ac_enable_libpq="yes"])
 
   AS_IF([test "x$ac_enable_libpq" = "xyes"],[
-    AC_CHECK_HEADERS([libpq-fe.h])
-    AC_LIB_HAVE_LINKFLAGS(pq,,[
-      #ifdef HAVE_LIBPQ_FE_H
-      # include <libpq-fe.h>
-      #else
-      # include <postgresql/libpq-fe.h>
-      #endif
+    AS_IF([test -z "$PG_CONFIG"], [
+      AC_PATH_PROG([PG_CONFIG], [pg_config], [])
+    ])
+
+    AS_IF([test ! -x "$PG_CONFIG"], [
+      AC_MSG_WARN([$PG_CONFIG does not exist or it is not an exectuable file])
+      PG_CONFIG="no"
+      ac_cv_libpq="no"
     ], [
-      PGconn *conn;
-      conn = PQconnectdb(NULL);
+      AC_MSG_CHECKING([for PostgreSQL libraries])
+
+      POSTGRESQL_CFLAGS="-I`$PG_CONFIG --includedir`"
+      POSTGRESQL_LIBS="-L`$PG_CONFIG --libdir` -lpq"
+      AC_SUBST([POSTGRESQL_CFLAGS])
+      AC_SUBST([POSTGRESQL_LIBS])
+
+      ac_cv_libpq=yes
+      AC_MSG_RESULT([yes])
     ])
-  ],[
-    ac_cv_libpq="no"
-  ])
-  
+  ],
+  [ac_cv_libpq=no])
+
   AM_CONDITIONAL(HAVE_LIBPQ, [test "x${ac_cv_libpq}" = "xyes"])
 ])