Thread: Compilation on Debian SID

Hi,

I'm using the GIT version of Pgadmin under Debian SID, and there is a compilation error (all was working fine a couple of week before) :
-------------------------------------------------
make[3]: entrant dans le répertoire « /home/pri/pgadmin3/pgadmin »
g++ -DHAVE_CONFIG_H -I. -I..  -DDATA_DIR=\"/usr/local/pgadmin3/share/pgadmin3/\" -I../pgadmin/include   -I/usr/local/pgsql9.1/include -I/usr/local/pgsql9.1/include/server -I/usr/local/pgsql9.1/include -DHAVE_CONNINFO_PARSE -I/usr/lib/wx/include/gtk2-unicode-release-2.8 -I/usr/include/wx-2.8 -D_FILE_OFFSET_BITS=64 -D_LARGE_FILES -D__WXGTK__ -DEMBED_XRC -I/usr/include/libxml2 -I/usr/include/libxml2  -O2 -MT pgAdmin3.o -MD -MP -MF .deps/pgAdmin3.Tpo -c -o pgAdmin3.o `test -f './pgAdmin3.cpp' || echo './'`./pgAdmin3.cpp
In file included from ../pgadmin/include/utils/sysSettings.h:15:0,
                 from ../pgadmin/include/pgAdmin3.h:37,
                 from ./pgAdmin3.cpp:13:
../pgadmin/include/utils/sysLogger.h:31:2: error: expected unqualified-id before ‘/’ token
../pgadmin/include/utils/sysLogger.h:31:2: error: expected constructor, destructor, or type conversion before ‘/’ token
../pgadmin/include/utils/sysLogger.h:76:1: error: expected initializer before ‘ATTRIBUTE_PRINTF_1’
../pgadmin/include/utils/sysLogger.h:77:1: error: expected initializer before ‘ATTRIBUTE_PRINTF_1’
../pgadmin/include/utils/sysLogger.h:78:1: error: expected initializer before ‘ATTRIBUTE_PRINTF_1’
../pgadmin/include/utils/sysLogger.h:79:1: error: expected initializer before ‘ATTRIBUTE_PRINTF_1’
../pgadmin/include/utils/sysLogger.h:80:1: error: expected initializer before ‘ATTRIBUTE_PRINTF_1’
In file included from ../pgadmin/include/pgAdmin3.h:37:0,
                 from ./pgAdmin3.cpp:13:
../pgadmin/include/utils/sysSettings.h: In member function ‘int sysSettings::GetLogLevel() const’:
../pgadmin/include/utils/sysSettings.h:42:29: error: ‘LOG_ERRORS’ was not declared in this scope
make[3]: *** [pgAdmin3.o] Erreur 1
-------------------------------------------------


Here is the output of ./configure
-------------------------------------------------
PostgreSQL directory:                   /usr/local/pgsql
PostgreSQL pg_config binary:            /usr/local/pgsql/bin/pg_config
PostgreSQL version:                     PostgreSQL 9.1.1

PostgreSQL PQconninfoParse support:     Present
PostgreSQL SSL support:                 Missing

wxWidgets directory:                    /usr
wxWidgets wx-config binary:             /usr/bin/wx-config
wxWidgets version:                      wxWidgets 2.8.12

libxml2 directory:                      /usr
libxml2 xml2-config binary:             /usr/bin/xml2-config
libxml2 version:                        libxml2 2.7.8

libxslt directory:                      /usr
libxslt xslt-config binary:             /usr/bin/xslt-config
libxslt version:                        libxslt 1.1.26

Building a debug version of pgAdmin:    No
Statically linking pgAdmin:             No
Building a Mac OS X appbundle:          No

sphinx-build executable:                /usr/bin/sphinx-build
-------------------------------------------------

