You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by cm...@apache.org on 2011/05/17 17:02:05 UTC
svn commit: r1104309 - in /subversion/trunk/subversion:
libsvn_repos/reporter.c tests/cmdline/lock_tests.py
tests/cmdline/update_tests.py
Author: cmpilato
Date: Tue May 17 15:02:05 2011
New Revision: 1104309
URL: http://svn.apache.org/viewvc?rev=1104309&view=rev
Log:
With rhuijben, avoid transmitting entry props for unmodified, locked
files when the client-provided lock token matches what is stored in
the repository. This fixes issue #3525 ("Locked file which is
scheduled for delete causes tree conflict") and issue #3471 ("svn up
touches file w/ lock & svn:keywords property").
NOTE: There is a remaining 3525-related test that is still failing
(update_tests.py 53), but that's because of out-of-date expectations
in the WC-NG world. (That will be fixed in a subsequent revision.)
* subversion/libsvn_repos/reporter.c
(update_entry): Return early not only when there's not a provided
lock token, but also when there's a provided lock token that matches
what's in the repository.
* subversion/tests/cmdline/lock_tests.py
(update_locked_deleted): Remove @XFail decorator.
* subversion/tests/cmdline/update_tests.py
(update_with_file_lock_and_keywords_property_set): Remove @XFail decorator.
Modified:
subversion/trunk/subversion/libsvn_repos/reporter.c
subversion/trunk/subversion/tests/cmdline/lock_tests.py
subversion/trunk/subversion/tests/cmdline/update_tests.py
Modified: subversion/trunk/subversion/libsvn_repos/reporter.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/reporter.c?rev=1104309&r1=1104308&r2=1104309&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_repos/reporter.c (original)
+++ subversion/trunk/subversion/libsvn_repos/reporter.c Tue May 17 15:02:05 2011
@@ -860,9 +860,23 @@ update_entry(report_baton_t *b, svn_revn
{
int distance = svn_fs_compare_ids(s_entry->id, t_entry->id);
if (distance == 0 && !any_path_info(b, e_path)
- && (!info || (!info->start_empty && !info->lock_token))
&& (requested_depth <= wc_depth || t_entry->kind == svn_node_file))
- return SVN_NO_ERROR;
+ {
+ if (!info)
+ return SVN_NO_ERROR;
+
+ if (!info->start_empty)
+ {
+ svn_lock_t *lock;
+
+ if (!info->lock_token)
+ return SVN_NO_ERROR;
+
+ SVN_ERR(svn_fs_get_lock(&lock, b->repos->fs, t_path, pool));
+ if (lock && (strcmp(lock->token, info->lock_token) == 0))
+ return SVN_NO_ERROR;
+ }
+ }
related = (distance != -1 || b->ignore_ancestry);
}
Modified: subversion/trunk/subversion/tests/cmdline/lock_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/lock_tests.py?rev=1104309&r1=1104308&r2=1104309&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/lock_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/lock_tests.py Tue May 17 15:02:05 2011
@@ -1650,7 +1650,6 @@ def cp_isnt_ro(sbox):
#----------------------------------------------------------------------
# Issue #3525: Locked file which is scheduled for delete causes tree
# conflict
-@XFail()
@Issue(3525)
def update_locked_deleted(sbox):
"updating locked scheduled-for-delete file"
Modified: subversion/trunk/subversion/tests/cmdline/update_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/update_tests.py?rev=1104309&r1=1104308&r2=1104309&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/update_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/update_tests.py Tue May 17 15:02:05 2011
@@ -5316,9 +5316,6 @@ def update_with_excluded_subdir(sbox):
#----------------------------------------------------------------------
# Test for issue #3471 'svn up touches file w/ lock & svn:keywords property'
-#
-# Marked as XFail until the issue is fixed.
-@XFail()
@Issue(3471)
def update_with_file_lock_and_keywords_property_set(sbox):
"""update with file lock & keywords property set"""
Re: svn commit: r1104309 - in /subversion/trunk/subversion:
libsvn_repos/reporter.c tests/cmdline/lock_tests.py tests/cmdline/update_tests.py
Posted by Johan Corveleyn <jc...@gmail.com>.
On Tue, May 17, 2011 at 7:13 PM, C. Michael Pilato <cm...@collab.net> wrote:
> On 05/17/2011 06:34 PM, Johan Corveleyn wrote:
>> On Tue, May 17, 2011 at 5:02 PM, <cm...@apache.org> wrote:
>>> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/reporter.c?rev=1104309&r1=1104308&r2=1104309&view=diff
>>> ==============================================================================
>>> --- subversion/trunk/subversion/libsvn_repos/reporter.c (original)
>>> +++ subversion/trunk/subversion/libsvn_repos/reporter.c Tue May 17 15:02:05 2011
>>> @@ -860,9 +860,23 @@ update_entry(report_baton_t *b, svn_revn
>>> {
>>> int distance = svn_fs_compare_ids(s_entry->id, t_entry->id);
>>> if (distance == 0 && !any_path_info(b, e_path)
>>> - && (!info || (!info->start_empty && !info->lock_token))
>>> && (requested_depth <= wc_depth || t_entry->kind == svn_node_file))
>>> - return SVN_NO_ERROR;
>>> + {
>>> + if (!info)
>>> + return SVN_NO_ERROR;
>>> +
>>> + if (!info->start_empty)
>>> + {
>>> + svn_lock_t *lock;
>>> +
>>> + if (!info->lock_token)
>>> + return SVN_NO_ERROR;
>>> +
>>> + SVN_ERR(svn_fs_get_lock(&lock, b->repos->fs, t_path, pool));
>>
>> Could t_path be NULL here (if only if a "malicious client" crafts a
>> special request)?
>>
>> The docstring of this function mentions that T_ENTRY and T_PATH may be
>> NULL. In this block we are sure that T_ENTRY is non-null, but what
>> about T_PATH?
>
> I think T_ENTRY and T_PATH come as a pair, either both NULL or both not.
Yes, it would seem so. And I've followed the callers of update_entry a
bit to assure myself that T_PATH can't be NULL (or if it would be,
earlier code would already have failed). Just making sure :-).
Thanks for fixing this issue.
--
Johan
Re: svn commit: r1104309 - in /subversion/trunk/subversion: libsvn_repos/reporter.c
tests/cmdline/lock_tests.py tests/cmdline/update_tests.py
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 05/17/2011 06:34 PM, Johan Corveleyn wrote:
> On Tue, May 17, 2011 at 5:02 PM, <cm...@apache.org> wrote:
>> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/reporter.c?rev=1104309&r1=1104308&r2=1104309&view=diff
>> ==============================================================================
>> --- subversion/trunk/subversion/libsvn_repos/reporter.c (original)
>> +++ subversion/trunk/subversion/libsvn_repos/reporter.c Tue May 17 15:02:05 2011
>> @@ -860,9 +860,23 @@ update_entry(report_baton_t *b, svn_revn
>> {
>> int distance = svn_fs_compare_ids(s_entry->id, t_entry->id);
>> if (distance == 0 && !any_path_info(b, e_path)
>> - && (!info || (!info->start_empty && !info->lock_token))
>> && (requested_depth <= wc_depth || t_entry->kind == svn_node_file))
>> - return SVN_NO_ERROR;
>> + {
>> + if (!info)
>> + return SVN_NO_ERROR;
>> +
>> + if (!info->start_empty)
>> + {
>> + svn_lock_t *lock;
>> +
>> + if (!info->lock_token)
>> + return SVN_NO_ERROR;
>> +
>> + SVN_ERR(svn_fs_get_lock(&lock, b->repos->fs, t_path, pool));
>
> Could t_path be NULL here (if only if a "malicious client" crafts a
> special request)?
>
> The docstring of this function mentions that T_ENTRY and T_PATH may be
> NULL. In this block we are sure that T_ENTRY is non-null, but what
> about T_PATH?
I think T_ENTRY and T_PATH come as a pair, either both NULL or both not.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: svn commit: r1104309 - in /subversion/trunk/subversion:
libsvn_repos/reporter.c tests/cmdline/lock_tests.py tests/cmdline/update_tests.py
Posted by Johan Corveleyn <jc...@gmail.com>.
On Tue, May 17, 2011 at 5:02 PM, <cm...@apache.org> wrote:
> Author: cmpilato
> Date: Tue May 17 15:02:05 2011
> New Revision: 1104309
>
> URL: http://svn.apache.org/viewvc?rev=1104309&view=rev
> Log:
> With rhuijben, avoid transmitting entry props for unmodified, locked
> files when the client-provided lock token matches what is stored in
> the repository. This fixes issue #3525 ("Locked file which is
> scheduled for delete causes tree conflict") and issue #3471 ("svn up
> touches file w/ lock & svn:keywords property").
>
> NOTE: There is a remaining 3525-related test that is still failing
> (update_tests.py 53), but that's because of out-of-date expectations
> in the WC-NG world. (That will be fixed in a subsequent revision.)
>
> * subversion/libsvn_repos/reporter.c
> (update_entry): Return early not only when there's not a provided
> lock token, but also when there's a provided lock token that matches
> what's in the repository.
>
> * subversion/tests/cmdline/lock_tests.py
> (update_locked_deleted): Remove @XFail decorator.
>
> * subversion/tests/cmdline/update_tests.py
> (update_with_file_lock_and_keywords_property_set): Remove @XFail decorator.
>
> Modified:
> subversion/trunk/subversion/libsvn_repos/reporter.c
> subversion/trunk/subversion/tests/cmdline/lock_tests.py
> subversion/trunk/subversion/tests/cmdline/update_tests.py
>
> Modified: subversion/trunk/subversion/libsvn_repos/reporter.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/reporter.c?rev=1104309&r1=1104308&r2=1104309&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_repos/reporter.c (original)
> +++ subversion/trunk/subversion/libsvn_repos/reporter.c Tue May 17 15:02:05 2011
> @@ -860,9 +860,23 @@ update_entry(report_baton_t *b, svn_revn
> {
> int distance = svn_fs_compare_ids(s_entry->id, t_entry->id);
> if (distance == 0 && !any_path_info(b, e_path)
> - && (!info || (!info->start_empty && !info->lock_token))
> && (requested_depth <= wc_depth || t_entry->kind == svn_node_file))
> - return SVN_NO_ERROR;
> + {
> + if (!info)
> + return SVN_NO_ERROR;
> +
> + if (!info->start_empty)
> + {
> + svn_lock_t *lock;
> +
> + if (!info->lock_token)
> + return SVN_NO_ERROR;
> +
> + SVN_ERR(svn_fs_get_lock(&lock, b->repos->fs, t_path, pool));
Could t_path be NULL here (if only if a "malicious client" crafts a
special request)?
The docstring of this function mentions that T_ENTRY and T_PATH may be
NULL. In this block we are sure that T_ENTRY is non-null, but what
about T_PATH?
--
Johan