Thread: configure "stuff"

Hi,

I received an email a few days ago (humm, weeks) about issues with
compiling pgAdmin.

====== extract ======
The file pgadmin/db/keywords.c has this statement in it:
#include <server/parser/kwlist.h>

 Now it comes that pg_config does spit out either
/usr/include/postgresql for the --includedir switch (for client things)
and /usr/include/postgresql/9.0/server for --includedir-server. Given
that both seems to work well for other stuff I'm unsure whether there is
a problem here and it should rather be #include <parser/kwlist.h> (or
the file be moved out of server context).
====== end of extract ======

I took a quick look at it, and it seems we forgot something. If I'm not
wrong, we don't use the --includedir-server switch for our includes in
the configure script (actually, in the acinclude.m4 file). And it breaks
the compilation on Debian when a user uses the PostgreSQL package on
Debian or any other .deb linux distributions. We already do this for the
package include dir (--pkgincludedir), I don't see any reason why we
shouldn't do it for the server one.

Comments?


--
Guillaume
  http://blog.guillaume.lelarge.info
  http://www.dalibo.com

On Sat, Jun 25, 2011 at 7:26 PM, Guillaume Lelarge
<guillaume@lelarge.info> wrote:
> Hi,
>
> I received an email a few days ago (humm, weeks) about issues with
> compiling pgAdmin.
>
> ====== extract ======
> The file pgadmin/db/keywords.c has this statement in it:
> #include <server/parser/kwlist.h>
>
>  Now it comes that pg_config does spit out either
> /usr/include/postgresql for the --includedir switch (for client things)
> and /usr/include/postgresql/9.0/server for --includedir-server. Given
> that both seems to work well for other stuff I'm unsure whether there is
> a problem here and it should rather be #include <parser/kwlist.h> (or
> the file be moved out of server context).
> ====== end of extract ======
>
> I took a quick look at it, and it seems we forgot something. If I'm not
> wrong, we don't use the --includedir-server switch for our includes in
> the configure script (actually, in the acinclude.m4 file). And it breaks
> the compilation on Debian when a user uses the PostgreSQL package on
> Debian or any other .deb linux distributions. We already do this for the
> package include dir (--pkgincludedir), I don't see any reason why we
> shouldn't do it for the server one.

Hmm, that would explain it - until now it's worked for some people and
not for others and we've assumed it was a broken -dev package. I
assume something like this does the trick:

