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

Re: Issue #2853, do_single_file_merge() lacks comprehensive "Replace-style merge" capability due to lack of attention to ancestry

From: C. Michael Pilato <cmpilato_at_collab.net>
Date: 2007-08-01 18:10:50 CEST

Daniel Rall wrote:
> On Tue, 31 Jul 2007, C. Michael Pilato wrote:
>
>> C. Michael Pilato wrote:
>>> Daniel Rall wrote:
>>>> Mike, r25891 doesn't appear to handle Replace support for the 'svn merge
>>>> -r2:1 url' case which is blocking issue #2822 (by causing the failure
>>>> for revert test #15 w/ the patch in that issue). This particular case
>>>> does an implicit peg of HEAD, which is handled in the block above the
>>>> one quoted below (in which you've fixed the Replace problem for
>>>> history-sensitive merging).
>>>>
>>>> In r25889, you changed the merge test case written by Senthil to avoid
>>>> this implicit use of a peg revision, and mentioned that "the jury is
>>>> still out (for me) on whether or not that use case is expected to
>>>> fail." Would you elaborate on this?
>>> Sure.
>>>
>>> Strictly speaking, and obeying the rules of the peg revision algorithm, 'svn
>>> merge -r2:1 URL[@HEAD]' means "please apply, in reverse, the change made to
>>> the line of history identified by URL@HEAD between revisions 1 and 2". In
>>> the test case mentioned, this causes a bit of a problem. URL@HEAD only has
>>> a single revision of history, r2 -- it doesn't exist in r1. A stretch in
>>> the interpretation of the peg-rev algorithm *might* let us allow to treat
>>> 'svn merge -r2:1 URL@HEAD' as a removal of all the lines in URL@2, but
>>> there's no support in the peg-rev algorithm whatsoever for referring to two
>>> different lines of the history. File-replace is, therefore, incompatible
>>> with that syntax.
>>>
>>> I'm looking over the code in discover_and_merge_children(), and what I see
>>> makes me suspicious. The code assumes that:
>>>
>>> "dir@PEG -rM:N" == "dir/child1@PEG -rM:N" + "dir/child2@PEG -rM:N" + ...
>>>
>>> Something tells me that simply isn't true.
>>>
>> Note that 'svn diff -r2:1 URL' produces the same kind of problem.
>>
>> But if at a theoretical level discover_and_merge_children() is sane in its
>> assumptions, there's no reason why do_single_file_merge() couldn't take a
>> flag to force the interpretation of the arguments into an "I'm a subset of a
>> directory merge" mode, used only when not called as part of a file-targeted
>> 'svn merge' invocation. In fact, I'll go ahead and add that mode now, and
>> we can deal with the theoretical implications later. This should solve your
>> revert test problem.
>
> That'd be great, Mike.

I thought more about this, and I've convinced myself that this would be a
wrong move. You are basically wanting to get non-peg-rev behavior with
peg-rev inputs, right? So, I set out to see how much this
dual-forms-of-input strategy permeates the merging logic. I was surprised
by what I found:

We have two public API starting points.

   svn_client_merge_peg() -- peg-rev-ful
   svn_client_merge() -- peg-rev-less

Both call the helpers do_merge() and do_single_file_merge(), depending on
whether or not the target of the merge is a directory or a file,
respectively. But here's where it gets confusing. do_merge() and
do_single_file_merge() each accept both forms of input -- pegged and
non-pegged. Why?! The non-pegged syntax can be derived from the pegged, so
why not make svn_client_merge_peg() just convert its input to non-pegged
forms before calling now-simplified do_merge() and do_single_file_merge()
functions?

I'm going to investigate this possibility further.

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

Received on Wed Aug 1 18:09:19 2007

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