You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Paul Burba <pt...@gmail.com> on 2008/04/01 01:50:34 UTC

Re: Re-merge a change from own history - corrupts svn:mergeinfo

On Mon, Mar 31, 2008 at 4:04 PM, Julian Foad <ju...@btopenworld.com> wrote:
> The following merge corrupts svn:mergeinfo:
>
>  svn merge --ignore-ancestry --accept=mine-full -c26169 \
>    subversion/tests/cmdline/merge_tests.py \
>
>    subversion/tests/cmdline/merge_tests.py
>
>  because (in subversion/libsvn_client/merge.c) do_file_merge() says:
>
>    if (honor_mergeinfo)
>      {
>        ...
>        calculate mergeinfo_path
>        ...
>      }
>    ...
>    if (record_mergeinfo ...)
>      {
>        ...
>        use mergeinfo_path
>        ...
>      }
>
>  and, in merges like this one, "record_mergeinfo" is true but "honor_mergeinfo"
>  isn't.
>
>  One interpretation of the bug is to say that we should never be recording
>  mergeinfo when we're not honouring it, because we decided that we never would
>  want to and that "ignore_ancestry" should prevent both things. In that case,
>  the following should fix it:
>
>  [[[
>   /* Set *HONOR_MERGEINFO and *RECORD_MERGEINFO (if non-NULL)
>  -   appropriately for MERGE_B. */
>  +   appropriately for MERGE_B.
>  +   One rule is that we shan't record mergeinfo if we're not honoring it. */
>   static APR_INLINE void
>   mergeinfo_behavior(svn_boolean_t *honor_mergeinfo,
>                      svn_boolean_t *record_mergeinfo,
>                      merge_cmd_baton_t *merge_b)
>   {
>     if (honor_mergeinfo)
>       *honor_mergeinfo = (merge_b->mergeinfo_capable
>                           && merge_b->sources_ancestral
>                           && merge_b->same_repos
>                           && (! merge_b->ignore_ancestry));
>
>     if (record_mergeinfo)
>       *record_mergeinfo = (merge_b->mergeinfo_capable
>                            && merge_b->sources_ancestral
>                            && merge_b->same_repos
>  +                         && (! merge_b->ignore_ancestry)
>                            && (! merge_b->dry_run));
>   }
>  ]]]
>
>  Paul, can you verify this is a correct fix? Can you see any knock-on effect?
>  The only other code path it affects is do_directory_merge, which is structured
>  a little differently and doesn't suffer the same problem.

Hi Julian,

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.

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.

>  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.

Paul

> (I don't
>  have a specific test for this. Symptom is sometimes/on-some-systems it crashes,
>  sometimes it produces bogus results...)
>
>  - Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Re-merge a change from own history - corrupts svn:mergeinfo

Posted by Julian Foad <ju...@btopenworld.com>.
Julian Foad wrote:
> I have now committed the patch with the tests tweaked, in r30162.
> 
> This issue should be resolved now.
> 
> Proposed for back-port to 1.5.x (including pburba's vote expressed in 
> IRC - just one more needed).

(Actually, no more votes were needed under the present relaxed rules.)

This fix is now back-ported to 1.5.x.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Re-merge a change from own history - corrupts svn:mergeinfo

Posted by Julian Foad <ju...@btopenworld.com>.
Paul Burba wrote:
> Sorry, I meant merge_tests.py 19 and *30*, not 60.  Patch to fix those
> tests with your change:

Thanks for that patch Paul: I had difficulty tweaking the test 30.

I have now committed the patch with the tests tweaked, in r30162.

This issue should be resolved now.

Proposed for back-port to 1.5.x (including pburba's vote expressed in IRC - 
just one more needed).

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Re-merge a change from own history - corrupts svn:mergeinfo

Posted by Paul Burba <pt...@gmail.com>.
On Tue, Apr 1, 2008 at 10:42 AM, Paul Burba <pt...@gmail.com> wrote:
> On Tue, Apr 1, 2008 at 9:49 AM, Julian Foad <ju...@btopenworld.com> wrote:
>  >

<snip>

>  >  > On Mon, Mar 31, 2008 at 9:50 PM, Paul Burba <pt...@gmail.com> wrote:
>  >  >> 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.

Sorry, I meant merge_tests.py 19 and *30*, not 60.  Patch to fix those
tests with your change:

[[[
* subversion/tests/cmdline/merge_tests.py
  (merge_skips_obstructions, property_merge_undo_redo): Tweak these tests
  to *not* expect mergeinfo changes when using --ignore-ancestry.
]]]

