You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@gmail.com> on 2010/03/11 02:05:01 UTC

Re: svn commit: r921179 - /subversion/trunk/subversion/libsvn_wc/adm_ops.c

On Tue, Mar 9, 2010 at 17:37,  <cm...@apache.org> wrote:
>...
> +++ subversion/trunk/subversion/libsvn_wc/adm_ops.c Tue Mar  9 22:37:06 2010
> @@ -1268,22 +1268,49 @@ svn_wc_get_ancestry2(const char **url,
>                                   result_pool, scratch_pool));
>  }
>
> -/* Recursively mark a tree DIR_ABSPATH with a COPIED flag, skip items
> -   scheduled for deletion. */
> +/* Helper for mark_tree_copied(), handling the property juggling and
> +   state changes for a single item LOCAL_ABSPATH (of kind LOCAL_KIND). */
> +static svn_error_t *
> +mark_item_copied(svn_wc__db_t *db,
> +                 const char *local_abspath,
> +                 svn_wc__db_kind_t local_kind,
> +                 apr_pool_t *pool)

This new function/param should be named scratch_pool. All new funcs in
libsvn_wc should use the result_pool and scratch_pool naming/paradigm.
Pushing that out to other libraries is also a Good Thing, but
definitely for wc.

> +{
> +  apr_hash_t *props;
> +  svn_wc_entry_t tmp_entry;
> +  svn_node_kind_t kind =
> +    local_kind == svn_wc__db_kind_dir ? svn_node_dir : svn_node_unknown;
> +
> +  /* Squirrel away the pristine properties to install them on
> +     working, because we might delete the base table */
> +  SVN_ERR(svn_wc__db_read_pristine_props(&props, db, local_abspath,
> +                                         pool, pool));
> +  tmp_entry.copied = TRUE;
> +  SVN_ERR(svn_wc__entry_modify2(db, local_abspath, kind, FALSE, &tmp_entry,
> +                                SVN_WC__ENTRY_MODIFY_COPIED, pool));

The old code also did this for parent_stub=TRUE. You've now lost that...

>...
> @@ -1307,64 +1335,38 @@ mark_tree_copied(svn_wc__db_t *db,
>       if (hidden)
>         continue;
>
> -      SVN_ERR(svn_wc__get_entry(&entry, db, child_abspath, FALSE,
> -                                svn_node_unknown, FALSE, iterpool, iterpool));
> +      SVN_ERR(svn_wc__db_read_info(&child_status, &child_kind, NULL, NULL,
> +                                   NULL, NULL, NULL, NULL, NULL, NULL, NULL,
> +                                   NULL, NULL, NULL, NULL, NULL, NULL, NULL,
> +                                   NULL, NULL, NULL, NULL, NULL, NULL,
> +                                   db, child_abspath, iterpool, iterpool));
>
>       /* Skip deleted items. */
> -      if (entry->schedule == svn_wc_schedule_delete)
> +      if ((child_status == svn_wc__db_status_deleted) ||
> +          (child_status == svn_wc__db_status_obstructed_delete))
>         continue;
>
> -      /* If this is a directory, recurse. */
> -      if (entry->kind == svn_node_dir)
> +      /* If this is a directory, recurse; otherwise, do real work. */
> +      if (child_kind == svn_wc__db_kind_dir)
>         {
> -          SVN_ERR(mark_tree_copied(db, child_abspath,
> -                            cancel_func, cancel_baton,
> -                            iterpool));
> +          SVN_ERR(mark_tree_copied(db, child_abspath, child_status,
> +                                   cancel_func, cancel_baton, iterpool));
> +        }
> +      else
> +        {
> +          SVN_ERR(mark_item_copied(db, child_abspath, child_kind, iterpool));
>         }
> -
> -      /* Store the pristine properties to install them on working, because
> -         we might delete the base table */
> -      if (entry->kind != svn_node_dir)
> -        SVN_ERR(svn_wc__db_read_pristine_props(&props, db, child_abspath,
> -                                               iterpool, iterpool));
> -      tmp_entry.copied = TRUE;
> -      SVN_ERR(svn_wc__entry_modify2(db, child_abspath, svn_node_unknown,
> -                            TRUE, &tmp_entry,
> -                            SVN_WC__ENTRY_MODIFY_COPIED,
> -                            iterpool));

This is the part which got lost.

>...

I'm assuming you got no test errors, so I'm wondering what is going
on. It is really hard to trace the ->copied flag thru the entries
writing code.

Cheers,
-g

Re: svn commit: r921179 - /subversion/trunk/subversion/libsvn_wc/adm_ops.c

Posted by "C. Michael Pilato" <cm...@collab.net>.
C. Michael Pilato wrote:
> Greg Stein wrote:
>> On Tue, Mar 9, 2010 at 17:37,  <cm...@apache.org> wrote:
>>> ...
>>> +++ subversion/trunk/subversion/libsvn_wc/adm_ops.c Tue Mar  9 22:37:06 2010
>>> @@ -1268,22 +1268,49 @@ svn_wc_get_ancestry2(const char **url,
>>>                                   result_pool, scratch_pool));
>>>  }
>>>
>>> -/* Recursively mark a tree DIR_ABSPATH with a COPIED flag, skip items
>>> -   scheduled for deletion. */
>>> +/* Helper for mark_tree_copied(), handling the property juggling and
>>> +   state changes for a single item LOCAL_ABSPATH (of kind LOCAL_KIND). */
>>> +static svn_error_t *
>>> +mark_item_copied(svn_wc__db_t *db,
>>> +                 const char *local_abspath,
>>> +                 svn_wc__db_kind_t local_kind,
>>> +                 apr_pool_t *pool)
>> This new function/param should be named scratch_pool. All new funcs in
>> libsvn_wc should use the result_pool and scratch_pool naming/paradigm.
>> Pushing that out to other libraries is also a Good Thing, but
>> definitely for wc.
> 
> Okey dokey.

