RE: svn commit: r1419560 - /subversion/trunk/subversion/libsvn_wc/wc_db_update_move.c

2012-12-10 Thread Bert Huijben


 -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=1419560view=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=1419560r1=1419559r2=1419560view=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_mergesomething() 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

2012-12-10 Thread Julian Foad
Bert Huijben wrote:

  URL: http://svn.apache.org/viewvc?rev=1419560view=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_mergesomething() 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

2012-12-10 Thread Daniel Shahaf
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