All work fine if i add those lines in pgadmin/include/utils/sysLogger.h (juste after #include <wx/wx.h>, line 16) :
-------------------------------------------------
#ifndef ATTRIBUTE_PRINTF
#   if defined(__GNUC__) && !wxUSE_UNICODE
#       define ATTRIBUTE_PRINTF(m, n) __attribute__ ((__format__ (__printf__, m, n)))
#   else
#       define ATTRIBUTE_PRINTF(m, n)
#   endif
 
#   define ATTRIBUTE_PRINTF_1 ATTRIBUTE_PRINTF(1, 2)
#   define ATTRIBUTE_PRINTF_2 ATTRIBUTE_PRINTF(2, 3)
#   define ATTRIBUTE_PRINTF_3 ATTRIBUTE_PRINTF(3, 4)
#   define ATTRIBUTE_PRINTF_4 ATTRIBUTE_PRINTF(4, 5)
#   define ATTRIBUTE_PRINTF_5 ATTRIBUTE_PRINTF(5, 6)
#endif /* !defined(ATTRIBUTE_PRINTF) */
-------------------------------------------------

I'm not a C++ dev, just a DBA/"googler" who try to make pgadmin work.

Regards

Philippe

On Thu, 2011-11-03 at 17:38 +0100, philippe rimbault wrote:
> [...]
> I'm using the GIT version of Pgadmin under Debian SID, and there is a
> compilation error (all was working fine a couple of week before) :

Have you changed anything in pgAdmin source code? because I compile it
without any issues.


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

Guillaume,

No change at all and it's the most recent version of the GIT repository.
Last commit :
commit 7b54af13039b59a9da7b326140b06cb434338889
Author: Guillaume Lelarge <guillaume@lelarge.info>
Date:   Thu Nov 3 12:19:03 2011 +0100

    Fix cache type lookup
...


I've "patched" my version to work after reading this post : http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=521924

There is also this bug (Mon, 24 Oct 2011) from debian FTBFS about pgadmin3 :
http://www.mail-archive.com/debian-bugs-dist@lists.debian.org/msg953221.html

And this thread about building pgadmin with wx2.9 (i'm using wx2.8 but there is the same error) :
http://archives.postgresql.org/pgadmin-hackers/2011-01/msg00125.php

On Thu, 2011-11-03 at 18:09 +0100, philippe rimbault wrote:
> There is also this bug (Mon, 24 Oct 2011) from debian FTBFS about pgadmin3 :
> http://www.mail-archive.com/debian-bugs-dist@lists.debian.org/msg953221.html
>
> And this thread about building pgadmin with wx2.9 (i'm using wx2.8 but
> there is the same error) :
> http://archives.postgresql.org/pgadmin-hackers/2011-01/msg00125.php
>

We don't use wxWidgets 2.9 because we aren't fully compatible with it
yet. Not sure why though.

About your issue, I tried this evening to install a Debian Sid. It
didn't work, the "aptitude full-upgrade" just failed. So, I cannot
check. Your issue could happen because of a new gcc version, ar new
headers, or something else. Would be weird, Fedora being pretty much in
advance on those things.


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

GCC : gcc (Debian 4.6.1-15) 4.6.1
GLIBC : glibc-2.13-1

I'have tried to compile on an other computer (fresh Debian SID) and it's the same issue.

Am I the only one who use the GIT version of Pgadmin3 on a Debian SID ? :)

On Fri, 2011-11-04 at 10:01 +0100, philippe rimbault wrote:
> GCC : gcc (Debian 4.6.1-15) 4.6.1

Same here.

> GLIBC : glibc-2.13-1


2.14 for me.

> I'have tried to compile on an other computer (fresh Debian SID) and it's
> the same issue.
>

How do you get a debian sid? I failed last evening. Here are my steps:
 * squeeze netinstall
 * sed -i -e 's/squeeze/unstable/' /etc/apt/sources.list
 * aptitude update
 * aptitude full-upgrade

Am I right? or is there a better way to do this?

> Am I the only one who use the GIT version of Pgadmin3 on a Debian SID ? :)
>

Probably not.


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

On Fri, 2011-11-04 at 10:15 +0100, philippe rimbault wrote:
> 2011/11/4 Guillaume Lelarge <guillaume@lelarge.info>
>
> > On Fri, 2011-11-04 at 10:01 +0100, philippe rimbault wrote:
> > > GCC : gcc (Debian 4.6.1-15) 4.6.1
> >
> > Same here.
> >
> > > GLIBC : glibc-2.13-1
> >
> >
> > 2.14 for me.
> >
> > > I'have tried to compile on an other computer (fresh Debian SID) and it's
> > > the same issue.
> > >
> >
> > How do you get a debian sid? I failed last evening. Here are my steps:
> >  * squeeze netinstall
> >  * sed -i -e 's/squeeze/unstable/' /etc/apt/sources.list
> >  * aptitude update
> >  * aptitude full-upgrade
> >
> >
> I always start with a testing ISO :
> http://cdimage.debian.org/cdimage/weekly-builds/
> and after :
> - change /etc/apt/sources.list
> - apt-get update
> - apt-get upgrade
> - apt-get dist-upgrade
>
> But yours steps seems ok too.
> What kind of error you get ?
>

First, sorry to answer that late.

I tried with a testing ISO, and I reproduce the error. No need to change
sources.list.

The only way I found to fix this is to change the file
pgadmin/include/utils/sysLogger.h, this way:

/*
#define DECLARE_INT_LOG_FUNCTION(level)                               \
extern void wxVLog##level(const wxChar *szFormat, va_list argptr);    \
extern void wxLog##level(const wxChar *szFormat, ...) ATTRIBUTE_PRINTF_1

DECLARE_INT_LOG_FUNCTION(Notice);
DECLARE_INT_LOG_FUNCTION(Sql);
DECLARE_INT_LOG_FUNCTION(QuietError);
DECLARE_INT_LOG_FUNCTION(Script);
DECLARE_INT_LOG_FUNCTION(ScriptVerbose);
*/
DECLARE_LOG_FUNCTION(Notice);
DECLARE_LOG_FUNCTION(Sql);
DECLARE_LOG_FUNCTION(QuietError);
DECLARE_LOG_FUNCTION(Script);
DECLARE_LOG_FUNCTION(ScriptVerbose);

In other words, I replace DECLARE_INT_LOG_FUNCTION with
DECLARE_LOG_FUNCTION, and comment the DECLARE_INT_LOG_FUNCTION
declaration.

AFAICT, this macro is here since the development of 1.14 to be able to
dynamically link with wxWidgets on Windows. This is commit
1270bcd192729d95cb5e2aa687e5ba1c366b7f5f. I don't quite understand why
the new macro was required, but it doesn't help on Linux. One possible
way to get rid of this is to put this code in place when compiling on
Linux, and not otherwise.

Not still sure if this is the right move. Something bothers me: why does
it work on Fedora without the patch, and not on Debian? how do the
Debian guys have a pgadmin 1.14 packaged without changing this file?
(yes, 1.14 shows also the issue, and yes there is no patch on this file
for their prebuilt package)

Well, it still needs more investigation, but at least, you can compile
it with this "patch".


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

Thanks for your investigation !

Philippe

