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

Re: svn commit: r1310581 - /subversion/branches/ev2-export/subversion/libsvn_client/copy.c

From: Greg Stein <gstein_at_gmail.com>
Date: Fri, 6 Apr 2012 17:31:42 -0400

On Fri, Apr 6, 2012 at 16:53, Hyrum K Wright <hyrum.wright_at_wandisco.com> wrote:
> On Fri, Apr 6, 2012 at 3:48 PM,  <hwright_at_apache.org> wrote:
>> Author: hwright
>> Date: Fri Apr  6 20:48:32 2012
>> New Revision: 1310581
>>
>> URL: http://svn.apache.org/viewvc?rev=1310581&view=rev
>> Log:
>> On the ev2-export branch:
>> Directly add any new directories, rather than through the path_info structs.
>>
>> * subversion/libsvn_client/copy.c
>>  (path_driver_info_t): Remove dir_add member.
>>  (drive_single_path): Don't check for the dir_add flag.
>>  (drive_editor): Handle the new dirs before the various copy/move paths.
>>  (repos_to_repos_copy): Don't bother putting the new directoris into the
>>    set of paths to be operated on, just give them to drive_editor().
>>
>> Modified:
>>    subversion/branches/ev2-export/subversion/libsvn_client/copy.c
>>
>> Modified: subversion/branches/ev2-export/subversion/libsvn_client/copy.c
>> URL: http://svn.apache.org/viewvc/subversion/branches/ev2-export/subversion/libsvn_client/copy.c?rev=1310581&r1=1310580&r2=1310581&view=diff
>> ==============================================================================
>> --- subversion/branches/ev2-export/subversion/libsvn_client/copy.c (original)
>> +++ subversion/branches/ev2-export/subversion/libsvn_client/copy.c Fri Apr  6 20:48:32 2012
>> @@ -532,7 +532,6 @@ typedef struct path_driver_info_t
>>   svn_node_kind_t src_kind;
>>   svn_revnum_t src_revnum;
>>   svn_boolean_t resurrection;
>> -  svn_boolean_t dir_add;
>>   svn_string_t *mergeinfo;  /* the new mergeinfo for the target */
>>  } path_driver_info_t;
>>
>> @@ -627,19 +626,9 @@ drive_single_path(svn_editor_t *editor,
>>                   apr_pool_t *scratch_pool)
>>  {
>>   apr_hash_t *props = apr_hash_make(scratch_pool);
>> -  apr_array_header_t *children = apr_array_make(scratch_pool, 1,
>> -                                                sizeof(const char *));
>>   svn_boolean_t do_delete = FALSE;
>>   svn_boolean_t do_add = FALSE;
>>
>> -  /* Check to see if we need to add the path as a directory. */
>> -  if (path_info->dir_add)
>> -    {
>> -      return svn_error_trace(svn_editor_add_directory(editor, path, children,
>> -                                                      props,
>> -                                                      SVN_INVALID_REVNUM));
>> -     }
>> -
>>   /* If this is a resurrection, we know the source and dest paths are
>>      the same, and that our driver will only be calling us once.  */
>>   if (path_info->resurrection)
>> @@ -702,6 +691,7 @@ static svn_error_t *
>>  drive_editor(svn_editor_t *editor,
>>              apr_array_header_t *paths,
>>              apr_hash_t *action_hash,
>> +             apr_array_header_t *new_dirs,
>>              svn_boolean_t is_move,
>>              svn_revnum_t youngest,
>>              apr_pool_t *scratch_pool)
>> @@ -710,6 +700,23 @@ drive_editor(svn_editor_t *editor,
>>   apr_pool_t *iterpool;
>>   int i;
>>
>> +  if (new_dirs)
>> +    {
>> +      apr_array_header_t *children = apr_array_make(scratch_pool, 1,
>> +                                                sizeof(const char *));
>> +      apr_hash_t *props = apr_hash_make(scratch_pool);
>> +
>> +      /* These are directories which we just need to add. */
>> +      for (i = 0; i < new_dirs->nelts; i++)
>> +        {
>> +          const char *dir = APR_ARRAY_IDX(new_dirs, i, const char *);
>> +
>> +          /* ### The children param needs to be populated correctly. */
>> +          SVN_ERR(svn_editor_add_directory(editor, dir, children, props,
>> +                                           SVN_INVALID_REVNUM));
>
> Greg,
>
> Just so you know, I would have expected this section to crash Ev2 in
> copy test 67, since there are several directories being added, but the
> children array is (currently) empty.  I wonder if there is a problem
> with the internal Ev2 state checking.

I'm suspicious that the checks are not working because of the "pass
URLs as relpath" that you're doing.

I should add some canonical assertions for all the relpath params, and
get you to fix that bug :-)

> Also, this currently feeds paths to Ev2 child-first.  For instance,
> copy test 67 calls this with 'X/Y/Z', 'X/Y', and 'X', in that order.
> The shims reorder things so that it gets done properly, but I wonder
> if there shouldn't be some state checks in Ev2 to ensure parents are
> added before children.

That wouldn't be too hard. We could mark the parent as "only allowed
to modify" (as long as it hasn't already been marked as
"added/unalterable").

Cheers,
-g
Received on 2012-04-06 23:32:13 CEST

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