Skip to content
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

Re-opening NamedTemporaryFile is not valid on Windows #222

Open
blais opened this issue Feb 26, 2018 · 3 comments
Open

Re-opening NamedTemporaryFile is not valid on Windows #222

blais opened this issue Feb 26, 2018 · 3 comments
Labels
BUG This is a bug, not a feature request; something isn't working. P1 High priority, not breaking but hindering or lessening existing functionality portability Component: Portability issues

Comments

@blais
Copy link
Member

blais commented Feb 26, 2018

Original report by Jeff Brantley (Bitbucket: jsbmsu, GitHub: jsbmsu).


Both test and production code are using tempfile.NamedTemporaryFile, which on non-Windows can be re-opened (by name) while the original handle is still open. For example, beancount.utils.test_utils.docfile decorator extracts a test's docstring, writes it to a file, and then---still holding the file open---passes the filename to the test, to be read by that test.

Searching the repo for NamedTemporaryFile turns up 10 files with "test" in the name, as well as beancount.parser.lexer and beancount.utils.text_utils.

It's unfortunate that this doesn't just work. Assuming all instances are in a with statement (not true, but perhaps that is a bug?), then it may be possible to workaround with a wrapper that passes delete=False and then manually deletes the file on __exit__. I'm not sure what flaws (if any) this may exhibit over the real thing.

@blais
Copy link
Member Author

blais commented Feb 26, 2018

Original comment by Martin Blais (Bitbucket: blais, GitHub: blais).


Great bug find. We'll have to look at this indeed.

@blais
Copy link
Member Author

blais commented Apr 3, 2018

Original comment by Bob Jordan (Bitbucket: bmjjr, GitHub: bmjjr).


Yes, I had to add delete=False, but I had to get it done and learn what I could about the library within a few day holiday break, so I didn't think about anything fancy like deleting the file on __exit__. Looks like I just made another function to clean up the temp file and call it a few places in the test suite and at the end of the tests.

#!python
def cleantmpdir():
    """clean tmp directory"""

    del_dir = r'c:\tmp'
    return subprocess.Popen('rmdir /S /Q %s' % del_dir, shell=True,
                            stdout=subprocess.PIPE, stderr=subprocess.PIPE)

@blais blais added P1 High priority, not breaking but hindering or lessening existing functionality portability Component: Portability issues BUG This is a bug, not a feature request; something isn't working. platform/windows and removed platform/windows labels May 22, 2020
@Ev2geny
Copy link
Contributor

Ev2geny commented Feb 27, 2024

I think there was another issue open for the same problem #550

Any way, in python 3.12 there is now new functionality, which allows to re-write the tests in a way that they will work on Windows as well:

The tempfile.NamedTemporaryFile function has a new optional parameter delete_on_close ( gh-58451)

But this is only in 3.12

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BUG This is a bug, not a feature request; something isn't working. P1 High priority, not breaking but hindering or lessening existing functionality portability Component: Portability issues
Projects
None yet
Development

No branches or pull requests

2 participants