On Mon, 2011-11-14 at 09:24 +0100, philippe rimbault wrote:
> 2011/11/13 Guillaume Lelarge <guillaume@lelarge.info>
>
> > On Fri, 2011-11-04 at 10:15 +0100, philippe rimbault wrote:
> > > 2011/11/4 Guillaume Lelarge <guillaume@lelarge.info>
> > >
> > > > On Fri, 2011-11-04 at 10:01 +0100, philippe rimbault wrote:
> > > > > GCC : gcc (Debian 4.6.1-15) 4.6.1
> > > >
> > > > Same here.
> > > >
> > > > > GLIBC : glibc-2.13-1
> > > >
> > > >
> > > > 2.14 for me.
> > > >
> > > > > I'have tried to compile on an other computer (fresh Debian SID) and
> > it's
> > > > > the same issue.
> > > > >
> > > >
> > > > How do you get a debian sid? I failed last evening. Here are my steps:
> > > >  * squeeze netinstall
> > > >  * sed -i -e 's/squeeze/unstable/' /etc/apt/sources.list
> > > >  * aptitude update
> > > >  * aptitude full-upgrade
> > > >
> > > >
> > > I always start with a testing ISO :
> > > http://cdimage.debian.org/cdimage/weekly-builds/
> > > and after :
> > > - change /etc/apt/sources.list
> > > - apt-get update
> > > - apt-get upgrade
> > > - apt-get dist-upgrade
> > >
> > > But yours steps seems ok too.
> > > What kind of error you get ?
> > >
> >
> > First, sorry to answer that late.
> >
> > I tried with a testing ISO, and I reproduce the error. No need to change
> > sources.list.
> >
> > The only way I found to fix this is to change the file
> > pgadmin/include/utils/sysLogger.h, this way:
> >
> > /*
> > #define DECLARE_INT_LOG_FUNCTION(level)                               \
> > extern void wxVLog##level(const wxChar *szFormat, va_list argptr);    \
> > extern void wxLog##level(const wxChar *szFormat, ...) ATTRIBUTE_PRINTF_1
> >
> > DECLARE_INT_LOG_FUNCTION(Notice);
> > DECLARE_INT_LOG_FUNCTION(Sql);
> > DECLARE_INT_LOG_FUNCTION(QuietError);
> > DECLARE_INT_LOG_FUNCTION(Script);
> > DECLARE_INT_LOG_FUNCTION(ScriptVerbose);
> > */
> > DECLARE_LOG_FUNCTION(Notice);
> > DECLARE_LOG_FUNCTION(Sql);
> > DECLARE_LOG_FUNCTION(QuietError);
> > DECLARE_LOG_FUNCTION(Script);
> > DECLARE_LOG_FUNCTION(ScriptVerbose);
> >
> > In other words, I replace DECLARE_INT_LOG_FUNCTION with
> > DECLARE_LOG_FUNCTION, and comment the DECLARE_INT_LOG_FUNCTION
> > declaration.
> >
> > AFAICT, this macro is here since the development of 1.14 to be able to
> > dynamically link with wxWidgets on Windows. This is commit
> > 1270bcd192729d95cb5e2aa687e5ba1c366b7f5f. I don't quite understand why
> > the new macro was required, but it doesn't help on Linux. One possible
> > way to get rid of this is to put this code in place when compiling on
> > Linux, and not otherwise.
> >
> > Not still sure if this is the right move. Something bothers me: why does
> > it work on Fedora without the patch, and not on Debian? how do the
> > Debian guys have a pgadmin 1.14 packaged without changing this file?
> > (yes, 1.14 shows also the issue, and yes there is no patch on this file
> > for their prebuilt package)
> >
> > Well, it still needs more investigation, but at least, you can compile
> > it with this "patch".
> >
> >
> > --
> > Guillaume
> >  http://blog.guillaume.lelarge.info
> >  http://www.dalibo.com
> >
> >
> Thanks for your investigation !
>

Still working on this. Gerfried Fuchs (the Debian packager for pgAdmin)
also has the issue. I tried a few things, namely install gcc 4.6.2, but
it still doesn't work without the patch.

One interesting thing is that I can compile wxWidgets which has the same
ATTRIBUTE_PRINTF_1 identifier. Maybe they do something smarter than us.
Problem is I don't know what yet :-/


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