[[[
Index: merge_tests.py
===================================================================
--- merge_tests.py	(revision 30158)
+++ merge_tests.py	(working copy)
@@ -2000,7 +2000,7 @@
   # this file, to understand why we use short_wc_dir and chdir() below.
   expected_output = wc.State(short_wc_dir, { })
   expected_disk.remove('A/B/lambda')
-  expected_status.tweak('A/B/lambda', status='!M')
+  expected_status.tweak('A/B/lambda', status='! ')
   os.chdir(svntest.main.work_dir)
   expected_status.tweak('', status='  ')
   # Why do we need to --ignore-ancestry?  Because the previous merge of r4,
@@ -3314,13 +3314,11 @@
   expected_output = wc.State(wc_dir, {'A/B/E/alpha'  : Item(status=' C'), })

   expected_disk = svntest.main.greek_state.copy()
-  expected_disk.add({'' : Item(props={SVN_PROP_MERGEINFO : '/:2'}), })
   expected_disk.add({'A/B/E/alpha.prej'
      : Item("Trying to create property 'foo' with value 'foo_val',\n"
             + "but it has been locally deleted.\n")})

   expected_status = svntest.actions.get_virginal_state(wc_dir, 2)
-  expected_status.tweak('', status=' M')
   expected_status.tweak('A/B/E/alpha', status=' C')

   expected_skip = wc.State('', { })

]]]

Paul

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Re-merge a change from own history - corrupts svn:mergeinfo

Posted by Paul Burba <pt...@gmail.com>.
On Tue, Apr 1, 2008 at 9:49 AM, Julian Foad <ju...@btopenworld.com> wrote:
>
> Mark Phippard wrote:
>  > On Mon, Mar 31, 2008 at 9:50 PM, Paul Burba <pt...@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@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Re-merge a change from own history - corrupts svn:mergeinfo

Posted by Julian Foad <ju...@btopenworld.com>.
Julian Foad wrote:
>> I think it should stay the way it is and just take your original patch
>> so that it works as it was intended.  I recall when Mike P. added some
>> of this that he was pretty clear that it had to ignore mergeinfo
>> anyway, and I really cannot see it being that controversial for it to
>> not write any mergeinfo when you use this flag.
>>
>> If we really did decide in the future that these need to be separated,
>> which I doubt, then I do not see why we could not just change the API
>> and introduce additional options at that point.
> 
> Right: maybe I'm making a mountain out of a molehill. The only essential 
> thing we have to do is ensure that we agree on the meaning of 
> "--ignore-ancestry". If and when Paul agrees that it should include "do 
> not record any new mergeinfo" as part of its meaning, then we're all set.

(Replying to myself)

Yes, Paul agrees and so there is nothing more to do here than to apply my 
little patch that I posted before that makes the meaning of "--ignore-ancestry" 
consistent with how we've defined it.

(All that talk of API changes and user interfaces was just speculation really. 
If we ever find a need to do it we can do it then.)

Will commit the fix soon.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Re-merge a change from own history - corrupts svn:mergeinfo

Posted by Julian Foad <ju...@btopenworld.com>.
Mark Phippard wrote:
> On Tue, Apr 1, 2008 at 9:49 AM, Julian Foad <ju...@btopenworld.com> wrote:
>> So shall we:
>>
>> (1) Separate those options in the API(s)?
>>
>> (2) Not add any more command-line options now, 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.
> 
> 
> I think it should stay the way it is and just take your original patch
> so that it works as it was intended.  I recall when Mike P. added some
> of this that he was pretty clear that it had to ignore mergeinfo
> anyway, and I really cannot see it being that controversial for it to
> not write any mergeinfo when you use this flag.
> 
> If we really did decide in the future that these need to be separated,
> which I doubt, then I do not see why we could not just change the API
> and introduce additional options at that point.

Right: maybe I'm making a mountain out of a molehill. The only essential thing 
we have to do is ensure that we agree on the meaning of "--ignore-ancestry". If 
and when Paul agrees that it should include "do not record any new mergeinfo" 
as part of its meaning, then we're all set.

Paul, can you take a look at my floundering comments and questions in the 
attached patch?

- Julian

Re: Re-merge a change from own history - corrupts svn:mergeinfo

Posted by Mark Phippard <ma...@gmail.com>.
On Tue, Apr 1, 2008 at 9:49 AM, Julian Foad <ju...@btopenworld.com> wrote:
>
> Mark Phippard wrote:
>  > On Mon, Mar 31, 2008 at 9:50 PM, Paul Burba <pt...@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.
>  >>
>  >> 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.
>
>
>  > 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)?
>
>  (2) Not add any more command-line options now, 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.

I think it should stay the way it is and just take your original patch
so that it works as it was intended.  I recall when Mike P. added some
of this that he was pretty clear that it had to ignore mergeinfo
anyway, and I really cannot see it being that controversial for it to
not write any mergeinfo when you use this flag.

If we really did decide in the future that these need to be separated,
which I doubt, then I do not see why we could not just change the API
and introduce additional options at that point.

-- 
Thanks

Mark Phippard
http://markphip.blogspot.com/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Re-merge a change from own history - corrupts svn:mergeinfo

Posted by Julian Foad <ju...@btopenworld.com>.
Mark Phippard wrote:
> On Mon, Mar 31, 2008 at 9:50 PM, Paul Burba <pt...@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.
>>
>> 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.

> 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)?

(2) Not add any more command-line options now, 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.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Re-merge a change from own history - corrupts svn:mergeinfo

Posted by Mark Phippard <ma...@gmail.com>.
On Mon, Mar 31, 2008 at 9:50 PM, Paul Burba <pt...@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.
>
>  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.
>
>
>  >  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 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.

-- 
Thanks

Mark Phippard
http://markphip.blogspot.com/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org