Patchwork [STABLE] httppeer: close the temporary bundle file after two-way streaming it

login
register
mail settings
Submitter Matt Harbison
Date Oct. 26, 2014, 1:42 a.m.
Message ID <fc3b5934a1b9e511f0b4.1414287773@Envy>
Download mbox | patch
Permalink /patch/6466/
State Accepted
Headers show

Comments

Matt Harbison - Oct. 26, 2014, 1:42 a.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1414287289 14400
#      Sat Oct 25 21:34:49 2014 -0400
# Branch stable
# Node ID fc3b5934a1b9e511f0b4f16cdfac3d1283e12c61
# Parent  eb763217152ab2b472416bcc57722451c317f282
httppeer: close the temporary bundle file after two-way streaming it

This fixes several push tests in test-bundle2-exchange.t that 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]
Adrian Buehlmann - Oct. 26, 2014, 6:32 a.m.
On 2014-10-26 02:42, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1414287289 14400
> #      Sat Oct 25 21:34:49 2014 -0400
> # Branch stable
> # Node ID fc3b5934a1b9e511f0b4f16cdfac3d1283e12c61
> # Parent  eb763217152ab2b472416bcc57722451c317f282
> httppeer: close the temporary bundle file after two-way streaming it
> 
> This fixes several push tests in test-bundle2-exchange.t that 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]
> 
> diff --git a/mercurial/httppeer.py b/mercurial/httppeer.py
> --- a/mercurial/httppeer.py
> +++ b/mercurial/httppeer.py
> @@ -214,6 +214,7 @@
>  
>      def _calltwowaystream(self, cmd, fp, **args):
>          fh = None
> +        fp_ = None
>          filename = None
>          try:
>              # dump bundle to disk
> @@ -225,10 +226,12 @@
>                  d = fp.read(4096)
>              fh.close()
>              # start http push
> -            fp = httpconnection.httpsendfile(self.ui, filename, "rb")
> +            fp_ = httpconnection.httpsendfile(self.ui, filename, "rb")
>              headers = {'Content-Type': 'application/mercurial-0.1'}
> -            return self._callstream(cmd, data=fp, headers=headers, **args)
> +            return self._callstream(cmd, data=fp_, headers=headers, **args)
>          finally:
> +            if fp_ is not None:
> +                fp_.close()
>              if fh is not None:
>                  fh.close()
>                  os.unlink(filename)

Congrats on finding a better way than just eating the exception. I won't
be able to review (and test) this patch, but it looks much better than
your first version.

(Nitpick for future patches: this patch here could have been flagged as
V2, which helps reviewers to recognize that they can drop earlier
versions of the same patch. But no need to send this particular patch
again right now).

(/me goes back to lurking mode)
Pierre-Yves David - Oct. 26, 2014, 11:23 a.m.
On 10/26/2014 02:42 AM, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1414287289 14400
> #      Sat Oct 25 21:34:49 2014 -0400
> # Branch stable
> # Node ID fc3b5934a1b9e511f0b4f16cdfac3d1283e12c61
> # Parent  eb763217152ab2b472416bcc57722451c317f282
> httppeer: close the temporary bundle file after two-way streaming it

Patch looks good. pushed to the clowncopter. Thanks.

Patch

diff --git a/mercurial/httppeer.py b/mercurial/httppeer.py
--- a/mercurial/httppeer.py
+++ b/mercurial/httppeer.py
@@ -214,6 +214,7 @@ 
 
     def _calltwowaystream(self, cmd, fp, **args):
         fh = None
+        fp_ = None
         filename = None
         try:
             # dump bundle to disk
@@ -225,10 +226,12 @@ 
                 d = fp.read(4096)
             fh.close()
             # start http push
-            fp = httpconnection.httpsendfile(self.ui, filename, "rb")
+            fp_ = httpconnection.httpsendfile(self.ui, filename, "rb")
             headers = {'Content-Type': 'application/mercurial-0.1'}
-            return self._callstream(cmd, data=fp, headers=headers, **args)
+            return self._callstream(cmd, data=fp_, headers=headers, **args)
         finally:
+            if fp_ is not None:
+                fp_.close()
             if fh is not None:
                 fh.close()
                 os.unlink(filename)