Incorrect merge

Incorrect merge

[Ken Brown]

Hi Michael,

It looks like the following change got merged with the trunk inadvertently:

revno: 100136
committer: Michael Albinus <michael.albinus@gmx.de>
branch nick: emacs-23
timestamp: Mon 2010-10-25 13:46:21 +0200
message:
   * dbusbind.c (Fdbus_call_method_asynchronously)
   (Fdbus_register_signal, Fdbus_register_method): Check, whether
   `dbus-registered-objects-table' is initialized.

   Must not be synchronized with the trunk.

Ken

Re: Incorrect merge

[Chong Yidong]

Ken Brown <kbrown@cornell.edu> writes:

> Hi Michael,
>
> It looks like the following change got merged with the trunk inadvertently:
>
> revno: 100136
> committer: Michael Albinus <michael.albinus@gmx.de>
> branch nick: emacs-23
> timestamp: Mon 2010-10-25 13:46:21 +0200
> message:
>   * dbusbind.c (Fdbus_call_method_asynchronously)
>   (Fdbus_register_signal, Fdbus_register_method): Check, whether
>   `dbus-registered-objects-table' is initialized.
>
>   Must not be synchronized with the trunk.

I just corrected this.  Thanks.

Re: Incorrect merge

[Chong Yidong]

Ken Brown <kbrown@cornell.edu> writes:

> It looks like the following change got merged with the trunk inadvertently:
>
> revno: 100136
> committer: Michael Albinus <michael.albinus@gmx.de>
> branch nick: emacs-23
> timestamp: Mon 2010-10-25 13:46:21 +0200
> message:
>   * dbusbind.c (Fdbus_call_method_asynchronously)
>   (Fdbus_register_signal, Fdbus_register_method): Check, whether
>   `dbus-registered-objects-table' is initialized.
>
>   Must not be synchronized with the trunk.

Should be fixed now, thanks.

Re: Incorrect merge

[Eli Zaretskii]

> From: Chong Yidong <cyd@stupidchicken.com>
> Date: Mon, 01 Nov 2010 12:02:31 -0400
> Cc: Michael Albinus <michael.albinus@gmx.de>, Emacs <emacs-devel@gnu.org>
> 
> Ken Brown <kbrown@cornell.edu> writes:
> 
> > It looks like the following change got merged with the trunk inadvertently:
> >
> > revno: 100136
> > committer: Michael Albinus <michael.albinus@gmx.de>
> > branch nick: emacs-23
> > timestamp: Mon 2010-10-25 13:46:21 +0200
> > message:
> >   * dbusbind.c (Fdbus_call_method_asynchronously)
> >   (Fdbus_register_signal, Fdbus_register_method): Check, whether
> >   `dbus-registered-objects-table' is initialized.
> >
> >   Must not be synchronized with the trunk.
> 
> Should be fixed now, thanks.

Should we have improved procedures to avoid such incidents in the
future?  This isn't the first time things get merged that shouldn't.
I wonder how many such cases are still there on the trunk, undetected.

Re: Incorrect merge

[Juanma Barranquero]

On Mon, Nov 1, 2010 at 18:54, Eli Zaretskii <eliz@gnu.org> wrote:

> Should we have improved procedures to avoid such incidents in the future?

Definitely.

> This isn't the first time things get merged that shouldn't.

Indeed.

    Juanma

Re: Incorrect merge

[Eli Zaretskii]

> From: Juanma Barranquero <lekktu@gmail.com>
> Date: Mon, 1 Nov 2010 21:21:04 +0100
> Cc: Chong Yidong <cyd@stupidchicken.com>, michael.albinus@gmx.de, kbrown@cornell.edu, 
> 	emacs-devel@gnu.org
> 
> On Mon, Nov 1, 2010 at 18:54, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> > Should we have improved procedures to avoid such incidents in the future?
> 
> Definitely.

Suggestions are welcome.

Re: Incorrect merge

[Juanma Barranquero]

On Mon, Nov 1, 2010 at 21:37, Eli Zaretskii <eliz@gnu.org> wrote:

> Suggestions are welcome.

I don't think there are magic bullets. Either we note somewhere (in a
file, for example) which revisions in the branches must not be
committed to the trunk, or we make sure that the info is conspicuous
enough in the patches themselves so the person doing the merge does
not miss it.

Adding a note to the commit log was suggested, and in fact followed in
this particular case, but during the merge is usually not necessary to
look at the commit log messages unless a conflict arises or you
suspect trouble somehow. A note as the first line in the ChangeLog
entry would be more visible, IMO. Perhaps also comments in the source,
though that would be messy/ugly for big patches.

