Re: PATCH RFA: Build system: Use AC_SYS_LARGEFILE

[Ian Lance Taylor <iant at google dot com>]

David Edelsohn <dje.gcc@gmail.com> writes:

> Also, given this problem, I want to try a meta-experiment: At the GCC
> Summit, some Google developers proposed that any patch causing
> breakage immediately be reverted, following the practice at Google.
> Jakub mentioned on IRC that reverting the patch will break bootstrap
> on i386-linux and other targets.  So Googlers, how do you want to
> proceed and demonstrate your own proposed policy in action?

Although I was regularly interrupted during that discussion at the
summit, I tried a few times to say that in my opinion the policy would
only apply during the first few days after the patch was committed.
It's been a week for this patch now.

Also there is a conflict in that this patch fixed bootstrap for
i686-unknown-linux-gnu, a primary platform, whereas you are telling us
that it breaks bootstrap for powerpc-ibm-aix5.3.0.0 a secondary
platform.  The right step would have been to revert the earlier
simple_object patch, rather than this one.

So on both those grounds I'm not sure an immediate reversion of this
patch is appropriate now, but I will do it if you ask again.


Another approach would be the appended patch.  Does it fix the problem?
Build maintainers, any opinion?

Ian


Index: configure.ac
===================================================================
--- configure.ac	(revision 166441)
+++ configure.ac	(working copy)
@@ -306,6 +306,14 @@ AC_C_INLINE
 
 AC_SYS_LARGEFILE
 
+# flex generated files do not include config.h first, so we need to
+# add _LARGE_FILES to CFLAGS on AIX.
+case $ac_cv_sys_large_files in
+  no | unknown) LFS_CFLAGS= ;;
+  *) LFS_CFLAGS="-D_LARGE_FILES=$ac_cv_sys_large_files" ;;
+esac
+AC_SUBST(LFS_CFLAGS)
+
 # sizeof(char) is 1 by definition.
 AC_CHECK_SIZEOF(void *)
 AC_CHECK_SIZEOF(short)
@@ -1768,7 +1776,7 @@ STMP_FIXINC=stmp-fixinc		AC_SUBST(STMP_F
 # And these apply if build != host, or we are generating coverage data
 if test x$build != x$host || test "x$coverage_flags" != x
 then
-    BUILD_CFLAGS='$(INTERNAL_CFLAGS) $(T_CFLAGS) $(CFLAGS_FOR_BUILD)'
+    BUILD_CFLAGS='$(LFS_CFLAGS) $(INTERNAL_CFLAGS) $(T_CFLAGS) $(CFLAGS_FOR_BUILD)'
     BUILD_LDFLAGS='$(LDFLAGS_FOR_BUILD)'
 fi
 
Index: Makefile.in
===================================================================
--- Makefile.in	(revision 166441)
+++ Makefile.in	(working copy)
@@ -374,7 +378,7 @@ GCC_FOR_TARGET = $(STAGE_CC_WRAPPER) ./x
 # This is used instead of ALL_CFLAGS when compiling with GCC_FOR_TARGET.
 # It specifies -B./.
 # It also specifies -isystem ./include to find, e.g., stddef.h.
-GCC_CFLAGS=$(CFLAGS_FOR_TARGET) $(INTERNAL_CFLAGS) $(T_CFLAGS) $(LOOSE_WARN) $(C_LOOSE_WARN) -Wold-style-definition $($@-warn) -isystem ./include $(TCFLAGS)
+GCC_CFLAGS=$(CFLAGS_FOR_TARGET) $(LFS_CFLAGS) $(INTERNAL_CFLAGS) $(T_CFLAGS) $(LOOSE_WARN) $(C_LOOSE_WARN) -Wold-style-definition $($@-warn) -isystem ./include $(TCFLAGS)
 
 # ---------------------------------------------------
 # Programs which produce files for the target machine
@@ -985,10 +989,10 @@ INTERNAL_CFLAGS = -DIN_GCC @CROSS@
 # This is the variable actually used when we compile. If you change this,
 # you probably want to update BUILD_CFLAGS in configure.ac
 ALL_CFLAGS = $(T_CFLAGS) $(CFLAGS-$@) \
-  $(CFLAGS) $(INTERNAL_CFLAGS) $(COVERAGE_FLAGS) $(WARN_CFLAGS) @DEFS@
+  $(CFLAGS) $(LFS_CFLAGS) $(INTERNAL_CFLAGS) $(COVERAGE_FLAGS) $(WARN_CFLAGS) @DEFS@
 
 # The C++ version.
-ALL_CXXFLAGS = $(T_CFLAGS) $(CXXFLAGS) $(INTERNAL_CFLAGS) \
+ALL_CXXFLAGS = $(T_CFLAGS) $(CXXFLAGS) $(LFS_CFLAGS) $(INTERNAL_CFLAGS) \
   $(COVERAGE_FLAGS) $(WARN_CXXFLAGS) @DEFS@
 
 # Likewise.  Put INCLUDES at the beginning: this way, if some autoconf macro