Patchwork [RESEND2] bookmarks: allow push to create a new remote head if it is about to be bookmarked via -B

login
register
mail settings
Submitter Stephen Lee
Date Nov. 24, 2013, 1:05 p.m.
Message ID <fc00a55e05989147ca62.1385298352@slee-desktop>
Download mbox | patch
Permalink /patch/3117/
State Superseded
Headers show

Comments

Stephen Lee - Nov. 24, 2013, 1:05 p.m.
# HG changeset patch
# User Stephen Lee <sphen.lee@gmail.com>
# Date 1384165014 -39600
#      Mon Nov 11 21:16:54 2013 +1100
# Node ID fc00a55e05989147ca622b44129c3b9cbbf250eb
# Parent  1c46b18b0e1c47fa4cecf21b78c083a54ae9903f
bookmarks: allow push to create a new remote head if it is about to be bookmarked via -B

Push is currently allowed to create a new head if there is a remote bookmark that will
be updated to point to the new head.  If the bookmark is not known remotely then
push aborts, even if a -B argument is about to push the bookmark. This change allows
push to continue in this case.  This does not require a wireproto force.
Sean Farley - Nov. 24, 2013, 4:10 p.m.
sphen.lee@gmail.com writes:

> # HG changeset patch
> # User Stephen Lee <sphen.lee@gmail.com>
> # Date 1384165014 -39600
> #      Mon Nov 11 21:16:54 2013 +1100
> # Node ID fc00a55e05989147ca622b44129c3b9cbbf250eb
> # Parent  1c46b18b0e1c47fa4cecf21b78c083a54ae9903f
> bookmarks: allow push to create a new remote head if it is about to be bookmarked via -B
>
> Push is currently allowed to create a new head if there is a remote bookmark that will
> be updated to point to the new head.  If the bookmark is not known remotely then
> push aborts, even if a -B argument is about to push the bookmark. This change allows
> push to continue in this case.  This does not require a wireproto force.
>
> diff --git a/mercurial/commands.py b/mercurial/commands.py
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -4679,6 +4679,7 @@
>      """
>  
>      if opts.get('bookmark'):
> +        ui.setconfig('bookmarks', 'pushing', opts['bookmark'])
>          for b in opts['bookmark']:
>              # translate -B options to -r so changesets get pushed
>              if b in repo._bookmarks:
> diff --git a/mercurial/discovery.py b/mercurial/discovery.py
> --- a/mercurial/discovery.py
> +++ b/mercurial/discovery.py
> @@ -219,7 +219,8 @@
>      unsynced = inc and set([None]) or set()
>      return {None: (oldheads, newheads, unsynced)}
>  
> -def checkheads(repo, remote, outgoing, remoteheads, newbranch=False, inc=False):
> +def checkheads(repo, remote, outgoing, remoteheads, newbranch=False, inc=False,
> +               newbookmarks=[]):
>      """Check that a push won't add any outgoing head
>  
>      raise Abort error and display ui message as needed.
> @@ -259,6 +260,9 @@
>              lctx, rctx = repo[bm], repo[rnode]
>              if bookmarks.validdest(repo, rctx, lctx):
>                  bookmarkedheads.add(lctx.node())
> +        else:
> +            if bm in newbookmarks:
> +                bookmarkedheads.add(repo[bm].node())
>  
>      # 3. Check for new heads.
>      # If there are more heads after the push than before, a suitable
> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -1864,9 +1864,10 @@
>                                      raise util.Abort(_(mst)
>                                                       % (ctx.troubles()[0],
>                                                          ctx))
> +                        newbm = self.ui.configlist('bookmarks', 'pushing')
>                          discovery.checkheads(unfi, remote, outgoing,
>                                               remoteheads, newbranch,
> -                                             bool(inc))
> +                                             bool(inc), newbm)
>  
>                      # TODO: get bundlecaps from remote
>                      bundlecaps = None

I like this patch a lot more than the previous two patches. It's smaller
and easier to review :-) Plus, it doesn't conflict with Fujiwara's patch
series, so that's another plus.

I just tested this out and it works swimmingly. I am definitely +1 on this.

> diff --git a/tests/test-bookmarks-pushpull.t b/tests/test-bookmarks-pushpull.t
> --- a/tests/test-bookmarks-pushpull.t
> +++ b/tests/test-bookmarks-pushpull.t
> @@ -424,4 +424,22 @@
>    remote: added 1 changesets with 1 changes to 1 files
>    exporting bookmark add-foo
>  
> +pushing a new bookmark on a new head does not require -f if -B is specified
> +
> +  $ hg up -q X
> +  $ hg book W
> +  $ echo c5 > f2
> +  $ hg ci -Am5
> +  created new head
> +  $ hg push -B W
> +  pushing to http://localhost:$HGPORT/
> +  searching for changes
> +  remote: adding changesets
> +  remote: adding manifests
> +  remote: adding file changes
> +  remote: added 1 changesets with 1 changes to 1 files (+1 heads)
> +  exporting bookmark W
> +  $ hg -R ../b id -r W
> +  cc978a373a53 tip W
> +
>    $ cd ..
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Katsunori FUJIWARA - Nov. 26, 2013, 2:12 p.m.
At Mon, 25 Nov 2013 00:05:52 +1100,
Stephen Lee wrote:
> 
> # HG changeset patch
> # User Stephen Lee <sphen.lee@gmail.com>
> # Date 1384165014 -39600
> #      Mon Nov 11 21:16:54 2013 +1100
> # Node ID fc00a55e05989147ca622b44129c3b9cbbf250eb
> # Parent  1c46b18b0e1c47fa4cecf21b78c083a54ae9903f
> bookmarks: allow push to create a new remote head if it is about to be bookmarked via -B
> 
> Push is currently allowed to create a new head if there is a remote bookmark that will
> be updated to point to the new head.  If the bookmark is not known remotely then
> push aborts, even if a -B argument is about to push the bookmark. This change allows
> push to continue in this case.  This does not require a wireproto force.

I like this allowing push itself.

But, in the other hand, I also worry that there is no way to examine
whether newly added head is bookmarked or not in "pretxnchangegroup"
(or any other pre-transaction) hooks on the remote repository side.

As far as I confirmed, "pushkey" hook can examine newly added (or
updated) bookmarks, but it can't know what changesets are brought from
another repository, and can't cancel already committed "changegroup".


> diff --git a/mercurial/commands.py b/mercurial/commands.py
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -4679,6 +4679,7 @@
>      """
>  
>      if opts.get('bookmark'):
> +        ui.setconfig('bookmarks', 'pushing', opts['bookmark'])
>          for b in opts['bookmark']:
>              # translate -B options to -r so changesets get pushed
>              if b in repo._bookmarks:
> diff --git a/mercurial/discovery.py b/mercurial/discovery.py
> --- a/mercurial/discovery.py
> +++ b/mercurial/discovery.py
> @@ -219,7 +219,8 @@
>      unsynced = inc and set([None]) or set()
>      return {None: (oldheads, newheads, unsynced)}
>  
> -def checkheads(repo, remote, outgoing, remoteheads, newbranch=False, inc=False):
> +def checkheads(repo, remote, outgoing, remoteheads, newbranch=False, inc=False,
> +               newbookmarks=[]):
>      """Check that a push won't add any outgoing head
>  
>      raise Abort error and display ui message as needed.
> @@ -259,6 +260,9 @@
>              lctx, rctx = repo[bm], repo[rnode]
>              if bookmarks.validdest(repo, rctx, lctx):
>                  bookmarkedheads.add(lctx.node())
> +        else:
> +            if bm in newbookmarks:
> +                bookmarkedheads.add(repo[bm].node())
>  
>      # 3. Check for new heads.
>      # If there are more heads after the push than before, a suitable
> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -1864,9 +1864,10 @@
>                                      raise util.Abort(_(mst)
>                                                       % (ctx.troubles()[0],
>                                                          ctx))
> +                        newbm = self.ui.configlist('bookmarks', 'pushing')
>                          discovery.checkheads(unfi, remote, outgoing,
>                                               remoteheads, newbranch,
> -                                             bool(inc))
> +                                             bool(inc), newbm)
>  
>                      # TODO: get bundlecaps from remote
>                      bundlecaps = None
> diff --git a/tests/test-bookmarks-pushpull.t b/tests/test-bookmarks-pushpull.t
> --- a/tests/test-bookmarks-pushpull.t
> +++ b/tests/test-bookmarks-pushpull.t
> @@ -424,4 +424,22 @@
>    remote: added 1 changesets with 1 changes to 1 files
>    exporting bookmark add-foo
>  
> +pushing a new bookmark on a new head does not require -f if -B is specified
> +
> +  $ hg up -q X
> +  $ hg book W
> +  $ echo c5 > f2
> +  $ hg ci -Am5
> +  created new head
> +  $ hg push -B W
> +  pushing to http://localhost:$HGPORT/
> +  searching for changes
> +  remote: adding changesets
> +  remote: adding manifests
> +  remote: adding file changes
> +  remote: added 1 changesets with 1 changes to 1 files (+1 heads)
> +  exporting bookmark W
> +  $ hg -R ../b id -r W
> +  cc978a373a53 tip W
> +
>    $ cd ..
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
> 

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp
Stephen Lee - Nov. 26, 2013, 11:33 p.m.
On Wed, Nov 27, 2013 at 1:12 AM, FUJIWARA Katsunori
<foozy@lares.dti.ne.jp> wrote:
> At Mon, 25 Nov 2013 00:05:52 +1100,
> Stephen Lee wrote:
>>
>> # HG changeset patch
>> # User Stephen Lee <sphen.lee@gmail.com>
>> # Date 1384165014 -39600
>> #      Mon Nov 11 21:16:54 2013 +1100
>> # Node ID fc00a55e05989147ca622b44129c3b9cbbf250eb
>> # Parent  1c46b18b0e1c47fa4cecf21b78c083a54ae9903f
>> bookmarks: allow push to create a new remote head if it is about to be bookmarked via -B
>>
>> Push is currently allowed to create a new head if there is a remote bookmark that will
>> be updated to point to the new head.  If the bookmark is not known remotely then
>> push aborts, even if a -B argument is about to push the bookmark. This change allows
>> push to continue in this case.  This does not require a wireproto force.
>
> I like this allowing push itself.
>
> But, in the other hand, I also worry that there is no way to examine
> whether newly added head is bookmarked or not in "pretxnchangegroup"
> (or any other pre-transaction) hooks on the remote repository side.