Less significant, but more error prone than porting unwanted patches
to the trunk is merging the ChangeLogs. In the merges I've done from
emacs-23, making sure the resulting ChangeLogs were updated with the
relevant entries (and nothing more and nothing less), was the most
time-consuming aspect of the merge.

    Juanma

Re: Incorrect merge

[Óscar Fuentes]

Eli Zaretskii <eliz@gnu.org> writes:

>> > Should we have improved procedures to avoid such incidents in the future?
>> 
>> Definitely.
>
> Suggestions are welcome.

The problem is that not everything that is committed on emacs-23 branch
is intended to be merged into trunk. The solution is obvious: don't
merge stuff from emacs-23 into trunk. Just use another branch (let's
call it `common-fixes') where patches intended for trunk and emacs-23
are applied. Then you merge `common-fixes' into trunk and emacs-23.

Re: Incorrect merge

[Jason Rumney]

On 02/11/2010 07:02, Óscar Fuentes wrote:

> The problem is that not everything that is committed on emacs-23 branch
> is intended to be merged into trunk. The solution is obvious: don't
> merge stuff from emacs-23 into trunk. Just use another branch (let's
> call it `common-fixes') where patches intended for trunk and emacs-23
> are applied. Then you merge `common-fixes' into trunk and emacs-23.
>    


How does the developer verify that their change is correct?  The 
common-fixes branch cannot be expected to build after some time has 
passed, as the changes commited to it might depend on other changes that 
differ between the trunk and emacs-23 branch, but are nonetheless required.

Re: Incorrect merge

[Stephen J. Turnbull]

Juanma Barranquero writes:
 > On Mon, Nov 1, 2010 at 21:37, Eli Zaretskii <eliz@gnu.org> wrote:
 > 
 > > Suggestions are welcome.
 > 
 > I don't think there are magic bullets.

In Python, they have a script called svnmerge.py.  This script did many
things, one of them was to keep track of blocked revisions, ie,
revisions that should not be merged to trunk (or to some given branch,
IIRC).

Re: Incorrect merge

[Óscar Fuentes]

Jason Rumney <jasonr@gnu.org> writes:

> On 02/11/2010 07:02, Óscar Fuentes wrote:
>
>> The problem is that not everything that is committed on emacs-23 branch
>> is intended to be merged into trunk. The solution is obvious: don't
>> merge stuff from emacs-23 into trunk. Just use another branch (let's
>> call it `common-fixes') where patches intended for trunk and emacs-23
>> are applied. Then you merge `common-fixes' into trunk and emacs-23.
>
> How does the developer verify that their change is correct?

By testing it, of course. More precisely, the developer must ensure that
everything he commits to common-fixes works on emacs-23 and trunk.

> The common-fixes branch cannot be expected to build after some time
> has passed, as the changes commited to it might depend on other
> changes that differ between the trunk and emacs-23 branch, but are
> nonetheless required.

This is equivalent to saying that a change on common-fixes could depend
on a change made to emacs-23 only. This does not qualify as a fix
intended for both emacs-23 and trunk, doesn't it?

Re: Incorrect merge

[Andreas Schwab]

Óscar Fuentes <ofv@wanadoo.es> writes:

> This is equivalent to saying that a change on common-fixes could depend
> on a change made to emacs-23 only.

How do you keep the branch up-to-date?

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Incorrect merge

[Óscar Fuentes]

Andreas Schwab <schwab@linux-m68k.org> writes:

> Óscar Fuentes <ofv@wanadoo.es> writes:
>
>> This is equivalent to saying that a change on common-fixes could depend
>> on a change made to emacs-23 only.
>
> How do you keep the branch up-to-date?

AFAIK almost all changes to emacs-23 are eventually merged into
trunk. This means that common-fixes would containt all changes intended
to emacs-23, except those few which are not intended for trunk. This
means that the divergence of emacs-23 from common-fixes will not be
significant.

Re: Incorrect merge

[Stefan Monnier]

>> > Suggestions are welcome.
>> I don't think there are magic bullets.
> In Python, they have a script called svnmerge.py.  This script did
> many things, one of them was to keep track of blocked revisions, ie,
> revisions that should not be merged to trunk (or to some given branch,
> IIRC).

Indeed, the only way I can think of to try and make sure such undesired
merges don't happen is to try and encode this fact into a merge script.
Having such a merge script is a good idea in any case since it can help
resolve the ChangeLog conflicts as well (they tend to require a lot of
work, but most of it can be automated).


        Stefan

Re: Incorrect merge

[Óscar Fuentes]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>> In Python, they have a script called svnmerge.py.  This script did
>> many things, one of them was to keep track of blocked revisions, ie,
>> revisions that should not be merged to trunk (or to some given branch,
>> IIRC).
>
> Indeed, the only way I can think of to try and make sure such undesired
> merges don't happen is to try and encode this fact into a merge
> script.

The *only* way? What abut having a branch for that purpose?

BTW, do you realize that the script would end cherry-picking commits and
that bzr has no support for tracking cherry-picks? This way it is not
possible to easily track commits across branches.

[snip]

Re: Incorrect merge

[Stephen J. Turnbull]

Óscar Fuentes writes:

 > The *only* way? What abut having a branch for that purpose?

Having a branch doesn't mean that people will use it properly.  Most
people do not understand branching and merging, or how to analyze
workflows, and don't want to learn.  Having a script allows them to
participate without learning.

 > BTW, do you realize that the script would end cherry-picking
 > commits

Not at all.  The whole point of svnmerge.py is to support
cherry-picking in SVN, as Python has up to 6 active mainlines at any
given time, along with sandbox branches of varying longevity and
proximity to a mainline.  Sure, that requires a fairly high level of
understanding and effort on the part of the script authors, but Python
considered it worth it.

It's not the only way, I'm sure, but it's one good, proven way.

Re: Incorrect merge

[Óscar Fuentes]

"Stephen J. Turnbull" <stephen@xemacs.org> writes:

>  > The *only* way? What abut having a branch for that purpose?
>
> Having a branch doesn't mean that people will use it properly.

They need to learn and remember how to insert the necessary
meta-information needed by the script.

> Most people do not understand branching and merging, or how to
> analyze workflows, and don't want to learn.  Having a script allows
> them to participate without learning.

Adding an extra branch does not increase the current workload. They
would commit to common-fixes the same way they do for emacs-23. The only
"new" thing to learn is that there is a branch for those changes shared
by emacs-23 and trunk, which is a concept quite easy to grasp. This
seems to me more clear than having to remember how to include some
magical spell on the commit message.

If the set of people who don't want to learn determines how the project
is managed, the most sensible way is to revert to CVS or even to
something more primitive.

>  > BTW, do you realize that the script would end cherry-picking
>  > commits
>
> Not at all.  The whole point of svnmerge.py is to support
> cherry-picking in SVN, as Python has up to 6 active mainlines at any
> given time, along with sandbox branches of varying longevity and
> proximity to a mainline.  Sure, that requires a fairly high level of
> understanding and effort on the part of the script authors, but Python
> considered it worth it.
>
> It's not the only way, I'm sure, but it's one good, proven way.

I used svnmerge.py. Is a hack that only seems "good" as long as you are
desperate enough. And I think that you'll find quite harder to implement
it on bzr than on SVN.

It is quite pathetic to even consider using hacks that workaround the
limitations of simplistic VCS like svn when there are standard practices
on dVCS for properly solving the problem at hand.

Re: Incorrect merge

[Stefan Monnier]

>>> In Python, they have a script called svnmerge.py.  This script did
>>> many things, one of them was to keep track of blocked revisions, ie,
>>> revisions that should not be merged to trunk (or to some given branch,
>>> IIRC).
>> Indeed, the only way I can think of to try and make sure such undesired
>> merges don't happen is to try and encode this fact into a merge
>> script.
> The *only* way? What abut having a branch for that purpose?

It's difficult enough to make sure patches get installed in the right
(emacs-23 or trunk) branch and to keep them in sync, that adding another
branch would just add to the work.
Using an ad-hoc script on the other hand would *reduce* the work.

> BTW, do you realize that the script would end cherry-picking commits and
> that bzr has no support for tracking cherry-picks? This way it is not
> possible to easily track commits across branches.

Of course that would end up doing cherry-picking, so what?
We already do that by hand, so doing in from a script can't be worse.


        Stefan

Re: Incorrect merge

[Stefan Monnier]

>>> This is equivalent to saying that a change on common-fixes could depend
>>> on a change made to emacs-23 only.
>> How do you keep the branch up-to-date?
> AFAIK almost all changes to emacs-23 are eventually merged into
> trunk. This means that common-fixes would containt all changes intended
> to emacs-23, except those few which are not intended for trunk. This
> means that the divergence of emacs-23 from common-fixes will not be
> significant.

Another problem with your branch approach is that a patch applied
(mistakenly) to emacs-23 rather than to emacs-common would end up lost.
It's a lot easier to detect excess patches and lost patches.


        Stefan

Re: Incorrect merge

[Óscar Fuentes]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

> It's difficult enough to make sure patches get installed in the right
> (emacs-23 or trunk) branch and to keep them in sync, that adding another
> branch would just add to the work.
> Using an ad-hoc script on the other hand would *reduce* the work.

Unless you end writing the Mythical Magic Script, the effects will be
the reverse: more complexity, more confusion, more mistakes.

>> BTW, do you realize that the script would end cherry-picking commits and
>> that bzr has no support for tracking cherry-picks? This way it is not
>> possible to easily track commits across branches.
>
> Of course that would end up doing cherry-picking, so what?
> We already do that by hand, so doing in from a script can't be worse.

The proposed branch would eliminate the need for cherry-picking, hence
improving the traceability of patches.

Re: Incorrect merge

[Óscar Fuentes]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

> Another problem with your branch approach is that a patch applied
> (mistakenly) to emacs-23 rather than to emacs-common would end up
> lost.

Not exactly lost, but applied to emacs-23 only.

> It's a lot easier to detect excess patches and lost patches.

If you see someone committing to emacs-23 (something that would be quite
infrequent) the usual reaction would be to take a look and, unless it
obviously pertains to emacs-23 only, ask the author if the change is on
the right place.

Re: Incorrect merge

[Davis Herring]

> Another problem with your branch approach is that a patch applied
> (mistakenly) to emacs-23 rather than to emacs-common would end up lost.
> It's a lot easier to detect excess patches and lost patches.

Could we restrict emacs-23 so that only the merges (and explicit
maintainer action) could update it?  The number of branch-only patches is
small; it might not be too much trouble to rely on just a few people to
commit them.

Davis

-- 
This product is sold by volume, not by mass.  If it appears too dense or
too sparse, it is because mass-energy conversion has occurred during
shipping.

Re: Incorrect merge

[Stephen J. Turnbull]

Óscar Fuentes writes:

 > Adding an extra branch does not increase the current workload.

Of course it does.  Most occasional developers work only on the branch
that they use every day, either trunk or stable/maintenance.  That's
where they test, that's where they refine.  Adding the common-fixes
branch means they need to clone and maintain that branch, and work in
a branch that they don't use.

Even for frequent contributors who help with porting patches back and
forth between the branches, this requires more thinking about where
you should do the work.

 > They would commit to common-fixes the same way they do for emacs-23.

You mean, stuff that doesn't belong there, as started this thread? ;-)

 > If the set of people who don't want to learn determines how the project
 > is managed,

It already has, in both the choice of BTS and the choice of dVCS.  In
the sense that in both cases the ability to continue with basically
the same flow, even if implemented with slightly different commands,
was a crucial requirement in choice of software.

 > And I think that you'll find quite harder to implement it on bzr
 > than on SVN.

Depends on what you mean.  The basic functionality we're talking about
here, blocking certain revisions from being merged or cherry-picked,
depends on a globally unique revision ID.  In a centralized system,
there's only one authority, so there's no problem.  In a dVCS you have
to contrive one, but all the dVCSes have one.  So I don't think it's
any harder in this sense.

On the other hand, this can really only be enforced at push time.  So
a developer could make several commits in a branch that is blocked
from merging before realizing it.  That would be inconvenient, and
it's not obvious to me how to work around.  But I don't think it would
be more than an inconvenience.

Finally, although right now Emacs doesn't have the skills AFAIK, in
the long run implementing for Bazaar might be far more powerful since
the script could be adapted from svnmerge.py, written in Python, and
thus have direct access to Bazaar's internal information.  Maybe even
as a plugin.

 > It is quite pathetic to even consider using hacks that workaround
 > the limitations of simplistic VCS like svn

Call it what you like; however, the fact is that Emacs has chosen to
use a workflow that emphasizes Bazaar's ability to work like a
centralized system.  Individual developers can use the decentralized
features as they like, but the project workflow does not mandate them.

 > when there are standard practices on dVCS for properly solving the
 > problem at hand.

But there aren't.  Only Darcs supports cherry-picking properly.  And
as an add-on hack, svnmerge.py does.  The revision-oriented systems
don't implement the necessary accounting.  What is available for the
revision-oriented dVCSes is a set of workflows that *mostly* avoid
creating a problem, but have their own inconveniences.

I suspect the Emacs community will prefer to implement a tool that
codifies their workflow and adds certain features such as true
cherry-picking and blocking revisions, to changing the workflow.

Re: Incorrect merge

[Eli Zaretskii]

> From: Óscar Fuentes <ofv@wanadoo.es>
> Date: Tue, 02 Nov 2010 16:57:37 +0100
> 
> BTW, do you realize that the script would end cherry-picking commits

Can't it instead do a merge and then reverse cherry-pick the revisions
that should not be merged?

Re: Incorrect merge

[Óscar Fuentes]

"Stephen J. Turnbull" <stephen@xemacs.org> writes:

>  > Adding an extra branch does not increase the current workload.
>
> Of course it does.  Most occasional developers work only on the branch
> that they use every day, either trunk or stable/maintenance.  That's
> where they test, that's where they refine.  Adding the common-fixes
> branch means they need to clone and maintain that branch, and work in
> a branch that they don't use.

Almost every change on emacs-23 is intended to trunk too. Right now
those changes that are exclusive to emacs-23 must be flagged
somehow. Adding common-fixes just means that people working on emacs-23
will work on common-fixes, except for those cases where they would flag
the change as belonging to emacs-23 only. Not so hard, IMO.

> Even for frequent contributors who help with porting patches back and
> forth between the branches, this requires more thinking about where
> you should do the work.

Uh? With common-fixes you merge the commits there into emacs-23 and
trunk. That's all.

>  > They would commit to common-fixes the same way they do for emacs-23.
>
> You mean, stuff that doesn't belong there, as started this thread? ;-)

The problem was that commits intended to emacs-23 were merged into
trunk. Yes, people can do all kinds of mistakes, but no script will
magically avoid them.

[snip]

>  > when there are standard practices on dVCS for properly solving the
>  > problem at hand.
>
> But there aren't.  Only Darcs supports cherry-picking properly.

You are missing the point. common-fixes will eliminate the need for
cherry-picking (and for examining each commit on emacs-23 before merging
into trunk). The maintainers save time and the VC history is consistent
(with commits maintaining its identity on the branches where they are
installed)

[snip]

Re: Incorrect merge

[Óscar Fuentes]

Eli Zaretskii <eliz@gnu.org> writes:

>> BTW, do you realize that the script would end cherry-picking commits
>
> Can't it instead do a merge and then reverse cherry-pick the revisions
> that should not be merged?

That's an option. It would be effective even for an human operator, as
the number of commits on emacs-23 not intented for trunk is so small
(that would not save the time required for reviewing the log messages
looking for some text flagging that condition, though)

IMO it is not a good thing to create revert commits as part of a
process. In this case, they will break bisecting and add potential
confussion to the VC history. For instance: the merged commits will be
hidden by default on level 1 of the DAG, but the reverting commits will
be always visible on the leftmost level of the DAG, unless you
complicate things by creating a branch containing the revert commits and
merge it into trunk.

It is desirable that patches not intended for trunk should never be on
trunk.

Re: Incorrect merge

[Stephen J. Turnbull]

Óscar Fuentes writes:

 > Almost every change on emacs-23 is intended to trunk too. Right now
 > those changes that are exclusive to emacs-23 must be flagged
 > somehow. Adding common-fixes just means that people working on emacs-23
 > will work on common-fixes, except for those cases where they would flag
 > the change as belonging to emacs-23 only. Not so hard, IMO.

Good luck to you in convincing the Emacs community, then.

 > > Even for frequent contributors who help with porting patches back and
 > > forth between the branches, this requires more thinking about where
 > > you should do the work.
 > 
 > Uh? With common-fixes you merge the commits there into emacs-23 and
 > trunk. That's all.

No, you have to choose whether to work in -23, trunk, or common-fixes.
This involves checking whether the bug affects both or not, and
whether the fix is the same.  Often trivial, but not always.

And what if you fix a bug in trunk, and only later realize it needs to
be backported?

Certainly, this is to some extent balanced by the extra work of
flagging -23 changes that shouldn't go into trunk.  The advantage of
the svnmerge-py workflow is that you make these decisions later, after
the work is done.

 > You are missing the point. common-fixes will eliminate the need for
 > cherry-picking (and for examining each commit on emacs-23 before merging
 > into trunk). The maintainers save time and the VC history is consistent
 > (with commits maintaining its identity on the branches where they are
 > installed)

It doesn't eliminate the need for cherry-picking as long as there's
more than one active branch: you can make a mistake about where to
work.  This should be a lot less frequent in the common-fixes
workflow.  It does require people who would otherwise focus on trunk
to switch between branches, and to be continuously thinking about
which branch they should be in.  This is probably a mildly bad thing;
work on the trunk is generally going to be more complex, and you'd
like people to concentrate there.

Every commit on common-fixes needs to be examined before making it to
be sure that it's appropriate for both release branches (common-fixes
is never released).  I don't think there's any saving, and in fact the
decision to work on common-fixes (before you have a patch) is probably
harder than the decision to merge to trunk or not (with the work
complete).  To some extent that decision needs to be made before doing
any work, which increases the burden and the likelihood of a mistake.

The VC history is consistent, but I don't think the maintainers save
much time and it's at a higher burden to the general contributors.
And it requires everybody to adapt a new workflow at all phases of
their work, instead of concentrating on the cherry pick only in the
cases where it's needed.

Re: Incorrect merge

[Óscar Fuentes]

"Stephen J. Turnbull" <stephen@xemacs.org> writes:

>  > Uh? With common-fixes you merge the commits there into emacs-23 and
>  > trunk. That's all.
>
> No, you have to choose whether to work in -23, trunk, or common-fixes.
> This involves checking whether the bug affects both or not, and
> whether the fix is the same.  Often trivial, but not always.
>
> And what if you fix a bug in trunk, and only later realize it needs to
> be backported?

And how is this different from the current workflow? Right now people
must decide the scope of the patch. Adding the common-fixes branch
simplifies the task from the conceptual POV: instead of

 * commit fixes for emacs-23 and trunk into emacs-23
 * commit fixes intended for trunk only into trunk.
 * commit fixes intented for emacs-23 only into emacs-23, put something
  into the log message and hope it is noticed.

you have

 * commit fixes for emacs-23 and trunk into common-fixes.
 * commit fixes intended for emacs-23 only into emacs-23.
 * same for trunk.

[snip]

>  > You are missing the point. common-fixes will eliminate the need for
>  > cherry-picking (and for examining each commit on emacs-23 before merging
>  > into trunk). The maintainers save time and the VC history is consistent
>  > (with commits maintaining its identity on the branches where they are
>  > installed)
>
> It doesn't eliminate the need for cherry-picking as long as there's
> more than one active branch: you can make a mistake about where to
> work.

If you come with "yes, but someone can make a mistake..." then any
schema we can think of can be dismissed with the same reasoning.

>  This should be a lot less frequent in the common-fixes
> workflow.  It does require people who would otherwise focus on trunk
> to switch between branches, and to be continuously thinking about
> which branch they should be in.

Again, the current workflow requires people to decide that.

[snip]

> Every commit on common-fixes needs to be examined before making it to
> be sure that it's appropriate for both release branches (common-fixes
> is never released).

If you want a fool-proof method, propose a gatekeeper workflow (with
several layers of verification, for enhanced security :-)

[snip]

> The VC history is consistent, but I don't think the maintainers save
> much time and it's at a higher burden to the general contributors.

The consistency on VC history is a huge win for me. The ability to
quickly decide which branches contain a given commit will turn more and
more important as feature branches proliferate. Right now we should put
the revision id (not revision number) of a fix on the respective bug
report when closing it.

> And it requires everybody to adapt a new workflow at all phases of
> their work,

This is an exaggeration. Only people who work on emacs-23 would need to
adapt their workflow (committing to common-fixes instead of emacs-23). I
admit that the big hurdle is to pass the word about the new workflow,
but this could be forced by making emacs-23 read-only for all except
some top maintainers, who would act as gatekeepers for some time.

> instead of concentrating on the cherry pick only in the
> cases where it's needed.

Doing the cherry-picking (with the current workflow) or the merge (with
my proposed branch) is something that only a few maintainers should care
about.

Re: Incorrect merge

[Stefan Monnier]

> It is desirable that patches not intended for trunk should never be on
> trunk.

Most/all patches on emacs-23 which shouldn't be merged to trunk are like
that because trunk already has those patches (tho typically in
a slightly different form, e.g. in a different file).


        Stefan

Re: Incorrect merge

[Stephen J. Turnbull]

Óscar Fuentes writes:

 > > And what if you fix a bug in trunk, and only later realize it needs to
 > > be backported?
 > 
 > And how is this different from the current workflow?

It's not.  That's precisely my point.  You can reduce cherry-picking,
but not eliminate it.

 > Right now people
 > must decide the scope of the patch. Adding the common-fixes branch
 > simplifies the task from the conceptual POV: instead of
 > 
 >  * commit fixes for emacs-23 and trunk into emacs-23
 >  * commit fixes intended for trunk only into trunk.
 >  * commit fixes intented for emacs-23 only into emacs-23, put something
 >   into the log message and hope it is noticed.

But the svnmerge.py workflow does a lot better than that; as long as
people use svnmerge.py, it does the accounting.  People grumble about
changing tools, but the workflow will be very similar; they'll get
over it quickly.  Changing the workflow is more effort, takes longer,
and some people will never get over it.

 > you have
 > 
 >  * commit fixes for emacs-23 and trunk into common-fixes.
 >  * commit fixes intended for emacs-23 only into emacs-23.
 >  * same for trunk.

You're counting bzr-level tasks, but my point is that these tasks are
more complex/difficult than the corresponding tasks in the other
workflow because the decisions are made in advance of any commit,
rather than with the actual commit to look at.

 > > It doesn't eliminate the need for cherry-picking as long as there's
 > > more than one active branch: you can make a mistake about where to
 > > work.
 > 
 > If you come with "yes, but someone can make a mistake..." then any
 > schema we can think of can be dismissed with the same reasoning.

I'm not dismissing your scheme yet.  I'm saying your accounting is way
too optimistic.

 > >  This should be a lot less frequent in the common-fixes
 > > workflow.  It does require people who would otherwise focus on trunk
 > > to switch between branches, and to be continuously thinking about
 > > which branch they should be in.
 > 
 > Again, the current workflow requires people to decide that.

Again you ignore the point, which is that the current workflow means
looking at an existing commit and making a decision.  The commit-fixes
workflow involves thinking about a prospective patch and making a
decision where to produce it, or creating yet another branch and
looking at the commit there.

Creating a temporary branch just for that fix is precisely what I do
with git, but doing that in Bazaar or Mercurial is too expensive for
me.

 > > Every commit on common-fixes needs to be examined before making it to
 > > be sure that it's appropriate for both release branches (common-fixes
 > > is never released).
 > 
 > If you want a fool-proof method, propose a gatekeeper workflow (with
 > several layers of verification, for enhanced security :-)