diff --git a/acinclude.m4 b/acinclude.m4
index e379c62..f5eeaa4 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -548,8 +548,9 @@ AC_DEFUN([SETUP_POSTGRESQL],
                AC_LANG_RESTORE

                PG_INCLUDE=`${PG_CONFIG} --includedir`
+               PG_SVRINCLUDE=`${PG_CONFIG} --includedir-server`
                PG_PKGINCLUDE=`${PG_CONFIG} --pkgincludedir`
-               CPPFLAGS="$CPPFLAGS -I${PG_INCLUDE} -I${PG_PKGINCLUDE}"
+               CPPFLAGS="$CPPFLAGS -I${PG_INCLUDE} -I${PG_SVRINCLUDE}
-I${PG_PKGINCLUDE}"

                PG_VERSION=`${PG_CONFIG} --version`




--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Sun, 2011-06-26 at 20:35 +0100, Dave Page wrote:
> On Sat, Jun 25, 2011 at 7:26 PM, Guillaume Lelarge
> <guillaume@lelarge.info> wrote:
> > Hi,
> >
> > I received an email a few days ago (humm, weeks) about issues with
> > compiling pgAdmin.
> >
> > ====== extract ======
> > The file pgadmin/db/keywords.c has this statement in it:
> > #include <server/parser/kwlist.h>
> >
> >  Now it comes that pg_config does spit out either
> > /usr/include/postgresql for the --includedir switch (for client things)
> > and /usr/include/postgresql/9.0/server for --includedir-server. Given
> > that both seems to work well for other stuff I'm unsure whether there is
> > a problem here and it should rather be #include <parser/kwlist.h> (or
> > the file be moved out of server context).
> > ====== end of extract ======
> >
> > I took a quick look at it, and it seems we forgot something. If I'm not
> > wrong, we don't use the --includedir-server switch for our includes in
> > the configure script (actually, in the acinclude.m4 file). And it breaks
> > the compilation on Debian when a user uses the PostgreSQL package on
> > Debian or any other .deb linux distributions. We already do this for the
> > package include dir (--pkgincludedir), I don't see any reason why we
> > shouldn't do it for the server one.
>
> Hmm, that would explain it - until now it's worked for some people and
> not for others and we've assumed it was a broken -dev package. I
> assume something like this does the trick:
>
> diff --git a/acinclude.m4 b/acinclude.m4
> index e379c62..f5eeaa4 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -548,8 +548,9 @@ AC_DEFUN([SETUP_POSTGRESQL],
>                 AC_LANG_RESTORE
>
>                 PG_INCLUDE=`${PG_CONFIG} --includedir`
> +               PG_SVRINCLUDE=`${PG_CONFIG} --includedir-server`
>                 PG_PKGINCLUDE=`${PG_CONFIG} --pkgincludedir`
> -               CPPFLAGS="$CPPFLAGS -I${PG_INCLUDE} -I${PG_PKGINCLUDE}"
> +               CPPFLAGS="$CPPFLAGS -I${PG_INCLUDE} -I${PG_SVRINCLUDE}
> -I${PG_PKGINCLUDE}"
>
>                 PG_VERSION=`${PG_CONFIG} --version`
>

Yes, that's pretty much what I was thinking. There is also another file
that needs to be updated:

diff --git a/pgadmin/db/keywords.c b/pgadmin/db/keywords.c
index 9370a9e..0362a71 100644
--- a/pgadmin/db/keywords.c
+++ b/pgadmin/db/keywords.c
@@ -30,7 +30,7 @@
  */
 #define PG_KEYWORD(a,b,c) {a,c},
 const ScanKeyword ScanKeywords[] = {
-#include <server/parser/kwlist.h>
+#include "parser/kwlist.h"
 };
 const int NumScanKeywords = lengthof(ScanKeywords);


Can I commit both changes?


--
Guillaume
  http://blog.guillaume.lelarge.info
  http://www.dalibo.com

On Mon, 2011-06-27 at 13:04 +0530, Ashesh Vashi wrote:
> Does this take of windows-build too? (Just question out of mind...)
>

Yes, I didn't think of that, but the changes on pgadmin/db/keywords.c
may need some changes on the Visual Studio project.


--
Guillaume
  http://blog.guillaume.lelarge.info
  http://www.dalibo.com

On Mon, Jun 27, 2011 at 8:32 AM, Guillaume Lelarge
<guillaume@lelarge.info> wrote:
>
> diff --git a/pgadmin/db/keywords.c b/pgadmin/db/keywords.c
> index 9370a9e..0362a71 100644
> --- a/pgadmin/db/keywords.c
> +++ b/pgadmin/db/keywords.c
> @@ -30,7 +30,7 @@
>  */
>  #define PG_KEYWORD(a,b,c) {a,c},
>  const ScanKeyword ScanKeywords[] = {
> -#include <server/parser/kwlist.h>
> +#include "parser/kwlist.h"
>  };
>  const int NumScanKeywords = lengthof(ScanKeywords);
>
>
> Can I commit both changes?

Along with an update for windows, yes.



--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Mon, 2011-06-27 at 08:50 +0100, Dave Page wrote:
> On Mon, Jun 27, 2011 at 8:32 AM, Guillaume Lelarge
> <guillaume@lelarge.info> wrote:
> >
> > diff --git a/pgadmin/db/keywords.c b/pgadmin/db/keywords.c
> > index 9370a9e..0362a71 100644
> > --- a/pgadmin/db/keywords.c
> > +++ b/pgadmin/db/keywords.c
> > @@ -30,7 +30,7 @@
> >  */
> >  #define PG_KEYWORD(a,b,c) {a,c},
> >  const ScanKeyword ScanKeywords[] = {
> > -#include <server/parser/kwlist.h>
> > +#include "parser/kwlist.h"
> >  };
> >  const int NumScanKeywords = lengthof(ScanKeywords);
> >
> >
> > Can I commit both changes?
>
> Along with an update for windows, yes.
>

Done.


--
Guillaume
  http://blog.guillaume.lelarge.info
  http://www.dalibo.com