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 21:14:31 CEST