autoconf: AC_SYS_LARGEFILE documentation misleading

[Thorsten Glaser]

found 158969 2.69-10
thanks

Hi *,

I have to reopen this because the documentation is plain wrong
and misleading still:

-- Macro: AC_SYS_LARGEFILE
Arrange for 64-bit file offsets, known as large-file support
(http://www.unix-systems.org/version2/whatsnew/lfs20mar.html). On
some hosts, one must use special compiler options to build programs
that can access large files. Append any such options to the output
variable 'CC'. Define '_FILE_OFFSET_BITS' and '_LARGE_FILES' if
necessary.

ii autoconf 2.69-10 all automatic configure script builder
ii autoconf-doc 2.69-10 all automatic configure script builder documentatio

Looking at the source, AC_SYS_LARGEFILE calls AC_DEFINE_UNQUOTED,
which appends to DEFS (which I agree with Tom Dickey is wrong) as
Florian Weimer said, but — even more confusingly — only when
AC_CONFIG_HEADERS is not used.

If AC_CONFIG_HEADERS is used, the definition is only written to
its output file (config.h by default).

This is even more unfortunate, because now, if any program includes
a glibc system header before "config.h", the definition will be
ignored, because it comes too late.

CPPFLAGS is r̲e̲a̲l̲l̲y̲ the correct place for this. Again.

But that’s a different bug. This issue is about the documentation
being wrong — please fix the documentation and raise this bug upstream
(and this time, don’t let them tell you it’s not a bug because it most
curtainly is).

Thanks,
//mirabilos (current hat: Debian Developer)

[Eric Blake]

Post by Thorsten Glaser
This is even more unfortunate, because now, if any program includes
a glibc system header before "config.h", the definition will be
ignored, because it comes too late.

This is wrong. The manual clearly states that config.h MUST be included
prior to any system headers, precisely because config.h can influence
what system headers do. In fact, gnulib offers a 'make syntax-check'
that, among other things, enforces that a project ALWAYS includes
config.h first.

Post by Thorsten Glaser
But that’s a different bug. This issue is about the documentation
being wrong — please fix the documentation and raise this bug upstream
(and this time, don’t let them tell you it’s not a bug because it most
curtainly is).

Please propose a patch to the documentation, rather than just telling me
that it is wrong, so that we have a concrete proposal for a wording
improvement that we can discuss.

[Thorsten Glaser]

Post by Eric Blake
Please propose a patch to the documentation, rather than just telling me
that it is wrong, so that we have a concrete proposal for a wording
improvement that we can discuss.

Oh okay, I’ll cater for lazy upstreams this time ;-)
See the attached git format-patch.

bye
//mirabilos

[Eric Blake]

Post by Thorsten Glaser

Post by Eric Blake
Please propose a patch to the documentation, rather than just telling me
that it is wrong, so that we have a concrete proposal for a wording
improvement that we can discuss.

Oh okay, I’ll cater for lazy upstreams this time ;-)

It's not necessarily laziness :)

Post by Thorsten Glaser
See the attached git format-patch.

Thanks; that's helpful. I'm still thinking we may want to iterate on

Post by Thorsten Glaser
+++ b/doc/autoconf.texi
@@ -8584,8 +8584,8 @@ if the system supports @samp{#!}, @samp{no} if not.
Arrange for 64-bit file offsets, known as
@uref{http://@/www.unix-systems@/.org/@/version2/@/whatsnew/@/lfs20mar.html,
large-file support}. On some hosts, one must use special compiler
-options to build programs that can access large files. Append any such

You're deleting all mention that the macro may modify CC (true, it
doesn't do it on most platforms, but the code is still there that does
it for Irix); I don't think we want the change to be that drastic.

Post by Thorsten Glaser
+options to build programs that can access large files. Define,
@code{_FILE_OFFSET_BITS} and @code{_LARGE_FILES} if necessary.

I also think we can try harder to point out the need for config.h to
appear first. How about the following counter-proposal:

