[PATCH] git-p4: close temporary file before removing

[PATCH] git-p4: close temporary file before removing

[Philip McGraw]

python os.remove() throws exceptions on Windows platform when attempting
to remove file while it is still open.  Need to grab filename while file open,
close file handle, then remove by name.  Apparently other platforms are more
permissive of removing files while busy.
reference: https://docs.python.org/3/library/os.html#os.remove
---
 git-p4.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/git-p4.py b/git-p4.py
index c71a6832e2..6b9d2a8317 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1161,12 +1161,14 @@ def exceedsLargeFileThreshold(self, relPath, contents):
                 return False
             contentTempFile = self.generateTempFile(contents)
             compressedContentFile = tempfile.NamedTemporaryFile(prefix='git-p4-large-file', delete=False)
+            compressedContentFileName = compressedContentFile.name
             zf = zipfile.ZipFile(compressedContentFile.name, mode='w')
             zf.write(contentTempFile, compress_type=zipfile.ZIP_DEFLATED)
             zf.close()
             compressedContentsSize = zf.infolist()[0].compress_size
             os.remove(contentTempFile)
-            os.remove(compressedContentFile.name)
+            compressedContentFile.close()
+            os.remove(compressedContentFileName)
             if compressedContentsSize > gitConfigInt('git-p4.largeFileCompressedThreshold'):
                 return True
         return False
--
2.21.0.windows.1

Re: [PATCH] git-p4: close temporary file before removing

[Andrey]

30.07.2019, 13:37, "Philip McGraw" <philip.mcgraw@bentley.com>:
> python os.remove() throws exceptions on Windows platform when attempting
> to remove file while it is still open. Need to grab filename while file open,
> close file handle, then remove by name. Apparently other platforms are more
> permissive of removing files while busy.
> reference: https://docs.python.org/3/library/os.html#os.remove
> ---
>  git-p4.py | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/git-p4.py b/git-p4.py
> index c71a6832e2..6b9d2a8317 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -1161,12 +1161,14 @@ def exceedsLargeFileThreshold(self, relPath, contents):
>                  return False
>              contentTempFile = self.generateTempFile(contents)
>              compressedContentFile = tempfile.NamedTemporaryFile(prefix='git-p4-large-file', delete=False)
> + compressedContentFileName = compressedContentFile.name
>              zf = zipfile.ZipFile(compressedContentFile.name, mode='w')
>              zf.write(contentTempFile, compress_type=zipfile.ZIP_DEFLATED)
>              zf.close()
>              compressedContentsSize = zf.infolist()[0].compress_size
>              os.remove(contentTempFile)
> - os.remove(compressedContentFile.name)
> + compressedContentFile.close()
> + os.remove(compressedContentFileName)

I'm not sure why NamedTemporaryFile() is called with delete=False above,
but it appears to me that it can have delete=True instead,
so that there is no need to call os.remove() explicitly
and thus worry about remove vs close ordering at all.

>              if compressedContentsSize > gitConfigInt('git-p4.largeFileCompressedThreshold'):
>                  return True
>          return False
> --
> 2.21.0.windows.1

Thank you,
Andrey.

RE: [PATCH] git-p4: close temporary file before removing

[Philip McGraw]

> 30.07.2019, 13:37, "Philip McGraw" <philip.mcgraw@bentley.com>:
> > python os.remove() throws exceptions on Windows platform when attempting
> > to remove file while it is still open. Need to grab filename while file open,
> > close file handle, then remove by name. Apparently other platforms are more
> > permissive of removing files while busy.
> > reference: https://docs.python.org/3/library/os.html#os.remove
> > ---
> >  git-p4.py | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/git-p4.py b/git-p4.py
> > index c71a6832e2..6b9d2a8317 100755
> > --- a/git-p4.py
> > +++ b/git-p4.py
> > @@ -1161,12 +1161,14 @@ def exceedsLargeFileThreshold(self, relPath, contents):
> >                  return False
> >              contentTempFile = self.generateTempFile(contents)
> >              compressedContentFile = tempfile.NamedTemporaryFile(prefix='git-p4-large-file', delete=False)
> > + compressedContentFileName = compressedContentFile.name
> >              zf = zipfile.ZipFile(compressedContentFile.name, mode='w')
> >              zf.write(contentTempFile, compress_type=zipfile.ZIP_DEFLATED)
> >              zf.close()
> >              compressedContentsSize = zf.infolist()[0].compress_size
> >              os.remove(contentTempFile)
> > - os.remove(compressedContentFile.name)
> > + compressedContentFile.close()
> > + os.remove(compressedContentFileName)
> 
> I'm not sure why NamedTemporaryFile() is called with delete=False above,
> but it appears to me that it can have delete=True instead,
> so that there is no need to call os.remove() explicitly
> and thus worry about remove vs close ordering at all.
> 
> >              if compressedContentsSize > gitConfigInt('git-p4.largeFileCompressedThreshold'):
> >                  return True
> >          return False
> > --
> > 2.21.0.windows.1
> 
> Thank you,
> Andrey.

