[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 17:40:18 -0400

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

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