Patchwork [2,of,2,V2] push: move handling of explicitly pushed bookmarks into localrepo.push()

login
register
mail settings
Submitter Kevin Bullock
Date Dec. 7, 2012, 7:43 p.m.
Message ID <64d29f52afb979d79992.1354909396@opendoor.mincava.umn.edu>
Download mbox | patch
Permalink /patch/32/
State Not Applicable
Headers show

Comments

Kevin Bullock - Dec. 7, 2012, 7:43 p.m.
# HG changeset patch
# User Kevin Bullock <kbullock at ringworld.org>
# Date 1354905590 21600
# Node ID 64d29f52afb979d79992cc9eaafc56690db372a3
# Parent  106b515749cf338617d24c2fae3850987bf46734
push: move handling of explicitly pushed bookmarks into localrepo.push()

This puts the handling of bookmarks pushed with -B/--bookmark in the
same place as already-shared bookmarks. It also avoids calling listkeys
on the remote repo twice (hence the change to test-hook.t).
Pierre-Yves David - Dec. 10, 2012, 10:49 a.m.
On Fri, Dec 07, 2012 at 01:43:16PM -0600, Kevin Bullock wrote:
> # HG changeset patch
> # User Kevin Bullock <kbullock at ringworld.org>
> # Date 1354905590 21600
> # Node ID 64d29f52afb979d79992cc9eaafc56690db372a3
> # Parent  106b515749cf338617d24c2fae3850987bf46734
> push: move handling of explicitly pushed bookmarks into localrepo.push()
> 
> This puts the handling of bookmarks pushed with -B/--bookmark in the
> same place as already-shared bookmarks. It also avoids calling listkeys
> on the remote repo twice (hence the change to test-hook.t).
> 
> diff --git a/hgext/largefiles/reposetup.py b/hgext/largefiles/reposetup.py
> --- a/hgext/largefiles/reposetup.py
> +++ b/hgext/largefiles/reposetup.py
> @@ -434,7 +434,7 @@ def reposetup(ui, repo):
>              finally:
>                  wlock.release()
>  
> -        def push(self, remote, force=False, revs=None, newbranch=False):
> +        def push(self, remote, force=False, revs=None, newbranch=False, marks=[]):
>              o = lfutil.findoutgoing(self, remote, force)
>              if o:
>                  toupload = set()
> @@ -465,7 +465,7 @@ def reposetup(ui, repo):
>                               if lfutil.isstandin(f) and f in ctx]))
>                  lfcommands.uploadlfiles(ui, self, remote, toupload)
>              return super(lfilesrepo, self).push(remote, force, revs,
> -                newbranch)
> +                newbranch, marks)
>  
>      repo.__class__ = lfilesrepo
>  
> diff --git a/mercurial/commands.py b/mercurial/commands.py
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -4766,31 +4766,9 @@ def push(ui, repo, dest=None, **opts):
>                  return False
>      finally:
>          del repo._subtoppath
> -    result = repo.push(other, opts.get('force'), revs=revs,
> -                       newbranch=opts.get('new_branch'))
> -
> -    result = not result
> -
> -    if opts.get('bookmark'):
> -        rb = other.listkeys('bookmarks')
> -        for b in opts['bookmark']:
> -            # explicit push overrides remote bookmark if any
> -            if b in repo._bookmarks:
> -                ui.status(_("exporting bookmark %s\n") % b)
> -                new = repo[b].hex()
> -            elif b in rb:
> -                ui.status(_("deleting remote bookmark %s\n") % b)
> -                new = '' # delete
> -            else:
> -                ui.warn(_('bookmark %s does not exist on the local '
> -                          'or remote repository!\n') % b)
> -                break
> -            old = rb.get(b, '')
> -            r = other.pushkey('bookmarks', b, old, new)
> -            if not r:
> -                ui.warn(_('updating bookmark %s failed!\n') % b)
> -
> -    return result
> +    return not repo.push(other, opts.get('force'), revs=revs,
> +                         newbranch=opts.get('new_branch'),
> +                         marks=opts.get('bookmark'))
>  
>  @command('recover', [])
>  def recover(ui, repo):
> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -1866,7 +1866,7 @@ class localrepository(object):
>          """
>          pass
>  
> -    def push(self, remote, force=False, revs=None, newbranch=False):
> +    def push(self, remote, force=False, revs=None, newbranch=False, marks=[]):
>          '''Push outgoing changesets (limited by revs) from the current
>          repository to remote. Return an integer:
>            - None means nothing to push
> @@ -2049,6 +2049,22 @@ class localrepository(object):
>                          else:
>                              self.ui.warn(_('updating bookmark %s'
>                                             ' failed!\n') % k)
> +        for b in marks:
> +            # explicit push overrides remote bookmark if any
> +            if b in unfi._bookmarks:
> +                unfi.ui.status(_("exporting bookmark %s\n") % b)
> +                new = unfi[b].hex()
> +            elif b in rb:
> +                unfi.ui.status(_("deleting remote bookmark %s\n") % b)
> +                new = '' # delete
> +            else:
> +                unfi.ui.warn(_('bookmark %s does not exist on the local '
> +                               'or remote repository!\n') % b)
> +                break
> +            old = rb.get(b, '')
> +            r = remote.pushkey('bookmarks', b, old, new)
> +            if not r:
> +                unfi.ui.warn(_('updating bookmark %s failed!\n') % b)

can't this be  factored with the sync of common bookmark?
(and break still want to be a continue)
Kevin Bullock - Dec. 10, 2012, 6:03 p.m.
On Dec 10, 2012, at 4:49 AM, Pierre-Yves David wrote:

> On Fri, Dec 07, 2012 at 01:43:16PM -0600, Kevin Bullock wrote:
>> +        for b in marks:
>> +            # explicit push overrides remote bookmark if any
>> +            if b in unfi._bookmarks:
>> +                unfi.ui.status(_("exporting bookmark %s\n") % b)
>> +                new = unfi[b].hex()
>> +            elif b in rb:
>> +                unfi.ui.status(_("deleting remote bookmark %s\n") % b)
>> +                new = '' # delete
>> +            else:
>> +                unfi.ui.warn(_('bookmark %s does not exist on the local '
>> +                               'or remote repository!\n') % b)
>> +                break
>> +            old = rb.get(b, '')
>> +            r = remote.pushkey('bookmarks', b, old, new)
>> +            if not r:
>> +                unfi.ui.warn(_('updating bookmark %s failed!\n') % b)
> 
> can't this be  factored with the sync of common bookmark?
> (and break still want to be a continue)

Yes. Again, just limiting my scope to the task at hand for now.

pacem in terris / ??? / ?????? / ????????? / ??
Kevin R. Bullock

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20121210/aeead26b/attachment.pgp>

Patch

diff --git a/hgext/largefiles/reposetup.py b/hgext/largefiles/reposetup.py
--- a/hgext/largefiles/reposetup.py
+++ b/hgext/largefiles/reposetup.py
@@ -434,7 +434,7 @@  def reposetup(ui, repo):
             finally:
                 wlock.release()
 
-        def push(self, remote, force=False, revs=None, newbranch=False):
+        def push(self, remote, force=False, revs=None, newbranch=False, marks=[]):
             o = lfutil.findoutgoing(self, remote, force)
             if o:
                 toupload = set()
@@ -465,7 +465,7 @@  def reposetup(ui, repo):
                              if lfutil.isstandin(f) and f in ctx]))
                 lfcommands.uploadlfiles(ui, self, remote, toupload)
             return super(lfilesrepo, self).push(remote, force, revs,
-                newbranch)
+                newbranch, marks)
 
     repo.__class__ = lfilesrepo
 
diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -4766,31 +4766,9 @@  def push(ui, repo, dest=None, **opts):
                 return False
     finally:
         del repo._subtoppath
-    result = repo.push(other, opts.get('force'), revs=revs,
-                       newbranch=opts.get('new_branch'))
-
-    result = not result
-
-    if opts.get('bookmark'):
-        rb = other.listkeys('bookmarks')
-        for b in opts['bookmark']:
-            # explicit push overrides remote bookmark if any
-            if b in repo._bookmarks:
-                ui.status(_("exporting bookmark %s\n") % b)
-                new = repo[b].hex()
-            elif b in rb:
-                ui.status(_("deleting remote bookmark %s\n") % b)
-                new = '' # delete
-            else:
-                ui.warn(_('bookmark %s does not exist on the local '
-                          'or remote repository!\n') % b)
-                break
-            old = rb.get(b, '')
-            r = other.pushkey('bookmarks', b, old, new)
-            if not r:
-                ui.warn(_('updating bookmark %s failed!\n') % b)
-
-    return result
+    return not repo.push(other, opts.get('force'), revs=revs,
+                         newbranch=opts.get('new_branch'),
+                         marks=opts.get('bookmark'))
 
 @command('recover', [])
 def recover(ui, repo):
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -1866,7 +1866,7 @@  class localrepository(object):
         """
         pass
 