On Mon, 2011-11-14 at 23:23 +0100, Guillaume Lelarge wrote:
> On Mon, 2011-11-14 at 09:24 +0100, philippe rimbault wrote:
> > 2011/11/13 Guillaume Lelarge <guillaume@lelarge.info>
> >
> > > On Fri, 2011-11-04 at 10:15 +0100, philippe rimbault wrote:
> > > > 2011/11/4 Guillaume Lelarge <guillaume@lelarge.info>
> > > >
> > > > > On Fri, 2011-11-04 at 10:01 +0100, philippe rimbault wrote:
> > > > > > GCC : gcc (Debian 4.6.1-15) 4.6.1
> > > > >
> > > > > Same here.
> > > > >
> > > > > > GLIBC : glibc-2.13-1
> > > > >
> > > > >
> > > > > 2.14 for me.
> > > > >
> > > > > > I'have tried to compile on an other computer (fresh Debian SID) and
> > > it's
> > > > > > the same issue.
> > > > > >
> > > > >
> > > > > How do you get a debian sid? I failed last evening. Here are my steps:
> > > > >  * squeeze netinstall
> > > > >  * sed -i -e 's/squeeze/unstable/' /etc/apt/sources.list
> > > > >  * aptitude update
> > > > >  * aptitude full-upgrade
> > > > >
> > > > >
> > > > I always start with a testing ISO :
> > > > http://cdimage.debian.org/cdimage/weekly-builds/
> > > > and after :
> > > > - change /etc/apt/sources.list
> > > > - apt-get update
> > > > - apt-get upgrade
> > > > - apt-get dist-upgrade
> > > >
> > > > But yours steps seems ok too.
> > > > What kind of error you get ?
> > > >
> > >
> > > First, sorry to answer that late.
> > >
> > > I tried with a testing ISO, and I reproduce the error. No need to change
> > > sources.list.
> > >
> > > The only way I found to fix this is to change the file
> > > pgadmin/include/utils/sysLogger.h, this way:
> > >
> > > /*
> > > #define DECLARE_INT_LOG_FUNCTION(level)                               \
> > > extern void wxVLog##level(const wxChar *szFormat, va_list argptr);    \
> > > extern void wxLog##level(const wxChar *szFormat, ...) ATTRIBUTE_PRINTF_1
> > >
> > > DECLARE_INT_LOG_FUNCTION(Notice);
> > > DECLARE_INT_LOG_FUNCTION(Sql);
> > > DECLARE_INT_LOG_FUNCTION(QuietError);
> > > DECLARE_INT_LOG_FUNCTION(Script);
> > > DECLARE_INT_LOG_FUNCTION(ScriptVerbose);
> > > */
> > > DECLARE_LOG_FUNCTION(Notice);
> > > DECLARE_LOG_FUNCTION(Sql);
> > > DECLARE_LOG_FUNCTION(QuietError);
> > > DECLARE_LOG_FUNCTION(Script);
> > > DECLARE_LOG_FUNCTION(ScriptVerbose);
> > >
> > > In other words, I replace DECLARE_INT_LOG_FUNCTION with
> > > DECLARE_LOG_FUNCTION, and comment the DECLARE_INT_LOG_FUNCTION
> > > declaration.
> > >
> > > AFAICT, this macro is here since the development of 1.14 to be able to
> > > dynamically link with wxWidgets on Windows. This is commit
> > > 1270bcd192729d95cb5e2aa687e5ba1c366b7f5f. I don't quite understand why
> > > the new macro was required, but it doesn't help on Linux. One possible
> > > way to get rid of this is to put this code in place when compiling on
> > > Linux, and not otherwise.
> > >
> > > Not still sure if this is the right move. Something bothers me: why does
> > > it work on Fedora without the patch, and not on Debian? how do the
> > > Debian guys have a pgadmin 1.14 packaged without changing this file?
> > > (yes, 1.14 shows also the issue, and yes there is no patch on this file
> > > for their prebuilt package)
> > >
> > > Well, it still needs more investigation, but at least, you can compile
> > > it with this "patch".
> > >
> > >
> > > --
> > > Guillaume
> > >  http://blog.guillaume.lelarge.info
> > >  http://www.dalibo.com
> > >
> > >
> > Thanks for your investigation !
> >
>
> Still working on this. Gerfried Fuchs (the Debian packager for pgAdmin)
> also has the issue. I tried a few things, namely install gcc 4.6.2, but
> it still doesn't work without the patch.
>
> One interesting thing is that I can compile wxWidgets which has the same
> ATTRIBUTE_PRINTF_1 identifier. Maybe they do something smarter than us.
> Problem is I don't know what yet :-/
>

Seems they do pretty much what we do.

Biggest issue I have is that I actually don't understand the statement:

#define DECLARE_INT_LOG_FUNCTION(level)                            \
extern void wxVLog##level(const wxChar *szFormat, va_list argptr); \
extern void wxLog##level(const wxChar *szFormat, ...) ATTRIBUTE_PRINTF_1

Dave, according to git, you're the one that added this line. Can you do
something to make it work on debian unstable?


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

On Tue, Nov 15, 2011 at 9:22 PM, Guillaume Lelarge
<guillaume@lelarge.info> wrote:
>
> Biggest issue I have is that I actually don't understand the statement:
>
> #define DECLARE_INT_LOG_FUNCTION(level)                            \
> extern void wxVLog##level(const wxChar *szFormat, va_list argptr); \
> extern void wxLog##level(const wxChar *szFormat, ...) ATTRIBUTE_PRINTF_1
>
> Dave, according to git, you're the one that added this line. Can you do
> something to make it work on debian unstable?

## is the concatenation operator, so:

DECLARE_INT_LOG_FUNCTION(Debug)

Should become:

extern void wxVLogDebug(const wxChar *szFormat, va_list argptr); \
extern void wxLogDebug(const wxChar *szFormat, ...) ATTRIBUTE_PRINTF_1

