[check_GNU_style.sh] More aggressively ignore dg-xxx directives

On 12/10/16 11:18, Kyrill Tkachov wrote:
>
> On 12/10/16 10:57, Kyrill Tkachov wrote:
>>
>> On 11/10/16 20:19, Jakub Jelinek wrote:
>>> On Tue, Oct 11, 2016 at 01:11:04PM -0600, Martin Sebor wrote:
>>>> Also, the pattern that starts with "/\+\+\+" looks like it's missing
>>>> the ^ anchor.  Presumably it should be "/^\+\+\+ \/testsuite\//".
>>> No, it will be almost never +++ /testsuite/
>>> There needs to be .* in between "+++ " and "/testsuite/", and perhaps
>>> it should also ignore "+++ testsuite/".
>>> So /^\+\+\+ (.*\/)?testsuite\// ?
>>> Also, normally (when matching $0) there won't be newlines in the text.
>>>
>>>     Jakub
>>
>> Thanks.
>> Here is the updated patch with your suggestions.
>>
>
> Actually, I've encountered a problem:
>
>  85 # Remove the testsuite part of the diff.  We don't care about GNU style
>  86 # in testcases and the dg-* directives give too many false positives.
>  87 remove_testsuite ()
>  88 {
>  89   awk 'BEGIN{testsuite=0} /\+\+\+ / && ! /testsuite\//{testsuite=0} \
>  90        {if (!testsuite) print} /^\+\+\+ (.*\/)?testsuite\//{testsuite=1}'
>  91 }
>  92
>  93 grep $format '^+' $files \
>  94     | remove_testsuite \
>  95     | grep -v ':+++' \
>  96     > $inp
>
>
> The /^\+\+\+ (.*\/)?testsuite\// doesn't ever match when the ^ anchor is used.
> The awk command matches fine by itself but not when fed from the "grep $format '^+' $files"
> command because grep adds the line numbers and file names.
> So is it okay to omit the ^ here?
>

Here is the patch omitting the ^.
Thanks,
Kyrill

2016-10-12  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     * check_GNU_style.sh (remove_testsuite): New function.
     Use it to remove testsuite from the diff.

> Thanks,
> Kyrill
>

On 10/12/2016 06:43 AM, Kyrill Tkachov wrote:
>
> On 12/10/16 11:18, Kyrill Tkachov wrote:
>>
>> On 12/10/16 10:57, Kyrill Tkachov wrote:
>>>
>>> On 11/10/16 20:19, Jakub Jelinek wrote:
>>>> On Tue, Oct 11, 2016 at 01:11:04PM -0600, Martin Sebor wrote:
>>>>> Also, the pattern that starts with "/\+\+\+" looks like it's missing
>>>>> the ^ anchor.  Presumably it should be "/^\+\+\+ \/testsuite\//".
>>>> No, it will be almost never +++ /testsuite/
>>>> There needs to be .* in between "+++ " and "/testsuite/", and perhaps
>>>> it should also ignore "+++ testsuite/".
>>>> So /^\+\+\+ (.*\/)?testsuite\// ?
>>>> Also, normally (when matching $0) there won't be newlines in the text.
>>>>
>>>>     Jakub
>>>
>>> Thanks.
>>> Here is the updated patch with your suggestions.
>>>
>>
>> Actually, I've encountered a problem:
>>
>>  85 # Remove the testsuite part of the diff.  We don't care about GNU
>> style
>>  86 # in testcases and the dg-* directives give too many false positives.
>>  87 remove_testsuite ()
>>  88 {
>>  89   awk 'BEGIN{testsuite=0} /\+\+\+ / && ! /testsuite\//{testsuite=0} \
>>  90        {if (!testsuite) print} /^\+\+\+
>> (.*\/)?testsuite\//{testsuite=1}'
>>  91 }
>>  92
>>  93 grep $format '^+' $files \
>>  94     | remove_testsuite \
>>  95     | grep -v ':+++' \
>>  96     > $inp
>>
>>
>> The /^\+\+\+ (.*\/)?testsuite\// doesn't ever match when the ^ anchor
>> is used.
>> The awk command matches fine by itself but not when fed from the "grep
>> $format '^+' $files"
>> command because grep adds the line numbers and file names.
>> So is it okay to omit the ^ here?

I think the AWK regex will not work correctly when the patch has
the line number prefix like "1234: " (AFAICT, this can only happen
in the second invocation of the remove_testsuite function which
also has the problem below making me wonder if your testing
exercised that mode).

I think the AWK regex needs to be changed to handle that.  It should
start with something like "^([1-9][0-9]*:)?\+\+\+"

I tried to test the patch but it doesn't seem to work.  When passed
a patch as an argument it hangs.  The hunk below isn't quite right:

  	# Don't reuse $inp, which may be generated using -H and thus contain a
-	# file prefix.
-	grep -n '^+' $f \
+	# file prefix.  Re-remove the testsuite since we're not using $inp.
+	remove_testsuite $f \
+	    | grep -n '^+' \
  	    | grep -v ':+++' \
  	    > $tmp

The remove_testsuite function ignores arguments so passing $f to it
won't do anything except hang waiting for input.  This should look
closer to this (it worked in my very limited testing):

         cat $f | remove_testsuite \

Martin