Patchwork [stable] dispatch: restore old cwd after dispatch

login
register
mail settings
Submitter Gregory Szorc
Date April 23, 2014, 3:35 a.m.
Message ID <cd65b532c59317be2f4e.1398224116@vm-ubuntu-main.gateway.sonic.net>
Download mbox | patch
Permalink /patch/4432/
State Changes Requested
Headers show

Comments

Gregory Szorc - April 23, 2014, 3:35 a.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1398223347 25200
#      Tue Apr 22 20:22:27 2014 -0700
# Node ID cd65b532c59317be2f4efcceded822b9f63bd858
# Parent  86596bc8f89201f9abd294ff6e8f858c4c4788ac
dispatch: restore old cwd after dispatch
Gregory Szorc - April 23, 2014, 4:01 a.m.
I thought this was innocent enough. It breaks test-rename.t:440 and
test-rename.t:490. The test looks somewhat suspicious. But I reckon we
need to catch the OSError.errno == errno.ENOENT in the finally.

On 4/22/2014 8:35 PM, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1398223347 25200
> #      Tue Apr 22 20:22:27 2014 -0700
> # Node ID cd65b532c59317be2f4efcceded822b9f63bd858
> # Parent  86596bc8f89201f9abd294ff6e8f858c4c4788ac
> dispatch: restore old cwd after dispatch
> 
> diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
> --- a/mercurial/dispatch.py
> +++ b/mercurial/dispatch.py
> @@ -60,20 +60,24 @@ def dispatch(req):
>                               (inst.args[1], inst.args[0]))
>          else:
>              ferr.write(_("hg: parse error: %s\n") % inst.args[0])
>          return -1
>  
>      msg = ' '.join(' ' in a and repr(a) or a for a in req.args)
>      starttime = time.time()
>      ret = None
> +    # _dispatch() may chdir(). For embeddable scenarios, revert so multiple
> +    # calls to dispatch() work.
> +    oldcwd = os.getcwd()
>      try:
>          ret = _runcatch(req)
>          return ret
>      finally:
> +        os.chdir(oldcwd)
>          duration = time.time() - starttime
>          req.ui.log("commandfinish", "%s exited %s after %0.2f seconds\n",
>                     msg, ret or 0, duration)
>  
>  def _runcatch(req):
>      def catchterm(*args):
>          raise error.SignalInterrupt
>  
>
Pierre-Yves David - April 23, 2014, 4:48 a.m.
On 04/22/2014 08:35 PM, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1398223347 25200
> #      Tue Apr 22 20:22:27 2014 -0700
> # Node ID cd65b532c59317be2f4efcceded822b9f63bd858
> # Parent  86596bc8f89201f9abd294ff6e8f858c4c4788ac
> dispatch: restore old cwd after dispatch
>
> diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
> --- a/mercurial/dispatch.py
> +++ b/mercurial/dispatch.py
> @@ -60,20 +60,24 @@ def dispatch(req):
>                                (inst.args[1], inst.args[0]))
>           else:
>               ferr.write(_("hg: parse error: %s\n") % inst.args[0])
>           return -1
>
>       msg = ' '.join(' ' in a and repr(a) or a for a in req.args)
>       starttime = time.time()
>       ret = None
> +    # _dispatch() may chdir(). For embeddable scenarios, revert so multiple
> +    # calls to dispatch() work.
> +    oldcwd = os.getcwd()
>       try:
>           ret = _runcatch(req)
>           return ret
>       finally:
> +        os.chdir(oldcwd)

If your operation deleted the cwd (update to old version, rebase with 
rename, etc) you can get in trouble.

We have a related test for rebase if you want to look at it. (the fix 
and the ends result are both kindof ugly)

Patch

diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
--- a/mercurial/dispatch.py
+++ b/mercurial/dispatch.py
@@ -60,20 +60,24 @@  def dispatch(req):
                              (inst.args[1], inst.args[0]))
         else:
             ferr.write(_("hg: parse error: %s\n") % inst.args[0])
         return -1
 
     msg = ' '.join(' ' in a and repr(a) or a for a in req.args)
     starttime = time.time()
     ret = None
+    # _dispatch() may chdir(). For embeddable scenarios, revert so multiple
+    # calls to dispatch() work.
+    oldcwd = os.getcwd()
     try:
         ret = _runcatch(req)
         return ret
     finally:
+        os.chdir(oldcwd)
         duration = time.time() - starttime
         req.ui.log("commandfinish", "%s exited %s after %0.2f seconds\n",
                    msg, ret or 0, duration)
 
 def _runcatch(req):
     def catchterm(*args):
         raise error.SignalInterrupt