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/12/10 17:38:46 UTC
RE: svn commit: r1419560 - /subversion/trunk/subversion/libsvn_wc/wc_db_update_move.c
> -----Original Message-----
> From: julianfoad@apache.org [mailto:julianfoad@apache.org]
> Sent: maandag 10 december 2012 17:29
> To: commits@subversion.apache.org
> Subject: svn commit: r1419560 -
> /subversion/trunk/subversion/libsvn_wc/wc_db_update_move.c
>
> Author: julianfoad
> Date: Mon Dec 10 16:28:34 2012
> New Revision: 1419560
>
> URL: http://svn.apache.org/viewvc?rev=1419560&view=rev
> Log:
> * subversion/libsvn_wc/wc_db_update_move.c
> (update_working_file): Update a comment.
>
> Modified:
> subversion/trunk/subversion/libsvn_wc/wc_db_update_move.c
>
> Modified: subversion/trunk/subversion/libsvn_wc/wc_db_update_move.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_
> db_update_move.c?rev=1419560&r1=1419559&r2=1419560&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_wc/wc_db_update_move.c
> (original)
> +++ subversion/trunk/subversion/libsvn_wc/wc_db_update_move.c Mon
> Dec 10 16:28:34 2012
> @@ -247,8 +247,8 @@ update_working_file(svn_skel_t **work_it
> old_version->props, old_version->props,
> actual_props, propchanges,
> scratch_pool, scratch_pool));
> - /* ### TODO: Make a WQ item in WORK_ITEMS to set new_actual_props
> ... */
> - /* ### Not a direct DB op like this... */
> + /* Install the new actual props. Don't set the conflict_skel yet, because
> + we might need to add a text conflict to it as well. */
> SVN_ERR(svn_wc__db_op_set_props(db, local_abspath,
> new_actual_props,
> svn_wc__has_magic_property(propchanges),
This tells me that the change is not atomic, so this really needs some fix-me comment after all.
The text and properties should be modified in one sqlite operation.
(Or in other words: the db should be updated to its final state, with the installing of the wq items in a single transaction)
I think the standard svn_wc_merge<something>() function should handle this for you in one step, so this function should do something similar.
It is not a problem to install a conflict skel and then to reinstall it later with more details. But it would be a problem for the client to crash between updating the state and installing the conflict. The sqlite transactions should catch that.
Bert
Re: svn commit: r1419560 -
/subversion/trunk/subversion/libsvn_wc/wc_db_update_move.c
Posted by Daniel Shahaf <da...@elego.de>.
Julian Foad wrote on Mon, Dec 10, 2012 at 16:57:26 +0000:
> svn_wc__db_op_set_property() uses a txn internally, but is being
> called within a larger 'outer txn' (sorry if the terminology is wrong)
"Savepoint"?
http://www.sqlite.org/lang_savepoint.html
Re: svn commit: r1419560 - /subversion/trunk/subversion/libsvn_wc/wc_db_update_move.c
Posted by Julian Foad <ju...@btopenworld.com>.
Bert Huijben wrote:
>> URL: http://svn.apache.org/viewvc?rev=1419560&view=rev
>> Log:
>> * subversion/libsvn_wc/wc_db_update_move.c
>> (update_working_file): Update a comment.
>> @@ -247,8 +247,8 @@ update_working_file(svn_skel_t **work_it
>> old_version->props, old_version->props,
>> actual_props, propchanges,
>> scratch_pool, scratch_pool));
>> - /* ### TODO: Make a WQ item in WORK_ITEMS to set new_actual_props
>> ... */
>> - /* ### Not a direct DB op like this... */
>> + /* Install the new actual props. Don't set the conflict_skel yet,
>> + because we might need to add a text conflict to it as well. */
>> SVN_ERR(svn_wc__db_op_set_props(db, local_abspath,
>> new_actual_props,
>> svn_wc__has_magic_property(propchanges),
>
> This tells me that the change is not atomic, so this really needs some fix-me
> comment after all.
>
> The text and properties should be modified in one sqlite operation.
> (Or in other words: the db should be updated to its final state, with the
> installing of the wq items in a single transaction)
The db_op_set_property() here is just one of a series of DB modifications being made here, all within a single DB txn.
svn_wc__db_op_set_property() uses a txn internally, but is being called within a larger 'outer txn' (sorry if the terminology is wrong) set up by the top-level function, svn_wc__db_update_moved_away_conflict_victim(), which executes the whole call tree in this file (wc_db_update_move.c) within svn_wc__db_with_txn().
- Julian
> I think the standard svn_wc_merge<something>() function should handle this
> for you in one step, so this function should do something similar.
>
>
> It is not a problem to install a conflict skel and then to reinstall it later
> with more details. But it would be a problem for the client to crash between
> updating the state and installing the conflict. The sqlite transactions should
> catch that.