You're missing the point yet again.

 > > The VC history is consistent, but I don't think the maintainers save
 > > much time and it's at a higher burden to the general contributors.
 > 
 > The consistency on VC history is a huge win for me. The ability to
 > quickly decide which branches contain a given commit will turn more and
 > more important as feature branches proliferate.

I don't agree; feature branches will branch from trunk, synch to it,
and eventually merge to it, being entirely unconcerned with what is or
is not in the maintenance branch or common-fixes, and vice-versa.
That's really the point of having a separate maintenance branch.

 > Right now we should put the revision id (not revision number) of a
 > fix on the respective bug report when closing it.
 > 
 > > And it requires everybody to adapt a new workflow at all phases of
 > > their work,
 > 
 > This is an exaggeration. Only people who work on emacs-23 would need to
 > adapt their workflow (committing to common-fixes instead of
 > emacs-23).

But that requires examining the commits of trunk-only workers for -23
relevance, and cherry-picking if they are relevant.  You can't have it
both ways.  Either you're serious about consistent history and
avoiding cherry-picking as much as possible, which requires everybody
to consider whether their fixes are -23 relevant, or you're not.

True, people working only on features can avoid this.

 > Doing the cherry-picking (with the current workflow) or the merge (with
 > my proposed branch) is something that only a few maintainers should care
 > about.

Doing it is Bazaar's problem; if Bazaar doesn't make that work as
convenient as possible, we made a big mistake.  The question is which
way is harder to think about.  Your way requires all committers who
fix bugs to worry about which branch to commit to.

Re: Incorrect merge

[Óscar Fuentes]

"Stephen J. Turnbull" <stephen@xemacs.org> writes:

[snip]

> Your way requires all committers who fix bugs to worry about which
> branch to commit to.

Yes, exactly as it happens right now.

Maybe you didn't notice that people commit to emacs-23 (and to emacs-23
only) whenever the fix applies to that release, and to trunk only
otherwise. Then, from time to time, some maintainer merges (or, more
precisely, cherry picks) emacs-23 into trunk. This means that people
already need to figure out where to commit their fixes. Replacing
emacs-23 with common-fixes here seems a tiny change on the workflow to
me, much less burdensome than waiting for the availability and using a
script that fakes cherry-pick tracking on bzr.

(Good luck writing that script. It looks much harder to do than on the
svn case, if possible at all without extending the repository format.)

Re: Incorrect merge

[Óscar Fuentes]

"Stephen J. Turnbull" <stephen@xemacs.org> writes:

>  > > And what if you fix a bug in trunk, and only later realize it needs to
>  > > be backported?
>  > 
>  > And how is this different from the current workflow?
>
> It's not.  That's precisely my point.  You can reduce cherry-picking,
> but not eliminate it.

