Re: [PATCH] Unify Solaris procfs and largefile handling

[Simon Marchi <simark@simark.ca>]

On 2020-07-30 5:17 a.m., Rainer Orth wrote:
> Hi Simon,
> 
>> On 2020-07-29 7:19 a.m., Rainer Orth wrote:
>>> It's even simpler: every configure script has code to parse
>>> --enable-foo/--disable-foo and turn the result into enable_foo=[yes|no].  
>>
>> Ok, nice!
>>
>>> No: the code has been (and should remain) like this.  It allows the user
>>> to override the automatic largefile detection with explicit
>>> --enable-largefile/--disable-largefile options without having to change
>>> the code.
>>
>> Ack.
>>
>>> I've now removed AC_ARG_ENABLE from largefile.m4.  Retested on
>>> i386-pc-solaris2.11 without and with --disable-gdb, checking that
>>> _FILE_OFFSET_BITS are set as expected, and amd64-pc-solaris2.11.
>>>
>>> Ok for master now?
>>
>> When I run `autoreconf -vf`, I get a lot of changes.  Make sure to run
>> it in any directory you touch that has a configure.ac and add the resulting
>> changes.
> 
> I didn't use autoreconf, but ran the appropriate
> autoconf/autoheader/aclocal/automake dance manually.  With one exception
> (LARGEFILE_CPPFLAGS in gnulib Makefile.in) I'd gotten things right :-)
> 
> I don't usually include generated files in patch submissions, though:
> they are heavily frowned upon at least over in GCC because they make
> review quite difficult, espcially in a case like this where the
> largefile.m4 change spreads to lots of configure scripts, obscuring the
> change proper.
> 
> Does GDB handle things differently here?

I don't think there's a hard rule.  If it makes the patch too big for sending on
the list, then it's fine for sure to not include them.  If you don't want to include
them, that's fine with my too, but in either case it's important to say that you've
omitted them on purpose so we know it's not an oversight (otherwise I'll complain
about them missing :)).

Maybe it can be inconvenient for people who read the diff directly to review... I
personally use meld to review patches, so it's simple to just skip over generated
files.

The patch LGTM, thanks.

Simon