Commit 2eb368c0 authored by Kai Ninomiya's avatar Kai Ninomiya Committed by Commit Bot

Implement FileSystem.rmtree for long paths on Windows

Changes blinkpy filesystem tests to use remove_contents instead of
rmtree. remove_contents calls into rmtree, but
  (1) retries several times to delete the files, and
  (2) throws by default instead of silently swallowing errors.
This improves all of the tests. Additionally, the retrying was
actually necessary to pass many tests (files were otherwise still
locked during teardown).

Extends the test_long_paths test to check spaces and apostrophes.

FileSystem.rmtree is now implemented on Windows by just calling
a shell with "rmdir /s /q". This is necessary, because
shutil.rmtree doesn't work for \\?\C:\ style "extended-length" paths
on Windows, and several other implementations I tried didn't work:
  - Loop using os.walk. os.walk doesn't support long paths, and
    though it seemed to work on the bots, it failed tests locally.
  - Recursive using os.listdir. os.listdir also doesn't support long
    paths.

Tested on dawn-win10-* try job on crrev.com/c/2050036/17.
Found in dawn-win10-* try jobs on crrev.com/c/2050036/4.

Change-Id: I7c67bb12305a41244829e35293edfc88901b76f4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2058065
Commit-Queue: Kai Ninomiya <kainino@chromium.org>
Reviewed-by: default avatarRobert Ma <robertma@chromium.org>
Cr-Commit-Position: refs/heads/master@{#742961}
parent 44db41a9
...@@ -41,6 +41,7 @@ import logging ...@@ -41,6 +41,7 @@ import logging
import os import os
import shutil import shutil
import stat import stat
import subprocess
import sys import sys
import tempfile import tempfile
import time import time
...@@ -74,6 +75,7 @@ class FileSystem(object): ...@@ -74,6 +75,7 @@ class FileSystem(object):
(https://msdn.microsoft.com/en-us/library/aa365247.aspx#maxpath) (https://msdn.microsoft.com/en-us/library/aa365247.aspx#maxpath)
""" """
if sys.platform == 'win32' and len(path) >= self.WINDOWS_MAX_PATH: if sys.platform == 'win32' and len(path) >= self.WINDOWS_MAX_PATH:
assert not path.startswith(r'\\'), "must not already be UNC"
return ur'\\?\%s' % (self.abspath(path),) return ur'\\?\%s' % (self.abspath(path),)
return path return path
...@@ -327,8 +329,26 @@ class FileSystem(object): ...@@ -327,8 +329,26 @@ class FileSystem(object):
def rmtree(self, path, ignore_errors=True, onerror=None): def rmtree(self, path, ignore_errors=True, onerror=None):
"""Deletes the directory rooted at path, whether empty or not.""" """Deletes the directory rooted at path, whether empty or not."""
# shutil.rmtree() uses os.path.join() which doesn't support UNC paths. if sys.platform == 'win32':
shutil.rmtree(path, ignore_errors=ignore_errors, onerror=onerror) # Ensure (hopefully) that the quoting done on the next line is safe.
assert '"' not in path
# Create a shell command to call rmdir. (Note rmdir is a shell
# built-in, so it cannot be called as an executable.)
cmd = 'rmdir /s /q "{}"'.format(self._path_for_access(path))
try:
subprocess.check_call(cmd, shell=True)
except subprocess.CalledProcessError as e:
if not ignore_errors:
if onerror:
# Unfortunately we can't know the exact file that failed,
# so we conjure up error info from the top level.
onerror('FileSystem.rmtree', path, sys.exc_info())
else:
raise e
else:
# shutil.rmtree() uses os.path.join() which doesn't support UNC paths.
shutil.rmtree(path, ignore_errors=ignore_errors, onerror=onerror)
def remove_contents(self, dirname): def remove_contents(self, dirname):
"""Attempts to remove the contents of a directory tree. """Attempts to remove the contents of a directory tree.
......
...@@ -56,9 +56,10 @@ class GenericFileSystemTests(object): ...@@ -56,9 +56,10 @@ class GenericFileSystemTests(object):
fs.chdir(self.orig_cwd) fs.chdir(self.orig_cwd)
def teardown_generic_test_dir(self): def teardown_generic_test_dir(self):
self.fs.rmtree(self.generic_test_dir) success = self.fs.remove_contents(self.generic_test_dir)
self.fs.chdir(self.orig_cwd) self.fs.chdir(self.orig_cwd)
self.generic_test_dir = None self.generic_test_dir = None
assert success
def test_glob__trailing_asterisk(self): def test_glob__trailing_asterisk(self):
self.fs.chdir(self.generic_test_dir) self.fs.chdir(self.generic_test_dir)
...@@ -112,10 +113,10 @@ class GenericFileSystemTests(object): ...@@ -112,10 +113,10 @@ class GenericFileSystemTests(object):
def test_rmtree(self): def test_rmtree(self):
self.fs.chdir(self.generic_test_dir) self.fs.chdir(self.generic_test_dir)
self.fs.rmtree('foo') self.fs.rmtree('doesntexist')
self.assertTrue(self.fs.exists('foodir')) self.assertTrue(self.fs.exists('foodir'))
self.assertTrue(self.fs.exists(self.fs.join('foodir', 'baz'))) self.assertTrue(self.fs.exists(self.fs.join('foodir', 'baz')))
self.fs.rmtree('foodir') self.fs.rmtree('foodir', ignore_errors=False)
self.assertFalse(self.fs.exists('foodir')) self.assertFalse(self.fs.exists('foodir'))
self.assertFalse(self.fs.exists(self.fs.join('foodir', 'baz'))) self.assertFalse(self.fs.exists(self.fs.join('foodir', 'baz')))
...@@ -361,3 +362,10 @@ class RealFileSystemTest(unittest.TestCase, GenericFileSystemTests): ...@@ -361,3 +362,10 @@ class RealFileSystemTest(unittest.TestCase, GenericFileSystemTests):
content = self.fs.read_text_file(file2) content = self.fs.read_text_file(file2)
self.fs.remove(file2) # No exception should be raised. self.fs.remove(file2) # No exception should be raised.
self.assertEqual(content, 'hello') self.assertEqual(content, 'hello')
long_path1 = self.fs.join(self.generic_test_dir, 'a' * 100, 'b' * 100 + " 'b", 'c' * 100)
long_path2 = self.fs.join(long_path1, 'd' * 100)
self.fs.maybe_make_directory(long_path2)
file1 = self.fs.join(long_path2, 'foo')
self.fs.write_text_file(file1, 'hello')
self.fs.rmtree(long_path2, ignore_errors=False)
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment