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 <gstein_at_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, <gstein_at_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)
>> {
>>
>>
>>
>
Received on 2010-04-02 23:40:46 CEST