-    def push(self, remote, force=False, revs=None, newbranch=False):
+    def push(self, remote, force=False, revs=None, newbranch=False, marks=[]):
         '''Push outgoing changesets (limited by revs) from the current
         repository to remote. Return an integer:
           - None means nothing to push
@@ -2049,6 +2049,22 @@  class localrepository(object):
                         else:
                             self.ui.warn(_('updating bookmark %s'
                                            ' failed!\n') % k)
+        for b in marks:
+            # explicit push overrides remote bookmark if any
+            if b in unfi._bookmarks:
+                unfi.ui.status(_("exporting bookmark %s\n") % b)
+                new = unfi[b].hex()
+            elif b in rb:
+                unfi.ui.status(_("deleting remote bookmark %s\n") % b)
+                new = '' # delete
+            else:
+                unfi.ui.warn(_('bookmark %s does not exist on the local '
+                               'or remote repository!\n') % b)
+                break
+            old = rb.get(b, '')
+            r = remote.pushkey('bookmarks', b, old, new)
+            if not r:
+                unfi.ui.warn(_('updating bookmark %s failed!\n') % b)
 
         return ret
 
diff --git a/tests/test-hook.t b/tests/test-hook.t
--- a/tests/test-hook.t
+++ b/tests/test-hook.t
@@ -214,7 +214,6 @@  test that prepushkey can prevent incomin
   no changes found
   listkeys hook: HG_NAMESPACE=phases HG_VALUES={'cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b': '1', 'publishing': 'True'}
   listkeys hook: HG_NAMESPACE=bookmarks HG_VALUES={'bar': '0000000000000000000000000000000000000000', 'foo': '0000000000000000000000000000000000000000'}
-  listkeys hook: HG_NAMESPACE=bookmarks HG_VALUES={'bar': '0000000000000000000000000000000000000000', 'foo': '0000000000000000000000000000000000000000'}
   exporting bookmark baz
   prepushkey.forbid hook: HG_KEY=baz HG_NAMESPACE=bookmarks HG_NEW=0000000000000000000000000000000000000000
   abort: prepushkey hook exited with status 1