Re: [PATCH] Fix building xfsprogs on 32-bit platforms

[Eric Biggers <ebiggers@google.com>]

Hi Dave,

On Wed, Nov 30, 2016 at 10:25:49AM +1100, Dave Chinner wrote:
> 
> So, builddefs.in should really detect if autoconf set a "large file
> required" flag, and in that case add the _FILE_OFFSET_BITS value
> that autoconf has generated to the PCFLAGS variable, right? (PCFLAGS
> are "Platform CFLAGS")
> 
> Or, if we are just going to unconditionally define
> -D_FILE_OFFSET_BITS=64 in CFLAGS, then the autoconf checks are
> completely useless and should be removed?
> 

I don't believe it's possible to use the results of AC_SYS_LARGEFILE without
using a config header that is included *first* in all .c files.

So the other option is to define _FILE_OFFSET_BITS=64 ourselves in CFLAGS.  (Or
PCFLAGS for just the platforms that need it; I know it's needed on 32-bit Linux,
but I don't know about other platforms.)  And yes, in that case the autoconf
macros are useless and should be removed.

Sorry for not explaining more in the commit message --- I hadn't looked into the
details of what was going on with the autoconf macros (or that they were
recently added, for that matter) until you pointed them out.

Eric

> > The reason AC_NEED_LFS still passes is that 'configure' includes a
> > temporary header confdefs.h rather than the final generated
> > header, and the temporary header includes the _FILE_OFFSET_BITS
> > definition.
> 
> IOWs, stupid code backed by ambiguous documentation means that
> developers that try to do the right thing still end up with broken
> builds.
> 
> Eric, to speed things up a bit next time, can you try to explain how
> something is broken in the commit message, not just say "it's
> broken, fix it". This avoids the need to go back and forth multiple
> times just so we can understand what the problem is that you are
> fixing....
> 
> Cheers,
> 
> -Dave.
> -- 
> Dave Chinner
> david@fromorbit.com