You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Shahaf <d....@daniel.shahaf.name> on 2010/12/18 21:29:40 UTC
Re: svn commit: r1050216 - in /subversion/trunk/subversion:
include/private/svn_ra_private.h libsvn_ra/util.c
svnrdump/load_editor.c svnsync/main.c
cmpilato@apache.org wrote on Thu, Dec 16, 2010 at 23:10:10 -0000:
> Author: cmpilato
> Date: Thu Dec 16 23:10:10 2010
> New Revision: 1050216
>
> URL: http://svn.apache.org/viewvc?rev=1050216&view=rev
> Log:
> Finish issue #3766 ("Unify svnsync and svnrdump repos-locking logic").
>
> * subversion/include/private/svn_ra_private.h
> (svn_ra__lock_retry_func_t): New callback type.
> (svn_ra__get_operational_lock, svn_ra__release_operational_lock):
> New semi-private functions.
>
> * subversion/libsvn_ra/util.c
> (is_atomicity_error): Moved here from svnsync/main.c.
> (svn_ra__release_operational_lock): New, abstracted from
> svnsync/main.c:maybe_unlock().
> (svn_ra__get_operational_lock): New, abstracted from
> svnsync/main.c:get_lock().
>
Not exactly the same as svnsync's versions, since you added the
'stolen_lock_p' parameter. (and the log message doesn't mention that)
> +svn_error_t *
> +svn_ra__release_operational_lock(svn_ra_session_t *session,
> + const char *lock_revprop_name,
> + const svn_string_t *mylocktoken,
> + apr_pool_t *scratch_pool)
> +{
> + svn_string_t *reposlocktoken;
> + svn_boolean_t be_atomic;
> +
> + SVN_ERR(svn_ra_has_capability(session, &be_atomic,
> + SVN_RA_CAPABILITY_ATOMIC_REVPROPS,
> + scratch_pool));
> + SVN_ERR(svn_ra_rev_prop(session, 0, lock_revprop_name,
> + &reposlocktoken, scratch_pool));
> + if (reposlocktoken && svn_string_compare(reposlocktoken, mylocktoken))
> + {
> + svn_error_t *err;
> +
> + err = svn_ra_change_rev_prop2(session, 0, lock_revprop_name,
> + be_atomic ? &mylocktoken : NULL, NULL,
> + scratch_pool);
> + if (is_atomicity_error(err))
> + return svn_error_quick_wrap(err,
> + _("Lock was stolen; unable to remove it"));
s/was stolen/was stolen by '%s'/ ?
> + }
> +
> + return SVN_NO_ERROR;
> +}
> +
> +svn_error_t *
> +svn_ra__get_operational_lock(const svn_string_t **lock_string_p,
> + const svn_string_t **stolen_lock_p,
> + svn_ra_session_t *session,
> + const char *lock_revprop_name,
> + svn_boolean_t steal_lock,
> + int num_retries,
> + svn_ra__lock_retry_func_t retry_func,
> + void *retry_baton,
> + svn_cancel_func_t cancel_func,
> + void *cancel_baton,
> + apr_pool_t *pool)
> +{
...
> +}
+1
Re: svn commit: r1050216 - in /subversion/trunk/subversion:
include/private/svn_ra_private.h libsvn_ra/util.c
svnrdump/load_editor.c svnsync/main.c
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
C. Michael Pilato wrote on Mon, Dec 20, 2010 at 11:04:04 -0500:
> On 12/18/2010 04:29 PM, Daniel Shahaf wrote:
> > cmpilato@apache.org wrote on Thu, Dec 16, 2010 at 23:10:10 -0000:
>
> [...]
>
> >> * subversion/libsvn_ra/util.c
> >> (is_atomicity_error): Moved here from svnsync/main.c.
> >> (svn_ra__release_operational_lock): New, abstracted from
> >> svnsync/main.c:maybe_unlock().
> >> (svn_ra__get_operational_lock): New, abstracted from
> >> svnsync/main.c:get_lock().
> >>
> >
> > Not exactly the same as svnsync's versions, since you added the
> > 'stolen_lock_p' parameter. (and the log message doesn't mention that)
>
> I'm not claiming they are the same. I'm claiming that essentially logic
> therein was culled from the svnsync functions. I note that they are "New",
> and it's not our practice to list the parameters of new functions. :-)
>
> If it was a simple function move, I would use the syntax as above with
> is_atomicity_error -- "Move here from..." or "Was ...".
>
When I read the log message, I assumed it was a function move+rename.
I didn't know we had just two hard-coded syntaxes whitelisted for use in
the event of moving a function :-)
Re: svn commit: r1050216 - in /subversion/trunk/subversion:
include/private/svn_ra_private.h libsvn_ra/util.c
svnrdump/load_editor.c svnsync/main.c
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
C. Michael Pilato wrote on Mon, Dec 20, 2010 at 11:04:04 -0500:
> On 12/18/2010 04:29 PM, Daniel Shahaf wrote:
> > cmpilato@apache.org wrote on Thu, Dec 16, 2010 at 23:10:10 -0000:
>
> [...]
>
> >> * subversion/libsvn_ra/util.c
> >> (is_atomicity_error): Moved here from svnsync/main.c.
> >> (svn_ra__release_operational_lock): New, abstracted from
> >> svnsync/main.c:maybe_unlock().
> >> (svn_ra__get_operational_lock): New, abstracted from
> >> svnsync/main.c:get_lock().
> >>
> >
> > Not exactly the same as svnsync's versions, since you added the
> > 'stolen_lock_p' parameter. (and the log message doesn't mention that)
>
> I'm not claiming they are the same. I'm claiming that essentially logic
> therein was culled from the svnsync functions. I note that they are "New",
> and it's not our practice to list the parameters of new functions. :-)
>
> If it was a simple function move, I would use the syntax as above with
> is_atomicity_error -- "Move here from..." or "Was ...".
>
When I read the log message, I assumed it was a function move+rename.
I didn't know we had just two hard-coded syntaxes whitelisted for use in
the event of moving a function :-)
Re: svn commit: r1050216 - in /subversion/trunk/subversion: include/private/svn_ra_private.h
libsvn_ra/util.c svnrdump/load_editor.c svnsync/main.c
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 12/18/2010 04:29 PM, Daniel Shahaf wrote:
> cmpilato@apache.org wrote on Thu, Dec 16, 2010 at 23:10:10 -0000:
[...]
>> * subversion/libsvn_ra/util.c
>> (is_atomicity_error): Moved here from svnsync/main.c.
>> (svn_ra__release_operational_lock): New, abstracted from
>> svnsync/main.c:maybe_unlock().
>> (svn_ra__get_operational_lock): New, abstracted from
>> svnsync/main.c:get_lock().
>>
>
> Not exactly the same as svnsync's versions, since you added the
> 'stolen_lock_p' parameter. (and the log message doesn't mention that)
I'm not claiming they are the same. I'm claiming that essentially logic
therein was culled from the svnsync functions. I note that they are "New",
and it's not our practice to list the parameters of new functions. :-)
If it was a simple function move, I would use the syntax as above with
is_atomicity_error -- "Move here from..." or "Was ...".
>> + if (is_atomicity_error(err))
>> + return svn_error_quick_wrap(err,
>> + _("Lock was stolen; unable to remove it"));
>
> s/was stolen/was stolen by '%s'/ ?
Ah yes, good suggestion. r1051157.
OOH! I just noticed a bug, though -- when I switched to using
svn_string_compare() (instead of strcmp()ing ->data elements) I didn't
switch the boolean sense. Will fix.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: svn commit: r1050216 - in /subversion/trunk/subversion: include/private/svn_ra_private.h
libsvn_ra/util.c svnrdump/load_editor.c svnsync/main.c
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 12/18/2010 04:29 PM, Daniel Shahaf wrote:
> cmpilato@apache.org wrote on Thu, Dec 16, 2010 at 23:10:10 -0000:
[...]
>> * subversion/libsvn_ra/util.c
>> (is_atomicity_error): Moved here from svnsync/main.c.
>> (svn_ra__release_operational_lock): New, abstracted from
>> svnsync/main.c:maybe_unlock().
>> (svn_ra__get_operational_lock): New, abstracted from
>> svnsync/main.c:get_lock().
>>
>
> Not exactly the same as svnsync's versions, since you added the
> 'stolen_lock_p' parameter. (and the log message doesn't mention that)
I'm not claiming they are the same. I'm claiming that essentially logic
therein was culled from the svnsync functions. I note that they are "New",
and it's not our practice to list the parameters of new functions. :-)
If it was a simple function move, I would use the syntax as above with
is_atomicity_error -- "Move here from..." or "Was ...".
>> + if (is_atomicity_error(err))
>> + return svn_error_quick_wrap(err,
>> + _("Lock was stolen; unable to remove it"));
>
> s/was stolen/was stolen by '%s'/ ?
Ah yes, good suggestion. r1051157.
OOH! I just noticed a bug, though -- when I switched to using
svn_string_compare() (instead of strcmp()ing ->data elements) I didn't
switch the boolean sense. Will fix.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand