Update: I have found a problem in our old entries-mapping code which incorrectly handles inherited copyfrom information. I believe that I've fixed that problem, but some other problems have rippled out from there.
Will work more on it later... On Fri, Apr 2, 2010 at 15:14, Greg Stein <gst...@gmail.com> wrote: > 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) >> { >> >> >> >
