want option to allow uncommit but disallow changing mainline

Bug #605067 reported by Loïc Minier
12
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Bazaar
Confirmed
Medium
Unassigned
bzr (Ubuntu)
Triaged
Medium
Unassigned

Bug Description

Binary package hint: bzr

Hey

I think we need a way to prevent bzr push from replacing the history when it think it can do so; this might not be the right thing to do for local branches, but it's certainly the default behavior we want in Launchpad.

Here's the story

16:43 < lool> poolie: So basically someone had a bzr branch of our trunk, did a
              local commit, pushed to a topic branch in launchpad, asked for a
              merge
16:43 < lool> I merged it, pushed to the trunk branch
16:44 < lool> this guy then did another merge, forgetting that he abused his
              local trunk branch as a topic branch
16:44 < lool> he pushed
16:44 < lool> Launchpad accepted that silently
16:44 < lool> but sent out an email that one revision had been removed
16:44 < lool> I was told this bzr behavior makes sense when working locally
16:44 < lool> If you look at the history now though, it misses my merge
              completely
16:45 < poolie> it's not on the mainline or it's not there at all?
16:45 < lool> it has the same data in the same timeline in the log, but one
              merge commit is missing around the change I merged
16:45 < lool> poolie: My merge was removed from mainline by a mere bzr push
16:45 < lool> the revision got superseded by his local commit
16:46 < poolie> lool, so from what mwh said we might want an option or a
                behaviour to allow uncommit but not allow other changes to the
                mainline
16:46 <@lool> Yes
16:47 <@lool> poolie: Cause it might be handy to be able to uncommit just after
              doing a bad commit, pushing to the wrong place or so
16:47 <@lool> but I dont want to allow silently replacing history
16:52 <@lool> poolie: Are you in agreement? am I expected to file a bug
              against bzr now, or will you folks hack and deploy it on the spot
              so that I never need to worry about it again? :-)
16:53 < poolie> lool, i do agree this makes sense
16:53 < poolie> i wonder if this should almost be on by default in some cases

Cheers,

ProblemType: Bug
DistroRelease: Ubuntu 10.10
Package: bzr 2.1.2-1
ProcVersionSignature: Ubuntu 2.6.35-6.9-generic 2.6.35-rc3
Uname: Linux 2.6.35-6-generic x86_64
Architecture: amd64
Date: Tue Jul 13 16:54:06 2010
ProcEnviron:
 LANGUAGE=fr_FR:fr:en_GB:en
 PATH=(custom, user)
 LANG=fr_FR.UTF-8
 SHELL=/bin/zsh
SourcePackage: bzr

Revision history for this message
Loïc Minier (lool) wrote :
Martin Pool (mbp)
summary: - Allows history rewrites
+ want option to allow uncommit but disallow changing mainline
Changed in bzr (Ubuntu):
status: New → Confirmed
Changed in bzr:
status: New → Confirmed
importance: Undecided → Medium
Revision history for this message
John A Meinel (jameinel) wrote : Re: [Bug 605067] [NEW] Allows history rewrites

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Loïc Minier wrote:
> Public bug reported:
>
> Binary package hint: bzr
>
> Hey
>
> I think we need a way to prevent bzr push from replacing the history
> when it think it can do so; this might not be the right thing to do for
> local branches, but it's certainly the default behavior we want in
> Launchpad.
>
> Here's the story
>
> 16:43 < lool> poolie: So basically someone had a bzr branch of our trunk, did a
> local commit, pushed to a topic branch in launchpad, asked for a
> merge
> 16:43 < lool> I merged it, pushed to the trunk branch
> 16:44 < lool> this guy then did another merge, forgetting that he abused his
> local trunk branch as a topic branch
> 16:44 < lool> he pushed
> 16:44 < lool> Launchpad accepted that silently
> 16:44 < lool> but sent out an email that one revision had been removed
> 16:44 < lool> I was told this bzr behavior makes sense when working locally
> 16:44 < lool> If you look at the history now though, it misses my merge
> completely