Thanks Andrey; simpler is certainly better!  I will test and re-submit v2 of patch with that approach.

Re: [PATCH] git-p4: close temporary file before removing

[Andrey]

31.07.2019, 09:53, "Philip McGraw" <philip.mcgraw@bentley.com>:
>>  30.07.2019, 13:37, "Philip McGraw" <philip.mcgraw@bentley.com>:
>>  > python os.remove() throws exceptions on Windows platform when attempting
>>  > to remove file while it is still open. Need to grab filename while file open,
>>  > close file handle, then remove by name. Apparently other platforms are more
>>  > permissive of removing files while busy.
>>  > reference: https://docs.python.org/3/library/os.html#os.remove
>>  > ---
>>  >  git-p4.py | 4 +++-
>>  >  1 file changed, 3 insertions(+), 1 deletion(-)
>>  >
>>  > diff --git a/git-p4.py b/git-p4.py
>>  > index c71a6832e2..6b9d2a8317 100755
>>  > --- a/git-p4.py
>>  > +++ b/git-p4.py
>>  > @@ -1161,12 +1161,14 @@ def exceedsLargeFileThreshold(self, relPath, contents):
>>  >                  return False
>>  >              contentTempFile = self.generateTempFile(contents)
>>  >              compressedContentFile = tempfile.NamedTemporaryFile(prefix='git-p4-large-file', delete=False)
>>  > + compressedContentFileName = compressedContentFile.name
>>  >              zf = zipfile.ZipFile(compressedContentFile.name, mode='w')
>>  >              zf.write(contentTempFile, compress_type=zipfile.ZIP_DEFLATED)
>>  >              zf.close()
>>  >              compressedContentsSize = zf.infolist()[0].compress_size
>>  >              os.remove(contentTempFile)
>>  > - os.remove(compressedContentFile.name)
>>  > + compressedContentFile.close()
>>  > + os.remove(compressedContentFileName)
>>
>>  I'm not sure why NamedTemporaryFile() is called with delete=False above,
>>  but it appears to me that it can have delete=True instead,
>>  so that there is no need to call os.remove() explicitly
>>  and thus worry about remove vs close ordering at all.
>>
>>  >              if compressedContentsSize > gitConfigInt('git-p4.largeFileCompressedThreshold'):
>>  >                  return True
>>  >          return False
>>  > --
>>  > 2.21.0.windows.1
>>
>>  Thank you,
>>  Andrey.
>
> Thanks Andrey; simpler is certainly better! I will test and re-submit v2 of patch with that approach.

Thank you, that would be great!

-- 
Andrey.

RE: [PATCH] git-p4: close temporary file before removing

[Philip McGraw]

