On Sep 19, 2009, at 8:47 PM, Bert Huijben wrote:
>> -----Original Message-----
>> From: Hyrum K. Wright [mailto:hyrum_at_hyrumwright.org]
>> Sent: donderdag 17 september 2009 16:58
>> To: svn_at_subversion.tigris.org
>> Subject: svn commit: r39399 - trunk/subversion/libsvn_wc
>>
>> Author: hwright
>> Date: Thu Sep 17 07:57:56 2009
>> New Revision: 39399
>>
>> Log:
>> * subversion/libsvn_wc/copy.c
>> (copy_dir_administratively): Remove an access baton by switching to
>> svn_wc__entry_modify2().
>>
>> Modified:
>> trunk/subversion/libsvn_wc/copy.c
>>
>> Modified: trunk/subversion/libsvn_wc/copy.c
>> URL:
>> http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/copy.c?path
>> rev=39399&r1=39398&r2=39399
>> =
>> =
>> =====================================================================
>> =======
>> --- trunk/subversion/libsvn_wc/copy.c Thu Sep 17 07:49:15 2009
>> (r39398)
>> +++ trunk/subversion/libsvn_wc/copy.c Thu Sep 17 07:57:56 2009
>> (r39399)
>> @@ -733,7 +733,6 @@ copy_dir_administratively(svn_wc_context
>> apr_pool_t *scratch_pool)
>> {
>> const svn_wc_entry_t *src_entry;
>> - svn_wc_adm_access_t *adm_access;
>> svn_wc__db_t *db = wc_ctx->db;
>>
>> /* Sanity check 1: You cannot make a copy of something that's not
>> @@ -772,9 +771,6 @@ copy_dir_administratively(svn_wc_context
>> scratch_pool));
>>
>> /* We've got some post-copy cleanup to do now. */
>> - SVN_ERR(svn_wc__adm_open_in_context(&adm_access, wc_ctx,
>> dst_abspath, TRUE,
>> - -1, cancel_func, cancel_baton,
>> - scratch_pool));
>> SVN_ERR(post_copy_cleanup(db,
>> dst_abspath,
>> scratch_pool));
>> @@ -802,9 +798,9 @@ copy_dir_administratively(svn_wc_context
>> will cause svn_wc_add4() below to fail. Set the URL to
>> the
>> URL of the first copy for now to prevent this. */
>> tmp_entry.url = apr_pstrdup(scratch_pool, copyfrom_url);
>> - SVN_ERR(svn_wc__entry_modify(adm_access, NULL, /* This Dir
>> */
>> - &tmp_entry,
>> - SVN_WC__ENTRY_MODIFY_URL,
>> scratch_pool));
>> + SVN_ERR(svn_wc__entry_modify2(db, dst_abspath, svn_node_dir,
>> FALSE,
>> + &tmp_entry,
>> SVN_WC__ENTRY_MODIFY_URL,
>> + scratch_pool));
>
> This deletes all entries in the wc database where dst_abspath lives
> without
> obtaining a write lock and then writes the in memory hash back. The
> outer
> access baton must still live until the entire operation is wrapped
> in some
> kind of transaction.
>
> I like that svn_wc__entry_modify2() doesn't need an access baton in
> its
> argument list, but it should still verify a write lock anyway.
> (Which is
> still handled by an access baton now).
>
> Only new operations that handle their own transactions in another
> way can
> avoid the lock. But in most cases they still need a lock because
> they can be
> reverted by another entry based operation that reads the entries
> before the
> transaction and writes them after the transaction.
Upon thinking more on this, I tend to agree. svn_wc__entry_modify2()
should still ensure it has a write lock on the directory in question
before performing the write (and error out otherwise).
This would seem to indicate to me that wc_db should be managing the
locks. I don't think or know if this is a long-term solution, since I
don't think we've full thought out the locking paradigm in the wc-ng
world. However, in the interim, we could add a temporary API inside
wc-ng which manages write-locks, and move that functionality out of
the access baton.
Or is this just crazy talk?
-Hyrum
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2398892
Received on 2009-09-23 16:00:16 CEST