You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Bert Huijben <be...@qqmail.nl> on 2012/10/30 11:46:36 UTC

RE: svn commit: r1403588 - in /subversion/trunk/subversion: mod_dav_svn/reports/update.c tests/cmdline/update_tests.py


> -----Original Message-----
> From: cmpilato@apache.org [mailto:cmpilato@apache.org]
> Sent: dinsdag 30 oktober 2012 01:24
> To: commits@subversion.apache.org
> Subject: svn commit: r1403588 - in /subversion/trunk/subversion:
> mod_dav_svn/reports/update.c tests/cmdline/update_tests.py
> 
> Author: cmpilato
> Date: Tue Oct 30 00:23:38 2012
> New Revision: 1403588
> 
> URL: http://svn.apache.org/viewvc?rev=1403588&view=rev
> Log:
> Tweak the server-side logic which validates update report source and
> target revision values so that they always get checked for validity,
> not only when doing non-client-pegged updates.
> 
> * subversion/mod_dav_svn/reports/update.c
>   (validate_input_revision): New helper function.
>   (dav_svn__update_report): Always query the youngest FS revision, and
>     use it (via validate_input_revision()) to raise errors when the
>     client requests an update to a revision younger than is available
>     or reports that it has objects whose revision is greater than the
>     youngest.

How hard would it be to port this one back to 1.7?

<snip>

> Modified: subversion/trunk/subversion/tests/cmdline/update_tests.py
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/
> update_tests.py?rev=1403588&r1=1403587&r2=1403588&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/tests/cmdline/update_tests.py (original)
> +++ subversion/trunk/subversion/tests/cmdline/update_tests.py Tue Oct 30
> 00:23:38 2012
> @@ -5516,7 +5516,7 @@ def update_to_HEAD_plus_1(sbox):
> 
>    svntest.actions.run_and_verify_update(wc_dir,
>                                          None, None, None,
> -                                        ".*No such revision",
> +                                        ".*E160006",

If possible, please also keep some part(s) of an error message in the regex, to make the testsuite verify that we keep failing for the same reason.
(In this case it checks for SVN_ERR_FS_NO_SUCH_REVISION, which already tells almost the whole story, but I don't see that in the test suite code)

	Bert
>                                          None, None,
>                                          None, None, None, wc_dir, '-r', '2')
> 
> 



Re: svn commit: r1403588 - in /subversion/trunk/subversion: mod_dav_svn/reports/update.c tests/cmdline/update_tests.py

Posted by "C. Michael Pilato" <cm...@collab.net>.
On 10/30/2012 08:58 AM, Bert Huijben wrote:
>>> How hard would it be to port this one back to 1.7?
>>
>> I don't think it would be hard at all.  I certainly plan to make the attempt
>> today.
> 
> One of the reasons I was suggesting is that I was thinking that this code
> might also relate to the errors reported by temporarily out of sync
> proxies, but I now think those originate in the reporter handling.

My changes to do affect temporarily out-of-sync proxies.

> Yes great.
> I think you could use "E160006.*No such.*revision". 
> (If I remember correctly we don't need a .* at the start.)

Done.  (And proposed alongside r1403588 for backport.)

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Enterprise Cloud Development


RE: svn commit: r1403588 - in /subversion/trunk/subversion: mod_dav_svn/reports/update.c tests/cmdline/update_tests.py

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: C. Michael Pilato [mailto:cmpilato@collab.net]
> Sent: dinsdag 30 oktober 2012 13:01
> To: Bert Huijben
> Cc: dev@subversion.apache.org
> Subject: Re: svn commit: r1403588 - in /subversion/trunk/subversion:
> mod_dav_svn/reports/update.c tests/cmdline/update_tests.py
> 
> On 10/30/2012 06:46 AM, Bert Huijben wrote:
> >
> >
> >> -----Original Message-----
> >> From: cmpilato@apache.org [mailto:cmpilato@apache.org]
> >> Sent: dinsdag 30 oktober 2012 01:24
> >> To: commits@subversion.apache.org
> >> Subject: svn commit: r1403588 - in /subversion/trunk/subversion:
> >> mod_dav_svn/reports/update.c tests/cmdline/update_tests.py
> >>
> >> Author: cmpilato
> >> Date: Tue Oct 30 00:23:38 2012
> >> New Revision: 1403588
> >>
> >> URL: http://svn.apache.org/viewvc?rev=1403588&view=rev
> >> Log:
> >> Tweak the server-side logic which validates update report source and
> >> target revision values so that they always get checked for validity,
> >> not only when doing non-client-pegged updates.
> >>
> >> * subversion/mod_dav_svn/reports/update.c
> >>   (validate_input_revision): New helper function.
> >>   (dav_svn__update_report): Always query the youngest FS revision, and
> >>     use it (via validate_input_revision()) to raise errors when the
> >>     client requests an update to a revision younger than is available
> >>     or reports that it has objects whose revision is greater than the
> >>     youngest.
> >
> > How hard would it be to port this one back to 1.7?
> 
> I don't think it would be hard at all.  I certainly plan to make the attempt
> today.

One of the reasons I was suggesting is that I was thinking that this code might also relate to the errors reported by temporarily out of sync proxies, but I now think those originate in the reporter handling.


> 
> >> Modified: subversion/trunk/subversion/tests/cmdline/update_tests.py
> >> URL:
> >>
> http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/
> >> update_tests.py?rev=1403588&r1=1403587&r2=1403588&view=diff
> >>
> ==========================================================
> >> ====================
> >> --- subversion/trunk/subversion/tests/cmdline/update_tests.py (original)
> >> +++ subversion/trunk/subversion/tests/cmdline/update_tests.py Tue
> Oct 30
> >> 00:23:38 2012
> >> @@ -5516,7 +5516,7 @@ def update_to_HEAD_plus_1(sbox):
> >>
> >>    svntest.actions.run_and_verify_update(wc_dir,
> >>                                          None, None, None,
> >> -                                        ".*No such revision",
> >> +                                        ".*E160006",
> >
> > If possible, please also keep some part(s) of an error message in the regex,
> to make the testsuite verify that we keep failing for the same reason.
> > (In this case it checks for SVN_ERR_FS_NO_SUCH_REVISION, which already
> tells almost the whole story, but I don't see that in the test suite code)
> 
> Good suggestion.  The "No such revision ..." is now (for DAV servers) "No
> such [target|reported] revision ...".  Would ".*No such.*revision" serve
> your desires here?

Yes great.
I think you could use "E160006.*No such.*revision". 
(If I remember correctly we don't need a .* at the start.)

	Bert



Re: svn commit: r1403588 - in /subversion/trunk/subversion: mod_dav_svn/reports/update.c tests/cmdline/update_tests.py

Posted by "C. Michael Pilato" <cm...@collab.net>.
On 10/30/2012 06:46 AM, Bert Huijben wrote:
> 
> 
>> -----Original Message-----
>> From: cmpilato@apache.org [mailto:cmpilato@apache.org]
>> Sent: dinsdag 30 oktober 2012 01:24
>> To: commits@subversion.apache.org
>> Subject: svn commit: r1403588 - in /subversion/trunk/subversion:
>> mod_dav_svn/reports/update.c tests/cmdline/update_tests.py
>>
>> Author: cmpilato
>> Date: Tue Oct 30 00:23:38 2012
>> New Revision: 1403588
>>
>> URL: http://svn.apache.org/viewvc?rev=1403588&view=rev
>> Log:
>> Tweak the server-side logic which validates update report source and
>> target revision values so that they always get checked for validity,
>> not only when doing non-client-pegged updates.
>>
>> * subversion/mod_dav_svn/reports/update.c
>>   (validate_input_revision): New helper function.
>>   (dav_svn__update_report): Always query the youngest FS revision, and
>>     use it (via validate_input_revision()) to raise errors when the
>>     client requests an update to a revision younger than is available
>>     or reports that it has objects whose revision is greater than the
>>     youngest.
> 
> How hard would it be to port this one back to 1.7?

I don't think it would be hard at all.  I certainly plan to make the attempt
today.

>> Modified: subversion/trunk/subversion/tests/cmdline/update_tests.py
>> URL:
>> http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/
>> update_tests.py?rev=1403588&r1=1403587&r2=1403588&view=diff
>> ==========================================================
>> ====================
>> --- subversion/trunk/subversion/tests/cmdline/update_tests.py (original)
>> +++ subversion/trunk/subversion/tests/cmdline/update_tests.py Tue Oct 30
>> 00:23:38 2012
>> @@ -5516,7 +5516,7 @@ def update_to_HEAD_plus_1(sbox):
>>
>>    svntest.actions.run_and_verify_update(wc_dir,
>>                                          None, None, None,
>> -                                        ".*No such revision",
>> +                                        ".*E160006",
> 
> If possible, please also keep some part(s) of an error message in the regex, to make the testsuite verify that we keep failing for the same reason.
> (In this case it checks for SVN_ERR_FS_NO_SUCH_REVISION, which already tells almost the whole story, but I don't see that in the test suite code)

Good suggestion.  The "No such revision ..." is now (for DAV servers) "No
such [target|reported] revision ...".  Would ".*No such.*revision" serve
your desires here?

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Enterprise Cloud Development