Patchwork [STABLE] httppeer: don't abort a push if the temporary bundle file cannot be deleted

login
register
mail settings
Submitter Matt Harbison
Date Oct. 25, 2014, 3:31 a.m.
Message ID <aa19b1e8464def0517d1.1414207905@Envy>
Download mbox | patch
Permalink /patch/6465/
State Superseded
Headers show

Comments

Matt Harbison - Oct. 25, 2014, 3:31 a.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1414201933 14400
#      Fri Oct 24 21:52:13 2014 -0400
# Branch stable
# Node ID aa19b1e8464def0517d171467bd772103abb57a2
# Parent  eb763217152ab2b472416bcc57722451c317f282
httppeer: don't abort a push if the temporary bundle file cannot be deleted

Several push tests in test-bundle2-exchange.t were failing on Windows with
messages like the following:

   $ hg -R main push http://localhost:$HGPORT2/ -r 32af7686d403 \
        --bookmark book_32af
   pushing to http://localhost:$HGPORT2/
   searching for changes
   remote: adding changesets
   remote: adding manifests
   remote: adding file changes
   remote: added 1 changesets with 1 changes to 1 files
   remote: 1 new obsolescence markers
   updating bookmark book_32af
   abort: The process cannot access the file because it is being used by another
            process: 'C:\path\to\tmp\bundle.hg'
   [255]

There's nothing obviously wrong with how the file is being handled, and I don't
like eating the error.  But this idiom is also used in util.mktempcopy(), so it
doesn't seem unreasonable here.  Prior to 978cce51cc5f three weeks ago, this
test was skipped on Windows.
Adrian Buehlmann - Oct. 25, 2014, 6:40 a.m.
On 2014-10-25 05:31, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1414201933 14400
> #      Fri Oct 24 21:52:13 2014 -0400
> # Branch stable
> # Node ID aa19b1e8464def0517d171467bd772103abb57a2
> # Parent  eb763217152ab2b472416bcc57722451c317f282
> httppeer: don't abort a push if the temporary bundle file cannot be deleted
> 
> Several push tests in test-bundle2-exchange.t were failing on Windows with
> messages like the following:
> 
>    $ hg -R main push http://localhost:$HGPORT2/ -r 32af7686d403 \
>         --bookmark book_32af
>    pushing to http://localhost:$HGPORT2/
>    searching for changes
>    remote: adding changesets
>    remote: adding manifests
>    remote: adding file changes
>    remote: added 1 changesets with 1 changes to 1 files
>    remote: 1 new obsolescence markers
>    updating bookmark book_32af
>    abort: The process cannot access the file because it is being used by another
>             process: 'C:\path\to\tmp\bundle.hg'
>    [255]
> 
> There's nothing obviously wrong with how the file is being handled, and I don't
> like eating the error.  But this idiom is also used in util.mktempcopy(), so it
> doesn't seem unreasonable here.  Prior to 978cce51cc5f three weeks ago, this
> test was skipped on Windows.
> 
> diff --git a/mercurial/httppeer.py b/mercurial/httppeer.py
> --- a/mercurial/httppeer.py
> +++ b/mercurial/httppeer.py
> @@ -231,7 +231,10 @@
>          finally:
>              if fh is not None:
>                  fh.close()
> -                os.unlink(filename)
> +                try:
> +                    os.unlink(filename)
> +                except OSError:
> +                    pass
>  
>      def _callcompressable(self, cmd, **args):
>          stream =  self._callstream(cmd, **args)

I'm not using recent Mercurial versions currently (stuck at an old one,
which works for me), so I don't have hands-on current knowledge, but
based on my previously gained knowledge when I was still hacking on
Mercurial using M$ Windows, I would say you should probably rather solve
the real issue, which seems to be that there is still another process
holding onto the very same file. I would try to find out why this is the
case. (The testsuite on Windows has often the problem that it fails to
kill "hg serve" processes, which might be an example reason for such an
abort).

So, on first sight, this patch looks like a non-fix to me. It appears to
just hide the real problem (which I currently can't help you to find).

Patch

diff --git a/mercurial/httppeer.py b/mercurial/httppeer.py
--- a/mercurial/httppeer.py
+++ b/mercurial/httppeer.py
@@ -231,7 +231,10 @@ 
         finally:
             if fh is not None:
                 fh.close()
-                os.unlink(filename)
+                try:
+                    os.unlink(filename)
+                except OSError:
+                    pass
 
     def _callcompressable(self, cmd, **args):
         stream =  self._callstream(cmd, **args)