msg254438 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2015-11-10 11:03 |
test_deleted_cwd in test_importlib is failed on AIX. http://buildbot.python.org/all/builders/PPC64%20AIX%203.x/builds/4318/steps/test/logs/stdio ====================================================================== ERROR: test_deleted_cwd (test.test_importlib.import_.test_path.Source_FinderTests) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/shager/cpython-buildarea/3.x.edelsohn-aix-ppc64/build/Lib/test/test_importlib/import_/test_path.py", line 169, in test_deleted_cwd os.chdir(path) File "/home/shager/cpython-buildarea/3.x.edelsohn-aix-ppc64/build/Lib/tempfile.py", line 807, in __exit__ self.cleanup() File "/home/shager/cpython-buildarea/3.x.edelsohn-aix-ppc64/build/Lib/tempfile.py", line 811, in cleanup _shutil.rmtree(self.name) File "/home/shager/cpython-buildarea/3.x.edelsohn-aix-ppc64/build/Lib/shutil.py", line 478, in rmtree onerror(os.rmdir, path, sys.exc_info()) File "/home/shager/cpython-buildarea/3.x.edelsohn-aix-ppc64/build/Lib/shutil.py", line 476, in rmtree os.rmdir(path) OSError: [Errno 16] Device busy: '/tmp/tmp2xdxtq6x' ---------------------------------------------------------------------- Proposed patch fixes the test. It also uses more verbose wording to create and remove temporary directory to be sure that caught exception was written by removing the directory, not by creating or changing CWD. See also . |
|
|
msg254439 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2015-11-10 11:15 |
-1, review on Rietveld. |
|
|
msg254442 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2015-11-10 12:55 |
Thank you Victor for your review. Here is fixed patch that implements your suggestions. |
|
|
msg254443 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2015-11-10 12:58 |
And here is simple alternative patch based on Martin's original suggestion in . |
|
|
msg254444 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2015-11-10 13:11 |
test_deleted_cwd_aix_alt.patch: I don't trust tempfile.TemporaryDirectory(), I prefer your patch. test_deleted_cwd_aix_2.patch looks good to me. |
|
|
msg254467 - (view) |
Author: Martin Panter (martin.panter) *  |
Date: 2015-11-11 00:07 |
Wow this is tricky to get right. Victor, you are right to not trust TemporaryDirectory, because when cleanup() fails you don’t get a second chance, and it will leave the directory behind. According to the Microsoft _mkdir() web page <https://msdn.microsoft.com/en-us/library/wt8es881.aspx>, Windows would raise ENOTEMPTY for removing the current directory. Maybe it is time to drop the win32 skip check as well? Here is a patch based on aix_2.patch, but also removing the redundant win32 skip. And “import errno” was no longer needed. I only tested it on Linux though, which supports removing the cwd. |
|
|
msg254476 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2015-11-11 06:24 |
Martin's original code looked nicer to me. It' a pity we can't use it. test_deleted_cwd_aix_3.patch LGTM. Thanks Martin! |
|
|
msg254477 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2015-11-11 06:27 |
New changeset d4dc36586f24 by Serhiy Storchaka in branch '3.5': Issue #25595: Fixed test_deleted_cwd in test_importlib on AIX. https://hg.python.org/cpython/rev/d4dc36586f24 New changeset 3f392050d519 by Serhiy Storchaka in branch 'default': Issue #25595: Fixed test_deleted_cwd in test_importlib on AIX. https://hg.python.org/cpython/rev/3f392050d519 |
|
|