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 Rall <dl...@collab.net> on 2006/03/25 00:58:29 UTC

[PATCH] Add ability to mark change sets as merged

This is effectively a way to manipulate the merge memory stored in the
"svnmerge-integrated" property.  It's the flip-side of the 'block'
concept.

Use case: A change set which is sufficiently different when backported
that use of 'svn merge' is no longer appropriate.  Example scenarios:

   o The actual change you want to apply to branch has no overlap with
   its incarnation on the source branch, yet is conceptually
   equivalent.

  o Only a subset of a change set warrants application.

  o The branch content has drifted far enough apart to make automatic
  merging impossible.

Typical usage is paired with manually-issued, targeted 'svn merge'
and/or 'svn diff' commands, stand-alone 'diff' and 'patch', and/or
manual edits to the target branch.
-- 

Daniel Rall


p.s. On a related note, I've got 'svnmerge.py rollback [--memory]'
stubbed out too.

Incidently, these changes are to implement the "Record Manual Merge"
and "Rollback Merge" requirements mentioned in Subversion's Merge
Tracking Requirements doc
<http://subversion.tigris.org/merge-tracking/requirements.html>

Re: [Svnmerge] [PATCH] Add ability to mark change sets as merged

Posted by Giovanni Bajo <ra...@develer.com>.
David James <dj...@collab.net> wrote:


>> """
>> [...]
>> The immediate effect is the same (the revision is not available for
>> merge anymore), but the behaviour will be more correct with respect
>> to merge across multiple branches.
>> """
>
> Really? Why would --record-only allow svnmerge.py to behave more
> correctly?

Lie to svnmerge.py and it will take its revenge :)

You manual-merge r100 from A to B, and block it in B (instead of
using --record-only). Then you merge all the changes from B to C: this
effectively brings in the manual merge of r100, but C doesn't know about it.
You then merge everything from A to C, and you get conflicts because r100 is
merged again.

I'm not sure svnmerge.py is *currently* smart enough that, if you
use --record-only, it understands that it doesn't need to merge r100 from A to
C. Surely, it is a feature that it is feasable. Even if it doesn't right now,
my point that you shouldn't lie to svnmerge still stands.

Anyway, I'm satisfied with what was committed right now as part of the help of
'block'. It is clear enough.

Giovanni Bajo


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

Re: [Svnmerge] [PATCH] Add ability to mark change sets as merged

Posted by Daniel Rall <dl...@collab.net>.
Giovanni, David, thanks for the excellent feedback.

On Sat, 25 Mar 2006, David James wrote:

> On 3/25/06, Giovanni Bajo <ra...@develer.com> wrote:
> > Daniel Rall <dl...@collab.net> wrote:
> >
> > > This is effectively a way to manipulate the merge memory stored in the
> > > "svnmerge-integrated" property.  It's the flip-side of the 'block'
> > > concept.
> >
> > Hi Dan, thanks for the patch. It looks ok.
> >
> > I don't specifically like the name of the option nor the documentation. I
> > thought of "--manual": what people think of it? I'm not really satisfied with
> > it either. "--record-only" is longer but possibly better? I don't think it's
> > too long as I don't expect it to be used so commonly to justify the need of a
> > shorter name (and there's the single-dash letter version anyway). [BTW, I don't
> > care about the single-dash letter, -M: that's always a non-informative
> > non-mnemonic version of the option, so really any letter works).
> >
> > So a proposal:
> >
> > """
> > --record-only: do not perform an actual merge of the changes, yet record that a
> > merge happened.
> > """
> 
> I definitely like these new names. "--manual" describes the use case:
> I did a manual merge; please update the merge metadata for me.
> "--record-only" describes what svnmerge.py actually does -- it records
> that we already did a manual merge. Both names are good, with the
> first being shorter, and the second being slightly more clear. Like
> you, I tend to favour clarity over brevity.

I actually considered both these names, and am happy with
--record-only too.

