| Submitter | Stephen Lee |
|---|---|
| Date | March 11, 2014, 11:20 a.m. |
| Message ID | <44db1ede5c65a0265203.1394536811@slee-desktop> |
| Download | mbox | patch |
| Permalink | /patch/3911/ |
| State | Deferred |
| Headers | show |
Comments
On Tue, 2014-03-11 at 22:20 +1100, Stephen Lee wrote: > # HG changeset patch > # User Stephen Lee <sphen.lee@gmail.com> > # Date 1394536638 -39600 > # Tue Mar 11 22:17:18 2014 +1100 > # Node ID 44db1ede5c65a0265203f5df063049813d2da8c8 > # Parent ff0ffb9855d8e88fbcce967867f2a373eaac581f > bookmarks: on a bare merge use config to choose target or choose @ > > When running "hg merge" if there is an active and current bookmark > look in config for a key "bookmarks.<name>.track" and it will name > the bookmark we should merge with. If the key is missing, and an > @ bookmark exists, merge with it instead. I have no idea what the rationale here is, but it sounds like a behavior change that's going to cause me to accidentally do horrible horrible things like merge @ into stable without realizing it. This is worrisome. Also, this sort of patch should probably be split into two patches: - create surprise merge accidents - do some mysterious tracking thing
On Wed, Mar 12, 2014 at 9:16 AM, Matt Mackall <mpm@selenic.com> wrote: > On Tue, 2014-03-11 at 22:20 +1100, Stephen Lee wrote: >> # HG changeset patch >> # User Stephen Lee <sphen.lee@gmail.com> >> # Date 1394536638 -39600 >> # Tue Mar 11 22:17:18 2014 +1100 >> # Node ID 44db1ede5c65a0265203f5df063049813d2da8c8 >> # Parent ff0ffb9855d8e88fbcce967867f2a373eaac581f >> bookmarks: on a bare merge use config to choose target or choose @ >> >> When running "hg merge" if there is an active and current bookmark >> look in config for a key "bookmarks.<name>.track" and it will name >> the bookmark we should merge with. If the key is missing, and an >> @ bookmark exists, merge with it instead. > > I have no idea what the rationale here is, but it sounds like a behavior > change that's going to cause me to accidentally do horrible horrible > things like merge @ into stable without realizing it. This is worrisome. I will try to explain the rationale (sorry, these two patches should have been marked RFC). Firstly this patch is the less important of the two - the update patch is fixing a real functionality hole. This patch was mostly to make the same changes to merge as I made to update. Currently there is no way to move the active bookmark to a descendant rev, and update to that rev in a single command _unless that rev happens to be tip_. (What Git calls a fast-forward merge). This is a common thing to want to do in a bookmark-as-feature-branch workflow. It is a sore point for Git people coming to Mercurial and causes confusion for beginners: "hg merge @" Just-Works, but "hg up @" usually is not what you meant... My idea is to allow a per-bookmark config setting to name the bookmark that should be used as the target of a bare "hg up". I called it a 'tracking' bookmark (which is also the term Git uses) because it implies that your bookmark is 'following behind' some other bookmark. A secondary idea is to assume @ if the setting is missing. In a bookmark workflow where @ is the mainline branch this is a sensible default, and much less confusing that the current behaviour. Also @ is already used as the update target of a clone. In a non-bookmark workflow, the new logic only triggers if you have an active bookmark and an @ bookmark (which should be rare in a non-bookmark workflow). The logic is this patch (for merge) is the same. If you have an active bookmark and run a bare merge, currently it will abort unless there is a divergent bookmark. In this patch it will select the 'tracked' bookmark, or @. If you have no active bookmark there is no behaviour change (it won't magically merge @ into stable). > > Also, this sort of patch should probably be split into two patches: > > - create surprise merge accidents > - do some mysterious tracking thing I can split the patch apart if you would prefer - but before that I would like to get comments/suggestions from other contributors about the entire idea. There were lots of people commenting on the thread last time I brought up this issue! The only conclusion I came to was "make a patch and go from there". Steve
On Wed, 2014-03-12 at 20:27 +1100, Stephen Lee wrote: > In a non-bookmark workflow, the new logic only triggers if you have an > active bookmark and an @ bookmark (which should be rare in a > non-bookmark workflow). It seems to me that you're failing to account the situation of both bookmarks and named branches being used intentionally, which is a common thing to do with Mercurial (named branches aren't anathema). When on a named branch, `hg update` keeps you on that branch and moves you to it head, if unique. I think this is what mpm is complaining about, what if you're on the stable named branch with no active bookmark? - Jordi G. H.
On Wed, Mar 19, 2014 at 1:05 AM, Jordi Gutiérrez Hermoso <jordigh@octave.org> wrote: > On Wed, 2014-03-12 at 20:27 +1100, Stephen Lee wrote: > >> In a non-bookmark workflow, the new logic only triggers if you have an >> active bookmark and an @ bookmark (which should be rare in a >> non-bookmark workflow). > > It seems to me that you're failing to account the situation of both > bookmarks and named branches being used intentionally, which is a > common thing to do with Mercurial (named branches aren't anathema). > When on a named branch, `hg update` keeps you on that branch and moves > you to it head, if unique. I think this is what mpm is complaining > about, what if you're on the stable named branch with no active > bookmark? As I said before if you have no active bookmark, there is no behaviour change - you will end up on the tip of the current branch. If you are using bookmarks with branches then that usually makes no sense - which branch head is the 'real' head and which are feature heads? The 'real' head is often not even at a branch head! Also note that if the destination selected by the new logic is not a descendent, then update will abort anyway. My patch currently requires bookmarks on a non-default branch to explicitly name the bookmark they want to track. By convention the @ bookmark is usually on default, so for other branches you would need a different bookmark for the 'real' head ("<branch name>@" has been suggested in the past). I can incorporate that behaviour into the patch(es). I know named branches are not anathema. The workflow I advocated at my work uses named branches for release branches and bookmarks for feature branches. I'm trying to improve bookmarks in general - with or without named branches. Steve
On Fri, 2014-03-21 at 14:29 +1100, Stephen Lee wrote: > On Wed, Mar 19, 2014 at 1:05 AM, Jordi Gutiérrez Hermoso > <jordigh@octave.org> wrote: > > On Wed, 2014-03-12 at 20:27 +1100, Stephen Lee wrote: > > > >> In a non-bookmark workflow, the new logic only triggers if you have an > >> active bookmark and an @ bookmark (which should be rare in a > >> non-bookmark workflow). > > > > It seems to me that you're failing to account the situation of both > > bookmarks and named branches being used intentionally, which is a > > common thing to do with Mercurial (named branches aren't anathema). > > When on a named branch, `hg update` keeps you on that branch and moves > > you to it head, if unique. I think this is what mpm is complaining > > about, what if you're on the stable named branch with no active > > bookmark? > > As I said before if you have no active bookmark, there is no behaviour > change - you will end up on the tip of the current branch. > If you are using bookmarks with branches then that usually makes no > sense - which branch head is the 'real' head and which are feature > heads? The 'real' head is often not even at a branch head! > > Also note that if the destination selected by the new logic is not a > descendent, then update will abort anyway. > > My patch currently requires bookmarks on a non-default branch to > explicitly name the bookmark they want to track. > By convention the @ bookmark is usually on default, so for other > branches you would need a different bookmark for the 'real' head > ("<branch name>@" has been suggested in the past). > I can incorporate that behaviour into the patch(es). > > I know named branches are not anathema. The workflow I advocated at my > work uses named branches for release branches and bookmarks for > feature branches. > I'm trying to improve bookmarks in general - with or without named branches. Sadly, this is all too complicated to discuss in this format because there are too many cases and it's not clear that we're covering them all. We really need to move this discussion into some summary table that covers all the possible cases and shows both the old and new behavior. I'm also wary of assigning any semantics to '@' beyond 'this is the default checkout on clone if it exists'.
Patch
diff --git a/mercurial/commands.py b/mercurial/commands.py --- a/mercurial/commands.py +++ b/mercurial/commands.py @@ -4248,9 +4248,11 @@ "please merge with an explicit rev or bookmark"), hint=_("run 'hg heads' to see all heads")) elif len(bmheads) <= 1: - raise util.Abort(_("no matching bookmark to merge - " - "please merge with an explicit rev or bookmark"), - hint=_("run 'hg heads' to see all heads")) + node = bookmarks.trackingbookmark(repo) + if node is None: + raise util.Abort(_("no matching bookmark to merge - " + "please merge with an explicit rev or bookmark"), + hint=_("run 'hg heads' to see all heads")) if not node and not repo._bookmarkcurrent: branch = repo[None].branch() diff --git a/tests/test-bookmarks-merge.t b/tests/test-bookmarks-merge.t --- a/tests/test-bookmarks-merge.t +++ b/tests/test-bookmarks-merge.t @@ -177,3 +177,53 @@ g 8:04dd21731d95 $ hg id 26bee9c5bcf3 @/c + +# test bookmark tracking for merge + + $ hg book -d @ + $ hg up -q g + $ hg --config bookmarks.g.track=e merge + 1 files updated, 0 files merged, 0 files removed, 0 files unresolved + (branch merge, don't forget to commit) + $ hg parents + changeset: 8:04dd21731d95 + bookmark: g + tag: tip + parent: 6:be381d1126a0 + user: test + date: Thu Jan 01 00:00:00 1970 +0000 + summary: g + + changeset: 7:ca784329f0ba + bookmark: e + parent: 5:26bee9c5bcf3 + parent: 4:a0546fcfe0fb + user: test + date: Thu Jan 01 00:00:00 1970 +0000 + summary: merge + + +# test bookmarks implicitly tracking @ for merge + + $ hg up -C -q g + $ hg book -r5 @ + $ hg merge + 1 files updated, 0 files merged, 0 files removed, 0 files unresolved + (branch merge, don't forget to commit) + $ hg parents + changeset: 8:04dd21731d95 + bookmark: g + tag: tip + parent: 6:be381d1126a0 + user: test + date: Thu Jan 01 00:00:00 1970 +0000 + summary: g + + changeset: 5:26bee9c5bcf3 + bookmark: @ + bookmark: c + parent: 3:b8f96cf4688b + user: test + date: Thu Jan 01 00:00:00 1970 +0000 + summary: e +