One thing is to resort to cherry-picking when some exceptional condition
is detected, and another thing is to use cherry-picking as a tool for an
ordinary task.

[snip]

Re: Incorrect merge

[Stephen J. Turnbull]

Óscar Fuentes writes:
 > "Stephen J. Turnbull" <stephen@xemacs.org> writes:
 > 
 > [snip]
 > 
 > > Your way requires all committers who fix bugs to worry about which
 > > branch to commit to.
 > 
 > Yes, exactly as it happens right now.

Which is missing the point.  One point of the svnmerge-based workflow
is that the branch to commit doesn't really matter, and the decision
to block merges/cherry-picks made after inspecting the commit.  So the
svnmerge approach is *better* in this respect.

But the main point is that cherry-picking, which is a natural workflow
in this context in general, and the historical practice of Emacs, is
well-supported by svnmerge.  The point of the common-fixes workflow is
to (unnaturally) avoid cherry-picking.  But (a) this is awkward for
the workers, especially at first, and (b) it is a change.

As I've said before, I personally use "emphemeral" branches in git for
this situation.  And I'm sure that for many communities, the common-
fixes approach works well too.  But the Python community strongly
prefers the svnmerge approach (porting svnmerge to Mercurial is one of
the blockers on their migration), and my feeling is that the Emacs
community is similar to the Python community in this respect.