Fixed and committed this.  Thanks.

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


Re: svn commit: r921179 - /subversion/trunk/subversion/libsvn_wc/adm_ops.c

Posted by "C. Michael Pilato" <cm...@collab.net>.
Greg Stein wrote:
> On Tue, Mar 9, 2010 at 17:37,  <cm...@apache.org> wrote:
>> ...
>> +++ subversion/trunk/subversion/libsvn_wc/adm_ops.c Tue Mar  9 22:37:06 2010
>> @@ -1268,22 +1268,49 @@ svn_wc_get_ancestry2(const char **url,
>>                                   result_pool, scratch_pool));
>>  }
>>
>> -/* Recursively mark a tree DIR_ABSPATH with a COPIED flag, skip items
>> -   scheduled for deletion. */
>> +/* Helper for mark_tree_copied(), handling the property juggling and
>> +   state changes for a single item LOCAL_ABSPATH (of kind LOCAL_KIND). */
>> +static svn_error_t *
>> +mark_item_copied(svn_wc__db_t *db,
>> +                 const char *local_abspath,
>> +                 svn_wc__db_kind_t local_kind,
>> +                 apr_pool_t *pool)
> 
> This new function/param should be named scratch_pool. All new funcs in
> libsvn_wc should use the result_pool and scratch_pool naming/paradigm.
> Pushing that out to other libraries is also a Good Thing, but
> definitely for wc.

Okey dokey.

>> +{
>> +  apr_hash_t *props;
>> +  svn_wc_entry_t tmp_entry;
>> +  svn_node_kind_t kind =
>> +    local_kind == svn_wc__db_kind_dir ? svn_node_dir : svn_node_unknown;
>> +
>> +  /* Squirrel away the pristine properties to install them on
>> +     working, because we might delete the base table */
>> +  SVN_ERR(svn_wc__db_read_pristine_props(&props, db, local_abspath,
>> +                                         pool, pool));
>> +  tmp_entry.copied = TRUE;
>> +  SVN_ERR(svn_wc__entry_modify2(db, local_abspath, kind, FALSE, &tmp_entry,
>> +                                SVN_WC__ENTRY_MODIFY_COPIED, pool));
> 
> The old code also did this for parent_stub=TRUE. You've now lost that...
[...]
>> -                                               iterpool, iterpool));
>> -      tmp_entry.copied = TRUE;
>> -      SVN_ERR(svn_wc__entry_modify2(db, child_abspath, svn_node_unknown,
>> -                            TRUE, &tmp_entry,
>> -                            SVN_WC__ENTRY_MODIFY_COPIED,
>> -                            iterpool));
> 
> This is the part which got lost.
> 
>> ...
> 
> I'm assuming you got no test errors, so I'm wondering what is going
> on. It is really hard to trace the ->copied flag thru the entries
> writing code.

I definitely realized that I was making this change.  I got no test errors,
and I tried to figure out why.  I was thinking that the parent stub was a
bare-minimal bit of information that perhaps didn't even carry the copied
flag (and related copyfrom-* bits).  Certainly in wc-1 that's the way it was
-- little more than a subdir name and node-kind flag.

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand