Patchwork push: do not require --force to push bookmarked head (issue2372)

login
register
mail settings
Submitter Prasoon Shukla
Date Dec. 19, 2013, 1:01 p.m.
Message ID <241543f591d7f40bb577.1387458105@jimmypage>
Download mbox | patch
Permalink /patch/3216/
State Rejected
Headers show

Comments

Prasoon Shukla - Dec. 19, 2013, 1:01 p.m.
# HG changeset patch
# User Prasoon Shukla <prasoon92.iitr@gmail.com>
# Date 1387389770 -19800
#      Wed Dec 18 23:32:50 2013 +0530
# Node ID 241543f591d7f40bb5777ce64174b420ae8e713a
# Parent  5ff0fd02385082433221d0c78a99d310257d27b3
push: do not require --force to push bookmarked head (issue2372)

When pushing, an error is raised if a new head is created at remote.
This is to avoid ambiguity for developers pulling from the remote. But,
if this head is bookmarked, then pushing shouldn't require --force.
This patch implements this function.
Prasoon Shukla - Dec. 19, 2013, 1:15 p.m.
A couple of things. The error was caused by the discovery.checkheads()
which raised the error. We can check inside discovery.checkheads()
whether the new head is bookmarked. But, just checking whether the
pushed head is bookmarked will also allow for the case where someone
can implicitly (and maybe without their knowledge) push a bookmarked
head upstream. So, we need to be know inside discovery.checkheads
whether the user is indeed trying to push the bookmark. That is why we
have added a new flag, bkmark, that would allow discovery.checkheads
to determine exactly this.
Matt Mackall - Dec. 19, 2013, 7 p.m.
On Thu, 2013-12-19 at 18:31 +0530, Prasoon Shukla wrote:
> # HG changeset patch
> # User Prasoon Shukla <prasoon92.iitr@gmail.com>
> # Date 1387389770 -19800
> #      Wed Dec 18 23:32:50 2013 +0530
> # Node ID 241543f591d7f40bb5777ce64174b420ae8e713a
> # Parent  5ff0fd02385082433221d0c78a99d310257d27b3
> push: do not require --force to push bookmarked head (issue2372)
> 
> When pushing, an error is raised if a new head is created at remote.
> This is to avoid ambiguity for developers pulling from the remote. But,
> if this head is bookmarked, then pushing shouldn't require --force.
> This patch implements this function.

You don't mention -B anywhere up here, so I wrote a rant about how this
was obviously wrong. Please write a more precise description.
Stephen Lee - Dec. 19, 2013, 11:06 p.m.
On Fri, Dec 20, 2013 at 6:00 AM, Matt Mackall <mpm@selenic.com> wrote:
> On Thu, 2013-12-19 at 18:31 +0530, Prasoon Shukla wrote:
>> # HG changeset patch
>> # User Prasoon Shukla <prasoon92.iitr@gmail.com>
>> # Date 1387389770 -19800
>> #      Wed Dec 18 23:32:50 2013 +0530
>> # Node ID 241543f591d7f40bb5777ce64174b420ae8e713a
>> # Parent  5ff0fd02385082433221d0c78a99d310257d27b3
>> push: do not require --force to push bookmarked head (issue2372)
>>
>> When pushing, an error is raised if a new head is created at remote.
>> This is to avoid ambiguity for developers pulling from the remote. But,
>> if this head is bookmarked, then pushing shouldn't require --force.
>> This patch implements this function.
>
> You don't mention -B anywhere up here, so I wrote a rant about how this
> was obviously wrong. Please write a more precise description.
>

I have already sent a patch for this issue (I didn't realise there was
an actual bug logged).
It allows a new head to be pushed if it has a bookmark FOO, and you
specified -B FOO.

See http://markmail.org/message/osli572jxcdlbsrj

Steve
Matt Mackall - Dec. 19, 2013, 11:18 p.m.
On Fri, 2013-12-20 at 10:06 +1100, Stephen Lee wrote:
> On Fri, Dec 20, 2013 at 6:00 AM, Matt Mackall <mpm@selenic.com> wrote:
> > On Thu, 2013-12-19 at 18:31 +0530, Prasoon Shukla wrote:
> >> # HG changeset patch
> >> # User Prasoon Shukla <prasoon92.iitr@gmail.com>
> >> # Date 1387389770 -19800
> >> #      Wed Dec 18 23:32:50 2013 +0530
> >> # Node ID 241543f591d7f40bb5777ce64174b420ae8e713a
> >> # Parent  5ff0fd02385082433221d0c78a99d310257d27b3
> >> push: do not require --force to push bookmarked head (issue2372)
> >>
> >> When pushing, an error is raised if a new head is created at remote.
> >> This is to avoid ambiguity for developers pulling from the remote. But,
> >> if this head is bookmarked, then pushing shouldn't require --force.
> >> This patch implements this function.
> >
> > You don't mention -B anywhere up here, so I wrote a rant about how this
> > was obviously wrong. Please write a more precise description.
> >
> 
> I have already sent a patch for this issue (I didn't realise there was
> an actual bug logged).
> It allows a new head to be pushed if it has a bookmark FOO, and you
> specified -B FOO.
> 
> See http://markmail.org/message/osli572jxcdlbsrj

Yep, that does appear to be the same feature.
Prasoon Shukla - Dec. 20, 2013, 7:11 a.m.
Oh well. I didn't know about Stephen's patch, hence this one. So, I'll
just let this lie here (without applying Matt's fix). And by the way,
I've linked Stephen's patch on the bugtracker.

Thread closed.

On 12/20/13, Matt Mackall <mpm@selenic.com> wrote:
> On Fri, 2013-12-20 at 10:06 +1100, Stephen Lee wrote:
>> On Fri, Dec 20, 2013 at 6:00 AM, Matt Mackall <mpm@selenic.com> wrote:
>> > On Thu, 2013-12-19 at 18:31 +0530, Prasoon Shukla wrote:
>> >> # HG changeset patch
>> >> # User Prasoon Shukla <prasoon92.iitr@gmail.com>
>> >> # Date 1387389770 -19800
>> >> #      Wed Dec 18 23:32:50 2013 +0530
>> >> # Node ID 241543f591d7f40bb5777ce64174b420ae8e713a
>> >> # Parent  5ff0fd02385082433221d0c78a99d310257d27b3
>> >> push: do not require --force to push bookmarked head (issue2372)
>> >>
>> >> When pushing, an error is raised if a new head is created at remote.
>> >> This is to avoid ambiguity for developers pulling from the remote.
>> >> But,
>> >> if this head is bookmarked, then pushing shouldn't require --force.
>> >> This patch implements this function.
>> >
>> > You don't mention -B anywhere up here, so I wrote a rant about how this
>> > was obviously wrong. Please write a more precise description.
>> >
>>
>> I have already sent a patch for this issue (I didn't realise there was
>> an actual bug logged).
>> It allows a new head to be pushed if it has a bookmark FOO, and you
>> specified -B FOO.
>>
>> See http://markmail.org/message/osli572jxcdlbsrj
>
> Yep, that does appear to be the same feature.
>
> --
> Mathematics is the supreme nostalgia of our time.
>
>
>

Patch

diff -r 5ff0fd023850 -r 241543f591d7 hgext/largefiles/reposetup.py
--- a/hgext/largefiles/reposetup.py	Mon Dec 16 12:59:32 2013 -0600
+++ b/hgext/largefiles/reposetup.py	Wed Dec 18 23:32:50 2013 +0530
@@ -406,7 +406,8 @@ 
             finally:
                 wlock.release()
 
-        def push(self, remote, force=False, revs=None, newbranch=False):
+        def push(self, remote, force=False, revs=None, newbranch=False,
+                 bkmark=False):
             if remote.local():
                 missing = set(self.requirements) - remote.local().supported
                 if missing:
diff -r 5ff0fd023850 -r 241543f591d7 mercurial/commands.py
--- a/mercurial/commands.py	Mon Dec 16 12:59:32 2013 -0600
+++ b/mercurial/commands.py	Wed Dec 18 23:32:50 2013 +0530
@@ -4705,10 +4705,10 @@ 
     finally:
         del repo._subtoppath
     result = repo.push(other, opts.get('force'), revs=revs,
-                       newbranch=opts.get('new_branch'))
+                       newbranch=opts.get('new_branch'),
+                       bkmark=bool(opts.get('bookmark')))
 
     result = not result
-
     if opts.get('bookmark'):
         bresult = bookmarks.pushtoremote(ui, repo, other, opts['bookmark'])
         if bresult == 2:
diff -r 5ff0fd023850 -r 241543f591d7 mercurial/discovery.py
--- a/mercurial/discovery.py	Mon Dec 16 12:59:32 2013 -0600
+++ b/mercurial/discovery.py	Wed Dec 18 23:32:50 2013 +0530
@@ -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,
+               bkmark=False):
     """Check that a push won't add any outgoing head
 
     raise Abort error and display ui message as needed.
@@ -321,7 +322,10 @@ 
         elif len(newhs) > len(oldhs):
             # strip updates to existing remote heads from the new heads list
             dhs = sorted(newhs - bookmarkedheads - oldhs)
-        if dhs:
+        # if new head is bookmarked, we don't raise an exception
+        if dhs and bkmark and dhs[0] in repo._bookmarks.values():
+            pass
+        elif dhs:
             if error is None:
                 if branch not in ('default', None):
                     error = _("push creates new remote head %s "
diff -r 5ff0fd023850 -r 241543f591d7 mercurial/localrepo.py
--- a/mercurial/localrepo.py	Mon Dec 16 12:59:32 2013 -0600
+++ b/mercurial/localrepo.py	Wed Dec 18 23:32:50 2013 +0530
@@ -1766,7 +1766,8 @@ 
         """
         pass
 
-    def push(self, remote, force=False, revs=None, newbranch=False):
+    def push(self, remote, force=False, revs=None, newbranch=False,
+             bkmark=False):
         '''Push outgoing changesets (limited by revs) from the current
         repository to remote. Return an integer:
           - None means nothing to push
@@ -1866,7 +1867,7 @@ 
                                                         ctx))
                         discovery.checkheads(unfi, remote, outgoing,
                                              remoteheads, newbranch,
-                                             bool(inc))
+                                             bool(inc), bkmark)
 
                     # TODO: get bundlecaps from remote
                     bundlecaps = None
diff -r 5ff0fd023850 -r 241543f591d7 tests/test-bookmarks-pushpull.t
--- a/tests/test-bookmarks-pushpull.t	Mon Dec 16 12:59:32 2013 -0600
+++ b/tests/test-bookmarks-pushpull.t	Wed Dec 18 23:32:50 2013 +0530
@@ -424,4 +424,18 @@ 
   remote: added 1 changesets with 1 changes to 1 files
   exporting bookmark add-foo
 
+When a bookmark is pushed, don't raise an exception if a new head is created
+  $ hg bookmark changefoo
+  $ echo 'foo foo' > foo
+  $ hg ci -m 'change foo'
+  created new head
+  $ hg push -B changefoo
+  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
+  exporting bookmark changefoo
+
   $ cd ..