This has been a problem for us too.
In particular we want to:
 * include bookmarks names in email notifications - the notify
extension can't do this because bookmarks arrive after changesets.
 * reject pushes that advance certain bookmarks (ie, only integrator
can advance @), we can reject the bookmark update, but the changesets
are already in the repo

Unfortunately I think we will have to wait for bundle2 to fix this.

>
> As far as I confirmed, "pushkey" hook can examine newly added (or
> updated) bookmarks, but it can't know what changesets are brought from
> another repository, and can't cancel already committed "changegroup".
>

Patch

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -4679,6 +4679,7 @@ 
     """
 
     if opts.get('bookmark'):
+        ui.setconfig('bookmarks', 'pushing', opts['bookmark'])
         for b in opts['bookmark']:
             # translate -B options to -r so changesets get pushed
             if b in repo._bookmarks:
diff --git a/mercurial/discovery.py b/mercurial/discovery.py
--- a/mercurial/discovery.py
+++ b/mercurial/discovery.py
@@ -219,7 +219,8 @@ 
     unsynced = inc and set([None]) or set()
     return {None: (oldheads, newheads, unsynced)}
 
-def checkheads(repo, remote, outgoing, remoteheads, newbranch=False, inc=False):
+def checkheads(repo, remote, outgoing, remoteheads, newbranch=False, inc=False,
+               newbookmarks=[]):
     """Check that a push won't add any outgoing head
 
     raise Abort error and display ui message as needed.
@@ -259,6 +260,9 @@ 
             lctx, rctx = repo[bm], repo[rnode]
             if bookmarks.validdest(repo, rctx, lctx):
                 bookmarkedheads.add(lctx.node())
+        else:
+            if bm in newbookmarks:
+                bookmarkedheads.add(repo[bm].node())
 
     # 3. Check for new heads.
     # If there are more heads after the push than before, a suitable
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -1864,9 +1864,10 @@ 
                                     raise util.Abort(_(mst)
                                                      % (ctx.troubles()[0],
                                                         ctx))
+                        newbm = self.ui.configlist('bookmarks', 'pushing')
                         discovery.checkheads(unfi, remote, outgoing,
                                              remoteheads, newbranch,
-                                             bool(inc))
+                                             bool(inc), newbm)
 
                     # TODO: get bundlecaps from remote
                     bundlecaps = None
diff --git a/tests/test-bookmarks-pushpull.t b/tests/test-bookmarks-pushpull.t
--- a/tests/test-bookmarks-pushpull.t
+++ b/tests/test-bookmarks-pushpull.t
@@ -424,4 +424,22 @@ 
   remote: added 1 changesets with 1 changes to 1 files
   exporting bookmark add-foo
 
+pushing a new bookmark on a new head does not require -f if -B is specified
+
+  $ hg up -q X
+  $ hg book W
+  $ echo c5 > f2
+  $ hg ci -Am5
+  created new head
+  $ hg push -B W
+  pushing to http://localhost:$HGPORT/
+  searching for changes
+  remote: adding changesets
+  remote: adding manifests
+  remote: adding file changes
+  remote: added 1 changesets with 1 changes to 1 files (+1 heads)
+  exporting bookmark W
+  $ hg -R ../b id -r W
+  cc978a373a53 tip W
+
   $ cd ..