FYI: After applying Bert's r930281 fix, this revision still breaks merge tests 34 and 134.
Investigating... On Fri, Apr 2, 2010 at 00:53, <gst...@apache.org> wrote: > Author: gstein > Date: Fri Apr 2 04:53:22 2010 > New Revision: 930162 > > URL: http://svn.apache.org/viewvc?rev=930162&view=rev > Log: > Remove ENTRY_MODIFY_INCOMPLETE from entry_modify2(). Remove LOG_ACCUM from > file attribute setting, and queue the work items directly. > > * subversion/libsvn_wc/entries.h: > (SVN_WC__ENTRY_MODIFY_INCOMPLETE): removed. no longer used. > > * subversion/libsvn_wc/entries.c: > (): remove a couple unused includes > (fold_entry): don't fold entry->incomplete > > * subversion/libsvn_wc/log.h: > (svn_wc__loggy_maybe_set_executable, svn_wc__loggy_maybe_set_readonly): > remove the LOG_ACCUM parameter, add a DB parameter. the work items > will be queued immediately, rather than placed into LOG_ACCUM > > * subversion/libsvn_wc/log.c: > (svn_wc__loggy_maybe_set_executable, svn_wc__loggy_maybe_set_readonly): > rejigger to queue immediately, rather than return the ops in a > LOG_ACCUM paramter. > > * subversion/libsvn_wc/merge.c: > (svn_wc__internal_merge): assert that DRY_RUN=TRUE will produce no work > items to queue. adjust file attr call signatures. > (svn_wc_merge4): do the write_check early. assert that internal_merge > queues its work. simplify the ending block, simply running the queue. > > * subversion/libsvn_wc/adm_ops.c: > (svn_wc_add4): avoid using SVN_WC__ENTRY_MODIFY_INCOMPLETE and use > svn_wc__temp_op_set_working_incomplete instead. > > * subversion/libsvn_wc/update_editor.c: > (merge_file): remove LOCK_REMOVED parameter. shift this logic "outward" > to the close_file function. after the __internal_merge call, assert > that it has queued all log operations. > (close_file): avoid passing LOCK_REMOVED param. shift the "set readonly" > logic to here. > > Modified: > subversion/trunk/subversion/libsvn_wc/adm_ops.c > subversion/trunk/subversion/libsvn_wc/entries.c > subversion/trunk/subversion/libsvn_wc/entries.h > subversion/trunk/subversion/libsvn_wc/log.c > subversion/trunk/subversion/libsvn_wc/log.h > subversion/trunk/subversion/libsvn_wc/merge.c > subversion/trunk/subversion/libsvn_wc/update_editor.c > > Modified: subversion/trunk/subversion/libsvn_wc/adm_ops.c > URL: > http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/adm_ops.c?rev=930162&r1=930161&r2=930162&view=diff > ============================================================================== > --- subversion/trunk/subversion/libsvn_wc/adm_ops.c (original) > +++ subversion/trunk/subversion/libsvn_wc/adm_ops.c Fri Apr 2 04:53:22 2010 > @@ -1584,15 +1584,16 @@ svn_wc_add4(svn_wc_context_t *wc_ctx, > This deletes the erroneous BASE_NODE for added directories and > adds a WORKING_NODE. */ > modify_flags |= SVN_WC__ENTRY_MODIFY_FORCE; > - modify_flags |= SVN_WC__ENTRY_MODIFY_INCOMPLETE; > tmp_entry.schedule = is_replace > ? svn_wc_schedule_replace > : svn_wc_schedule_add; > - tmp_entry.incomplete = FALSE; > SVN_ERR(svn_wc__entry_modify2(db, local_abspath, svn_node_dir, > FALSE /* parent_stub */, > &tmp_entry, modify_flags, pool)); > > + SVN_ERR(svn_wc__db_temp_op_set_working_incomplete( > + db, local_abspath, FALSE, pool)); > + > if (copyfrom_url) > { > /* If this new directory has ancestry, it's not enough to > > Modified: subversion/trunk/subversion/libsvn_wc/entries.c > URL: > http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/entries.c?rev=930162&r1=930161&r2=930162&view=diff > ============================================================================== > --- subversion/trunk/subversion/libsvn_wc/entries.c (original) > +++ subversion/trunk/subversion/libsvn_wc/entries.c Fri Apr 2 04:53:22 2010 > @@ -37,7 +37,6 @@ > > #include "wc.h" > #include "adm_files.h" > -#include "adm_ops.h" > #include "entries.h" > #include "lock.h" > #include "tree_conflicts.h" > @@ -47,7 +46,6 @@ > #include "svn_private_config.h" > #include "private/svn_wc_private.h" > #include "private/svn_sqlite.h" > -#include "private/svn_skel.h" > > #define MAYBE_ALLOC(x,p) ((x) ? (x) : apr_pcalloc((p), sizeof(*(x)))) > > @@ -2565,10 +2563,6 @@ fold_entry(apr_hash_t *entries, > if (modify_flags & SVN_WC__ENTRY_MODIFY_ABSENT) > cur_entry->absent = entry->absent; > > - /* Incomplete state */ > - if (modify_flags & SVN_WC__ENTRY_MODIFY_INCOMPLETE) > - cur_entry->incomplete = entry->incomplete; > - > /* Text/prop modification times */ > if (modify_flags & SVN_WC__ENTRY_MODIFY_TEXT_TIME) > cur_entry->text_time = entry->text_time; > > Modified: subversion/trunk/subversion/libsvn_wc/entries.h > URL: > http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/entries.h?rev=930162&r1=930161&r2=930162&view=diff > ============================================================================== > --- subversion/trunk/subversion/libsvn_wc/entries.h (original) > +++ subversion/trunk/subversion/libsvn_wc/entries.h Fri Apr 2 04:53:22 2010 > @@ -109,7 +109,6 @@ svn_error_t *svn_wc__atts_to_entry(svn_w > #define SVN_WC__ENTRY_MODIFY_CONFLICT_WRK > APR_INT64_C(0x0000000000004000) > #define SVN_WC__ENTRY_MODIFY_PREJFILE > APR_INT64_C(0x0000000000008000) > /* OPEN */ > -#define SVN_WC__ENTRY_MODIFY_INCOMPLETE > APR_INT64_C(0x0000000000100000) > #define SVN_WC__ENTRY_MODIFY_ABSENT > APR_INT64_C(0x0000000000200000) > /* OPEN */ > #define SVN_WC__ENTRY_MODIFY_WORKING_SIZE > APR_INT64_C(0x0000000100000000) > > Modified: subversion/trunk/subversion/libsvn_wc/log.c > URL: > http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/log.c?rev=930162&r1=930161&r2=930162&view=diff > ============================================================================== > --- subversion/trunk/subversion/libsvn_wc/log.c (original) > +++ subversion/trunk/subversion/libsvn_wc/log.c Fri Apr 2 04:53:22 2010 > @@ -1062,44 +1062,46 @@ svn_wc__loggy_move(svn_stringbuf_t **log > } > > svn_error_t * > -svn_wc__loggy_maybe_set_executable(svn_stringbuf_t **log_accum, > +svn_wc__loggy_maybe_set_executable(svn_wc__db_t *db, > const char *adm_abspath, > const char *path, > - apr_pool_t *result_pool, > apr_pool_t *scratch_pool) > { > + svn_stringbuf_t *log_accum = NULL; > const char *loggy_path1; > > SVN_ERR(loggy_path(&loggy_path1, path, adm_abspath, scratch_pool)); > - svn_xml_make_open_tag(log_accum, > - result_pool, > + svn_xml_make_open_tag(&log_accum, > + scratch_pool, > svn_xml_self_closing, > SVN_WC__LOG_MAYBE_EXECUTABLE, > SVN_WC__LOG_ATTR_NAME, loggy_path1, > NULL); > > - return SVN_NO_ERROR; > + return svn_error_return(svn_wc__wq_add_loggy(db, adm_abspath, log_accum, > + scratch_pool)); > } > > svn_error_t * > -svn_wc__loggy_maybe_set_readonly(svn_stringbuf_t **log_accum, > +svn_wc__loggy_maybe_set_readonly(svn_wc__db_t *db, > const char *adm_abspath, > const char *path, > - apr_pool_t *result_pool, > apr_pool_t *scratch_pool) > { > + svn_stringbuf_t *log_accum = NULL; > const char *loggy_path1; > > SVN_ERR(loggy_path(&loggy_path1, path, adm_abspath, scratch_pool)); > - svn_xml_make_open_tag(log_accum, > - result_pool, > + svn_xml_make_open_tag(&log_accum, > + scratch_pool, > svn_xml_self_closing, > SVN_WC__LOG_MAYBE_READONLY, > SVN_WC__LOG_ATTR_NAME, > loggy_path1, > NULL); > > - return SVN_NO_ERROR; > + return svn_error_return(svn_wc__wq_add_loggy(db, adm_abspath, log_accum, > + scratch_pool)); > } > > svn_error_t * > > Modified: subversion/trunk/subversion/libsvn_wc/log.h > URL: > http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/log.h?rev=930162&r1=930161&r2=930162&view=diff > ============================================================================== > --- subversion/trunk/subversion/libsvn_wc/log.h (original) > +++ subversion/trunk/subversion/libsvn_wc/log.h Fri Apr 2 04:53:22 2010 > @@ -192,37 +192,35 @@ svn_wc__loggy_move(svn_stringbuf_t **log > > > > -/* Extend **LOG_ACCUM with log instructions to set permissions of PATH > - to 'executable' if it has the 'executable' property set. > +/* Queue instructions to set permissions of PATH to 'executable' if it has > + the 'executable' property set. > + > ADM_ABSPATH is the absolute path for the admin directory for PATH. > > The property is tested at log run time, within this log instruction. > > - Allocate *LOG_ACCUM in RESULT_POOL if it is NULL. Use SCRATCH_POOL for > - temporary allocations. > + Use SCRATCH_POOL for temporary allocations. > */ > svn_error_t * > -svn_wc__loggy_maybe_set_executable(svn_stringbuf_t **log_accum, > +svn_wc__loggy_maybe_set_executable(svn_wc__db_t *db, > const char *adm_abspath, > const char *path, > - apr_pool_t *result_pool, > apr_pool_t *scratch_pool); > > -/* Extend **LOG_ACCUM with log instructions to set permissions of PATH > - to 'readonly' if it has the 'needs-lock' property set and there is > - no lock for the file in the working copy. > +/* Queue instructions to set permissions of PATH to 'readonly' if it has > + the 'needs-lock' property set and there is no lock for the file in the > + working copy. > + > ADM_ABSPATH is the absolute path for the admin directory for PATH. > > The tests are made at log run time, within this log instruction. > > - Allocate *LOG_ACCUM in RESULT_POOL if it is NULL. Use SCRATCH_POOL for > - temporary allocations. > + Use SCRATCH_POOL for temporary allocations. > */ > svn_error_t * > -svn_wc__loggy_maybe_set_readonly(svn_stringbuf_t **log_accum, > +svn_wc__loggy_maybe_set_readonly(svn_wc__db_t *db, > const char *adm_abspath, > const char *path, > - apr_pool_t *result_pool, > apr_pool_t *scratch_pool); > > > > Modified: subversion/trunk/subversion/libsvn_wc/merge.c > URL: > http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/merge.c?rev=930162&r1=930161&r2=930162&view=diff > ============================================================================== > --- subversion/trunk/subversion/libsvn_wc/merge.c (original) > +++ subversion/trunk/subversion/libsvn_wc/merge.c Fri Apr 2 04:53:22 2010 > @@ -1253,23 +1253,33 @@ svn_wc__internal_merge(svn_stringbuf_t * > cancel_baton, > pool)); > > + SVN_ERR_ASSERT(!dry_run > + || *log_accum == NULL > + || svn_stringbuf_isempty(*log_accum)); > + > + /* Queue everything that has been accumulated. */ > + if (*log_accum != NULL) > + { > + SVN_ERR(svn_wc__wq_add_loggy(db, dir_abspath, *log_accum, pool)); > + svn_stringbuf_setempty(*log_accum); > + } > + > /* Merging is complete. Regardless of text or binariness, we might > need to tweak the executable bit on the new working file, and > possibly make it read-only. */ > if (! dry_run) > { > - SVN_ERR(svn_wc__loggy_maybe_set_executable(log_accum, dir_abspath, > + SVN_ERR(svn_wc__loggy_maybe_set_executable(db, dir_abspath, > target_abspath, > - pool, pool)); > - SVN_ERR(svn_wc__loggy_maybe_set_readonly(log_accum, dir_abspath, > + pool)); > + SVN_ERR(svn_wc__loggy_maybe_set_readonly(db, dir_abspath, > target_abspath, > - pool, pool)); > + pool)); > } > > return SVN_NO_ERROR; > } > > - > > svn_error_t * > svn_wc_merge4(enum svn_wc_merge_outcome_t *merge_outcome, > @@ -1292,8 +1302,14 @@ svn_wc_merge4(enum svn_wc_merge_outcome_ > void *cancel_baton, > apr_pool_t *scratch_pool) > { > - svn_stringbuf_t *log_accum = svn_stringbuf_create("", scratch_pool); > + svn_stringbuf_t *log_accum = NULL; > + const char *dir_abspath = svn_dirent_dirname(target_abspath, scratch_pool); > + > + /* Before we do any work, make sure we hold a write lock. */ > + if (!dry_run) > + SVN_ERR(svn_wc__write_check(wc_ctx->db, dir_abspath, scratch_pool)); > > + /* Queue all the work. */ > SVN_ERR(svn_wc__internal_merge(&log_accum, merge_outcome, > wc_ctx->db, > left_abspath, left_version, > @@ -1309,23 +1325,14 @@ svn_wc_merge4(enum svn_wc_merge_outcome_ > cancel_func, cancel_baton, > scratch_pool)); > > - /* Write our accumulation of log entries into a log file */ > - if (!dry_run) > - { > - const char *dir_abspath = svn_dirent_dirname(target_abspath, > - scratch_pool); > - > - /* Verify that we're holding this directory's write lock. */ > - SVN_ERR(svn_wc__write_check(wc_ctx->db, dir_abspath, scratch_pool)); > + /* svn_wc__internal_merge() should have queued all of its work. */ > + SVN_ERR_ASSERT(log_accum == NULL || svn_stringbuf_isempty(log_accum)); > > - /* Add all the work items to the queue, then run it. */ > - /* ### add_loggy takes a DIR, but wq_run is a simple WRI_ABSPATH */ > - SVN_ERR(svn_wc__wq_add_loggy(wc_ctx->db, dir_abspath, > - log_accum, scratch_pool)); > - SVN_ERR(svn_wc__wq_run(wc_ctx->db, target_abspath, > - cancel_func, cancel_baton, > - scratch_pool)); > - } > + /* If this isn't a dry run, then run the work! */ > + if (!dry_run) > + SVN_ERR(svn_wc__wq_run(wc_ctx->db, target_abspath, > + cancel_func, cancel_baton, > + scratch_pool)); > > return SVN_NO_ERROR; > } > > Modified: subversion/trunk/subversion/libsvn_wc/update_editor.c > URL: > http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/update_editor.c?rev=930162&r1=930161&r2=930162&view=diff > ============================================================================== > --- subversion/trunk/subversion/libsvn_wc/update_editor.c (original) > +++ subversion/trunk/subversion/libsvn_wc/update_editor.c Fri Apr 2 04:53:22 > 2010 > @@ -4260,7 +4260,6 @@ merge_file(svn_stringbuf_t **log_accum, > struct file_baton *fb, > const char *new_text_base_abspath, > const svn_checksum_t *actual_checksum, > - svn_boolean_t lock_removed, > apr_pool_t *pool) > { > struct edit_baton *eb = fb->edit_baton; > @@ -4523,10 +4522,11 @@ merge_file(svn_stringbuf_t **log_accum, > eb->cancel_func, eb->cancel_baton, > pool)); > > - /* ### now that we're past the internal_merge() (which might > - ### cause an exit), we can start writing work items. */ > - SVN_WC__FLUSH_LOG_ACCUM(eb->db, pb->local_abspath, *log_accum, > - pool); > + /* svn_wc__internal_merge() should have queued all of > + its work (including a bunch of stuff that we pre-loaded > + into the log accumulator). */ > + SVN_ERR_ASSERT(*log_accum == NULL > + || svn_stringbuf_isempty(*log_accum)); > > /* If we created a temporary left merge file, get rid of it. */ > if (delete_left) > @@ -4597,16 +4597,7 @@ merge_file(svn_stringbuf_t **log_accum, > work_item, pool)); > } > } > - > - if (lock_removed) > - { > - /* If a lock was removed and we didn't update the text contents, we > - might need to set the file read-only. */ > - SVN_ERR(svn_wc__loggy_maybe_set_readonly(log_accum, > pb->local_abspath, > - fb->local_abspath, pool, > - pool)); > - SVN_WC__FLUSH_LOG_ACCUM(eb->db, pb->local_abspath, *log_accum, > pool); > - } > + /* ### */ > } > > /* Deal with installation of the new textbase, if appropriate. */ > @@ -4954,13 +4945,25 @@ close_file(void *file_baton, > SVN_ERR(merge_file(&delayed_log_accum, > &content_state, entry, > fb, new_base_abspath, md5_actual_checksum, > - lock_state == svn_wc_notify_lock_state_unlocked, > pool)); > > /* Queue all operations. */ > SVN_WC__FLUSH_LOG_ACCUM(eb->db, fb->dir_baton->local_abspath, > delayed_log_accum, pool); > > + /* Now that all the state has settled, should we update the readonly > + status of the working file? The LOCK_STATE will signal what we should > + do for this node. */ > + if (new_base_abspath == NULL > + && lock_state == svn_wc_notify_lock_state_unlocked) > + { > + /* If a lock was removed and we didn't update the text contents, we > + might need to set the file read-only. */ > + SVN_ERR(svn_wc__loggy_maybe_set_readonly(eb->db, > + fb->dir_baton->local_abspath, > + fb->local_abspath, pool)); > + } > + > /* Clean up any temporary files. */ > if (fb->copied_text_base) > { > > >