2019.07.31 10:09 Andrey <ahippo@yandex.ru> 
>31.07.2019, 09:53, "Philip McGraw" <philip.mcgraw@bentley.com>:
>>>  30.07.2019, 13:37, "Philip McGraw" <philip.mcgraw@bentley.com>:
>>>  > python os.remove() throws exceptions on Windows platform when attempting
>>>  > to remove file while it is still open. Need to grab filename while file open,
>>>  > close file handle, then remove by name. Apparently other platforms are more
>>>  > permissive of removing files while busy.
>>>  > reference: 
>>>  > ---
>>>  >  git-p4.py | 4 +++-
>>>  >  1 file changed, 3 insertions(+), 1 deletion(-)
>>>  >
>>>  > diff --git a/git-p4.py b/git-p4.py
>>>  > index c71a6832e2..6b9d2a8317 100755
>>>  > --- a/git-p4.py
>>>  > +++ b/git-p4.py
>>>  > @@ -1161,12 +1161,14 @@ def exceedsLargeFileThreshold(self, relPath, contents):
>>>  >                  return False
>>>  >              contentTempFile = self.generateTempFile(contents)
>>>  >              compressedContentFile = tempfile.NamedTemporaryFile(prefix='git-p4-large-file', delete=False)
>>>  > + compressedContentFileName = compressedContentFile.name
>>>  >              zf = zipfile.ZipFile(compressedContentFile.name, mode='w')
>>>  >              zf.write(contentTempFile, compress_type=zipfile.ZIP_DEFLATED)
>>>  >              zf.close()
>>>  >              compressedContentsSize = zf.infolist()[0].compress_size
>>>  >              os.remove(contentTempFile)
>>>  > - os.remove(compressedContentFile.name)
>>>  > + compressedContentFile.close()
>>>  > + os.remove(compressedContentFileName)
>>>
>>>  I'm not sure why NamedTemporaryFile() is called with delete=False above,
>>>  but it appears to me that it can have delete=True instead,
>>>  so that there is no need to call os.remove() explicitly
>>>  and thus worry about remove vs close ordering at all.
>>>
>>>  >              if compressedContentsSize > gitConfigInt('git-p4.largeFileCompressedThreshold'):
>>>  >                  return True
>>>  >          return False
>>>  > --
>>>  > 2.21.0.windows.1
>>>
>>>  Thank you,
>>>  Andrey.
>>
>> Thanks Andrey; simpler is certainly better! I will test and re-submit v2 of patch with that approach.
>
>Thank you, that would be great!
>
>-- 
>Andrey.

Unfortunately it wasn't as simple it seemed: upon testing with only changing delete=True, 
found that the problem was not solved.  Upon further debugging, recoded/refactored slightly adding 
allocateTempFileName() locally scoped function to try to clarify how the NamedTemporaryFile()
was actually being used.

We can't depend on the delete-on-close because the NamedTemporaryFile() is merely allocating 
a temporary name for real use by the zipfile open-for-write which fails (on Windows) if file
was not explicitly closed first.  

