On Wed, Jul 23, 2008 at 10:14 AM, Paul Burba <ptburba_at_gmail.com> wrote:
> On Tue, Jul 22, 2008 at 7:23 PM, Karl Fogel <kfogel_at_red-bean.com> wrote:
>> pburba_at_tigris.org writes:
>>> Log:
>>> Fix remaining bug from issue #2825 'Handle --depth option for 'merge'
>>> operations in terms of both content and merge info'.
>>>
>>> An inoperative merge to a directory at --depth files should set
>>> non-inheritable mergeinfo on the merge target *and* normal mergeinfo on
>>> any of the targets file children. This fits with the current disposition of
>>> issue #2883. See item D) in
>>> http://subversion.tigris.org/issues/show_bug.cgi?id=2825#desc5.
>>>
>>> * subversion/libsvn_client/merge.c
>>> (THE CHILDREN_WITH_MERGEINFO ARRAY): Minor tweak to this global comment.
>>> (get_mergeinfo_walk_cb): Simplify check for paths which are immediate
>>> children of the merge target when depth is 'immediates'. Also check for
>>> file paths which are immediate children of the merge target when depth is
>>> 'files'.
>>> (get_mergeinfo_paths): Update doc string noting one case we've handled
>>> for a long time and one new case.
>>>
>>> --- trunk/subversion/libsvn_client/merge.c (r31657)
>>> +++ trunk/subversion/libsvn_client/merge.c (r31658)
>>> @@ -3091,8 +3091,9 @@ get_mergeinfo_walk_cb(const char *path,
>>>
>>> /* Store PATHs with explict mergeinfo, which are switched, are missing
>>> children due to a sparse checkout, are scheduled for deletion are absent
>>> - from the WC, and/or are first level sub directories relative to merge
>>> - target if depth is immediates. */
>>> + from the WC, are first level sub directories relative to merge target if
>>> + depth is immediates, and/or are file children of the merge target if
>>> + depth is files. */
>>> if (path_is_merge_target
>>> || has_mergeinfo_from_merge_src
>>> || entry->schedule == svn_wc_schedule_delete
>>> @@ -3102,8 +3103,11 @@ get_mergeinfo_walk_cb(const char *path,
>>> || entry->absent
>>> || ((wb->depth == svn_depth_immediates) &&
>>> (entry->kind == svn_node_dir) &&
>>> - (strcmp(parent_path, path) != 0) &&
>>> - (strcmp(parent_path, wb->merge_target_path) == 0)))
>>> + (strcmp(parent_path, wb->merge_target_path) == 0))
>>> + || ((wb->depth == svn_depth_files) &&
>>> + (entry->kind == svn_node_file) &&
>>> + (strcmp(parent_path, wb->merge_target_path) == 0))
>>> + )
>>> {
>>> svn_client__merge_path_t *child =
>>> apr_pcalloc(wb->children_with_mergeinfo->pool, sizeof(*child));
>>
>> I thought maybe this further patch would be appropriate:
>>
>> [[[
>> * subversion/libsvn_client/merge.c
>> (get_mergeinfo_walk_cb): Let svn_depth_immediates reach file
>> children too, when storing paths with explicit mergeinfo.
>> This follows up to r31658.
>> ]]]
>>
>> Index: subversion/libsvn_client/merge.c
>> ===================================================================
>> --- subversion/libsvn_client/merge.c (revision 32232)
>> +++ subversion/libsvn_client/merge.c (working copy)
>> @@ -3410,7 +3410,7 @@
>> || entry->depth == svn_depth_files
>> || entry->absent
>> || ((wb->depth == svn_depth_immediates) &&
>> - (entry->kind == svn_node_dir) &&
>> + (entry->kind == svn_node_dir || entry->kind == svn_node_file) &&
>> (strcmp(parent_path, wb->merge_target_path) == 0))
>> || ((wb->depth == svn_depth_files) &&
>> (entry->kind == svn_node_file) &&
>>
>> But then I saw these comments later in r31658:
>>
>>> @@ -3399,6 +3403,10 @@ insert_parent_and_sibs_of_sw_absent_del_
>>> a sparse checkout.
>>> 6) Path is absent from disk due to an authz restriction.
>>> 7) Path is equal to MERGE_CMD_BATON->TARGET.
>>> + 8) Path is an immediate *directory* child of MERGE_CMD_BATON->TARGET and
>>> + DEPTH is svn_depth_immediates.
>>> + 9) Path is an immediate *file* child of MERGE_CMD_BATON->TARGET and
>>> + DEPTH is svn_depth_files.
>>
>> Hmm, well, that matches what the code change does.
>>
>> Is the idea that when depth=immediates, we'll catch file children later
>> in the descent anyway?
>
> Hi Karl
>
> We don't need to catch the file children of the merge target if
> depth=immediates because those file children will inherit the
> inheritable mergeinfo that will be set on the merge target. The merge
> target's directory children need non-inheritable mergeinfo set on them
> though, which is why we do "catch" those.
>
> Your proposed change would set explicit mergeinfo on the merge
> target's file children which is equivalent to that set on the merge
> target. You won't see this in practice because the post-merge elision
> process would remove it, but it is not necessary.
>
> Does that make things any clearer?
>
> r31658 was primarily about fixing case where depth=files* In that
> case the merge target get's non-inheritable mergeinfo and its file
> children get inheritable mergeinfo. Is that part of the change clear?
> Your proposed change is only tenuously related to r31658.
>
> Paul
>
> * Though I did remove a check "(strcmp(parent_path, path) != 0)" for
> the immediates depth case which should have been a separate commit.
>
>> If so, won't we still not catch them, because
>> depth will still be immediates?
>>
>> I'd like to understand this better before voting for r31658/r31659...
Hi Karl,
I just wanted to ping you on this and see if you still had any
outstanding questions. I know you are busy, so no rush, I would only
like to get it in 1.5.2 if possible (which Hyrum is looking to roll
8/28).
Thanks,
Paul
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-08-15 16:21:27 CEST