| Submitter | Kostia Balytskyi |
|---|---|
| Date | May 30, 2016, 5:31 p.m. |
| Message ID | <330fa585c8b9a272f51e.1464629480@ikostia-mbp> |
| Download | mbox | patch |
| Permalink | /patch/15265/ |
| State | Deferred |
| Headers | show |
Comments
On 05/30/2016 07:31 PM, Kostia Balytskyi wrote: > # HG changeset patch > # User Kostia Balytskyi <ikostia@fb.com> > # Date 1464629336 -3600 > # Mon May 30 18:28:56 2016 +0100 > # Node ID 330fa585c8b9a272f51e6b82a8f65d9b01e0207e > # Parent 318534bb5dfd37b1c95f10ba90b73c5ce562713f > rebase: split _checkobsrebase into two parts for better wrapping > > Currently _checkobsrebase does two checks: one aborts if divergence is going > to be caused by the rebase operation (and this can be turned off by a config), > another aborts if all changesets have successors in the destination (and this > cannot be turned off). I have suspicions that some rebase users would like it > not to abort when there's nothing to rebase, but rather to exit with code 0. > Splitting _checkobsrebase is a step in this direction. > > This is unrelated to the other refactoring series. I actually lean toward have that alternate behavior as the default, I can't remember the discussion with Laurent that led to the current default (if we had one), but the current abort feel strange to me and I would rather see the rebase proceed successfully (it will have to update the working copy parent and related bookmarks) I've CC Laurent who might have valuable insight on why the current behavior was picked.
On Thu, Jun 02, 2016 at 03:25:58AM +0200, Pierre-Yves David wrote: > > > On 05/30/2016 07:31 PM, Kostia Balytskyi wrote: > > # HG changeset patch > > # User Kostia Balytskyi <ikostia@fb.com> > > # Date 1464629336 -3600 > > # Mon May 30 18:28:56 2016 +0100 > > # Node ID 330fa585c8b9a272f51e6b82a8f65d9b01e0207e > > # Parent 318534bb5dfd37b1c95f10ba90b73c5ce562713f > > rebase: split _checkobsrebase into two parts for better wrapping > > > > Currently _checkobsrebase does two checks: one aborts if divergence is going > > to be caused by the rebase operation (and this can be turned off by a config), > > another aborts if all changesets have successors in the destination (and this > > cannot be turned off). I have suspicions that some rebase users would like it > > not to abort when there's nothing to rebase, but rather to exit with code 0. > > Splitting _checkobsrebase is a step in this direction. > > > > This is unrelated to the other refactoring series. > > I actually lean toward have that alternate behavior as the default, I > can't remember the discussion with Laurent that led to the current > default (if we had one), but the current abort feel strange to me and I > would rather see the rebase proceed successfully (it will have to update > the working copy parent and related bookmarks) I'm agreed on the current behavior as a bug, for what it's worth. It's a deviation from rebase-without-obsolete, and I've found it surprising more than once as a long-time rebase enthusiast. > > I've CC Laurent who might have valuable insight on why the current > behavior was picked. > > -- > Pierre-Yves David > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Patch
diff --git a/hgext/rebase.py b/hgext/rebase.py --- a/hgext/rebase.py +++ b/hgext/rebase.py @@ -737,6 +737,33 @@ else: return None +def _abortifdivergencecaused(repo, ui, rebaseobsrevs, rebaseobsskipped): + """Abort if rebasing will cuase divergence""" + # Obsolete node with successors not in dest leads to divergence + divergenceok = ui.configbool('experimental', 'allowdivergence') + divergencebasecandidates = rebaseobsrevs - rebaseobsskipped + + if divergencebasecandidates and not divergenceok: + divhashes = (str(repo[r]) for r in divergencebasecandidates) + msg = _("this rebase will cause " + "divergences from: %s") + h = _("to force the rebase please set " + "experimental.allowdivergence=True") + raise error.Abort(msg % (",".join(divhashes),), hint=h) + +def _abortifnothingtorebase(repo, ui, rebasesetrevs, rebaseobsskipped): + """Abort if every changeset has a successor in the destination""" + # - plain prune (no successor) changesets are rebased + # - split changesets are not rebased if at least one of the + # changeset resulting from the split is an ancestor of dest + rebaseset = rebasesetrevs - rebaseobsskipped + if rebasesetrevs and not rebaseset: + msg = _('all requested changesets have equivalents ' + 'or were marked as obsolete') + hint = _('to force the rebase, set the config ' + 'experimental.rebaseskipobsolete to False') + raise error.Abort(msg, hint=hint) + def _checkobsrebase(repo, ui, rebaseobsrevs, rebasesetrevs, @@ -749,30 +776,8 @@ `rebaseobsskipped`: set of revisions from source skipped because they have successors in destination """ - # Obsolete node with successors not in dest leads to divergence - divergenceok = ui.configbool('experimental', - 'allowdivergence') - divergencebasecandidates = rebaseobsrevs - rebaseobsskipped - - if divergencebasecandidates and not divergenceok: - divhashes = (str(repo[r]) - for r in divergencebasecandidates) - msg = _("this rebase will cause " - "divergences from: %s") - h = _("to force the rebase please set " - "experimental.allowdivergence=True") - raise error.Abort(msg % (",".join(divhashes),), hint=h) - - # - plain prune (no successor) changesets are rebased - # - split changesets are not rebased if at least one of the - # changeset resulting from the split is an ancestor of dest - rebaseset = rebasesetrevs - rebaseobsskipped - if rebasesetrevs and not rebaseset: - msg = _('all requested changesets have equivalents ' - 'or were marked as obsolete') - hint = _('to force the rebase, set the config ' - 'experimental.rebaseskipobsolete to False') - raise error.Abort(msg, hint=hint) + _abortifdivergencecaused(repo, ui, rebaseobsrevs, rebaseobsskipped) + _abortifnothingtorebase(repo, ui, rebasesetrevs, rebaseobsskipped) def defineparents(repo, rev, target, state, targetancestors, obsoletenotrebased):