RE: svn commit: r1419560 - /subversion/trunk/subversion/libsvn_wc/wc_db_update_move.c
> -Original Message- > From: julianf...@apache.org [mailto:julianf...@apache.org] > Sent: maandag 10 december 2012 17:29 > To: comm...@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() 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
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() 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.
Re: svn commit: r1419560 - /subversion/trunk/subversion/libsvn_wc/wc_db_update_move.c
Julian Foad wrote on Mon, Dec 10, 2012 at 16:57:26 +: > 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
