msg95256 - (view) |
Author: Roy Smith (roysmith) |
Date: 2009-11-14 19:27 |
The docs (http://www.python.org/doc/2.5.1/lib/module-tempfile.html) specify that
mkdtemp(), "returns the absolute pathname of the new directory". It does that in
the default case, but if you specify a relative path for 'dir', you get back a
relative path.
$ python
Python 2.5.1 (r251:54863, Oct 17 2008, 14:39:09)
[GCC 3.4.6 20060404 (Red Hat 3.4.6-10)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import tempfile
>>> tempfile.mkdtemp(dir='.')
'./tmpHk1pBD'
similar results were obtained on:
Python 2.5.1 (r251:54863, Feb 6 2009, 19:02:12)
[GCC 4.0.1 (Apple Inc. build 5465)] on darwin
Note that mkstemp() gets it right:
>>> tempfile.mkdtemp(dir='.')
'./tmpoPXdL7'
>>> tempfile.mkstemp(dir='.')
(3, '/Users/roy2/tmpwTGZ2y')
>>>
|
msg95257 - (view) |
Author: R. David Murray (r.david.murray) *  |
Date: 2009-11-14 19:51 |
This is true on trunk and py3k as well. 2.5 is in security fix only
mode, so I've removed it from the versions list.
Since mkstemp does return in the absolute path in this case, I think
this is a code rather than a documentation bug. However, changing it
would be backward incompatible, so it may not be possible to fix it in
2.6 and 3.1.
|
msg95307 - (view) |
Author: Brett Cannon (brett.cannon) *  |
Date: 2009-11-15 20:56 |
It's a border case for backporting. The docs do say it should be returning
an absolute path, so that is a bug. And chances are anyone who has
discovered this and is working around it simply called os.path.abspath()
on the value instead of some manual calculation (if at all; bigger chance
no one simply noticed).
My vote is to backport. Otherwise the docs should be updated for 2.6/3.1
to mention the bug.
|
msg95308 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2009-11-15 21:03 |
I think we should be conservative. People may be relying on it *not*
returning an absolute path; besides, the bug itself wasn't a problem in
practice, so backporting the fix doesn't bring any added value.
|
msg98316 - (view) |
Author: Thomas Holmes (thomas.holmes) |
Date: 2010-01-26 06:57 |
I have created a patch for this for Python 3.1 and included an update to the unit tests. The tests were never checking for a relative path and if they did would pas it even when it would have failed due to liberal use of os.path.abspath()
The test fix is probably not as clean as it could be by I did not want to severely alter its state. The change to tempfile.mkdtemp() was clear and followed the current method tempfile.mkstemp() uses.
|
msg98317 - (view) |
Author: Thomas Holmes (thomas.holmes) |
Date: 2010-01-26 07:07 |
As a side note, this was done mostly as an exercise for myself and as a learning experience. Any feedback would be appreciated regardless of any decision on the status of this bug.
|
msg98342 - (view) |
Author: R. David Murray (r.david.murray) *  |
Date: 2010-01-26 16:17 |
First of all, thanks for working on this. Now for some feedback :)
It is most helpful if you provide patches against trunk (which is 2.7 alpha right now). We then forward port them to py3k. (This will change after the release of 2.7, when py3k will become trunk.)
Isn't the assert you added to nameCheck redundant? It seems to me it would be better just to remove the abspath call on ndir from the existing assert.
I haven't applied and tested the patch, but the other changes to the tests look odd to me. Why are you obtaining the abspath of the current directory? And why do those tests need to be changed at all?
Your additional test case looks like it might be some exploration code you forgot to delete before generating the patch, maybe those other changes are as well?
|
msg98405 - (view) |
Author: Thomas Holmes (thomas.holmes) |
Date: 2010-01-27 03:39 |
I agree, I do not feel like the precise changes to the tests feel completely ideal. I feel that this problem stems from the fact that the nameCheck function as originally written doesn't seem to completely serve its originally intended purpose.
The original issue that caused the modifications of the tests were as follows:
* After adding "test_abs_path()" namecheck would pass the test when it should actually fail due to the original assert performing os.path.abspath() on both paths. The obviously solution to this seemed to be to take the abspath of the user supplied path (the variable dir in this function) relative or otherwise and compare to ndir which is derived from the passed in "name" variable, populated by the mkdtemp call.
* The second issue is a somewhat complex case of those asserts passing the atypical odd calls of nameCheck from the "test__RandomNameSequence" suite. Without absolute pathing of both parameters in the new first assert this test began to fail. This is because the original call was self.namecheck(<file name>, '', '', ''). These empty parameters when called with os.path.abspath() would return valid paths and these asserts would succeed. As a result, the abspath(join()) call built a proper path into the "name" parameter allowing the tests to pass in this case.
* For the sake of cleanliness I decided it made more sense to split the two apart. Make one assert that specifically verifies the path the file/folder is being placed in is both absolute and matching and one that will pass the file name. Perhaps the old first assert (or second assert in the patch) can actually be removed but I think it may be getting properly tested against in one of the other test classes.
I am quite confident there is a much better way to accomplish this but I did not wish to change _too_ much of the test on my first stab at this.
I appreciate your feedback very much. I will work on setting up the 2.7 environment for working on issues that span the 2x/3x gap.
|
msg98406 - (view) |
Author: Thomas Holmes (thomas.holmes) |
Date: 2010-01-27 03:46 |
One other thing that crossed my mind while I was thinking over this today. Instead of just relative pathing to '.' I should probably change my path to gettempdir() and then relative path to it since the location of the python executable may not be writable.
Any thoughts on how to go about testing this in a way that is most likely to succeed in all cases and respect OS permissions?
|
msg98457 - (view) |
Author: Thomas Holmes (thomas.holmes) |
Date: 2010-01-28 04:58 |
I made a new patch off of the 2.7 trunk version. I think I have handled some of the issues more cleanly.
Please see Py(27)7325.diff
I addressed the issue of using a relative path in the tempdir to achieve the 'guaranteed' ability to write rather than assuming the location of the python executable was writable.
|
msg111828 - (view) |
Author: Mark Lawrence (BreamoreBoy) * |
Date: 2010-07-28 16:05 |
Tried this on windows against 2.7 don't see why it can't go forward.
|
msg121214 - (view) |
Author: Jessica McKellar (jesstess) *  |
Date: 2010-11-15 04:04 |
Thomas, I think the weirdness you were sensing when trying to adapt the nameCheck calls in test__RandomNameSequence after adding the abspath check is that those test cases really don't need nameCheck.
For example, test_get_six_char_str should be testing that _RandomNameSequence returns something that is both a string and 6 characters, not properties of some path manufactured around it.
I've attached a patch based on Thomas's that factors out the string check in nameCheck and has the tests in test__RandomNameSequence just use the string check.
The patch is against release27-maint.
|
msg121734 - (view) |
Author: Rafael dos Santos Gonçalves (elesbom) |
Date: 2010-11-20 19:36 |
Hi people,
I insert a simple code in tempfile.py, that verify if the return starts with ./, and concatenate using the abspath function.
I changed the variable name file to temp_dir_abspath, because file is a built-in function and it's not recommended, overwrite it with local variables.
And at least i add one test, that verify if temp_directory returns an absolut path.
thanks.
|
msg121764 - (view) |
Author: Éric Araujo (eric.araujo) *  |
Date: 2010-11-20 20:43 |
Looks good to me, with one exception:
if temp_dir_abspath.startswith('./'):
Wouldn’t this be better:
if not _os.path.isabs(temp_dir_abspath)
(P.S. file is not a builtin anymore in 3.x, it’s used for example as an argument to the print function.)
|
msg121906 - (view) |
Author: Rafael dos Santos Gonçalves (elesbom) |
Date: 2010-11-21 12:00 |
You is right, i put this command on test and not the test.
thankss
|
msg122038 - (view) |
Author: Éric Araujo (eric.araujo) *  |
Date: 2010-11-22 00:14 |
os.path.abspath actually checks for isabs first, so I think you can leave the test out and always call abspath.
|
msg178291 - (view) |
Author: Hynek Schlawack (hynek) *  |
Date: 2012-12-27 10:26 |
I think we should resolve this one line change.
Jessica’s patch looks just fine, so I tend to apply it. However, I’d like to document the current behavior in 2.7, 3.2, 3.3 and 3.4.
Am I missing anything?
|
msg209838 - (view) |
Author: Yury Selivanov (yselivanov) *  |
Date: 2014-01-31 22:21 |
bump? see also #20267
|
msg407816 - (view) |
Author: Irit Katriel (iritkatriel) *  |
Date: 2021-12-06 14:33 |
It's still the same in 3.11. I don't know whether this is something to fix in the code or in the docs.
>> import tempfile
>>> tempfile.mkdtemp(dir='.')
'./tmplvkt14za'
>>> tempfile.mkstemp(dir='.')
(3, '/Users/iritkatriel/src/cpython/tmp86ljh34k')
|
|
Date |
User |
Action |
Args |
2022-04-11 14:56:54 | admin | set | github: 51574 |
2021-12-06 14:33:45 | iritkatriel | set | nosy:
+ iritkatriel
messages:
+ msg407816 versions:
+ Python 3.9, Python 3.10, Python 3.11, - Python 2.7, Python 3.2, Python 3.3, Python 3.4 |
2014-02-03 15:33:39 | BreamoreBoy | set | nosy:
- BreamoreBoy
|
2014-01-31 22:21:08 | yselivanov | set | nosy:
+ yselivanov messages:
+ msg209838
|
2012-12-28 20:02:04 | brett.cannon | set | nosy:
- brett.cannon
|
2012-12-27 10:26:37 | hynek | set | nosy:
+ hynek
messages:
+ msg178291 versions:
+ Python 2.7, Python 3.3, Python 3.4 |
2010-11-22 00:14:55 | eric.araujo | set | messages:
+ msg122038 |
2010-11-21 12:00:29 | elesbom | set | files:
+ issue7325.patch
messages:
+ msg121906 |
2010-11-21 11:59:09 | elesbom | set | files:
- issue7325.diff |
2010-11-20 20:43:41 | eric.araujo | set | messages:
+ msg121764 |
2010-11-20 19:36:08 | elesbom | set | files:
+ issue7325.diff nosy:
+ elesbom messages:
+ msg121734
|
2010-11-17 22:42:29 | eric.araujo | set | versions:
- Python 3.1, Python 2.7 |
2010-11-17 22:41:57 | eric.araujo | set | versions:
+ Python 3.1, Python 2.7 |
2010-11-16 09:48:56 | eric.araujo | set | nosy:
+ eric.araujo
versions:
- Python 2.6, Python 3.1, Python 2.7 |
2010-11-15 04:04:33 | jesstess | set | files:
+ issue7325.patch title: tempfile.mkdtemp() does not return absolute pathname when dir is specified -> tempfile.mkdtemp() does not return absolute pathname when relative dir is specified nosy:
+ jesstess
messages:
+ msg121214
|
2010-07-31 18:26:28 | dstanek | set | nosy:
+ dstanek
|
2010-07-28 16:05:38 | BreamoreBoy | set | nosy:
+ BreamoreBoy messages:
+ msg111828
|
2010-01-28 04:58:17 | thomas.holmes | set | files:
+ Py(27)7325.diff
messages:
+ msg98457 |
2010-01-27 03:46:35 | thomas.holmes | set | messages:
+ msg98406 |
2010-01-27 03:39:17 | thomas.holmes | set | messages:
+ msg98405 |
2010-01-26 16:17:18 | r.david.murray | set | messages:
+ msg98342 stage: test needed -> patch review |
2010-01-26 07:07:59 | thomas.holmes | set | messages:
+ msg98317 |
2010-01-26 06:57:56 | thomas.holmes | set | files:
+ Py7325.diff
nosy:
+ thomas.holmes messages:
+ msg98316
keywords:
+ patch |
2009-11-15 21:03:30 | pitrou | set | nosy:
+ pitrou messages:
+ msg95308
|
2009-11-15 20:56:34 | brett.cannon | set | nosy:
+ brett.cannon messages:
+ msg95307
|
2009-11-14 19:51:30 | r.david.murray | set | priority: normal
versions:
+ Python 2.6, Python 3.1, Python 2.7, Python 3.2, - Python 2.5 keywords:
+ easy nosy:
+ r.david.murray
messages:
+ msg95257 stage: test needed |
2009-11-14 19:27:34 | roysmith | create | |