| Submitter | Durham Goode |
|---|---|
| Date | April 10, 2013, 7:55 p.m. |
| Message ID | <4f8c54c2540d1a28773f.1365623739@dev350.prn1.facebook.com> |
| Download | mbox | patch |
| Permalink | /patch/1272/ |
| State | Deferred |
| Headers | show |
Comments
On Wed, Apr 10, 2013 at 12:55:39PM -0700, Durham Goode wrote: > # HG changeset patch > # User Durham Goode <durham@fb.com> > # Date 1365614989 25200 > # Wed Apr 10 10:29:49 2013 -0700 > # Node ID 4f8c54c2540d1a28773f0dd48e6e2b92163259e4 > # Parent dfd94b32f8731515a0e6e90642609343dc1d6d4d > update: default update should move as far forward as possible (issue3883) > > DO NOT QUEUE THIS CHANGE! It's an rfc. > > Previously the default behavior for 'hg up' was to move to the tip of the > current named branch. This meant if you were on a different fork of the > DAG from the tip you got an "abort: crosses branches" error. > > This changes 'hg up' to update to the farthest descendant of your current > working copy parent within your current named branch. Ex: > > o 3 > | > | o 2 > | | > | @ 1 > |/ > | > o 0 > > 'hg update' will now put you on commit 2. If you were already on commit 2, > it would be a no-op. If you were on commit 0, it would put you on 3. Where would it put you before this change? > > This only affects hg updates that would have previously failed. hg updates > that would have succeeded before will continue to succeed and land on tip. > > I considered making this only happen if they have an active bookmark, but > it seemed weird to have different behavior based on if your bookmark was > active or not, and it seems like the new behavior is desirable even in non > bookmark scenarios. > > If people agree with this change, I'll update the tests and help documentation > and resubmit. > > diff --git a/mercurial/merge.py b/mercurial/merge.py > --- a/mercurial/merge.py > +++ b/mercurial/merge.py > @@ -652,14 +652,17 @@ > try: > wc = repo[None] > if node is None: > - # tip of current branch > - try: > - node = repo.branchtip(wc.branch()) > - except error.RepoLookupError: > - if wc.branch() == "default": # no default branch! > - node = repo.lookup("tip") # update to tip > - else: > - raise util.Abort(_("branch %s not found") % wc.branch()) > + # move as far forward as possible > + wcbranch = wc.branch() > + nodectx = wc.parents()[0] > + descendants = nodectx.descendants() > + for descendant in descendants: > + if (descendant.branch() == wcbranch and > + descendant.rev() > nodectx.rev()): > + nodectx = descendant > + > + node = nodectx.node() > + > overwrite = force and not branchmerge > pl = wc.parents() > p1, p2 = pl[0], repo[node] > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@selenic.com > http://selenic.com/mailman/listinfo/mercurial-devel
On 4/10/13 1:22 PM, "Augie Fackler" <raf@durin42.com> wrote: >On Wed, Apr 10, 2013 at 12:55:39PM -0700, Durham Goode wrote: >> >> o 3 >> | >> | o 2 >> | | >> | @ 1 >> |/ >> | >> o 0 >> >> 'hg update' will now put you on commit 2. If you were already on >>commit 2, >> it would be a no-op. If you were on commit 0, it would put you on 3. > >Where would it put you before this change? It would give you an error "abort: crosses branches" because it tried to go to 3.
On Wed, Apr 10, 2013 at 4:24 PM, Durham Goode <durham@fb.com> wrote: > It would give you an error "abort: crosses branches" because it tried to > go to 3. In that case, this seems like it's strictly an improvement given our bookmark-using present.
On Wed, Apr 10, 2013 at 1:31 PM, Augie Fackler <raf@durin42.com> wrote: > In that case, this seems like it's strictly an improvement given our > bookmark-using present. > Agreed.
On 10 avr. 2013, at 22:22, Augie Fackler wrote: > On Wed, Apr 10, 2013 at 12:55:39PM -0700, Durham Goode wrote: >> o 3 >> | >> | o 2 >> | | >> | @ 1 >> |/ >> | >> o 0 >> >> 'hg update' will now put you on commit 2. If you were already on commit 2, >> it would be a no-op. If you were on commit 0, it would put you on 3. > > Where would it put you before this change? Before: max(branch(.)) After: max(.:: and branch(.))
On 11 April 2013 07:27, Kevin Bullock <kbullock+mercurial@ringworld.org>wrote: > On Apr 10, 2013, at 3:31 PM, Augie Fackler wrote: > > > On Wed, Apr 10, 2013 at 4:24 PM, Durham Goode <durham@fb.com> wrote: > >> It would give you an error "abort: crosses branches" because it tried to > >> go to 3. > > > > In that case, this seems like it's strictly an improvement given our > > bookmark-using present. > > Yeah, I think this is probably the right thing to do. It's definitely (BC) > though. > > I'm still wondering about whether this is the right thing if you're > working on a series of changes, push, then pull and get a divergent head > (and tangentially, possibly a divergent bookmark). Specifically whether > it's the right thing in -all- of the usual workflows (anonymous branches, > named branches, bookmarked branches). > I don't use bookmarks at all - my workflows are primarily named branch per task (I end up with a *lot* of named branches). To me this change seems to be exactly the right thing to do. When I pull in changesets, if there is a divergent changeset on the same named branch I will normally want to either: 1. merge it into my existing branch immediately; or 2. ignore it until I'm ready to merge my existing branch back to it. In both cases, updating to the latest version of the stuff *I'm* working on is the correct thing to do. Tim Delaney
On Wed, 2013-04-10 at 20:24 +0000, Durham Goode wrote: > On 4/10/13 1:22 PM, "Augie Fackler" <raf@durin42.com> wrote: > > >On Wed, Apr 10, 2013 at 12:55:39PM -0700, Durham Goode wrote: > >> > >> o 3 > >> | > >> | o 2 > >> | | > >> | @ 1 > >> |/ > >> | > >> o 0 > >> > >> 'hg update' will now put you on commit 2. If you were already on > >>commit 2, > >> it would be a no-op. If you were on commit 0, it would put you on 3. > > > >Where would it put you before this change? > > It would give you an error "abort: crosses branches" because it tried to > go to 3. So.. user on 2 will now be given the false impression they were up-to-date? Prepare for a flood of "where did my incoming changes go?" or even more clueless bug reports. It's underspecified here whether you're talking about this being bookmark-specific behavior, and it seems from the patch that it's not. It's definitely not the right answer if there are no bookmarks involved.
On 11 April 2013 10:17, Matt Mackall <mpm@selenic.com> wrote: > On Wed, 2013-04-10 at 20:24 +0000, Durham Goode wrote: > > On 4/10/13 1:22 PM, "Augie Fackler" <raf@durin42.com> wrote: > > > > >On Wed, Apr 10, 2013 at 12:55:39PM -0700, Durham Goode wrote: > > >> > > >> o 3 > > >> | > > >> | o 2 > > >> | | > > >> | @ 1 > > >> |/ > > >> | > > >> o 0 > > >> > > >> 'hg update' will now put you on commit 2. If you were already on > > >>commit 2, > > >> it would be a no-op. If you were on commit 0, it would put you on 3. > > > > > >Where would it put you before this change? > > > > It would give you an error "abort: crosses branches" because it tried to > > go to 3. > > So.. user on 2 will now be given the false impression they were > up-to-date? Prepare for a flood of "where did my incoming changes go?" > or even more clueless bug reports. > Perhaps that could be avoided by a message when updating - something like "there are more recent divergent heads on the same branch - run hg heads <branchname>". This would only appear then updating to a changeset that is a head and has a lower revision than a divergent changeset on the same named branch. In the above scenario the warning would appear when updating to rev 2, but not when updating to any of rev 0, 1, or 3. Tim Delaney
On 4/10/13 5:17 PM, "Matt Mackall" <mpm@selenic.com> wrote: >On Wed, 2013-04-10 at 20:24 +0000, Durham Goode wrote: >> On 4/10/13 1:22 PM, "Augie Fackler" <raf@durin42.com> wrote: >> >> >On Wed, Apr 10, 2013 at 12:55:39PM -0700, Durham Goode wrote: >> >> >> >> o 3 >> >> | >> >> | o 2 >> >> | | >> >> | @ 1 >> >> |/ >> >> | >> >> o 0 >> >> >> >> 'hg update' will now put you on commit 2. If you were already on >> >>commit 2, >> >> it would be a no-op. If you were on commit 0, it would put you on 3. >> > >> >Where would it put you before this change? >> >> It would give you an error "abort: crosses branches" because it tried to >> go to 3. > >So.. user on 2 will now be given the false impression they were >up-to-date? Prepare for a flood of "where did my incoming changes go?" >or even more clueless bug reports. > >It's underspecified here whether you're talking about this being >bookmark-specific behavior, and it seems from the patch that it's not. >It's definitely not the right answer if there are no bookmarks involved. To better specify: this change is not bookmark specific. It always applies. If a user sees "abort: crosses branches" they're going to have to investigate anyway to learn why it failed. That same investigation holds true with my change if hg update reports nothing changed. I think the new behavior is more globally useful and intuitive (for anonymous branch workflows, named branch flows, and bookmark flows). Would it help to output a message when it's not going to the absolute tip? I'd hesitate to print anything named-branch specific since it would just confuse people using bookmark workflows.
On Thu, 2013-04-11 at 17:23 +0000, Durham Goode wrote: > On 4/10/13 5:17 PM, "Matt Mackall" <mpm@selenic.com> wrote: > > >On Wed, 2013-04-10 at 20:24 +0000, Durham Goode wrote: > >> On 4/10/13 1:22 PM, "Augie Fackler" <raf@durin42.com> wrote: > >> > >> >On Wed, Apr 10, 2013 at 12:55:39PM -0700, Durham Goode wrote: > >> >> > >> >> o 3 > >> >> | > >> >> | o 2 > >> >> | | > >> >> | @ 1 > >> >> |/ > >> >> | > >> >> o 0 > >> >> > >> >> 'hg update' will now put you on commit 2. If you were already on > >> >>commit 2, > >> >> it would be a no-op. If you were on commit 0, it would put you on 3. > >> > > >> >Where would it put you before this change? > >> > >> It would give you an error "abort: crosses branches" because it tried to > >> go to 3. > > > >So.. user on 2 will now be given the false impression they were > >up-to-date? Prepare for a flood of "where did my incoming changes go?" > >or even more clueless bug reports. > > > >It's underspecified here whether you're talking about this being > >bookmark-specific behavior, and it seems from the patch that it's not. > >It's definitely not the right answer if there are no bookmarks involved. > > To better specify: this change is not bookmark specific. It always applies. > > If a user sees "abort: crosses branches" they're going to have to > investigate anyway to learn why it failed. That same investigation holds > true with my change if hg update reports nothing changed. First, I'm happy to agree that the old message sucks. But it does tell the user that a) they have an issue and b) it's somehow related to branching. But "hg update" doing this: $ hg update 0 files updated, 0 files merged, 0 files removed, 0 files unresolved says "everything's fine" to all but the most clueful, attentive users. We have maybe a dozen of those. The average user instead has only the vaguest idea of how the DAG works (ie "it's magic") and it's not going to occur to them to merge[1] unless you give them a big fat hint. Instead, they'll keep working for a few days and then wonder why they didn't get any of the bugfixes they were supposed to. [1] what typical non-expert, non-bookmark-and-rebase workflows should probably do here
On 4/11/13 1:15 PM, "Matt Mackall" <mpm@selenic.com> wrote: >First, I'm happy to agree that the old message sucks. But it does tell >the user that a) they have an issue and b) it's somehow related to >branching. > >But "hg update" doing this: > > $ hg update > 0 files updated, 0 files merged, 0 files removed, 0 files unresolved > >says "everything's fine" to all but the most clueful, attentive users. What about something like: $ hg update newer commits exist on a different branch - merge or rebase to reach them 0 files updated, 0 files merged, 0 files removed, 0 files unresolved It's more explicit than the current message, it's relatively applicable to both branch and bookmark flows, and hg update is still useful for moving to descendants.
Den 2013-04-12 21:07:14 skrev Durham Goode <durham@fb.com>: > On 4/11/13 1:15 PM, "Matt Mackall" <mpm@selenic.com> wrote: > >> First, I'm happy to agree that the old message sucks. But it does tell >> the user that a) they have an issue and b) it's somehow related to >> branching. >> >> But "hg update" doing this: >> >> $ hg update >> 0 files updated, 0 files merged, 0 files removed, 0 files unresolved >> >> says "everything's fine" to all but the most clueful, attentive users. > > What about something like: > > $ hg update > newer commits exist on a different branch - merge or rebase to reach them I think that something like "divergent" is better than "different", and "branch" should better be avoided. One can easily read the message as "on some other (named) branch there's something newer" and not recognize as relevant to one's case. What about "line of development" or "thread" (as in "forum thread")? "merge or rebase to reach them" seems (at least to me) to imply that you *should* merge or rebase (and the sooner the better), otherwise those commits remain unreachable (at least 'update' command is powerless). But I'm not native English speaker (yeah, shocking truth), so I may be wrong here.
On Fri, 2013-04-12 at 17:07 +0000, Durham Goode wrote: > On 4/11/13 1:15 PM, "Matt Mackall" <mpm@selenic.com> wrote: > > >First, I'm happy to agree that the old message sucks. But it does tell > >the user that a) they have an issue and b) it's somehow related to > >branching. > > > >But "hg update" doing this: > > > > $ hg update > > 0 files updated, 0 files merged, 0 files removed, 0 files unresolved > > > >says "everything's fine" to all but the most clueful, attentive users. > > What about something like: > > $ hg update > newer commits exist on a different branch - merge or rebase to reach them > 0 files updated, 0 files merged, 0 files removed, 0 files unresolved > It's more explicit than the current message, it's relatively applicable to > both branch and bookmark flows, and hg update is still useful for moving > to descendants. That looks promising. However, I think we need to look at the big picture here. Update has -lots- of different cases and we need to make sure that our changes are coherent. We've got this table hiding in merge.py:update: -c -C dirty rev | linear same cross n n n n | ok (1) x n n n y | ok ok ok n n y * | merge (2) (2) n y * * | --- discard --- y n y * | --- (3) --- y n n * | --- ok --- y y * * | --- (4) --- x = can't happen * = don't-care 1 = abort: crosses branches (use 'hg merge' or 'hg update -c') 2 = abort: crosses branches (use 'hg merge' to merge or use 'hg update -C' to discard changes) 3 = abort: uncommitted local changes 4 = incompatible options (checked in commands.py) Is this table correct and complete? The first line, 'same' column seems to say "hg update with no args and clean working directory that wants to go to a head that's on the same branch but isn't a linear descendant.. gets error message (1)". Do we not get that hint? Is there bookmark-specific behavior that should be in this table? Should this table live in 'hg help update -v'? Also, I kindof like abort+hint. Abort says "pay attention, n00b, I didn't do what you told me to", while hint says "try this". Lastly, mentioning non-core commands like "rebase" in the output of core commands is problematic.
On Saturday 13 April 2013 12:15:49 am Matt Mackall wrote: > On Fri, 2013-04-12 at 17:07 +0000, Durham Goode wrote: > > On 4/11/13 1:15 PM, "Matt Mackall" <mpm@selenic.com> wrote: > > >First, I'm happy to agree that the old message sucks. But it does tell > > >the user that a) they have an issue and b) it's somehow related to > > >branching. > > > > > >But "hg update" doing this: > > > > > > $ hg update > > > 0 files updated, 0 files merged, 0 files removed, 0 files unresolved > > > > > >says "everything's fine" to all but the most clueful, attentive users. > > > > What about something like: > > > > $ hg update > > newer commits exist on a different branch - merge or rebase to reach them > > 0 files updated, 0 files merged, 0 files removed, 0 files unresolved > > > > It's more explicit than the current message, it's relatively applicable > > to both branch and bookmark flows, and hg update is still useful for > > moving to descendants. > > That looks promising. However, I think we need to look at the big > picture here. Update has -lots- of different cases and we need to make > sure that our changes are coherent. > > We've got this table hiding in merge.py:update: > > -c -C dirty rev | linear same cross > n n n n | ok (1) x > n n n y | ok ok ok > n n y * | merge (2) (2) > n y * * | --- discard --- > y n y * | --- (3) --- > y n n * | --- ok --- > y y * * | --- (4) --- > > x = can't happen > * = don't-care > 1 = abort: crosses branches (use 'hg merge' or 'hg update -c') > 2 = abort: crosses branches (use 'hg merge' to merge or > use 'hg update -C' to discard changes) > 3 = abort: uncommitted local changes > 4 = incompatible options (checked in commands.py) > > Is this table correct and complete? The first line, 'same' column seems > to say "hg update with no args and clean working directory that wants to > go to a head that's on the same branch but isn't a linear descendant.. > gets error message (1)". Do we not get that hint? > > Is there bookmark-specific behavior that should be in this table? > > Should this table live in 'hg help update -v'? > > Also, I kindof like abort+hint. Abort says "pay attention, n00b, I > didn't do what you told me to", while hint says "try this". > > Lastly, mentioning non-core commands like "rebase" in the output of core > commands is problematic. The problem with this patch is that "hg up -c" is still forcing the latest rev in the current branch in commands.py, so that it will need to be fixed as well. This is symptomatic of few design problems in this area: - The logic is split between commands.update and merge.update. This table in merge.py explains behavior depending on flags parsed in commands.py. We cannot even directly sense anymore what was the content of those flags. - This table was originally for developers, due to the complexity of the different cases. But it probably ought to go in the help. - When I was doing training to my team, explaining all those different cases was a real pain. I would like it to be simplified such that: * there is no distinction between named branch and anonymous branches, and here this patch would help as the first line would be only linear by construction (provided some warnings are added): -c -C dirty rev | linear same cross n n n n | ok x x * there is no distinction between linear and non-linear, so that we always merge local changes by default (provided there are some undo mechanism in place), so that the 3rd line looks like: -c -C dirty rev | linear same cross n n y * | merge merge merge * --check promises that it only checks for local changes, but it does more than that by subtly changing the target to the latest rev of the branch. It shall not do that. With all that, the right side of the table just contains 1 column, line 1 and 6 ('hg up' and 'hg up -c') behave the same. However, this might be too much and violates the backward compatibility / safety principles. Regards. Gilles.
On 4/12/13 3:15 PM, "Matt Mackall" <mpm@selenic.com> wrote: >On Fri, 2013-04-12 at 17:07 +0000, Durham Goode wrote: >> >> What about something like: >> >> $ hg update >> newer commits exist on a different branch - merge or rebase to reach >>them >> 0 files updated, 0 files merged, 0 files removed, 0 files unresolved > > -c -C dirty rev | linear same cross > > > n n n n | ok (1) x > > > n n n y | ok ok ok > > > n n y * | merge (2) (2) > > > n y * * | --- discard --- > > > y n y * | --- (3) --- > > > y n n * | --- ok --- > > > y y * * | --- (4) --- > > > > > > x = can't happen > > > * = don't-care > > > 1 = abort: crosses branches (use 'hg merge' or 'hg update -c') > > > 2 = abort: crosses branches (use 'hg merge' to merge or > > > use 'hg update -C' to discard changes) > > > 3 = abort: uncommitted local changes > > > 4 = incompatible options (checked in commands.py) > > > >Is this table correct and complete? The first line, 'same' column seems >to say "hg update with no args and clean working directory that wants to >go to a head that's on the same branch but isn't a linear descendant.. >gets error message (1)". Do we not get that hint? With my proposal (1) would become a warning instead of an abort. Something like: "newer commits exist on another branch (use 'hg update -c' to go there)". The rest of the chart should be unchanged since my change only affects the case where 'rev == n' > >Is there bookmark-specific behavior that should be in this table? Not at the moment. My change (and the table as a whole, afaik) do not have bookmark specific behavior. >Should this table live in 'hg help update -v'? The help -v already explains the rules (albeit in prose, not a table). If anything I think we should make update have less rules so we have less explaining to do (as Gilles proposed in their email). >Also, I kindof like abort+hint. Abort says "pay attention, n00b, I >didn't do what you told me to", while hint says "try this". I'm not sure what you mean here. The whole purpose of my change is to avoid an abort. We could add a 'warning:' in front of the message to give it more emphasis I guess: "warning: newer commits exist on another branch (use 'hg update -c' to go there)" But it's not actually a problem in a rebase workflow so I'm hesitant to flag it as 'warning'.
On 16 April 2013 03:33, Durham Goode <durham@fb.com> wrote: > On 4/12/13 3:15 PM, "Matt Mackall" <mpm@selenic.com> wrote: > > >Is this table correct and complete? The first line, 'same' column seems > >to say "hg update with no args and clean working directory that wants to > >go to a head that's on the same branch but isn't a linear descendant.. > >gets error message (1)". Do we not get that hint? > > With my proposal (1) would become a warning instead of an abort. Something > like: > "newer commits exist on another branch (use 'hg update -c' to go there)". > The rest of the chart should be unchanged since my change only affects the > case where 'rev == n' I would prefer to only get this warning when updating to a head. If I'm deliberately updating to a non-head changeset, then presumably I know what I'm doing and shouldn't be told about divergent changesets. But it's not actually a problem in a rebase workflow so I'm hesitant to > flag it as 'warning'. Yeah - I think it's more of a hint than a warning in either case. Tim Delaney
On 4/15/13 2:07 PM, "Tim Delaney" <timothy.c.delaney@gmail.com> wrote: > >I would prefer to only get this warning when updating to a head. If I'm >deliberately updating to a non-head changeset, then presumably I know >what I'm doing and shouldn't be told about divergent changesets. The warning only occurs when you run 'hg update' without specifying a rev. So updating to non heads would never show this.
On 16 April 2013 07:56, Durham Goode <durham@fb.com> wrote: > On 4/15/13 2:07 PM, "Tim Delaney" <timothy.c.delaney@gmail.com> wrote: > > > >I would prefer to only get this warning when updating to a head. If I'm > >deliberately updating to a non-head changeset, then presumably I know > >what I'm doing and shouldn't be told about divergent changesets. > > The warning only occurs when you run 'hg update' without specifying a rev. > So updating to non heads would never show this. > Ah - I missed that. Works for me. Tim Delaney
Patch
diff --git a/mercurial/merge.py b/mercurial/merge.py --- a/mercurial/merge.py +++ b/mercurial/merge.py @@ -652,14 +652,17 @@ try: wc = repo[None] if node is None: - # tip of current branch - try: - node = repo.branchtip(wc.branch()) - except error.RepoLookupError: - if wc.branch() == "default": # no default branch! - node = repo.lookup("tip") # update to tip - else: - raise util.Abort(_("branch %s not found") % wc.branch()) + # move as far forward as possible + wcbranch = wc.branch() + nodectx = wc.parents()[0] + descendants = nodectx.descendants() + for descendant in descendants: + if (descendant.branch() == wcbranch and + descendant.rev() > nodectx.rev()): + nodectx = descendant + + node = nodectx.node() + overwrite = force and not branchmerge pl = wc.parents() p1, p2 = pl[0], repo[node]