-
Notifications
You must be signed in to change notification settings - Fork 141
git-p4: auto-delete named temporary file #303
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
git-p4: auto-delete named temporary file #303
Conversation
/submit |
Submitted as [email protected] |
Please do use the GitHub feature where you update PRs by force-pushing the PR branch instead of renaming the branches. Git is really good at versioning, file/branch names are not. |
On the Git mailing list, Andrey wrote (reply to this):
|
Avoid double-open exceptions on Windows platform when calculating for lfs compressed size threshold (git-p4.largeFileCompressedThreshold) comparisons. Take new approach using the NamedTemporaryFile() file-like object as input to the ZipFile() which auto-deletes after implicit close leaving with scope. Original code had double-open exception on Windows platform because file still open from NamedTemporaryFile() using generated filename instead of object. Thanks to Andrey for patiently suggesting several iterations on this change for avoiding exceptions! Also print error details after resulting IOError to make debugging cause of exception less mysterious when it has nothing to do with "git version recent enough." Signed-off-by: Philip.McGraw <[email protected]> Reviewed-by: Andrey Mazo <[email protected]>
1b270ef
to
7e59b5c
Compare
Thanks; sorry for the confusion over branch renaming. |
/submit |
Submitted as [email protected] |
I wonder why the actual patch didn't make it to the mailing list -- only the cover letter did. @dscho, do you have any ideas? |
I wondered what else to do to get the patch included. Without the patch, everyone using git-p4 with LFS on Windows would get double-open exceptions. |
According to |
Sent: Wednesday, 21 August, 2019 14:09
To: Philip McGraw ***@***.***>
Cc: Philip McGraw ***@***.***>; Author ***@***.***>
Subject: Re: [gitgitgadget/git] git-p4: auto-delete named temporary file (#303)
According to git shortlog -nse --since=2.years.ago git-p4.py, our resident git-p4 expert is Luke Diamand ***@***.*** Maybe reply to the thread, Cc:ing Luke, requesting a review?
Thanks Johannes! Luke, would you be able to review this patch?
#303
Thanks!
Philip
|
Junio may pick up the patch even without Luke's review. |
By "reply" I meant on the mailing list ;-)
It made it to the list: https://public-inbox.org/git/[email protected]/#r If you follow the advice on the bottom of that page, you can reply to the mails themselves, Cc:ing Luke. That should get their attention. |
Only the cover letter made it, but not the actual patch, right? |
Yes :-( I don't know, why, though. |
I tried to re-send it, and it kinda worked, but it used a different Message-Id... sigh |
On the Git mailing list, Git Gadget wrote (reply to this):
|
On the Git mailing list, Junio C Hamano wrote (reply to this):
|
@dscho, thank you! I guess, I'll just resend @philip-mcgraw's change using send-email. |
On the Git mailing list, Andrey Mazo wrote (reply to this):
|
On the Git mailing list, Junio C Hamano wrote (reply to this):
|
On the Git mailing list, Luke Diamand wrote (reply to this):
|
On the Git mailing list, Johannes Schindelin wrote (reply to this):
|
On the Git mailing list, Junio C Hamano wrote (reply to this):
|
On the Git mailing list, Johannes Schindelin wrote (reply to this):
|
On the Git mailing list, Junio C Hamano wrote (reply to this):
|
This branch is now known as |
This patch series was integrated into pu via git@3ec4a7a. |
This patch series was integrated into pu via git@9edfe2c. |
This patch series was integrated into next via git@4f45be7. |
This patch series was integrated into pu via git@9c7ca4a. |
This patch series was integrated into pu via git@328caa9. |
This patch series was integrated into pu via git@2e956f7. |
This patch series was integrated into next via git@2e956f7. |
This patch series was integrated into master via git@2e956f7. |
Closed via 2e956f7. |
Take new approach using the NamedTemporaryFile()
file-like object as input to the ZipFile() which
auto-deletes after implicit close leaving with scope.
Original code produced double-open problems on Windows
platform from using already open NamedTemporaryFile()
generated filename instead of object.
Thanks to Andrey for patiently suggesting several
iterations on this change for avoiding exceptions!
Also print error details after resulting IOError to make
debugging cause of exception less mysterious when it has
nothing to do with "git version recent enough."
Signed-off-by: Philip.McGraw [email protected]