You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Shahaf <da...@elego.de> on 2011/09/23 07:06:14 UTC

Does this need to be added to the #3668/#3669 revert?

[[[
Index: subversion/libsvn_client/merge.c
===================================================================
--- subversion/libsvn_client/merge.c	(revision 1174351)
+++ subversion/libsvn_client/merge.c	(working copy)
@@ -6822,7 +6822,7 @@ do_file_merge(svn_mergeinfo_catalog_t result_catal
                                &inherited, svn_mergeinfo_inherited,
                                merge_b->ra_session1, target_abspath,
                                MAX(revision1, revision2),
-                               0, /* Get all implicit mergeinfo */
+                               MIN(revision1, revision2),
                                ctx, scratch_pool, iterpool);
 
       if (err)
]]]

This reverts a hunk of r1035894 (the revision merging the
issue-3668-3669 branch to trunk).

Re: Does this need to be added to the #3668/#3669 revert?

Posted by Paul Burba <pt...@gmail.com>.
On Fri, Sep 23, 2011 at 10:35 AM, Daniel Shahaf <da...@elego.de> wrote:
> Paul Burba wrote on Fri, Sep 23, 2011 at 10:24:07 -0400:
>> On Fri, Sep 23, 2011 at 1:06 AM, Daniel Shahaf <da...@elego.de> wrote:
>> > [[[
>> > Index: subversion/libsvn_client/merge.c
>> > ===================================================================
>> > --- subversion/libsvn_client/merge.c    (revision 1174351)
>> > +++ subversion/libsvn_client/merge.c    (working copy)
>> > @@ -6822,7 +6822,7 @@ do_file_merge(svn_mergeinfo_catalog_t result_catal
>> >                                &inherited, svn_mergeinfo_inherited,
>> >                                merge_b->ra_session1, target_abspath,
>> >                                MAX(revision1, revision2),
>> > -                               0, /* Get all implicit mergeinfo */
>> > +                               MIN(revision1, revision2),
>> >                                ctx, scratch_pool, iterpool);
>> >
>> >       if (err)
>> > ]]]
>> >
>> > This reverts a hunk of r1035894 (the revision merging the
>> > issue-3668-3669 branch to trunk).
>>
>> Hi Daniel,
>>
>> Yes, that should have been reverted, must have messed up some conflict
>> resolution.  Thanks for spotting that.  Committed the above patch in
>> r1174797 , nominating for backport in a moment.
>>
>> FWIW it's fairly harmless since after r1035894 was reverted,
>> get_full_mergeinfo() no longer filtered implicit mergeinfo and any
>> implicit mergeinfo obtained which is older than the requested merge of
>> REV1:REV2 isn't used anywhere else in do_file_merge.
>>
>> Paul
>
> In other words, the old revisions' mergeinfo would have been fetched
> from the server and then ignored/discarded?

Yes, the older revisions are ignored.  The only other place they are
used is in do_file_merge's call to
filter_natural_history_from_mergeinfo(), but that *only* deals in
possible tweaks to the rangelist described by REV1:REV2 (I'm referring
to the args of those names to do_file_merge), so the older ranges have
no impact.

Paul

> Thanks for the fix/information,
>
> Daniel
>

Re: Does this need to be added to the #3668/#3669 revert?

Posted by Daniel Shahaf <da...@elego.de>.
Paul Burba wrote on Fri, Sep 23, 2011 at 10:24:07 -0400:
> On Fri, Sep 23, 2011 at 1:06 AM, Daniel Shahaf <da...@elego.de> wrote:
> > [[[
> > Index: subversion/libsvn_client/merge.c
> > ===================================================================
> > --- subversion/libsvn_client/merge.c    (revision 1174351)
> > +++ subversion/libsvn_client/merge.c    (working copy)
> > @@ -6822,7 +6822,7 @@ do_file_merge(svn_mergeinfo_catalog_t result_catal
> >                                &inherited, svn_mergeinfo_inherited,
> >                                merge_b->ra_session1, target_abspath,
> >                                MAX(revision1, revision2),
> > -                               0, /* Get all implicit mergeinfo */
> > +                               MIN(revision1, revision2),
> >                                ctx, scratch_pool, iterpool);
> >
> >       if (err)
> > ]]]
> >
> > This reverts a hunk of r1035894 (the revision merging the
> > issue-3668-3669 branch to trunk).
> 
> Hi Daniel,
> 
> Yes, that should have been reverted, must have messed up some conflict
> resolution.  Thanks for spotting that.  Committed the above patch in
> r1174797 , nominating for backport in a moment.
> 
> FWIW it's fairly harmless since after r1035894 was reverted,
> get_full_mergeinfo() no longer filtered implicit mergeinfo and any
> implicit mergeinfo obtained which is older than the requested merge of
> REV1:REV2 isn't used anywhere else in do_file_merge.
> 
> Paul

In other words, the old revisions' mergeinfo would have been fetched
from the server and then ignored/discarded?

Thanks for the fix/information,

Daniel

Re: Does this need to be added to the #3668/#3669 revert?

Posted by Paul Burba <pt...@gmail.com>.
On Fri, Sep 23, 2011 at 1:06 AM, Daniel Shahaf <da...@elego.de> wrote:
> [[[
> Index: subversion/libsvn_client/merge.c
> ===================================================================
> --- subversion/libsvn_client/merge.c    (revision 1174351)
> +++ subversion/libsvn_client/merge.c    (working copy)
> @@ -6822,7 +6822,7 @@ do_file_merge(svn_mergeinfo_catalog_t result_catal
>                                &inherited, svn_mergeinfo_inherited,
>                                merge_b->ra_session1, target_abspath,
>                                MAX(revision1, revision2),
> -                               0, /* Get all implicit mergeinfo */
> +                               MIN(revision1, revision2),
>                                ctx, scratch_pool, iterpool);
>
>       if (err)
> ]]]
>
> This reverts a hunk of r1035894 (the revision merging the
> issue-3668-3669 branch to trunk).

Hi Daniel,

Yes, that should have been reverted, must have messed up some conflict
resolution.  Thanks for spotting that.  Committed the above patch in
r1174797 , nominating for backport in a moment.

FWIW it's fairly harmless since after r1035894 was reverted,
get_full_mergeinfo() no longer filtered implicit mergeinfo and any
implicit mergeinfo obtained which is older than the requested merge of
REV1:REV2 isn't used anywhere else in do_file_merge.

Paul