[PATCH,contrib] Add option to patch tester to avoid caching baseline test results.

[PATCH,contrib] Add option to patch tester to avoid caching baseline test results.

[Dave Korn]

[-- Attachment #1: Type: text/plain, Size: 1562 bytes --]


    Hi,

  I discovered a problem with the patch_tester.sh script in the contrib/
directory.  By design, the script caches the results of the clean test run it
performs against any given branch/revision combination, which saves a lot of
time when testing a whole bunch of patches.  However, it can give false
results in the case when testing one patch against a variety of different
configure options.  Different configuration options can alter the baseline set
of FAILs that the testsuite will produce, meaning that the cached results of
the clean test run may falsely fail to match the results from the test run
done against the patched build if they originally came from a run with
different configure options - the particular case that bit me was compiling
with --disable-shared, which causes a load of mudflap FAILs to appear(*).

  This patch addresses that problem by adding a command-line option that
prevents the use of cached results, forcing the baseline results to be
regenerated anew for each patch that is tested.

contrib/ChangeLog:

	* patch_tester.sh (nopristinecache): New shell var, set according
	to presence or absence of new -nopristinecache option.
	(usage): Document new option.
	(bootntest_pristine): Implement it.

  OK?

    cheers,
      DaveK
-- 
(*) - owing to a similar problem as occurs for lto-plugin in PR42690: a
compiler built with --disable-shared effectively has -static in effect for all
compilations, but since there is no explicit -static on the command-line, any
spec strings keyed off that option don't get activated.

[-- Attachment #2: patch-tester-nocache.diff --]
[-- Type: text/x-c, Size: 1949 bytes --]

Index: contrib/patch_tester.sh
===================================================================
--- contrib/patch_tester.sh	(revision 166694)
+++ contrib/patch_tester.sh	(working copy)
@@ -36,13 +36,14 @@ standby=$default_standby
 default_watermark=0.60
 watermark=$default_watermark
 savecompilers=false
+nopristinecache=false
 nogpg=false
 stop=false
 
 usage() {
     cat <<EOF
 patch_tester.sh [-j<N>] [-standby N] [-watermark N] [-savecompilers] [-nogpg]
-                [-svnpath URL] [-stop]
+                [-svnpath URL] [-stop] [-nopristinecache]
                 <source_dir> [patches_dir [state_dir [build_dir]]]
 
     J is the flag passed to make.  Default is empty string.
@@ -56,6 +57,12 @@ patch_tester.sh [-j<N>] [-standby N] [-watermark N
     SAVECOMPILERS copies the compilers in the same directory as the
     test results for the non patched version.  Default is not copy.
 
+    NOPRISTINECACHE prevents use of cached test results from any earlier
+    test runs on the pristine version of the branch and revision under
+    test (the default behaviour).  This should be used when testing the
+    same revision and patch with multiple sets of configure options, as
+    these may affect the set of baseline failures.
+
     NOGPG can be used to avoid checking the GPG signature of patches.
 
     URL is the location of the GCC SVN repository.  The default is
@@ -103,6 +110,9 @@ while [ $# -ne 0 ]; do
 	-savecompilers)
 	    savecompilers=true; shift
 	    ;;
+	-nopristinecache)
+	    nopristinecache=true; shift
+	    ;;
 	-nogpg)
 	    nogpg=true; shift
 	    ;;
@@ -366,6 +376,9 @@ bootntest_pristine () {
     current_version=`svn info $SOURCE | grep "^Revision:" | sed -e "s/^Revision://g" -e "s/ //g"`
     PRISTINE=$STATE/$current_branch/$current_version
 
+    if [ $nopristinecache = true ]; then
+      rm -rf $PRISTINE
+    fi
     if [ -d $PRISTINE ]; then
 	ln -s $PRISTINE $TESTING/pristine
 	return 0

Re: [PATCH,contrib] Add option to patch tester to avoid caching baseline test results.

[Dave Korn]

On 15/11/2010 15:43, Dave Korn wrote:

>   I discovered a problem with the patch_tester.sh script in the contrib/
> directory.  By design, the script caches the results of the clean test run it
> performs against any given branch/revision combination, which saves a lot of
> time when testing a whole bunch of patches.  However, it can give false
> results in the case when testing one patch against a variety of different
> configure options.  Different configuration options can alter the baseline set
> of FAILs that the testsuite will produce, meaning that the cached results of
> the clean test run may falsely fail to match the results from the test run
> done against the patched build if they originally came from a run with
> different configure options - the particular case that bit me was compiling
> with --disable-shared, which causes a load of mudflap FAILs to appear(*).
> 
>   This patch addresses that problem by adding a command-line option that
> prevents the use of cached results, forcing the baseline results to be
> regenerated anew for each patch that is tested.
> 
> contrib/ChangeLog:
> 
> 	* patch_tester.sh (nopristinecache): New shell var, set according
> 	to presence or absence of new -nopristinecache option.
> 	(usage): Document new option.
> 	(bootntest_pristine): Implement it.
> 
>   OK?

  Ping?

    cheers,
      DaveK

[PING^2] Re: [PATCH,contrib] Add option to patch tester to avoid caching baseline test results.

[Dave Korn]

On 02/12/2010 04:51, Dave Korn wrote:
> On 15/11/2010 15:43, Dave Korn wrote:
> 
>>   I discovered a problem with the patch_tester.sh script in the contrib/
>> directory.  By design, the script caches the results of the clean test run it
>> performs against any given branch/revision combination, which saves a lot of
>> time when testing a whole bunch of patches.  However, it can give false
>> results in the case when testing one patch against a variety of different
>> configure options.  Different configuration options can alter the baseline set
>> of FAILs that the testsuite will produce, meaning that the cached results of
>> the clean test run may falsely fail to match the results from the test run
>> done against the patched build if they originally came from a run with
>> different configure options - the particular case that bit me was compiling
>> with --disable-shared, which causes a load of mudflap FAILs to appear(*).
>>
>>   This patch addresses that problem by adding a command-line option that
>> prevents the use of cached results, forcing the baseline results to be
>> regenerated anew for each patch that is tested.
>>
>> contrib/ChangeLog:
>>
>> 	* patch_tester.sh (nopristinecache): New shell var, set according
>> 	to presence or absence of new -nopristinecache option.
>> 	(usage): Document new option.
>> 	(bootntest_pristine): Implement it.
>>
>>   OK?
> 
>   Ping?

  Re-ping?

    cheers,
      DaveK

Re: [PING^2] Re: [PATCH,contrib] Add option to patch tester to avoid caching baseline test results.

[Ralf Wildenhues]

* Dave Korn wrote on Sat, Dec 18, 2010 at 01:04:26PM CET:
> On 02/12/2010 04:51, Dave Korn wrote:
> > On 15/11/2010 15:43, Dave Korn wrote:
> >> contrib/ChangeLog:
> >>
> >> 	* patch_tester.sh (nopristinecache): New shell var, set according
> >> 	to presence or absence of new -nopristinecache option.
> >> 	(usage): Document new option.
> >> 	(bootntest_pristine): Implement it.
> >>
> >>   OK?
> > 
> >   Ping?
> 
>   Re-ping?

Who can review this?  You could Cc: them.  I think the patch looked
syntactically ok (but I only looked at it briefly a month ago, and
didn't check semantics).

Cheers,
Ralf

Re: [PING^2] Re: [PATCH,contrib] Add option to patch tester to avoid caching baseline test results.

[Mike Stump]

On Dec 18, 2010, at 3:42 AM, Ralf Wildenhues wrote:

> * Dave Korn wrote on Sat, Dec 18, 2010 at 01:04:26PM CET:
>> On 02/12/2010 04:51, Dave Korn wrote:
>>> On 15/11/2010 15:43, Dave Korn wrote:
>>>> contrib/ChangeLog:
>>>> 
>>>> 	* patch_tester.sh (nopristinecache): New shell var, set according
>>>> 	to presence or absence of new -nopristinecache option.
>>>> 	(usage): Document new option.
>>>> 	(bootntest_pristine): Implement it.
>>>> 
>>>>  OK?
>>> 
>>>  Ping?
>> 
>>  Re-ping?
> 
> Who can review this?

svn log will help identify them...   Sebastian seems like a good candidate.

> You could Cc: them.  I think the patch looked
> syntactically ok (but I only looked at it briefly a month ago, and
> didn't check semantics).

Failing that, I think review from the people that use the script would be the next best thing.

Re: [PING^2] Re: [PATCH,contrib] Add option to patch tester to avoid caching baseline test results.

[Sebastian Pop]

On Sat, Dec 18, 2010 at 11:49, Mike Stump <mikestump@comcast.net> wrote:
>
> On Dec 18, 2010, at 3:42 AM, Ralf Wildenhues wrote:
>
>> * Dave Korn wrote on Sat, Dec 18, 2010 at 01:04:26PM CET:
>>> On 02/12/2010 04:51, Dave Korn wrote:
>>>> On 15/11/2010 15:43, Dave Korn wrote:
>>>>> contrib/ChangeLog:
>>>>>
>>>>>    * patch_tester.sh (nopristinecache): New shell var, set according
>>>>>    to presence or absence of new -nopristinecache option.
>>>>>    (usage): Document new option.
>>>>>    (bootntest_pristine): Implement it.
>>>>>
>>>>>  OK?

The patch looks good to me.  Please apply to trunk.

>>>>
>>>>  Ping?
>>>
>>>  Re-ping?
>>
>> Who can review this?
>
> svn log will help identify them...   Sebastian seems like a good candidate.
>

Thanks for pointing me to this patch.
I updated my filters to catch this kind of patches next time.

Sebastian

>> You could Cc: them.  I think the patch looked
>> syntactically ok (but I only looked at it briefly a month ago, and
>> didn't check semantics).
>
> Failing that, I think review from the people that use the script would be the next best thing.

Re: [PING^2] Re: [PATCH,contrib] Add option to patch tester to avoid caching baseline test results.

[Dave Korn]

On 19/12/2010 16:11, Sebastian Pop wrote:
> On Sat, Dec 18, 2010 at 11:49, Mike Stump <mikestump@comcast.net> wrote:
>> On Dec 18, 2010, at 3:42 AM, Ralf Wildenhues wrote:
>>
>>> * Dave Korn wrote on Sat, Dec 18, 2010 at 01:04:26PM CET:
>>>> On 02/12/2010 04:51, Dave Korn wrote:
>>>>> On 15/11/2010 15:43, Dave Korn wrote:
>>>>>> contrib/ChangeLog:
>>>>>>
>>>>>>    * patch_tester.sh (nopristinecache): New shell var, set according
>>>>>>    to presence or absence of new -nopristinecache option.
>>>>>>    (usage): Document new option.
>>>>>>    (bootntest_pristine): Implement it.
>>>>>>
>>>>>>  OK?
> 
> The patch looks good to me.  Please apply to trunk.

  (Finally!) Committed revision 169273.

    cheers,
      DaveK