Patchwork PATCH RFA: Build system: Use AC_SYS_LARGEFILE

login
register
mail settings
Submitter Ian Taylor
Date Nov. 9, 2010, 3:21 p.m.
Message ID <mcriq061q1h.fsf@google.com>
Download mbox | patch
Permalink /patch/70552/
State New
Headers show

Comments

Ian Taylor - Nov. 9, 2010, 3:21 p.m.
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
David Edelsohn - Nov. 9, 2010, 3:33 p.m.
Ian,

With Paolo's patch, my bootstrap has progressed into stage 2.
Hopefully the Flex-generated file is the only case where header files
are included in the wrong order.  Paolo's patch definitely is a step
in the right direction.

Thanks, David

On Tue, Nov 9, 2010 at 10:21 AM, Ian Lance Taylor <iant@google.com> wrote:
> 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
>
>
>
Ian Taylor - Nov. 9, 2010, 4:46 p.m.
David Edelsohn <dje.gcc@gmail.com> writes:

> With Paolo's patch, my bootstrap has progressed into stage 2.
> Hopefully the Flex-generated file is the only case where header files
> are included in the wrong order.  Paolo's patch definitely is a step
> in the right direction.

Paolo's patch makes me a little bit uncomfortable because it means that
gengtype-lex.l is compiled with a different ABI than the rest of the
compiler.  Of course it's a very simple file and it doesn't use any
aspect of the ABI.  But it does call fopen and fclose.  If a call to
fseeko ever sneaks in there somehow, it will break on AIX in a rather
mysterious manner.

Ian

Patch

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