Pre-approval for #include adjustments to the testsuite

Pre-approval for #include adjustments to the testsuite

[Zack Weinberg]

I'm working on a patch to compile as much of the testsuite as possible
in _ISOMAC mode, as a step toward compiling the testsuite against
installed headers.  This has a lot of tentacles and isn't even close to
ready for review yet, but the bulk of the changes required turn out to
be boring and repetitive: the wrapper headers include each other
casually, so lots of test programs are missing #includes that they
really should have.  For instance, many of the io/tst-*at.c programs are
getting away with not including sys/stat.h because include/stdlib.h does
it for them.  Here's an excerpt from the changelog for the current
iteration of the patch:

    	* dirent/opendir-tst1.c: Include sys/stat.h.
    	* dirent/tst-fdopendir.c: Include sys/stat.h.
    	* dirent/tst-fdopendir2.c: Include stdlib.h.
    	* dirent/tst-scandir.c: Include stdbool.h.
    	* elf/tst-auditmod1.c: Include link.h and stddef.h.
    	* elf/tst-tls15.c: Include stdlib.h.
    	* elf/tst-tls16.c: Include stdlib.h.
    	* elf/tst-tls17.c: Include stdlib.h.
    	* elf/tst-tls18.c: Include stdlib.h.
    	* iconv/tst-iconv6.c: Include endian.h.
    	* iconvdata/bug-iconv11.c: Include limits.h.
    	* io/test-utime.c: Include stdint.h.
	...

You get the idea.  To reduce churn and make the core patch easier to
review, I would like to commit changes like these piecemeal, ahead of
the main patch.  So I'm asking for blanket pre-approval to commit
additions and/or removals of #include lines to files that are part of
the testsuite, making no other changes.  Is that OK?

zw

Re: Pre-approval for #include adjustments to the testsuite

[Joseph Myers]

On Thu, 16 Feb 2017, Zack Weinberg wrote:

> You get the idea.  To reduce churn and make the core patch easier to
> review, I would like to commit changes like these piecemeal, ahead of
> the main patch.  So I'm asking for blanket pre-approval to commit
> additions and/or removals of #include lines to files that are part of
> the testsuite, making no other changes.  Is that OK?

We already have 
<https://sourceware.org/glibc/wiki/Consensus#Trivial_Bug-Fix_Changes> 
"Anyone can commit a change adding missing #include directives where it is 
clear what the right header is for functionality used in a source file. 
Post the patch and ChangeLog to libc-alpha with a short message and then 
push the commit.".  The intent was for the situation "code compiles on one 
architecture where the sysdeps headers include the required header 
indirectly, doesn't compile on another architecture where the sysdeps 
headers don't have that include".  But I think it's equally reasonable to 
apply it to cases where code is relying on implicit inclusions by wrapper 
headers.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: Pre-approval for #include adjustments to the testsuite

[Carlos O'Donell]

On 02/16/2017 10:12 AM, Joseph Myers wrote:
> On Thu, 16 Feb 2017, Zack Weinberg wrote:
> 
>> You get the idea.  To reduce churn and make the core patch easier to
>> review, I would like to commit changes like these piecemeal, ahead of
>> the main patch.  So I'm asking for blanket pre-approval to commit
>> additions and/or removals of #include lines to files that are part of
>> the testsuite, making no other changes.  Is that OK?
> 
> We already have 
> <https://sourceware.org/glibc/wiki/Consensus#Trivial_Bug-Fix_Changes> 
> "Anyone can commit a change adding missing #include directives where it is 
> clear what the right header is for functionality used in a source file. 
> Post the patch and ChangeLog to libc-alpha with a short message and then 
> push the commit.".  The intent was for the situation "code compiles on one 
> architecture where the sysdeps headers include the required header 
> indirectly, doesn't compile on another architecture where the sysdeps 
> headers don't have that include".  But I think it's equally reasonable to 
> apply it to cases where code is relying on implicit inclusions by wrapper 
> headers.
 
Agreed.

-- 
Cheers,
Carlos.

Re: Pre-approval for #include adjustments to the testsuite

[Zack Weinberg]

On Thu, Feb 16, 2017 at 11:53 AM, Carlos O'Donell <carlos@redhat.com> wrote:
> On 02/16/2017 10:12 AM, Joseph Myers wrote:
>> On Thu, 16 Feb 2017, Zack Weinberg wrote:
>>> I'm asking for blanket pre-approval to commit
>>> additions and/or removals of #include lines to files that are part of
>>> the testsuite, making no other changes.
>>
>> We already have
>> <https://sourceware.org/glibc/wiki/Consensus#Trivial_Bug-Fix_Changes>
>> "Anyone can commit a change adding missing #include directives where it is
>> clear what the right header is for functionality used in a source file.
>
> Agreed.

OK, I will proceed with these patches.

Follow-up question: In several cases, include/ wrappers include
secondary headers that they do not themselves appear to need.  What do
people think of removing such inclusions?  Useful, waste of time,
harmful?  I would not consider such patches to be trivial.

zw

Re: Pre-approval for #include adjustments to the testsuite

[Carlos O'Donell]

On 02/16/2017 11:59 AM, Zack Weinberg wrote:
> On Thu, Feb 16, 2017 at 11:53 AM, Carlos O'Donell <carlos@redhat.com> wrote:
>> On 02/16/2017 10:12 AM, Joseph Myers wrote:
>>> On Thu, 16 Feb 2017, Zack Weinberg wrote:
>>>> I'm asking for blanket pre-approval to commit
>>>> additions and/or removals of #include lines to files that are part of
>>>> the testsuite, making no other changes.
>>>
>>> We already have
>>> <https://sourceware.org/glibc/wiki/Consensus#Trivial_Bug-Fix_Changes>
>>> "Anyone can commit a change adding missing #include directives where it is
>>> clear what the right header is for functionality used in a source file.
>>
>> Agreed.
> 
> OK, I will proceed with these patches.
> 
> Follow-up question: In several cases, include/ wrappers include
> secondary headers that they do not themselves appear to need.  What do
> people think of removing such inclusions?  Useful, waste of time,
> harmful?  I would not consider such patches to be trivial.

I would avoid it for now, only because it detracts from your awesome goal :-)

-- 
Cheers,
Carlos.