This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: TemporaryDirectory does not resolve path when created using a relative path
Type: Stage:
Components: Library (Lib) Versions: Python 3.5
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Antony.Lee, georg.brandl, ncoghlan, pitrou, serhiy.storchaka, yselivanov
Priority: normal Keywords: patch

Created on 2014-01-15 04:48 by Antony.Lee, last changed 2022-04-11 14:57 by admin.

Files
File name Uploaded Description Edit
tempfile_01.patch yselivanov, 2014-01-20 19:47 review
tempfile_02.patch yselivanov, 2014-09-26 21:02 review
Messages (13)
msg208141 - (view) Author: Antony Lee (Antony.Lee) * Date: 2014-01-15 04:48
Essentially, the following fails:

t = tempfile.TemporaryDirectory(dir=".")
os.chdir(some_other_dir)
t.cleanup()

because t.name is a relative path.  Resolving the "dir" argument when the directory is created should(?) fix the issue.
msg208571 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2014-01-20 19:47
This looks like a bug to me (or we should complain if a relative 'dir' was passed).

I'm attaching a patch that fixes this, although I'm not sure if the unittest I wrote will work on windows.

Moreover, 'mkstemp' and 'mktemp' have the same bug (separate issue for them?)
msg208591 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2014-01-20 23:51
The patch is ok for windows.
msg210581 - (view) Author: Antony Lee (Antony.Lee) * Date: 2014-02-08 01:28
Thanks for the fix.  The same fix seems should also work for mkstemp and mktemp -- is it really worth creating a new issue for them?
msg210589 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2014-02-08 05:05
> Thanks for the fix.  The same fix seems should also work for mkstemp and mktemp -- is it really worth creating a new issue for them?

No, no new issue is necessary. I think I'll update the patch and merge it in 3.5, as it's too late for 3.4
msg227649 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2014-09-26 21:02
A second version of the patch (tempfile_02), fixing more tempfile functions to properly support relative paths. Please review.
msg227653 - (view) Author: Antony Lee (Antony.Lee) * Date: 2014-09-26 21:35
This looks reasonable.  Note that the output of gettempdir is always passed as first argument to os.path.join (possibly via _mkstemp_inner), so perhaps you should rather define something like

def _absolute_child_of_parent_or_tmpdir(parent, *args):
    """Return the absolute child of parent, or gettempdir() if parent is None, given by *args.
    """
    if parent is None:
        parent = <_sanitize_dir> # inline the code here
    return _os.path.join(parent, *args)

and use that function instead.

This factorizes the code a little bit more and makes intent clearer (I don't think _sanitize_dir is a very clear name).
msg227659 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2014-09-26 21:54
Antony, I agree regarding the poor naming of '_sanitize_dir()' helper. As for your other suggestion, I think such a refactoring will actually make code harder to follow (+ it's more invasive). Generally, I'm in favour of transforming parameters like 'dir' closer to the beginning of the method's code, so that it's immediately obvious what's going on, and is also easier to put debug code [like 'print("mkdtemp call for: ", dir)'].
msg227662 - (view) Author: Antony Lee (Antony.Lee) * Date: 2014-09-26 22:08
I don't feel strongly about this, so either way is fine.
msg227665 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-09-27 08:38
Note that abspath() can return incorrect result in case of symbolic links to directories and pardir components. I.e. abspath('symlink/..').
msg227701 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2014-09-27 16:46
> Note that abspath() can return incorrect result in case of symbolic links to directories and pardir components. I.e. abspath('symlink/..').

Good catch.. Should I use os.path.realpath?
msg228459 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-10-04 13:45
> Should I use os.path.realpath?

I don't see other way.

But there is other issue. Following code works now, but will fail with the patch:

import os, tempfile
os.mkdir('parent')
os.chdir('parent')
t = tempfile.TemporaryDirectory(dir=".")
os.rename('../parent', '../parent2')
t.cleanup()
msg228513 - (view) Author: Antony Lee (Antony.Lee) * Date: 2014-10-05 00:20
The change would be backwards-incompatible but also mimics the behavior of NamedTemporaryFile (which also fails to delete the file if the containing folder has been renamed -- this is easy to verify manually).
I guess the other option would be to use fd-based semantics?  (but it'd be preferable if the behavior was kept the same between TemporaryDirectory and NamedTemporaryFile).
History
Date User Action Args
2022-04-11 14:57:56adminsetgithub: 64466
2014-10-05 00:20:00Antony.Leesetmessages: + msg228513
2014-10-04 13:45:51serhiy.storchakasetmessages: + msg228459
2014-09-27 16:46:27yselivanovsetmessages: + msg227701
2014-09-27 08:39:24serhiy.storchakasetnosy: + pitrou
2014-09-27 08:38:34serhiy.storchakasetmessages: + msg227665
2014-09-26 22:08:58Antony.Leesetmessages: + msg227662
2014-09-26 21:54:18yselivanovsetmessages: + msg227659
2014-09-26 21:35:20Antony.Leesetmessages: + msg227653
2014-09-26 21:02:50yselivanovsetfiles: + tempfile_02.patch
nosy: + serhiy.storchaka
messages: + msg227649

2014-02-08 05:05:03yselivanovsetmessages: + msg210589
versions: + Python 3.5, - Python 3.4
2014-02-08 01:28:40Antony.Leesetmessages: + msg210581
2014-01-20 23:51:18yselivanovsetmessages: + msg208591
2014-01-20 19:48:23yselivanovsetnosy: + georg.brandl, ncoghlan
2014-01-20 19:47:51yselivanovsetfiles: + tempfile_01.patch

nosy: + yselivanov
messages: + msg208571

keywords: + patch
2014-01-15 04:48:28Antony.Leecreate