diff --git a/gclient_utils.py b/gclient_utils.py index 7182848527..8e2d1c315e 100644 --- a/gclient_utils.py +++ b/gclient_utils.py @@ -182,10 +182,10 @@ def FileWrite(filename, content, mode='w'): f.close() -def RemoveDirectory(*path): - """Recursively removes a directory, even if it's marked read-only. +def rmtree(path): + """shutil.rmtree() on steroids. - Remove the directory located at *path, if it exists. + Recursively removes a directory, even if it's marked read-only. shutil.rmtree() doesn't work on Windows if any of the files or directories are read-only, which svn repositories and some .svn files are. We need to @@ -208,63 +208,55 @@ def RemoveDirectory(*path): In the ordinary case, this is not a problem: for our purposes, the user will never lack write permission on *path's parent. """ - logging.debug(path) - file_path = os.path.join(*path) - if not os.path.exists(file_path): + if not os.path.exists(path): return - if os.path.islink(file_path) or not os.path.isdir(file_path): - raise Error('RemoveDirectory asked to remove non-directory %s' % file_path) + if os.path.islink(path) or not os.path.isdir(path): + raise Error('Called rmtree(%s) in non-directory' % path) - has_win32api = False if sys.platform == 'win32': - has_win32api = True # Some people don't have the APIs installed. In that case we'll do without. try: win32api = __import__('win32api') win32con = __import__('win32con') except ImportError: - has_win32api = False + pass else: # On POSIX systems, we need the x-bit set on the directory to access it, # the r-bit to see its contents, and the w-bit to remove files from it. # The actual modes of the files within the directory is irrelevant. - os.chmod(file_path, stat.S_IRUSR | stat.S_IWUSR | stat.S_IXUSR) - for fn in os.listdir(file_path): - fullpath = os.path.join(file_path, fn) + os.chmod(path, stat.S_IRUSR | stat.S_IWUSR | stat.S_IXUSR) + def remove(func, subpath): + if sys.platform == 'win32': + os.chmod(subpath, stat.S_IWRITE) + if win32api and win32con: + win32api.SetFileAttributes(subpath, win32con.FILE_ATTRIBUTE_NORMAL) + try: + func(subpath) + except OSError, e: + if e.errno != errno.EACCES or sys.platform != 'win32': + raise + # Failed to delete, try again after a 100ms sleep. + time.sleep(0.1) + func(subpath) + + for fn in os.listdir(path): # If fullpath is a symbolic link that points to a directory, isdir will # be True, but we don't want to descend into that as a directory, we just # want to remove the link. Check islink and treat links as ordinary files # would be treated regardless of what they reference. + fullpath = os.path.join(path, fn) if os.path.islink(fullpath) or not os.path.isdir(fullpath): - if sys.platform == 'win32': - os.chmod(fullpath, stat.S_IWRITE) - if has_win32api: - win32api.SetFileAttributes(fullpath, win32con.FILE_ATTRIBUTE_NORMAL) - try: - os.remove(fullpath) - except OSError, e: - if e.errno != errno.EACCES or sys.platform != 'win32': - raise - print 'Failed to delete %s: trying again' % fullpath - time.sleep(0.1) - os.remove(fullpath) + remove(os.remove, fullpath) else: - RemoveDirectory(fullpath) + # Recurse. + rmtree(fullpath) - if sys.platform == 'win32': - os.chmod(file_path, stat.S_IWRITE) - if has_win32api: - win32api.SetFileAttributes(file_path, win32con.FILE_ATTRIBUTE_NORMAL) - try: - os.rmdir(file_path) - except OSError, e: - if e.errno != errno.EACCES or sys.platform != 'win32': - raise - print 'Failed to remove %s: trying again' % file_path - time.sleep(0.1) - os.rmdir(file_path) + remove(os.rmdir, path) + +# TODO(maruel): Rename the references. +RemoveDirectory = rmtree def CheckCallAndFilterAndHeader(args, always=False, **kwargs): diff --git a/tests/fake_repos.py b/tests/fake_repos.py index 4998575842..5f8c54ef82 100755 --- a/tests/fake_repos.py +++ b/tests/fake_repos.py @@ -7,20 +7,18 @@ import atexit import datetime -import errno import logging import os import pprint import re import socket -import stat import subprocess import sys import tempfile -import time # trial_dir must be first for non-system libraries. from tests import trial_dir +import gclient_utils import scm ## Utility functions @@ -66,71 +64,6 @@ def add_kill(): subprocess.Popen.kill = kill_nix -def rmtree(*path): - """Recursively removes a directory, even if it's marked read-only. - - Remove the directory located at *path, if it exists. - - shutil.rmtree() doesn't work on Windows if any of the files or directories - are read-only, which svn repositories and some .svn files are. We need to - be able to force the files to be writable (i.e., deletable) as we traverse - the tree. - - Even with all this, Windows still sometimes fails to delete a file, citing - a permission error (maybe something to do with antivirus scans or disk - indexing). The best suggestion any of the user forums had was to wait a - bit and try again, so we do that too. It's hand-waving, but sometimes it - works. :/ - """ - file_path = os.path.join(*path) - if not os.path.exists(file_path): - return - - def RemoveWithRetry_win(rmfunc, path): - os.chmod(path, stat.S_IWRITE) - if win32_api_avail: - win32api.SetFileAttributes(path, win32con.FILE_ATTRIBUTE_NORMAL) - try: - return rmfunc(path) - except EnvironmentError, e: - if e.errno != errno.EACCES: - raise - print 'Failed to delete %s: trying again' % repr(path) - time.sleep(0.1) - return rmfunc(path) - - def RemoveWithRetry_non_win(rmfunc, path): - if os.path.islink(path): - return os.remove(path) - else: - return rmfunc(path) - - win32_api_avail = False - remove_with_retry = None - if sys.platform.startswith('win'): - # Some people don't have the APIs installed. In that case we'll do without. - try: - win32api = __import__('win32api') - win32con = __import__('win32con') - win32_api_avail = True - except ImportError: - pass - remove_with_retry = RemoveWithRetry_win - else: - remove_with_retry = RemoveWithRetry_non_win - - for root, dirs, files in os.walk(file_path, topdown=False): - # For POSIX: making the directory writable guarantees removability. - # Windows will ignore the non-read-only bits in the chmod value. - os.chmod(root, 0770) - for name in files: - remove_with_retry(os.remove, os.path.join(root, name)) - for name in dirs: - remove_with_retry(os.rmdir, os.path.join(root, name)) - - remove_with_retry(os.rmdir, file_path) - - def write(path, content): f = open(path, 'wb') f.write(content) @@ -309,9 +242,9 @@ class FakeReposBase(object): self.svnserve = None if not self.trial.SHOULD_LEAK: logging.debug('Removing %s' % self.svn_repo) - rmtree(self.svn_repo) + gclient_utils.rmtree(self.svn_repo) logging.debug('Removing %s' % self.svn_checkout) - rmtree(self.svn_checkout) + gclient_utils.rmtree(self.svn_checkout) else: return False return True @@ -330,7 +263,7 @@ class FakeReposBase(object): self.wait_for_port_to_free(self.git_port) if not self.trial.SHOULD_LEAK: logging.debug('Removing %s' % self.git_root) - rmtree(self.git_root) + gclient_utils.rmtree(self.git_root) else: return False return True diff --git a/tests/gclient_utils_test.py b/tests/gclient_utils_test.py index 4b9ebf2a57..8027675b0e 100755 --- a/tests/gclient_utils_test.py +++ b/tests/gclient_utils_test.py @@ -9,7 +9,8 @@ import StringIO # Fixes include path. from super_mox import SuperMoxTestBase - +import trial_dir +import os import gclient_utils @@ -32,8 +33,8 @@ class GclientUtilsUnittest(GclientUtilBase): 'ParseXML', 'Popen', 'PrintableObject', 'RemoveDirectory', 'SoftClone', 'SplitUrlRevision', 'SyntaxErrorToError', 'WorkItem', - 'errno', 'hack_subprocess', 'logging', 'os', 'Queue', 're', 'stat', - 'subprocess', 'sys','threading', 'time', 'xml', + 'errno', 'hack_subprocess', 'logging', 'os', 'Queue', 're', 'rmtree', + 'stat', 'subprocess', 'sys','threading', 'time', 'xml', ] # If this test fails, you should add the relevant test. self.compareMembers(gclient_utils, members) @@ -181,6 +182,24 @@ class SplitUrlRevisionTestCase(GclientUtilBase): self.assertEquals(out_url, url) +class GClientUtilsTest(trial_dir.TestCase): + def testHardToDelete(self): + # Use the fact that tearDown will delete the directory to make it hard to do + # so. + l1 = os.path.join(self.root_dir, 'l1') + l2 = os.path.join(l1, 'l2') + l3 = os.path.join(l2, 'l3') + f3 = os.path.join(l3, 'f3') + os.mkdir(l1) + os.mkdir(l2) + os.mkdir(l3) + gclient_utils.FileWrite(f3, 'foo') + os.chmod(f3, 0) + os.chmod(l3, 0) + os.chmod(l2, 0) + os.chmod(l1, 0) + + if __name__ == '__main__': import unittest unittest.main() diff --git a/tests/scm_unittest.py b/tests/scm_unittest.py index 31d0a7c46e..5b21c6f17e 100755 --- a/tests/scm_unittest.py +++ b/tests/scm_unittest.py @@ -276,7 +276,7 @@ class RealSvnTest(fake_repos.FakeReposTestBase): self._capture( ['propset', 'foo', 'bar', scm.os.path.join(self.svn_root, 'prout', 'origin')]) - fake_repos.rmtree(scm.os.path.join(self.svn_root, 'prout')) + fake_repos.gclient_utils.rmtree(scm.os.path.join(self.svn_root, 'prout')) with open(scm.os.path.join(self.svn_root, 'faa'), 'w') as f: f.write('eh') with open(scm.os.path.join(self.svn_root, 'faala'), 'w') as f: