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

Adrian Buehlmann adrian at cadifra.com
Sun Oct 26 01:32:09 CDT 2014


On 2014-10-26 02:42, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison at 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)


More information about the Mercurial-devel mailing list