Thread: Severe regression in autoconf 2.61
There seems to have been a bit of a brain cramp upstream :-(.
Previously, AC_FUNC_FSEEKO did this to test if fseeko was available:
return !fseeko;
Now it does this:
return fseeko (stdin, 0, 0) && (fseeko) (stdin, 0, 0);
Unfortunately, that gives the compiler enough of a syntactic clue
to guess that fseeko is probably an undeclared function, and therefore
*it will not error out*, only generate a warning, if it's not seen
a declaration for fseeko.
The proximate result of this in our HEAD is that configure fails to
detect that _LARGEFILE_SOURCE is needed, resulting in a rather broken
build on platforms where that really is needed. I had mis-blamed this
on Bruce's recent NetBSD/BSDi hack, but I think it's been there since
we installed 2.61 autoconf. I suspect that in fact the problem Bruce
was seeing was due to this very bug, and that what we need to do is
revert his BSD-specific patch and find a different solution.
regards, tom lane
Tom Lane wrote: > There seems to have been a bit of a brain cramp upstream :-(. > Previously, AC_FUNC_FSEEKO did this to test if fseeko was available: > > return !fseeko; > > Now it does this: > > return fseeko (stdin, 0, 0) && (fseeko) (stdin, 0, 0); > > Unfortunately, that gives the compiler enough of a syntactic clue > to guess that fseeko is probably an undeclared function, and therefore > *it will not error out*, only generate a warning, if it's not seen > a declaration for fseeko. > > The proximate result of this in our HEAD is that configure fails to > detect that _LARGEFILE_SOURCE is needed, resulting in a rather broken > build on platforms where that really is needed. I had mis-blamed this > on Bruce's recent NetBSD/BSDi hack, but I think it's been there since > we installed 2.61 autoconf. I suspect that in fact the problem Bruce > was seeing was due to this very bug, and that what we need to do is > revert his BSD-specific patch and find a different solution. I am not sure this explains the BSD case. NetBSD/BSDi uses fsetpos/fgetpos to implement fseeko/ftello. What I found with autoconf 2.59 was that setting ac_cv_func_fseeko=yes was enough so AC_FUNC_FSEEKO thought fseeko exists. What I am finding now is that the AC_FUNC_FSEEKO macro doesn't use ac_cv_func_fseeko in the same way and I have to force HAVE_FSEEKO to 1, while in previous versions of autoconf this was done for me. The change that is causing me problems I think is: if test $ac_cv_func_fseeko = yes; then AC_DEFINE(HAVE_FSEEKO, 1, [Define to 1 if fseeko (and presumably ftello) existsand is declared.])fi changed to: if test $ac_cv_sys_largefile_source != unknown; then AC_DEFINE(HAVE_FSEEKO, 1, [Define to 1 if fseeko (and presumablyftello) exists and is declared.])fi I don't really understand why ac_cv_sys_largefile_source is now being tested. I put back the patch now that you are saying it isn't based on the BSD* fix. Once you find a more general fix I will test the BSD* cases again. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://postgres.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian <bruce@momjian.us> writes:
> I am not sure this explains the BSD case. NetBSD/BSDi uses
> fsetpos/fgetpos to implement fseeko/ftello.
What exactly do you mean by "uses" --- are fseeko and ftello declared
as macros that call the other two, or what?
I'd kinda have thought that the new coding of AC_FUNC_FSEEKO would
work well with macros. What it *doesn't* work well, or at all,
with is
#ifdef _LARGEFILE_SOURCEextern int fseeko(...#endif
which is exactly what the test was originally supposed to find out
about :-(
> I don't really understand why ac_cv_sys_largefile_source is now being
> tested.
I think the idea is that by this point, ac_cv_sys_largefile_source is
set to 1 if you need _LARGEFILE_SOURCE to see a definition of fseeko
(as above), or to "no" if you see a definition of fseeko without
_LARGEFILE_SOURCE, or to "unknown" if you don't get fseeko either way.
So that test makes sense in context. The problem is that someone
subsequently broke the method for testing whether fseeko is declared.
regards, tom lane
Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > I am not sure this explains the BSD case. NetBSD/BSDi uses > > fsetpos/fgetpos to implement fseeko/ftello. > > What exactly do you mean by "uses" --- are fseeko and ftello declared > as macros that call the other two, or what? There are ftello/ftello implementions in our port/fseeko.c; prototypes are in our include/port.h. > I'd kinda have thought that the new coding of AC_FUNC_FSEEKO would > work well with macros. What it *doesn't* work well, or at all, > with is > > #ifdef _LARGEFILE_SOURCE > extern int fseeko(... > #endif > > which is exactly what the test was originally supposed to find out > about :-( > > > I don't really understand why ac_cv_sys_largefile_source is now being > > tested. > > I think the idea is that by this point, ac_cv_sys_largefile_source is > set to 1 if you need _LARGEFILE_SOURCE to see a definition of fseeko > (as above), or to "no" if you see a definition of fseeko without > _LARGEFILE_SOURCE, or to "unknown" if you don't get fseeko either way. > So that test makes sense in context. The problem is that someone > subsequently broke the method for testing whether fseeko is declared. OK, based on the way they are doing things now I can't inject a ac_cv_sys_largefile_source=no from outside that function so the BSD* workaround I just did is necessary, and probably less prone to breakage. Have you see these lines lower in configure.in? if test $ac_cv_func_fseeko = yes; thenAC_SYS_LARGEFILEfi Is this broken too? It just seemed more straight-forward when the defined HAVE_FSEEKO based on ac_cv_func_fseeko rather than ac_cv_sys_largefile_source. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://postgres.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian <bruce@momjian.us> writes:
> Have you see these lines lower in configure.in?
> if test $ac_cv_func_fseeko = yes; then
> AC_SYS_LARGEFILE
> fi
> Is this broken too?
Yeah, I thought so at first, but looking closer I think it's not too
relevant to the problem. This is for testing whether a couple of *other*
macros need to be defined, it's not about _LARGEFILE_SOURCE.
> It just seemed more straight-forward when the defined HAVE_FSEEKO based
> on ac_cv_func_fseeko rather than ac_cv_sys_largefile_source.
Well, I think 2.61's treatment of fseeko is simply broken. I'm tempted
to propose fixing this by defining PGAC_FUNC_SEEKO the same way 2.59
defined AC_FUNC_SEEKO, and then we wouldn't need the changes you've been
making.
But someone ought to kick this upstream and ask what they were thinking.
regards, tom lane
On Mon, 18 Feb 2008, Tom Lane wrote: > There seems to have been a bit of a brain cramp upstream :-(. > Previously, AC_FUNC_FSEEKO did this to test if fseeko was available: > > return !fseeko; > > Now it does this: > > return fseeko (stdin, 0, 0) && (fseeko) (stdin, 0, 0); > > Unfortunately, that gives the compiler enough of a syntactic clue > to guess that fseeko is probably an undeclared function, and therefore > *it will not error out*, only generate a warning, if it's not seen > a declaration for fseeko. > So that's what that was. I had the same problem in another project I was working on (which I used some PostgreSQL configure code in). I had to add this in the gcc section of configure: PGAC_PROG_CC_CFLAGS_OPT([-Werror-implicit-function-declaration]) But it would be nice to find a better fix. I don't understand how calling a function that has not been defined yet is ever not an error. -- In 1915 pancake make-up was invented but most people still preferred syrup.
Am Dienstag, 19. Februar 2008 schrieb Tom Lane: > Previously, AC_FUNC_FSEEKO did this to test if fseeko was available: > return !fseeko; > Now it does this: > return fseeko (stdin, 0, 0) && (fseeko) (stdin, 0, 0); > > Unfortunately, that gives the compiler enough of a syntactic clue > to guess that fseeko is probably an undeclared function, and therefore > *it will not error out*, only generate a warning, if it's not seen > a declaration for fseeko. Please try the attached patch. What is currently the consequence of the problem? Does it fail to build, fail to run, or does it fail with large files? -- Peter Eisentraut http://developer.postgresql.org/~petere/
Peter Eisentraut <peter_e@gmx.net> writes:
> Please try the attached patch.
Shortly.
> What is currently the consequence of the problem? Does it fail to build, fail
> to run, or does it fail with large files?
The consequence of the problem is that pg_dump/pg_restore are compiled
without any visible prototypes for fseeko/ftello, which has implications
that'd depend on the architecture's rules for passing/returning off_t
as opposed to int. I'd say "not work at all" is possible and "not work
for large files" is certain. The backend doesn't seem to use these
functions so it shouldn't be affected.
regards, tom lane
Peter Eisentraut <peter_e@gmx.net> writes:
> Am Dienstag, 19. Februar 2008 schrieb Tom Lane:
>> Unfortunately, that gives the compiler enough of a syntactic clue
>> to guess that fseeko is probably an undeclared function, and therefore
>> *it will not error out*, only generate a warning, if it's not seen
>> a declaration for fseeko.
> Please try the attached patch.
Seems to fix the problem, please apply.
regards, tom lane