when it goes through the preprocessor (ignoring ATTRIBUTE_PRINTF_1 of course).

I would probably suggest something like Philippe's proposed fix,
though I'm not sure why it's needed (I don't have an affected system
here to play with at the moment).

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

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

philippe rimbault <philippe@rimbault.org> writes:
> There is also this bug (Mon, 24 Oct 2011) from debian FTBFS about pgadmin3:
> http://www.mail-archive.com/debian-bugs-dist@lists.debian.org/msg953221.html
> And this thread about building pgadmin with wx2.9 (i'm using wx2.8 but there
> is the same error):
> http://archives.postgresql.org/pgadmin-hackers/2011-01/msg00125.php

The reason this code has stopped building is that wxwidgets2.8 in Debian
unstable was recently patched to stop defining the ATTRIBUTE_PRINTF* macros
(which are too generically named and cause clashes with other software)
and to instead define WX_ATTRIBUTE_PRINTF* (as wx2.9 already does).  This
patch can be seen here:

http://patch-tracker.debian.org/patch/series/view/wxwidgets2.8/2.8.12.1-3/fix-
macro-namespace-pollution.patch

The bug motivating this change is here:

http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=521924

My take on this is that the ATTRIBUTE_PRINTF* macros weren't intended to be
used outside of the wx headers - there aren't publicly documented anywhere,
and wx upstream has renamed them in 2.9 without mentioning the fact in
docs/changes.txt (which is intended to document all wx API changes).

So I would say it's a bug in pgadmin that it makes use of these macros -
it should define them itself if it needs them (and to avoid clashes with
everyone else who carelessly defines them without thinking through the
implications, it ought to define something like PG_ATTRIBUTE_PRINTF*
instead of ATTRIBUTE_PRINTF*).

Cheers,
    Olly

On Sat, Nov 26, 2011 at 12:38 AM, Olly Betts <olly@survex.com> wrote:
> philippe rimbault <philippe@rimbault.org> writes:
>> There is also this bug (Mon, 24 Oct 2011) from debian FTBFS about pgadmin3:
>> http://www.mail-archive.com/debian-bugs-dist@lists.debian.org/msg953221.html
>> And this thread about building pgadmin with wx2.9 (i'm using wx2.8 but there
>> is the same error):
>> http://archives.postgresql.org/pgadmin-hackers/2011-01/msg00125.php
>
> The reason this code has stopped building is that wxwidgets2.8 in Debian
> unstable was recently patched to stop defining the ATTRIBUTE_PRINTF* macros
> (which are too generically named and cause clashes with other software)
> and to instead define WX_ATTRIBUTE_PRINTF* (as wx2.9 already does).  This
> patch can be seen here:
>
> http://patch-tracker.debian.org/patch/series/view/wxwidgets2.8/2.8.12.1-3/fix-
> macro-namespace-pollution.patch
>
> The bug motivating this change is here:
>
> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=521924

Hmm, thanks for the info. I really thought the Debian guys had stopped
unilaterally breaking wxWidgets like that (downstream patching used to
cause other problems with Debian in years gone by). Reminds me of why
I prefer Redhat.

> My take on this is that the ATTRIBUTE_PRINTF* macros weren't intended to be
> used outside of the wx headers - there aren't publicly documented anywhere,
> and wx upstream has renamed them in 2.9 without mentioning the fact in
> docs/changes.txt (which is intended to document all wx API changes).
>
> So I would say it's a bug in pgadmin that it makes use of these macros -
> it should define them itself if it needs them (and to avoid clashes with
> everyone else who carelessly defines them without thinking through the
> implications, it ought to define something like PG_ATTRIBUTE_PRINTF*
> instead of ATTRIBUTE_PRINTF*).

There are a lot of things that aren't documented in wxWidgets. If we
removed all of them from pgAdmin... well, we just couldn't.
Unfortunately we can't just copy them because the licences aren't
compatible.

Anyway, thanks again for the info - I guess we'll just check to see if
WX_ATTRIBUTE_PRINTF* is defined, and if so use that instead of
ATTRIBUTE_PRINTF*.

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

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

