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...
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-07-23 16:14:57 CEST