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<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.
Received on 2012-12-10 17:58:05 CET