pushvars: do not mangle repo state

Setting `repo._shellvars` works but is not a clean way to pass the pushvars
information from the push command to the exchange operation. Therefore
change it to actually pass `pushvars` as a push operation argument instead.

This makes third party extension like remotenames easier to support pushvars
cleanly. The key value parsing and verification code has been moved to a
lower level so it's harder to be bypassed and easier to be used in
remotenames which could replace `push` command entirely.

Differential Revision: https://phab.mercurial-scm.org/D423

mercurial/commands.py

@@ -4082,26 +4082,13 b' def push(ui, repo, dest=None, **opts):'
     finally:
         del repo._subtoppath
 
-    pushvars = opts.get('pushvars')
-    if pushvars:
-        shellvars = {}
-        for raw in pushvars:
-            if '=' not in raw:
-                msg = ("unable to parse variable '%s', should follow "
-                        "'KEY=VALUE' or 'KEY=' format")
-                raise error.Abort(msg % raw)
-            k, v = raw.split('=', 1)
-            shellvars[k] = v
-
-        repo._shellvars = shellvars
+    opargs = dict(opts.get('opargs', {})) # copy opargs since we may mutate it
+    opargs.setdefault('pushvars', []).extend(opts.get('pushvars', []))
 
     pushop = exchange.push(repo, other, opts.get('force'), revs=revs,
                            newbranch=opts.get('new_branch'),
                            bookmarks=opts.get('bookmark', ()),
-                           opargs=opts.get('opargs'))
-
-    if pushvars:
-        del repo._shellvars
+                           opargs=opargs)
 
     result = not pushop.cgresult
 

mercurial/exchange.py

@@ -294,7 +294,7 b' class pushoperation(object):'
     """
 
     def __init__(self, repo, remote, force=False, revs=None, newbranch=False,
-                 bookmarks=()):
+                 bookmarks=(), pushvars=None):
         # repo we push from
         self.repo = repo
         self.ui = repo.ui
@@ -352,6 +352,8 b' class pushoperation(object):'
         # map { pushkey partid -> callback handling failure}
         # used to handle exception from mandatory pushkey part failure
         self.pkfailcb = {}
+        # an iterable of pushvars or None
+        self.pushvars = pushvars
 
     @util.propertycache
     def futureheads(self):
@@ -876,10 +878,20 b' def _pushb2bookmarks(pushop, bundler):'
 @b2partsgenerator('pushvars', idx=0)
 def _getbundlesendvars(pushop, bundler):
     '''send shellvars via bundle2'''
-    if getattr(pushop.repo, '_shellvars', ()):
+    pushvars = pushop.pushvars
+    if pushvars:
+        shellvars = {}
+        for raw in pushvars:
+            if '=' not in raw:
+                msg = ("unable to parse variable '%s', should follow "
+                        "'KEY=VALUE' or 'KEY=' format")
+                raise error.Abort(msg % raw)
+            k, v = raw.split('=', 1)
+            shellvars[k] = v
+
         part = bundler.newpart('pushvars')
 
-        for key, value in pushop.repo._shellvars.iteritems():
+        for key, value in shellvars.iteritems():
             part.addparam(key, value, mandatory=False)
 
 def _pushbundle2(pushop):

tests/test-pushvars.t

@@ -66,5 +66,6 b' Test pushing bad vars'
   $ hg commit -Aqm b
   $ hg push --pushvars "DEBUG"
   pushing to $TESTTMP/repo (glob)
+  searching for changes
   abort: unable to parse variable 'DEBUG', should follow 'KEY=VALUE' or 'KEY=' format
   [255]