I don't know about others, but you don't need to prove to me that your
approach works.  Although I haven't actually used it in practice, the
theory looks good enough.  The question I have is whether it's well-
adapted to Emacs.  So far the Emacs maintainers (including Richard)
have been *very* conservative about changes to workflow.  svnmerge (or
a port of it) *supports* the current workflow.  That's going to be
tough to beat.

Re: Incorrect merge

[Stephen J. Turnbull]

Óscar Fuentes writes:

 > One thing is to resort to cherry-picking when some exceptional condition
 > is detected, and another thing is to use cherry-picking as a tool for an
 > ordinary task.

No, from the point of view of history consistency they're the same.
Cross-branch histories aren't "a little bit inconsistent"; they're
consistent or they're not.  In the first case you can depend on tools
like Bazaar and git to DTRT, and in the latter you can't.

Re: Incorrect merge

[Stefan Monnier]

> I don't know about others, but you don't need to prove to me that your
> approach works.  Although I haven't actually used it in practice, the
> theory looks good enough.  The question I have is whether it's well-
> adapted to Emacs.

Exactly.  And even more so if you consider our use of ChangeLog files:

Not only do I want a "merge script" in any case to help me resolve those
spurious conflicts, so making this script look for commits with a "don't
merge" flag and reverse apply their diffs before committing the merge is
just a natural extension (in the sense that, just like for resolving
conflicts in ChangeLog, all it does is automatize what we do manually).