Hopefully the new patch (https://github.com/gitgitgadget/git/pull/301) will make this more clear.

Open to other suggestions if still not clear.

Thanks again,
Philip

Re: [PATCH] git-p4: close temporary file before removing

[Andrey]

31.07.2019, 17:52, "Philip McGraw" <philip.mcgraw@bentley.com>:
> 2019.07.31 10:09 Andrey <ahippo@yandex.ru>
>> 31.07.2019, 09:53, "Philip McGraw" <philip.mcgraw@bentley.com>:
>>>>   30.07.2019, 13:37, "Philip McGraw" <philip.mcgraw@bentley.com>:
>>>>   > python os.remove() throws exceptions on Windows platform when attempting
>>>>   > to remove file while it is still open. Need to grab filename while file open,
>>>>   > close file handle, then remove by name. Apparently other platforms are more
>>>>   > permissive of removing files while busy.
>>>>   > reference:
>>>>   > ---
>>>>   >  git-p4.py | 4 +++-
>>>>   >  1 file changed, 3 insertions(+), 1 deletion(-)
>>>>   >
>>>>   > diff --git a/git-p4.py b/git-p4.py
>>>>   > index c71a6832e2..6b9d2a8317 100755
>>>>   > --- a/git-p4.py
>>>>   > +++ b/git-p4.py
>>>>   > @@ -1161,12 +1161,14 @@ def exceedsLargeFileThreshold(self, relPath, contents):
>>>>   >                  return False
>>>>   >              contentTempFile = self.generateTempFile(contents)
>>>>   >              compressedContentFile = tempfile.NamedTemporaryFile(prefix='git-p4-large-file', delete=False)
>>>>   > + compressedContentFileName = compressedContentFile.name
>>>>   >              zf = zipfile.ZipFile(compressedContentFile.name, mode='w')
>>>>   >              zf.write(contentTempFile, compress_type=zipfile.ZIP_DEFLATED)
>>>>   >              zf.close()
>>>>   >              compressedContentsSize = zf.infolist()[0].compress_size
>>>>   >              os.remove(contentTempFile)
>>>>   > - os.remove(compressedContentFile.name)
>>>>   > + compressedContentFile.close()
>>>>   > + os.remove(compressedContentFileName)
>>>>
>>>>   I'm not sure why NamedTemporaryFile() is called with delete=False above,
>>>>   but it appears to me that it can have delete=True instead,
>>>>   so that there is no need to call os.remove() explicitly
>>>>   and thus worry about remove vs close ordering at all.
>>>>
>>>>   >              if compressedContentsSize > gitConfigInt('git-p4.largeFileCompressedThreshold'):
>>>>   >                  return True
>>>>   >          return False
>>>>   > --
>>>>   > 2.21.0.windows.1
>>>>
>>>>   Thank you,
>>>>   Andrey.
>>>
>>>  Thanks Andrey; simpler is certainly better! I will test and re-submit v2 of patch with that approach.
>>
>> Thank you, that would be great!
>>
>> --
>> Andrey.
>
> Unfortunately it wasn't as simple it seemed: upon testing with only changing delete=True,
> found that the problem was not solved. Upon further debugging, recoded/refactored slightly adding
> allocateTempFileName() locally scoped function to try to clarify how the NamedTemporaryFile()
> was actually being used.
>
> We can't depend on the delete-on-close because the NamedTemporaryFile() is merely allocating
> a temporary name for real use by the zipfile open-for-write which fails (on Windows) if file
> was not explicitly closed first.

Oh, sorry for misguiding you!
I didn't think of this aspect.

> Hopefully the new patch (https://github.com/gitgitgadget/git/pull/301) will make this more clear.

The new changeset looks good to me.
(I'll post a reply in that thread too)

> Open to other suggestions if still not clear.

Just as a thought, ZipFile() can take a file-like object instead of a file name,
so can be passed the NamedTemporaryFile() object directly instead of its file name.
This should hopefully avoid double-open issue on Windows.

However, I'm good with your allocateTempFileName() changeset,
so it's up to you to give it a try or not.

> Thanks again,
> Philip

Thank you,
Andrey.

RE: [PATCH] git-p4: close temporary file before removing

[Philip McGraw]

> From: Andrey <ahippo@yandex.ru>
> Sent: Wednesday, 31 July, 2019 21:35
> To: Philip McGraw <Philip.McGraw@bentley.com>
> Cc: git@vger.kernel.org; luke@diamand.org
> Subject: Re: [PATCH] git-p4: close temporary file before removing
> 
> 31.07.2019, 17:52, "Philip McGraw" <philip.mcgraw@bentley.com>:
> > 2019.07.31 10:09 Andrey <ahippo@yandex.ru>
> >> 31.07.2019, 09:53, "Philip McGraw" <philip.mcgraw@bentley.com>:
> >>>>   30.07.2019, 13:37, "Philip McGraw" <philip.mcgraw@bentley.com>:
> >>>>   > python os.remove() throws exceptions on Windows platform when
> attempting
> >>>>   > to remove file while it is still open. Need to grab filename
> while file open,
> >>>>   > close file handle, then remove by name. Apparently other
> platforms are more
> >>>>   > permissive of removing files while busy.
> >>>>   > reference:
> >>>>   > ---
> >>>>   >  git-p4.py | 4 +++-
> >>>>   >  1 file changed, 3 insertions(+), 1 deletion(-)
> >>>>   >
> >>>>   > diff --git a/git-p4.py b/git-p4.py
> >>>>   > index c71a6832e2..6b9d2a8317 100755
> >>>>   > --- a/git-p4.py
> >>>>   > +++ b/git-p4.py
> >>>>   > @@ -1161,12 +1161,14 @@ def exceedsLargeFileThreshold(self,
> relPath, contents):
> >>>>   >                  return False
> >>>>   >              contentTempFile = self.generateTempFile(contents)
> >>>>   >              compressedContentFile =
> tempfile.NamedTemporaryFile(prefix='git-p4-large-file', delete=False)
> >>>>   > + compressedContentFileName = compressedContentFile.name
> >>>>   >              zf = zipfile.ZipFile(compressedContentFile.name,
> mode='w')
> >>>>   >              zf.write(contentTempFile,
> compress_type=zipfile.ZIP_DEFLATED)
> >>>>   >              zf.close()
> >>>>   >              compressedContentsSize =
> zf.infolist()[0].compress_size
> >>>>   >              os.remove(contentTempFile)
> >>>>   > - os.remove(compressedContentFile.name)
> >>>>   > + compressedContentFile.close()
> >>>>   > + os.remove(compressedContentFileName)
> >>>>
> >>>>   I'm not sure why NamedTemporaryFile() is called with delete=False
> above,
> >>>>   but it appears to me that it can have delete=True instead,
> >>>>   so that there is no need to call os.remove() explicitly
> >>>>   and thus worry about remove vs close ordering at all.
> >>>>
> >>>>   >              if compressedContentsSize > gitConfigInt('git-
> p4.largeFileCompressedThreshold'):
> >>>>   >                  return True
> >>>>   >          return False
> >>>>   > --
> >>>>   > 2.21.0.windows.1
> >>>>
> >>>>   Thank you,
> >>>>   Andrey.
> >>>
> >>>  Thanks Andrey; simpler is certainly better! I will test and re-submit
> v2 of patch with that approach.
> >>
> >> Thank you, that would be great!
> >>
> >> --
> >> Andrey.
> >
> > Unfortunately it wasn't as simple it seemed: upon testing with only
> changing delete=True,
> > found that the problem was not solved. Upon further debugging,
> recoded/refactored slightly adding
> > allocateTempFileName() locally scoped function to try to clarify how the
> NamedTemporaryFile()
> > was actually being used.
> >
> > We can't depend on the delete-on-close because the NamedTemporaryFile()
> is merely allocating
> > a temporary name for real use by the zipfile open-for-write which fails
> (on Windows) if file
> > was not explicitly closed first.
> 
> Oh, sorry for misguiding you!
> I didn't think of this aspect.

No worries! I probably just misunderstood the implementation of your idea.
> 
> > Hopefully the new patch
> (https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__github.com_gitgitgadget_git_pull_301&d=DwIDaQ&c=hmGTLOph1qd_VnCqj81HzE
> WkDaxmYdIWRBdoFggzhj8&r=b0ikFMJGw7xxhF3yjexiWJpLuNxlAh1SvUDuUJ-
> pHmE&m=1jGOrV_I1Mg5ajkJ7yFEcNlyLnD6zYNXqXB9Z5SIPyE&s=TdT4WHyQCk5WZty_CvajH
> XgrZJbmIOl1gbMcngmjmAs&e= ) will make this more clear.
> 
> The new changeset looks good to me.
> (I'll post a reply in that thread too)
> 
> > Open to other suggestions if still not clear.
> 
> Just as a thought, ZipFile() can take a file-like object instead of a file
> name,
> so can be passed the NamedTemporaryFile() object directly instead of its
> file name.
> This should hopefully avoid double-open issue on Windows.

Another excellent idea that minimizes changes.  I am testing this approach
now and will submit v3 of the patch soon.
> 
> However, I'm good with your allocateTempFileName() changeset,
> so it's up to you to give it a try or not.
> 
> > Thanks again,
> > Philip
> 
> Thank you,
> Andrey.

Thanks,
Philip

Re: [PATCH] git-p4: close temporary file before removing

[Andrey]

01.08.2019, 11:30, "Philip McGraw" <philip.mcgraw@bentley.com>:
>>  From: Andrey <ahippo@yandex.ru>
>>  Sent: Wednesday, 31 July, 2019 21:35
>>  To: Philip McGraw <Philip.McGraw@bentley.com>
>>  Cc: git@vger.kernel.org; luke@diamand.org
>>  Subject: Re: [PATCH] git-p4: close temporary file before removing
>>
>>  31.07.2019, 17:52, "Philip McGraw" <philip.mcgraw@bentley.com>:
>>  > 2019.07.31 10:09 Andrey <ahippo@yandex.ru>
>>  >> 31.07.2019, 09:53, "Philip McGraw" <philip.mcgraw@bentley.com>:
>>  >>>>   30.07.2019, 13:37, "Philip McGraw" <philip.mcgraw@bentley.com>:
>>  >>>>   > python os.remove() throws exceptions on Windows platform when
>>  attempting
>>  >>>>   > to remove file while it is still open. Need to grab filename
>>  while file open,
>>  >>>>   > close file handle, then remove by name. Apparently other
>>  platforms are more
>>  >>>>   > permissive of removing files while busy.
>>  >>>>   > reference:
>>  >>>>   > ---
>>  >>>>   >  git-p4.py | 4 +++-
>>  >>>>   >  1 file changed, 3 insertions(+), 1 deletion(-)
>>  >>>>   >
>>  >>>>   > diff --git a/git-p4.py b/git-p4.py
>>  >>>>   > index c71a6832e2..6b9d2a8317 100755
>>  >>>>   > --- a/git-p4.py
>>  >>>>   > +++ b/git-p4.py
>>  >>>>   > @@ -1161,12 +1161,14 @@ def exceedsLargeFileThreshold(self,
>>  relPath, contents):
>>  >>>>   >                  return False
>>  >>>>   >              contentTempFile = self.generateTempFile(contents)
>>  >>>>   >              compressedContentFile =
>>  tempfile.NamedTemporaryFile(prefix='git-p4-large-file', delete=False)
>>  >>>>   > + compressedContentFileName = compressedContentFile.name
>>  >>>>   >              zf = zipfile.ZipFile(compressedContentFile.name,
>>  mode='w')
>>  >>>>   >              zf.write(contentTempFile,
>>  compress_type=zipfile.ZIP_DEFLATED)
>>  >>>>   >              zf.close()
>>  >>>>   >              compressedContentsSize =
>>  zf.infolist()[0].compress_size
>>  >>>>   >              os.remove(contentTempFile)
>>  >>>>   > - os.remove(compressedContentFile.name)
>>  >>>>   > + compressedContentFile.close()
>>  >>>>   > + os.remove(compressedContentFileName)
>>  >>>>
>>  >>>>   I'm not sure why NamedTemporaryFile() is called with delete=False
>>  above,
>>  >>>>   but it appears to me that it can have delete=True instead,
>>  >>>>   so that there is no need to call os.remove() explicitly
>>  >>>>   and thus worry about remove vs close ordering at all.
>>  >>>>
>>  >>>>   >              if compressedContentsSize > gitConfigInt('git-
>>  p4.largeFileCompressedThreshold'):
>>  >>>>   >                  return True
>>  >>>>   >          return False
>>  >>>>   > --
>>  >>>>   > 2.21.0.windows.1
>>  >>>>
>>  >>>>   Thank you,
>>  >>>>   Andrey.
>>  >>>
>>  >>>  Thanks Andrey; simpler is certainly better! I will test and re-submit
>>  v2 of patch with that approach.
>>  >>
>>  >> Thank you, that would be great!
>>  >>
>>  >> --
>>  >> Andrey.
>>  >
>>  > Unfortunately it wasn't as simple it seemed: upon testing with only
>>  changing delete=True,
>>  > found that the problem was not solved. Upon further debugging,
>>  recoded/refactored slightly adding
>>  > allocateTempFileName() locally scoped function to try to clarify how the
>>  NamedTemporaryFile()
>>  > was actually being used.
>>  >
>>  > We can't depend on the delete-on-close because the NamedTemporaryFile()
>>  is merely allocating
>>  > a temporary name for real use by the zipfile open-for-write which fails
>>  (on Windows) if file
>>  > was not explicitly closed first.
>>
>>  Oh, sorry for misguiding you!
>>  I didn't think of this aspect.
>
> No worries! I probably just misunderstood the implementation of your idea.

No, you understood what I was saying correctly.
It's just that I didn't think of opening the file twice.
(or rather that it would be a problem)

>>  > Hopefully the new patch
>>  (https://urldefense.proofpoint.com/v2/url?u=https-
>>  3A__github.com_gitgitgadget_git_pull_301&d=DwIDaQ&c=hmGTLOph1qd_VnCqj81HzE
>>  WkDaxmYdIWRBdoFggzhj8&r=b0ikFMJGw7xxhF3yjexiWJpLuNxlAh1SvUDuUJ-
>>  pHmE&m=1jGOrV_I1Mg5ajkJ7yFEcNlyLnD6zYNXqXB9Z5SIPyE&s=TdT4WHyQCk5WZty_CvajH
>>  XgrZJbmIOl1gbMcngmjmAs&e= ) will make this more clear.
>>
>>  The new changeset looks good to me.
>>  (I'll post a reply in that thread too)
>>
>>  > Open to other suggestions if still not clear.
>>
>>  Just as a thought, ZipFile() can take a file-like object instead of a file
>>  name,
>>  so can be passed the NamedTemporaryFile() object directly instead of its
>>  file name.
>>  This should hopefully avoid double-open issue on Windows.
>
> Another excellent idea that minimizes changes. I am testing this approach
> now and will submit v3 of the patch soon.

Thank you for willing to try the new approach!

>>  However, I'm good with your allocateTempFileName() changeset,
>>  so it's up to you to give it a try or not.
>>
>>  > Thanks again,
>>  > Philip
>>
>>  Thank you,
>>  Andrey.
>
> Thanks,
> Philip

-- 
Andrey.