Isn't the original fix for this setting "append_revisions_only" on the
trunk? (Preventing this sort of behavior).

In the short term, doing:

 bzr branch lp:.../trunk -r -2
 bzr merge XXX
 bzr commit -m "Merging X correctly"
 bzr push --overwrite lp:.../trunk

There are other ways, but it seems reasonable.

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkw9fAQACgkQJdeBCYSNAAMiawCfcbsuF0YbEFWDgA7wgtgh7SR4
gaMAnRcNnGpm3HjuXCZcWv0cCtJuYWcT
=rzkq
-----END PGP SIGNATURE-----

Revision history for this message
Martin Pool (mbp) wrote :

On 14 July 2010 10:57, John A Meinel <email address hidden> wrote:
> Isn't the original fix for this setting "append_revisions_only" on the
> trunk? (Preventing this sort of behavior).

That would prohibit uncommits. Perhaps it is reasonable to want
explicit uncommits but not changing the mainline.

--
Martin

Revision history for this message
Loïc Minier (lool) wrote :

Exactly; we want to allow uncommits because these are explicit but disable the implicit default of replacing/overwriting history.

The main use case is Launchpad code hosting for me here, but I'm happy if that also covers other bzr workflows.

Revision history for this message
John A Meinel (jameinel) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Martin Pool wrote:
> On 14 July 2010 10:57, John A Meinel <email address hidden> wrote:
>> Isn't the original fix for this setting "append_revisions_only" on the
>> trunk? (Preventing this sort of behavior).
>
> That would prohibit uncommits. Perhaps it is reasonable to want
> explicit uncommits but not changing the mainline.
>

It does, but if you prevent getting there in the first place, you might
be okay.

Uncommit is pretty dangerous on a trunk that people integrate around,
since suddenly a revision in the history disappears, and everyone has to
figure out how to resync.

Given the nature of it, is there a problem making it 2-phase?

1) Unset the append_revisions_only flag, indicating that for the next
bit it is okay to mutate history a bit.
2) Uncommit
3) set append_revisions_only back to True.

I think not having that automated is a reasonable tradeoff between being
possible, and shooting yourself in the foot by doing it accidentally.

However, we certainly need to make it easier to set (and unset) the
append_revisions_only flag.

John
=;->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkw+urkACgkQJdeBCYSNAAOUIACeIW+/KcYrzUurM3GZGKYSV+OB
h/AAoLZV7Zj3IPlmlzAKzMX8uBz+Vzg+
=G0yA
-----END PGP SIGNATURE-----

Revision history for this message
Loïc Minier (lool) wrote :

We agree uncommit is dangerous, but it can't be triggered by accident; people will have to explicitly type uncommit. I have a problem when a regular "bzr push" in the default configuration can change history though.

If you propose that whenever I mean to "bzr uncommit" I have to "bzr disable-append-revision-only", "bzr uncommit", and "bzr reenable-append-revision-only", then I'm going to as for a "bzr really-uncommit" instead! :-)

Revision history for this message
Vincent Ladeuil (vila) wrote :

@lool, I think John's point is that bzr really-uncommit should also send an email to all subscribers of the branch explaining how to resync their branch which is (finding the subscribers) not trivial.

You may also want to me-too bug #491196 that is aimed at addressing last John's remark about an easy way to set/unset the apprend_revisions_only flag.

Revision history for this message
Loïc Minier (lool) wrote :

Vincent, thanks for the pointer. I don't think the uncommit case might not always require a fix from subscribers. The rare cases where I uncommited were usually immediately after a push or commit, presumably letting only a short window of time for people to base of the uncommitted revision. But I guess it would be nice in some cases to indeed have this notification.

Jelmer Vernooij (jelmer)
Changed in bzr (Ubuntu):
status: Confirmed → Triaged
importance: Undecided → Medium
Jelmer Vernooij (jelmer)
tags: added: uncommit
Jelmer Vernooij (jelmer)
tags: added: check-for-breezy
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.