On Sat, Nov 26, 2011 at 08:18:31PM +0000, Dave Page wrote:
> On Sat, Nov 26, 2011 at 12:38 AM, Olly Betts <olly@survex.com> wrote:
> > The reason this code has stopped building is that wxwidgets2.8 in Debian
> > unstable was recently patched to stop defining the ATTRIBUTE_PRINTF* macros
> > (which are too generically named and cause clashes with other software)
> > and to instead define WX_ATTRIBUTE_PRINTF* (as wx2.9 already does).  This
> > patch can be seen here:
> >
> > http://patch-tracker.debian.org/patch/series/view/wxwidgets2.8/2.8.12.1-3/fix-
> > macro-namespace-pollution.patch
> >
> > The bug motivating this change is here:
> >
> > http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=521924
>
> Hmm, thanks for the info. I really thought the Debian guys had stopped
> unilaterally breaking wxWidgets like that (downstream patching used to
> cause other problems with Debian in years gone by). Reminds me of why
> I prefer Redhat.

It's hardly unilateral - wxWidgets 2.9 has made exactly the same change.
So if you view it as "breaking wxWidgets", then it's only backporting
breakage from the next wx release series...

> > My take on this is that the ATTRIBUTE_PRINTF* macros weren't intended to be
> > used outside of the wx headers - there aren't publicly documented anywhere,
> > and wx upstream has renamed them in 2.9 without mentioning the fact in
> > docs/changes.txt (which is intended to document all wx API changes).
> >
> > So I would say it's a bug in pgadmin that it makes use of these macros -
> > it should define them itself if it needs them (and to avoid clashes with
> > everyone else who carelessly defines them without thinking through the
> > implications, it ought to define something like PG_ATTRIBUTE_PRINTF*
> > instead of ATTRIBUTE_PRINTF*).
>
> There are a lot of things that aren't documented in wxWidgets. If we
> removed all of them from pgAdmin... well, we just couldn't.

OTOH, you shouldn't be too surprised if such features disappear from
underneath you.

> Unfortunately we can't just copy them because the licences aren't
> compatible.

This isn't original wx code - a number of projects define equivalent
macros (and often with the same names, which is where the problems lie
when they do it in API headers and you want to use two or more such
projects at once).

So just find one with a licence that's compatible:

http://www.google.com/codesearch#search/&q=ATTRIBUTE_PRINTF&type=cs

E.g. libxml is MIT/X:

http://www.google.com/codesearch#y1w3ejFL0VQ/include/libxml/xmlversion.h&q=ATTRIBUTE_PRINTF&type=cs&l=431

> Anyway, thanks again for the info - I guess we'll just check to see if
> WX_ATTRIBUTE_PRINTF* is defined, and if so use that instead of
> ATTRIBUTE_PRINTF*.

Practically that'll fix it (and will work for 2.9 too), but you might
get bitten again in the future.

Cheers,
    Olly

On Sat, Nov 26, 2011 at 11:05 PM, Olly Betts <olly@survex.com> wrote:
>
>> Hmm, thanks for the info. I really thought the Debian guys had stopped
>> unilaterally breaking wxWidgets like that (downstream patching used to
>> cause other problems with Debian in years gone by). Reminds me of why
>> I prefer Redhat.
>
> It's hardly unilateral - wxWidgets 2.9 has made exactly the same change.
> So if you view it as "breaking wxWidgets", then it's only backporting
> breakage from the next wx release series...

wxWidgets 2.9 is the development code for 3.0. The wx guys have
changed a *lot* of the API there, and intentionally did it in the
2.9/master branch only to avoid breaking the stable 2.8 branch for
existing apps - which is exactly what Debian has now done.

Every wxWidgets developer is expecting to do work to support 2.9/3.0
in their apps - we've already made a lot of changes for example. They
don't expect the downstream distros to cherry-pick changes and
backport them into the stable versions however - if every distro did
that, it would make it nigh-on impossible to support them.

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

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

On Sat, Nov 26, 2011 at 11:05 PM, Olly Betts <olly@survex.com> wrote:
> On Sat, Nov 26, 2011 at 08:18:31PM +0000, Dave Page wrote:
>>
> This isn't original wx code - a number of projects define equivalent
> macros (and often with the same names, which is where the problems lie
> when they do it in API headers and you want to use two or more such
> projects at once).
>
> So just find one with a licence that's compatible:

