Thread: plperl warnings on win32
When building with MingW, we get a ton of warnings of the type: C:/Perl/lib/CORE/config.h:39:20: warning: "/*" within comment (see http://pgbuildfarm.org/cgi-bin/show_stage_log.pl?nm=vaquita&dt=2007-07-23%20200011&stg=make) Attached patch removes this by disabling the warning specifically for plperl on mingw. Any objections? If not, I'll go ahead and apply... //Magnus
Attachment
"Magnus Hagander" <magnus@hagander.net> writes: > When building with MingW, we get a ton of warnings of the type: > C:/Perl/lib/CORE/config.h:39:20: warning: "/*" within comment > > (see > http://pgbuildfarm.org/cgi-bin/show_stage_log.pl?nm=vaquita&dt=2007-07-23%20200011&stg=make) Huh, what's actually in your /Perl/lib/CORE/config.h ? Perhaps older versions of perl weren't so careful but on my machine perl generates a config.h like this: /* HAS_GETPGRP2: * This symbol, if defined, indicates that the getpgrp2() (as in DG/UX) * routine is available to get the current process group. */ /*#define HAS_GETPGRP2 / **/ /* HAS_GETPPID: * This symbol, if defined, indicates that the getppid routine is * available to get the parent process ID. */ #define HAS_GETPPID /**/ -- Gregory Stark EnterpriseDB http://www.enterprisedb.com
On Tue, Jul 24, 2007 at 12:42:38PM +0100, Gregory Stark wrote: > > "Magnus Hagander" <magnus@hagander.net> writes: > > > When building with MingW, we get a ton of warnings of the type: > > C:/Perl/lib/CORE/config.h:39:20: warning: "/*" within comment > > > > (see > > http://pgbuildfarm.org/cgi-bin/show_stage_log.pl?nm=vaquita&dt=2007-07-23%20200011&stg=make) > > Huh, what's actually in your /Perl/lib/CORE/config.h ? /*#define HAS_BCMP /**/ and similar. > Perhaps older versions of perl weren't so careful but on my machine perl > generates a config.h like this: The difference is in the space. This is with perl 5.8.8.819 from ActiveState - the latest is 5.8.8.820, and I doubt they've changed that there. //Magnus
"Magnus Hagander" <magnus@hagander.net> writes: > /*#define HAS_BCMP /**/ > > The difference is in the space. This is with perl 5.8.8.819 from > ActiveState - the latest is 5.8.8.820, and I doubt they've changed > that there. Huh, I'm on 5.8.8 here as well. I suppose ActiveState has to generate the config.h file themselves and they missed this detail. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com
Magnus Hagander wrote: > When building with MingW, we get a ton of warnings of the type: > C:/Perl/lib/CORE/config.h:39:20: warning: "/*" within comment > > (see > http://pgbuildfarm.org/cgi-bin/show_stage_log.pl?nm=vaquita&dt=2007-07-23%20200011&stg=make) > > Attached patch removes this by disabling the warning specifically for > plperl on mingw. > > Any objections? If not, I'll go ahead and apply... > > I recall I hacked the perl header on one machine to silence this. I guess this is acceptable. We really should grumble loudly at the perl/AS people, but then we cater for all sorts of header oddities elsewhere too, so this isn't much different. cheers andrew
Magnus Hagander <magnus@hagander.net> writes:
> override CPPFLAGS += -DPLPERL_HAVE_UID_GID
> + # Perl on win32 contains /* within comment all over the header file,
> + # so disable this warning.
> + override CFLAGS += -Wno-comment
> endif
If you insist you can apply that in some way that makes it Windows-only.
I object strongly to the patch in this form, because it will mask our
own mistakes as well as one particular Perl build's mistakes.
regards, tom lane
On Tue, Jul 24, 2007 at 09:55:57AM -0400, Tom Lane wrote: > Magnus Hagander <magnus@hagander.net> writes: > > override CPPFLAGS += -DPLPERL_HAVE_UID_GID > > + # Perl on win32 contains /* within comment all over the header file, > > + # so disable this warning. > > + override CFLAGS += -Wno-comment > > endif > > If you insist you can apply that in some way that makes it Windows-only. > I object strongly to the patch in this form, because it will mask our > own mistakes as well as one particular Perl build's mistakes. Uh, it's already inside a win32-only block. So it'll only affect mingw. Do you know of a way to do it from inside gcc, like the #pragma that exists for win32 to disable specific warnings? If so we could just disable it on the line before we #include the perl header, and re-enable it after... //Magnus
Magnus Hagander wrote: > On Tue, Jul 24, 2007 at 09:55:57AM -0400, Tom Lane wrote: > >> Magnus Hagander <magnus@hagander.net> writes: >> >>> override CPPFLAGS += -DPLPERL_HAVE_UID_GID >>> + # Perl on win32 contains /* within comment all over the header file, >>> + # so disable this warning. >>> + override CFLAGS += -Wno-comment >>> endif >>> >> If you insist you can apply that in some way that makes it Windows-only. >> I object strongly to the patch in this form, because it will mask our >> own mistakes as well as one particular Perl build's mistakes. >> > > Uh, it's already inside a win32-only block. So it'll only affect mingw. > > Do you know of a way to do it from inside gcc, like the #pragma that exists > for win32 to disable specific warnings? If so we could just disable it on > the line before we #include the perl header, and re-enable it after... > see http://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Pragmas.html#Diagnostic-Pragmas cheers andrew
On Tue, Jul 24, 2007 at 10:11:52AM -0400, Andrew Dunstan wrote: > > > Magnus Hagander wrote: > >On Tue, Jul 24, 2007 at 09:55:57AM -0400, Tom Lane wrote: > > > >>Magnus Hagander <magnus@hagander.net> writes: > >> > >>> override CPPFLAGS += -DPLPERL_HAVE_UID_GID > >>>+ # Perl on win32 contains /* within comment all over the header file, > >>>+ # so disable this warning. > >>>+ override CFLAGS += -Wno-comment > >>> endif > >>> > >>If you insist you can apply that in some way that makes it Windows-only. > >>I object strongly to the patch in this form, because it will mask our > >>own mistakes as well as one particular Perl build's mistakes. > >> > > > >Uh, it's already inside a win32-only block. So it'll only affect mingw. > > > >Do you know of a way to do it from inside gcc, like the #pragma that exists > >for win32 to disable specific warnings? If so we could just disable it on > >the line before we #include the perl header, and re-enable it after... > > > > see > > http://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Pragmas.html#Diagnostic-Pragmas This seems to be new in gcc 4.2, so we can't really use that :( //Magnus
Magnus Hagander wrote: > On Tue, Jul 24, 2007 at 09:55:57AM -0400, Tom Lane wrote: > >> Magnus Hagander <magnus@hagander.net> writes: >> >>> override CPPFLAGS += -DPLPERL_HAVE_UID_GID >>> + # Perl on win32 contains /* within comment all over the header file, >>> + # so disable this warning. >>> + override CFLAGS += -Wno-comment >>> endif >>> >> If you insist you can apply that in some way that makes it Windows-only. >> I object strongly to the patch in this form, because it will mask our >> own mistakes as well as one particular Perl build's mistakes. >> > > Uh, it's already inside a win32-only block. So it'll only affect mingw. > > Do you know of a way to do it from inside gcc, like the #pragma that exists > for win32 to disable specific warnings? If so we could just disable it on > the line before we #include the perl header, and re-enable it after... > > An alternative might be to provide a perl script in the tools directory which would fix the perl header file. cheers andrew
Magnus Hagander <magnus@hagander.net> writes:
> On Tue, Jul 24, 2007 at 09:55:57AM -0400, Tom Lane wrote:
>> If you insist you can apply that in some way that makes it Windows-only.
> Uh, it's already inside a win32-only block. So it'll only affect mingw.
Doh ... not enough caffeine yet ... sorry for the noise.
regards, tom lane
Andrew Dunstan <andrew@dunslane.net> writes:
> An alternative might be to provide a perl script in the tools directory
> which would fix the perl header file.
Seems like more work than it's worth. I'm OK with Magnus' proposed
Makefile hack --- though someone should also complain to ActiveState
so maybe they'll fix it sometime.
regards, tom lane
On Tue, Jul 24, 2007 at 12:11:03PM -0400, Tom Lane wrote: > Andrew Dunstan <andrew@dunslane.net> writes: > > An alternative might be to provide a perl script in the tools directory > > which would fix the perl header file. > > Seems like more work than it's worth. I'm OK with Magnus' proposed > Makefile hack --- though someone should also complain to ActiveState > so maybe they'll fix it sometime. Applied and complained, activestate bugid 71303. //Magnus