[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r930162 - in /subversion/trunk/subversion/libsvn_wc: adm_ops.c entries.c entries.h log.c log.h merge.c update_editor.c

From: Greg Stein <gstein_at_gmail.com>
Date: Fri, 2 Apr 2010 15:14:03 -0400

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

This is an archived mail posted to the Subversion Dev mailing list.