| Submitter | Katsunori FUJIWARA |
|---|---|
| Date | Sept. 9, 2013, 1:51 p.m. |
| Message ID | <f0edc4e70f976b20d0c8.1378734690@feefifofum> |
| Download | mbox | patch |
| Permalink | /patch/2415/ |
| State | Superseded |
| Headers | show |
Comments
On Thu, 2013-09-12 at 22:33 -0500, Kevin Bullock wrote: > On 9 Sep 2013, at 8:51 AM, FUJIWARA Katsunori wrote: > > > # HG changeset patch > > # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp> > > # Date 1378733863 -32400 > > # Mon Sep 09 22:37:43 2013 +0900 > > # Node ID f0edc4e70f976b20d0c886a287b472a629789329 > > # Parent 1d07bf106c2ad1c7ef5e257e754ca8d858bd04b0 > > histedit: abort if there are multiple roots in "--outgoing" revisions > > LGTM except that I think we need to bikeshed the messages we give the > user here. Telling them the outgoing changesets have "multiple roots" > will be terrifying. I don't have any better suggestions, though (yet). Can we bikeshed by follow-up patch then? This appears to be a bugfix that belongs on stable, yes?
At Thu, 12 Sep 2013 22:33:04 -0500, Kevin Bullock wrote: > > On 9 Sep 2013, at 8:51 AM, FUJIWARA Katsunori wrote: > > > # HG changeset patch > > # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp> > > # Date 1378733863 -32400 > > # Mon Sep 09 22:37:43 2013 +0900 > > # Node ID f0edc4e70f976b20d0c886a287b472a629789329 > > # Parent 1d07bf106c2ad1c7ef5e257e754ca8d858bd04b0 > > histedit: abort if there are multiple roots in "--outgoing" revisions > > LGTM except that I think we need to bikeshed the messages we give the user here. Telling them the outgoing changesets have "multiple roots" will be terrifying. I don't have any better suggestions, though (yet). What about the idea blow ? - show abort message "there are outgoing revisions which may confuse users" (and the hint to lead users to "hg help histedit"), and - explain more detail about "--outgoing", like: For safety, this command is aborted, also if there are outgoing revisions which may confuse users: for example, there may be another unexpected outgoing branches, if number of "root revisions in outgoings" is two or more. Use "min(outgoing() and ::.)" or similar revset expression as ANCESTOR instead of "--outgoing" option, to specify only one root of outgoing ancestors of the working directory in such ambiguous situation. Please see :hg:`help revsets` for detail about selecting revisions. I hope revise for messages by native speakers :-) ---------------------------------------------------------------------- [FUJIWARA Katsunori] foozy@lares.dti.ne.jp
At Sat, 14 Sep 2013 15:12:13 -0500, Matt Mackall wrote: > > On Thu, 2013-09-12 at 22:33 -0500, Kevin Bullock wrote: > > On 9 Sep 2013, at 8:51 AM, FUJIWARA Katsunori wrote: > > > > > # HG changeset patch > > > # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp> > > > # Date 1378733863 -32400 > > > # Mon Sep 09 22:37:43 2013 +0900 > > > # Node ID f0edc4e70f976b20d0c886a287b472a629789329 > > > # Parent 1d07bf106c2ad1c7ef5e257e754ca8d858bd04b0 > > > histedit: abort if there are multiple roots in "--outgoing" revisions > > > > LGTM except that I think we need to bikeshed the messages we give the > > user here. Telling them the outgoing changesets have "multiple roots" > > will be terrifying. I don't have any better suggestions, though (yet). > > Can we bikeshed by follow-up patch then? > > This appears to be a bugfix that belongs on stable, yes? I don't think strongly that this should be fiexed on stable, because (1) no issue has been reported yet about "histedit --outgoing" AFAIK (according to BTS search result), and (2) this problem doesn't seem so serious. And, I dropped "STABLE" flag from this patch series. But I don't have any objection to apply this on stable, of course :-) ---------------------------------------------------------------------- [FUJIWARA Katsunori] foozy@lares.dti.ne.jp
On Tue, 2013-09-17 at 05:25 +0900, FUJIWARA Katsunori wrote: > At Sat, 14 Sep 2013 15:12:13 -0500, > Matt Mackall wrote: > > > > On Thu, 2013-09-12 at 22:33 -0500, Kevin Bullock wrote: > > > On 9 Sep 2013, at 8:51 AM, FUJIWARA Katsunori wrote: > > > > > > > # HG changeset patch > > > > # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp> > > > > # Date 1378733863 -32400 > > > > # Mon Sep 09 22:37:43 2013 +0900 > > > > # Node ID f0edc4e70f976b20d0c886a287b472a629789329 > > > > # Parent 1d07bf106c2ad1c7ef5e257e754ca8d858bd04b0 > > > > histedit: abort if there are multiple roots in "--outgoing" revisions > > > > > > LGTM except that I think we need to bikeshed the messages we give the > > > user here. Telling them the outgoing changesets have "multiple roots" > > > will be terrifying. I don't have any better suggestions, though (yet). > > > > Can we bikeshed by follow-up patch then? > > > > This appears to be a bugfix that belongs on stable, yes? > > I don't think strongly that this should be fiexed on stable, because > (1) no issue has been reported yet about "histedit --outgoing" AFAIK > (according to BTS search result), and (2) this problem doesn't seem so > serious. And, I dropped "STABLE" flag from this patch series. Having a BTS issue is not a requirement for fixing a bug. If histedit already does something vaguely reasonable in this case, then perhaps it's not a bug. But it seems like it's doing something silly currently.
On Mon, Sep 16, 2013 at 04:01:01PM -0500, Matt Mackall wrote: > On Tue, 2013-09-17 at 05:25 +0900, FUJIWARA Katsunori wrote: > > At Sat, 14 Sep 2013 15:12:13 -0500, > > Matt Mackall wrote: > > > > > > On Thu, 2013-09-12 at 22:33 -0500, Kevin Bullock wrote: > > > > On 9 Sep 2013, at 8:51 AM, FUJIWARA Katsunori wrote: > > > > > > > > > # HG changeset patch > > > > > # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp> > > > > > # Date 1378733863 -32400 > > > > > # Mon Sep 09 22:37:43 2013 +0900 > > > > > # Node ID f0edc4e70f976b20d0c886a287b472a629789329 > > > > > # Parent 1d07bf106c2ad1c7ef5e257e754ca8d858bd04b0 > > > > > histedit: abort if there are multiple roots in "--outgoing" revisions > > > > > > > > LGTM except that I think we need to bikeshed the messages we give the > > > > user here. Telling them the outgoing changesets have "multiple roots" > > > > will be terrifying. I don't have any better suggestions, though (yet). > > > > > > Can we bikeshed by follow-up patch then? > > > > > > This appears to be a bugfix that belongs on stable, yes? > > > > I don't think strongly that this should be fiexed on stable, because > > (1) no issue has been reported yet about "histedit --outgoing" AFAIK > > (according to BTS search result), and (2) this problem doesn't seem so > > serious. And, I dropped "STABLE" flag from this patch series. > > Having a BTS issue is not a requirement for fixing a bug. If histedit > already does something vaguely reasonable in this case, then perhaps > it's not a bug. But it seems like it's doing something silly currently. Stable seems appropriate for this, yes. > > -- > Mathematics is the supreme nostalgia of our time. > > > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@selenic.com > http://selenic.com/mailman/listinfo/mercurial-devel
Patch
diff --git a/hgext/histedit.py b/hgext/histedit.py --- a/hgext/histedit.py +++ b/hgext/histedit.py @@ -422,7 +422,12 @@ outgoing = discovery.findcommonoutgoing(repo, other, revs, force=force) if not outgoing.missing: raise util.Abort(_('no outgoing ancestors')) - return outgoing.missing[0] + roots = list(repo.set("roots(%ln)", outgoing.missing)) + if 1 < len(roots): + msg = _('there are multiple roots in outgoing revisions') + hint = _('see "hg help histedit" for detail about --outgoing') + raise util.Abort(msg, hint=hint) + return repo.lookup(roots[0]) actiontable = {'p': pick, 'pick': pick, @@ -457,6 +462,13 @@ With --outgoing, this edits changesets not found in the destination repository. If URL of the destination is omitted, the 'default-push' (or 'default') path will be used. + + This command is aborted, if there are multiple roots in revisions + specified by --outgoing, to avoid editing unexpected + revisions. Use "min(outgoing() and ::.)" or similar revset + specification instead of --outgoing to specify the root of + outgoing ancestors of the working directory in such ambiguous + situation. """ # TODO only abort if we try and histedit mq patches, not just # blanket if mq patches are applied somewhere diff --git a/tests/test-histedit-outgoing.t b/tests/test-histedit-outgoing.t --- a/tests/test-histedit-outgoing.t +++ b/tests/test-histedit-outgoing.t @@ -102,4 +102,38 @@ # m, mess = edit message without changing commit content # 0 files updated, 0 files merged, 0 files removed, 0 files unresolved + +test to check number of roots in outgoing revisions + + $ hg -q outgoing -G --template '{node|short}({branch})' '../r' + @ f26599ee3441(foo) + + o 652413bf663e(default) + | + o e860deea161a(default) + | + o 055a42cdd887(default) + + $ HGEDITOR=cat hg -q histedit --outgoing '../r' + abort: there are multiple roots in outgoing revisions + (see "hg help histedit" for detail about --outgoing) + [255] + + $ hg -q update -C 2 + $ echo aa >> a + $ hg -q commit -m 'another head on default' + $ hg -q outgoing -G --template '{node|short}({branch})' '../r#default' + @ 3879dc049647(default) + + o 652413bf663e(default) + | + o e860deea161a(default) + | + o 055a42cdd887(default) + + $ HGEDITOR=cat hg -q histedit --outgoing '../r#default' + abort: there are multiple roots in outgoing revisions + (see "hg help histedit" for detail about --outgoing) + [255] + $ cd ..