diff --git i/doc/autoconf.texi w/doc/autoconf.texi
index 55f96a3..2901937 100644
--- i/doc/autoconf.texi
+++ w/doc/autoconf.texi
@@ -8584,9 +8584,10 @@ System Services
Arrange for 64-bit file offsets, known as
@uref{http://@/www.unix-systems@/.org/@/version2/@/whatsnew/@/lfs20mar.html,
large-file support}. On some hosts, one must use special compiler
-options to build programs that can access large files. Define,
-by calling @code{AC_DEFINE_UNQUOTED},
-@code{_FILE_OFFSET_BITS} and @code{_LARGE_FILES} if necessary.
+options to build programs that can access large files, and this macro
+modifies the output variable @code{CC} in that case. This macro will
+additionally define @code{_FILE_OFFSET_BITS} and @code{_LARGE_FILES} if
+necessary, by calling @code{AC_DEFINE_UNQUOTED}.

Large-file support can be disabled by configuring with the
@option{--disable-largefile} option.
@@ -8595,7 +8596,9 @@ System Services
@code{off_t} is wider than @code{long int}, since this is common when
large-file support is enabled. For example, it is not correct to print
an arbitrary @code{off_t} value @code{X} with @code{printf ("%ld",
-(long int) X)}.
+(long int) X)}. Also, when using this macro in concert with
+@code{AC_CONFIG_HEADERS}, be sure that @file{config.h} is included
+before any system header.

The LFS introduced the @code{fseeko} and @code{ftello} functions to
replace their C counterparts @code{fseek} and @code{ftell} that do not

[Zack Weinberg]

Post by Eric Blake
I also think we can try harder to point out the need for config.h to

...

If we're going to warn people about this in the context of specific
macros we should do the same for AC_USE_SYSTEM_EXTENSIONS as well. I
don't *think* there are any other stock macros that put
feature-selection #defines into config.h but I could have missed
something.

zw

[Thorsten Glaser]

Post by Eric Blake
Thanks; that's helpful. I'm still thinking we may want to iterate on

Yes, of course; you know autoconf much better than I do.

Post by Eric Blake
You're deleting all mention that the macro may modify CC (true, it
doesn't do it on most platforms, but the code is still there that does
it for Irix)

Ah, okay. Let’s change that then.

Post by Eric Blake
I also think we can try harder to point out the need for config.h to
diff --git i/doc/autoconf.texi w/doc/autoconf.texi
index 55f96a3..2901937 100644
--- i/doc/autoconf.texi
+++ w/doc/autoconf.texi
@@ -8584,9 +8584,10 @@ System Services
Arrange for 64-bit file offsets, known as
@uref{http://@/www.unix-systems@/.org/@/version2/@/whatsnew/@/lfs20mar.html,
large-file support}. On some hosts, one must use special compiler
-options to build programs that can access large files. Define,
+options to build programs that can access large files, and this macro

Nice, but I think I can improve on that a bit (later, I’m almost
dropping down due to tiredness), it still confuses me a bit ;)

Post by Eric Blake
Large-file support can be disabled by configuring with the
@option{--disable-largefile} option.
@@ -8595,7 +8596,9 @@ System Services
@code{off_t} is wider than @code{long int}, since this is common when
large-file support is enabled. For example, it is not correct to print
-(long int) X)}.
+(long int) X)}. Also, when using this macro in concert with
+before any system header.

Making that explicit is good, yes. Thank you.

Post by Eric Blake
If we're going to warn people about this in the context of specific
macros we should do the same for AC_USE_SYSTEM_EXTENSIONS as well. I

Feel free to… but that’s outside of my scope; I never heard of that
particular macro ;)

bye,
//mirabilos

[Thorsten Glaser]

Dixi quod…

Post by Thorsten Glaser

Post by Eric Blake
I also think we can try harder to point out the need for config.h to

[…]

Post by Thorsten Glaser
Nice, but I think I can improve on that a bit (later, I’m almost
dropping down due to tiredness), it still confuses me a bit ;)

What do you think about this? I tried several variants and ended
up making it a bullet point list, and I think the result is quite
less ambiguous (especially if one is not a native English speaker)
than the original text.

I’ve also changed “be sure” to an imperative “ensure” in the
usage notes.

bye,
//mirabilos

[Thorsten Glaser]

Dixi quod


Post by Thorsten Glaser
What do you think about this? I tried several variants and ended

Arrgh, forgot the attachment, sorry.

bye,
//mirabilos