The irony is that on closer examination we don't actually need the
macro at all, as in a Unicode build of wxWidgets (pgAdmin requires
Unicode support), it gets defined away to nothing. So, the patch ends
up looking like the attached.

Guillaume, please give it a quick check in case I missed anything.

Thanks!

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

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

It seems this is now moot, but meanwhile I asked the wx devs whether
these are undocumented public API or private helper methods, and it
seems worth relaying the reply - at least it will inform anyone
researching this issue who finds this thread:

<tierra> ojwb: that isn't intended to be a publically used
macro, though it is used to help define some public API methods

(tierra is Bryan Petty)

Cheers,
    Olly

On Mon, 2011-11-28 at 17:40 +0000, Dave Page wrote:
> On Sat, Nov 26, 2011 at 11:05 PM, Olly Betts <olly@survex.com> wrote:
> > On Sat, Nov 26, 2011 at 08:18:31PM +0000, Dave Page wrote:
> >>
> > This isn't original wx code - a number of projects define equivalent
> > macros (and often with the same names, which is where the problems lie
> > when they do it in API headers and you want to use two or more such
> > projects at once).
> >
> > So just find one with a licence that's compatible:
>
> The irony is that on closer examination we don't actually need the
> macro at all, as in a Unicode build of wxWidgets (pgAdmin requires
> Unicode support), it gets defined away to nothing. So, the patch ends
> up looking like the attached.
>
> Guillaume, please give it a quick check in case I missed anything.
>

Trying it now...


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

On Sun, 2011-12-04 at 10:37 +0100, Guillaume Lelarge wrote:
> On Mon, 2011-11-28 at 17:40 +0000, Dave Page wrote:
> > On Sat, Nov 26, 2011 at 11:05 PM, Olly Betts <olly@survex.com> wrote:
> > > On Sat, Nov 26, 2011 at 08:18:31PM +0000, Dave Page wrote:
> > >>
> > > This isn't original wx code - a number of projects define equivalent
> > > macros (and often with the same names, which is where the problems lie
> > > when they do it in API headers and you want to use two or more such
> > > projects at once).
> > >
> > > So just find one with a licence that's compatible:
> >
> > The irony is that on closer examination we don't actually need the
> > macro at all, as in a Unicode build of wxWidgets (pgAdmin requires
> > Unicode support), it gets defined away to nothing. So, the patch ends
> > up looking like the attached.
> >
> > Guillaume, please give it a quick check in case I missed anything.
> >
>
> Trying it now...
>

Compiles fine on Fedora 16, Debian Sid, Ubuntu 11.04, Windows XP, and
Mac OS X. So I commited the fix.

Thanks to all who participate in this thread.


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

Thanks!

On Sunday, December 4, 2011, Guillaume Lelarge <guillaume@lelarge.info> wrote:
> On Sun, 2011-12-04 at 10:37 +0100, Guillaume Lelarge wrote:
>> On Mon, 2011-11-28 at 17:40 +0000, Dave Page wrote:
>> > On Sat, Nov 26, 2011 at 11:05 PM, Olly Betts <olly@survex.com> wrote:
>> > > On Sat, Nov 26, 2011 at 08:18:31PM +0000, Dave Page wrote:
>> > >>
>> > > This isn't original wx code - a number of projects define equivalent
>> > > macros (and often with the same names, which is where the problems lie
>> > > when they do it in API headers and you want to use two or more such
>> > > projects at once).
>> > >
>> > > So just find one with a licence that's compatible:
>> >
>> > The irony is that on closer examination we don't actually need the
>> > macro at all, as in a Unicode build of wxWidgets (pgAdmin requires
>> > Unicode support), it gets defined away to nothing. So, the patch ends
>> > up looking like the attached.
>> >
>> > Guillaume, please give it a quick check in case I missed anything.
>> >
>>
>> Trying it now...
>>
>
> Compiles fine on Fedora 16, Debian Sid, Ubuntu 11.04, Windows XP, and
> Mac OS X. So I commited the fix.
>
> Thanks to all who participate in this thread.
>
>
> --
> Guillaume
>  http://blog.guillaume.lelarge.info
>  http://www.dalibo.com
>
>

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

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

Thanks to all !
The last GIT version compile fine on my SID.