It's nice when the short option also reflects the long option.  I
considering switching the short option from -M to -O (-R reminds me
too much of Subverion's own "recursive"), but after conversation with
David, left it as -M on the grounds that it's a better mnemonic for
"manual" and "merge".

> > As for the main description at the top:
> >
> > """
> > When manually merging changes across branches, --record-only can be used to
> > instruct svnmerge that a manual merge of a certain revision already happened,
> > so that it can record it and not offer that revision for merge anymore.
> > """
> 
> Very clear. Great! Thanks for writing this documentation as it will be
> very helpful for users.

Looks great, thanks Giovanni!

> Should we also refer back to the "block" subcommand, which allows you
> to ask svnmerge *not* to merge a particular set of revisions? In some
> cases, if you didn't exactly "merge" the revision but the
> functionality is available on the branch with similar code, it might
> be tough to choose between the two.

I mentioned it.

> We may want to mention somewhere that svnmerge.py does not record
> "indirect" metadata (i.e. it does not record that, if you merge a
> change from branchA to branchB, you might indirectly be merging
> changes from trunk). For now, you can correct this metadata using
> merge --record-only. In future, it might be possible for us to be a
> bit better about merging the indirect metadata, but I don't think
> we'll ever get it perfect, so this feature will always be handy.
> 
> > I'd also add a note to the svnmerge block help page:
> >
> > """
> > Do not use this option to hide revisions that were manually merged into the
> > branch. Instead, use "svnmerge merge --record-only".
> 
> Great! Good idea to refer back to "--record-only".

I used this text.

> > The immediate effect is the same (the revision is not available for merge anymore),
> > but the behaviour will be more correct with respect to merge across multiple branches.
> 
> Really? Why would --record-only allow svnmerge.py to behave more
> correctly? I find this sentence a bit confusing. I think we should
> simply say that "--record-only" records that a merge happened, whereas
> "block" records that a merge should not happen.

I used something closer to David's suggestion, and committed the
changes to Subversion trunk in r19051.  If you'd like any additional
tweaks, let me know.
-- 

Daniel Rall

Re: [Svnmerge] [PATCH] Add ability to mark change sets as merged

Posted by David James <dj...@collab.net>.
On 3/25/06, Giovanni Bajo <ra...@develer.com> wrote:
> Daniel Rall <dl...@collab.net> wrote:
>
> > This is effectively a way to manipulate the merge memory stored in the
> > "svnmerge-integrated" property.  It's the flip-side of the 'block'
> > concept.
>
> Hi Dan, thanks for the patch. It looks ok.
>
> I don't specifically like the name of the option nor the documentation. I
> thought of "--manual": what people think of it? I'm not really satisfied with
> it either. "--record-only" is longer but possibly better? I don't think it's
> too long as I don't expect it to be used so commonly to justify the need of a
> shorter name (and there's the single-dash letter version anyway). [BTW, I don't
> care about the single-dash letter, -M: that's always a non-informative
> non-mnemonic version of the option, so really any letter works).
>
> So a proposal:
>
> """
> --record-only: do not perform an actual merge of the changes, yet record that a
> merge happened.
> """

I definitely like these new names. "--manual" describes the use case:
I did a manual merge; please update the merge metadata for me.
"--record-only" describes what svnmerge.py actually does -- it records
that we already did a manual merge. Both names are good, with the
first being shorter, and the second being slightly more clear. Like
you, I tend to favour clarity over brevity.

> As for the main description at the top:
>
> """
> When manually merging changes across branches, --record-only can be used to
> instruct svnmerge that a manual merge of a certain revision already happened,
> so that it can record it and not offer that revision for merge anymore.
> """

Very clear. Great! Thanks for writing this documentation as it will be
very helpful for users.

Should we also refer back to the "block" subcommand, which allows you
to ask svnmerge *not* to merge a particular set of revisions? In some
cases, if you didn't exactly "merge" the revision but the
functionality is available on the branch with similar code, it might
be tough to choose between the two.

We may want to mention somewhere that svnmerge.py does not record
"indirect" metadata (i.e. it does not record that, if you merge a
change from branchA to branchB, you might indirectly be merging
changes from trunk). For now, you can correct this metadata using
merge --record-only. In future, it might be possible for us to be a
bit better about merging the indirect metadata, but I don't think
we'll ever get it perfect, so this feature will always be handy.

> I'd also add a note to the svnmerge block help page:
>
> """
> Do not use this option to hide revisions that were manually merged into the
> branch. Instead, use "svnmerge merge --record-only".

Great! Good idea to refer back to "--record-only".

> The immediate effect is the same (the revision is not available for merge anymore),
> but the behaviour will be more correct with respect to merge across multiple branches.

Really? Why would --record-only allow svnmerge.py to behave more
correctly? I find this sentence a bit confusing. I think we should
simply say that "--record-only" records that a merge happened, whereas
"block" records that a merge should not happen.

Cheers,

David

--
David James -- http://www.cs.toronto.edu/~james

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


Re: [Svnmerge] [PATCH] Add ability to mark change sets as merged

Posted by Giovanni Bajo <ra...@develer.com>.
Daniel Rall <dl...@collab.net> wrote:

> This is effectively a way to manipulate the merge memory stored in the
> "svnmerge-integrated" property.  It's the flip-side of the 'block'
> concept.

Hi Dan, thanks for the patch. It looks ok.

I don't specifically like the name of the option nor the documentation. I
thought of "--manual": what people think of it? I'm not really satisfied with
it either. "--record-only" is longer but possibly better? I don't think it's
too long as I don't expect it to be used so commonly to justify the need of a
shorter name (and there's the single-dash letter version anyway). [BTW, I don't
care about the single-dash letter, -M: that's always a non-informative
non-mnemonic version of the option, so really any letter works).

So a proposal:

"""
--record-only: do not perform an actual merge of the changes, yet record that a
merge happened.
"""

As for the main description at the top:

"""
When manually merging changes across branches, --record-only can be used to
instruct svnmerge that a manual merge of a certain revision already happened,
so that it can record it and not offer that revision for merge anymore.
"""

I'd also add a note to the svnmerge block help page:

"""
Do not use this option to hide revisions that were manually merged into the
branch. Instead, use "svnmerge merge --record-only". The immediate effect is
the same (the revision is not available for merge anymore), but the behaviour
will be more correct with respect to merge across multiple branches.
"""

Giovanni Bajo


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

Re: [PATCH] Add ability to mark change sets as merged

Posted by David James <dj...@collab.net>.
On 3/24/06, Daniel Rall <dl...@collab.net> wrote:
> This is effectively a way to manipulate the merge memory stored in the
> "svnmerge-integrated" property.  It's the flip-side of the 'block'
> concept.

Excellent work, Dan! I'm especially impressed with your clearly
spelled out use cases. Feel free to commit.

If further feedback from the svnmerge list determines we want to make
tweaks to the names of the command-line options, we can make them as
needed.

( svnmerge developers, see http://svn.haxx.se/dev/archive-2006-03/1097.shtml
for Dan's original post, which hasn't shown up on the svnmerge list yet. )

> Use case: A change set which is sufficiently different when backported
> that use of 'svn merge' is no longer appropriate.

This happens to me all the time! When I merge from trunk to 1.3.x, I
often run into conflicts, so I create a temporary merge branch. If I
tried to merge from the temporary merge branch to 1.3.x, the merge
metadata would be lost because svnmerge.py does not track indirect
metadata. To correct this metadata, I'd use svnmerge.py merge
--memory.

As a side note, I'm hoping to extend svnmerge.py to track metadata
across merge branches, but this is quite tricky to implement
correctly.

> [snip clearly spelled out use cases ]

> p.s. On a related note, I've got 'svnmerge.py rollback [--memory]'
> stubbed out too.

Great! This will be a very useful feature. I'm looking forward to your patch.

Cheers,

David


--
David James -- http://www.cs.toronto.edu/~james

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