autoconf: AC_SYS_LARGEFILE should output to CPPFLAGS

[Thorsten Glaser]

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.

Let’s keep track of t̲h̲i̲s̲ bug here <***@bugs.debian.org>.
Again, please forward this upstream, too.

The workaround I was forced to use in a real-world package is thus:
https://anonscm.debian.org/cgit/pkg-remote/xrdp.git/plain/debian/patches/lfs.diff?id=e17430063641d44f5596b5bfc1b32ac4ba39f9f1

This feels completely wrong (and breaks with nōn-GCC compilers).

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

[Eric Blake]

Post by Thorsten Glaser

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.

Again, please forward this upstream, too.
https://anonscm.debian.org/cgit/pkg-remote/xrdp.git/plain/debian/patches/lfs.diff?id=e17430063641d44f5596b5bfc1b32ac4ba39f9f1

If the real-world package is not including config.h first, then that is
a bug in the real-world package that should be fixed there. Autoconf
clearly documents that if you create config.h, it MUST be included
first, before any system headers.

[Thorsten Glaser]

severity 852617 wishlist
tags 852617 = upstream
thanks

Post by Eric Blake
If the real-world package is not including config.h first, then that is
a bug in the real-world package that should be fixed there. Autoconf

OK, I’ve (separately) reported the bug to xrdp upstream.

Post by Eric Blake
clearly documents that if you create config.h, it MUST be included
first, before any system headers.

Still not happy with this… it used to “just work”, and this change
is very likely to break existing software (unless — actually, even
if — that requirement was documented “always”, i.e. not added later).

Would you at least *consider* moving the definition back to some
command line argument? (Changing severity to wishlist now; if not,
we can likely close the bug, but I’d still like you to please at
least consider doing the change.)

bye,
//mirabilos

[Zack Weinberg]

Post by Thorsten Glaser
Would you at least *consider* moving the definition back to some
command line argument? (Changing severity to wishlist now; if not,
we can likely close the bug, but I’d still like you to please at
least consider doing the change.)

As far as I can tell from the Git history, AC_SYS_LARGEFILE has
*always* used AC_DEFINE_UNQUOTED to define the various preprocessor
macros that it can define (_FILE_OFFSET_BITS, _LARGE_FILES, and
_DARWIN_USE_64_BIT_INODE). That part of the code has been unchanged
since AC_SYS_LARGEFILE was added to specific.m4.

I can see how you got the impression from the documentation that these
are added to CC instead, but that's actually talking about a different
thing that AC_SYS_LARGEFILE can do -- on *one* system, namely IRIX 6,
it may try adding "-n32" to CC. As a general rule, whenever the
Autoconf manual says that something defines preprocessor macros, you
can assume that it does that with AC_DEFINE(_UNQUOTED).

You're talking about this like it's a regression. Can you please
verify that it really was an Autoconf upgrade that broke the build of
whatever program you're having trouble with, and if so, report what
the older version was?

zw

[Thorsten Glaser]

Post by Zack Weinberg
As far as I can tell from the Git history, AC_SYS_LARGEFILE has
*always* used AC_DEFINE_UNQUOTED to define the various preprocessor
macros that it can define (_FILE_OFFSET_BITS, _LARGE_FILES, and
_DARWIN_USE_64_BIT_INODE).

Interesting, as I recall seeing -D_FILE_OFFSET_BITS=64 on various
compiler command lines when working under GNU. (I normally work
under BSD at home, so I don’t know where exactly.)

Post by Zack Weinberg
That part of the code has been unchanged
since AC_SYS_LARGEFILE was added to specific.m4.

Interesting, when was that? (That is, before 2.13? Anything before
that, even I consider ancient ☻)

bye,
//mirabilos

[Zack Weinberg]

Post by Thorsten Glaser

Post by Zack Weinberg
As far as I can tell from the Git history, AC_SYS_LARGEFILE has
*always* used AC_DEFINE_UNQUOTED to define the various preprocessor
macros that it can define (_FILE_OFFSET_BITS, _LARGE_FILES, and
_DARWIN_USE_64_BIT_INODE).

Interesting, as I recall seeing -D_FILE_OFFSET_BITS=64 on various
compiler command lines when working under GNU. (I normally work
under BSD at home, so I don’t know where exactly.)

Is it possible that those programs were not using a config.h?

Post by Thorsten Glaser

Post by Zack Weinberg
That part of the code has been unchanged
since AC_SYS_LARGEFILE was added to specific.m4.

Interesting, when was that? (That is, before 2.13? Anything before
that, even I consider ancient ☻)

2000-06-08. According to NEWS, it was not in 2.13 (which was released
1999-05-01), but it was in the very next release, 2.50 (2001-05-21).
The ChangeLog entry for the addition says "Import AC_SYS_LARGEFILE
from largefile.m4 serial 12", so that sounds like there was an add-on
.m4 file with the same functionality floating around prior to that - I
don't know where to find copies of that file.

zw

[Paul Eggert]

Post by Zack Weinberg
The ChangeLog entry for the addition says "Import AC_SYS_LARGEFILE
from largefile.m4 serial 12", so that sounds like there was an add-on
.m4 file with the same functionality floating around prior to that - I
don't know where to find copies of that file.

It's from gnulib, which in turn got it from coreutils.

Gnulib still has m4/largefile.m4, although now it's merely a copy of
what's in (the next version of) Autoconf. That is, people use the Gnulib
largefile.m4 because they don't want to wait for the next release of
Autoconf to come out.

[Thorsten Glaser]

Post by Zack Weinberg

Post by Thorsten Glaser
Interesting, as I recall seeing -D_FILE_OFFSET_BITS=64 on various
compiler command lines when working under GNU. (I normally work
under BSD at home, so I don’t know where exactly.)

Is it possible that those programs were not using a config.h?

Hm right, that could have been possible… although I also vaguely
recall seeing it on short compile lines, so there would have been
very few -DHAVE_* on it (but that’s not impossible either).

Post by Zack Weinberg
2000-06-08. According to NEWS, it was not in 2.13 (which was released
1999-05-01), but it was in the very next release, 2.50 (2001-05-21).
The ChangeLog entry for the addition says "Import AC_SYS_LARGEFILE
from largefile.m4 serial 12", so that sounds like there was an add-on
.m4 file with the same functionality floating around prior to that - I

Hm, possible as well then. (I mostly use 2.13 for old software,
2.61 for new software, when building on my BSD. Debian has more
up-to-date autoconf, and only that, nowadays.)

Post by Zack Weinberg
don't know where to find copies of that file.

Me either. Well, if it’s this obscure… I could almost say this
issue can be closed, especially since we seem to be going for‐
wards with the documentation issue. (It can still be confusing
for people switching from no config.h to using config.h, but
the suggested documentation change addresses this somewhat
explicitly.)

bye,
//mirabilos