On Tue, Apr 1, 2008 at 9:49 AM, Julian Foad <julianfoad_at_btopenworld.com> wrote:
>
> Mark Phippard wrote:
> > On Mon, Mar 31, 2008 at 9:50 PM, Paul Burba <ptburba_at_gmail.com> wrote:
> >
> >> I agree that we need to clarify the effect --ignore-ancestry has
> >> regarding mergeinfo. Your fix is certainly correct in that it fixes
> >> the bogus mergeinfo/segfault, but I'm not sure that it is correct in
> >> terms of expanding the meaning of --ignore-ancestry to mean *not*
> >> setting mergeinfo. To get back to what Karl said earlier in this
> >> thread, maybe we need a --no-record option so we can do all of the
> >> following:
> >>
> >> A) Ignore mergeinfo when calculating what to merge and don't set any mergeinfo
> >>
> >> B) Ignore mergeinfo when calculating what to merge but set mergeinfo
> >> describing the merge
> >>
> >> C) Consider mergeinfo when calculating what to merge but don't set any mergeinfo
> >>
> >> D) Consider mergeinfo when calculating what to merge and set mergeinfo
> >> describing the merge
> >>
> >> Question is, are there valid use cases for all of these? Obviously
> >> there is for D, that's Merge Tracking(TM)! B is what using
> >> --ignore-ancestry today does and it seems potentially useful in cases
> >> like those described in issue #2898 - Imagine a case like that
> >> described in http://subversion.tigris.org/issues/show_bug.cgi?id=2898#desc7
> >> where there are other subtrees with mergeinfo that we want
> >> updated/elided.
Scratch that thought on issue #2898. Even if --ignore-ancestry
doesn't consider *or* set mergeinfo (option C) it can still be used to
compel a merge that otherwise wouldn't happen due to mergeinfo. Then
an inoperative merge *without* --ignore-ancestry could be used to
cleanup (elide) subtree mergeinfo. So there is a perfectly acceptable
work-around here.
> >> But for A and C I can't come up with a valid use case...Given that B
> >> and D are handled today, for 1.5 I'd be happy just fixing
> >> do_file_merge() and waiting to see if we need a --no-record option to
> >> enable A and C and putting it in 1.6.
>
> Hmm, yes. The suppression of merge tracking info is something I can't
> immediately imagine wanting. That doesn't mean there won't be good cases for
> it, but like you say we don't have to provide that option until we're shown a
> need for it.
>
> Today, it seems we're not clear on what the "--ignore-ancestry" flag does or
> should do. Now I come to try to describe it, I'm not even sure precisely what
> it has always done. It's something like, "diff the content of the two source
> trees path by path without taking advantage of any "copy from" history to
> compose the diff". But maybe the definition is a bit different from that.
>
>
> >> > It certainly seems to fix the problem. I'm running "make check" now.
> >>
> >> FWIW It will break merge_tests.py 19 and 60, but in both cases it
> >> would just be a matter of tweaking the tests to agree with the new
> >> meaning of --ignore-ancestry.
>
> I think my "make check" ran without failures, though I seem to have lost the
> proof of having run it.
Those tests fail only after r30145 (start recording mergeinfo for
no-op merges) so you probably didn't see it.
> > I am confused. Why would we want --ignore-ancestry to record
> > mergeinfo? If we are not considering mergeinfo as part of the merge,
> > then it seems like we should not be recording it either. It seems not
> > unlike the case of merging from a foreign repository, where we also do
> > not consider or record mergeinfo.
>
> Mark,
>
> Without answering your question, let me split it into two:
>
> (1) It feels like the API should have separate options for whether to
>
> * notice ancestral relationship (affects how each diff is created)
> * honor existing mergeinfo (affects which diffs are created)
> * record new mergeinfo
>
> because they are all logically separate (and somewhat independent*). There
> might well be valid use cases for all sensible combinations within some client
> software that helps users to perform a series of merges, for example. Even if
> there aren't valid use cases for all combinations, that doesn't mean the API
> shouldn't express the options separately.
>
> (* Both honoring existing mergeinfo and recording new mergeinfo are dependent
> on the idea of there being a "source branch" for the info to refer to.
> Recognising a "source branch" is dependent on having recognised an ancestral
> relationship between the two source trees, so there is a dependeny between
> these options.)
>
> But
>
> (2) The command-line client need not and probably should not support all of
> these combinations. It certainly need not support them all in v1.5.
>
> So shall we:
>
> (1) Separate those options in the API(s)?
For 1.5 I'd say no. We can always rev the API if it becomes clear we
need this finer grained control no?
> (2) Not add any more command-line options now,
+1
> but decide on a meaning for the
> existing "--ignore-ancestry" flag now, that will set some combination of the
> above options, and accept that we might want to supplement that with additional
> flags in a later version. The meaning of the "--ignore-ancestry" flag now
> should be one that closely fits its historical (pre-merge-tracking) *usage*
> (rather than its implementation) and that also is not too confusing in the
> presence of other options that we might want to provide later.
As I mentioned above, I'm backing away from the need for
--ignore-ancestry to no consider mergeinfo when deciding what to merge
but to still set mergeinfo. Making --ignore-ancestry not consider and
not set mergeinfo seems more logical and I suspect will be less
surprising to users. So +1 on your patch, which does just that.
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-04-01 16:42:32 CEST