But on top of that, until we have a "merge script" I'd rather not add
yet more branches to merge, since merging from emacs-common to emacs-23
will be yet another merge where I'll need to manually fix the ChangeLog
conflicts.

Of course, your argument will be that once we have a good "merge script"
to resolve ChangeLog conflicts (or once we get rid of the ChangeLog
files), we can use an emacs-common branch without suffering when
merging from it into emacs-23.  It's not like your point is not valid,
but I don't think the difference matters nearly as much as this thread
makes it out to be.


        Stefan

Re: Incorrect merge

[Lars Magne Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> (or once we get rid of the ChangeLog files)

You probably hoped nobody would notice that parenthesis, but is getting
rid of the ChangeLog files in the cards?

1) They're a pain in the ass, both to write and due to the constant
merge conflicts.

2) But they give a better overview over what's happening, and who's
doing what, than any VC log I've seen.

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: Incorrect merge

[Andreas Schwab]

Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

> 1) They're a pain in the ass, both to write and due to the constant
> merge conflicts.

git-merge-changelog can help a lot here.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Incorrect merge

[Eli Zaretskii]

> From: Andreas Schwab <schwab@linux-m68k.org>
> Date: Fri, 05 Nov 2010 10:30:26 +0100
> 
> Lars Magne Ingebrigtsen <larsi@gnus.org> writes:
> 
> > 1) They're a pain in the ass, both to write and due to the constant
> > merge conflicts.
> 
> git-merge-changelog can help a lot here.

As can this bzr plugin:

  bzr branch lp:bzr-changelog-merge ~/.bazaar/plugins/changelog_merge

It is still experimental, see
https://lists.ubuntu.com/archives/bazaar/2010q3/070157.html and the
rest of that thread (it continues in 2010q4) for details.

Re: Incorrect merge

[Stefan Monnier]

>> (or once we get rid of the ChangeLog files)
> You probably hoped nobody would notice that parenthesis, but is getting
> rid of the ChangeLog files in the cards?

It's been requested many times already, and seeing how it's the main
source of pain when merging branches, there's clearly some incentives.
What makes it more so is that the switch away from CVS removed one of
the barriers: CVS's inability to generate good logs.

> 1) They're a pain in the ass, both to write and due to the constant
> merge conflicts.

I don't find them painful at all to write, quite the contrary: I find it
currently inconvenient to write commit logs without going through
a ChangeLog file.  So to get rid of ChangeLogs, we need to make "C-x
4 a" and friends work for *VC-Log* or something like that.

But yes, the merging conflicts are annoying.

> 2) But they give a better overview over what's happening, and who's
> doing what, than any VC log I've seen.

That's another issue, indeed: "bzr log" is OK but is not as good as the
ChangeLog files in many cases.  Some of the issues I see with it:
- Not editable: bzr does not support fixing commit messages.
- Slowish.
OTOH, it has some advantages as well:
- you can get a log for a specific set of files.
- no issue when a commit spans several directories with different
  ChangeLog files, e.g. lisp/net/imap.el and lisp/gnus/nnimap.el.

FWIW, overall, I'm in favor of ditching the ChangeLog,


        Stefan