Commit 85686aef authored by Kai Ninomiya's avatar Kai Ninomiya Committed by Commit Bot

Reland "Implement FileSystem.rmtree for long paths on Windows"

This is a reland of 2eb368c0

Changed to not allow long paths for the root of the tree
being rmtree'd, so it works on Windows 7.

Original change's description:
> 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: Robert Ma <robertma@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#742961}

Bug: 1054331
Change-Id: Ib1f7a7ac0109fb77e1d26f1bdb292ad2c95cea24
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2092946
Commit-Queue: Kai Ninomiya <kainino@chromium.org>
Reviewed-by: default avatarRobert Ma <robertma@chromium.org>
Cr-Commit-Position: refs/heads/master@{#748886}
parent eb91d635
...@@ -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,34 @@ class FileSystem(object): ...@@ -327,8 +329,34 @@ 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) assert not path.startswith(r'\\'), 'root path cannot be UNC-style'
path_abs = self.abspath(path)
# Ensure the root of the tree being rmtree'd is not a long path.
# We can't convert it to a long path (using _path_for_access),
# because long paths are not supported in 'rmdir' on Windows 7.
assert len(path_abs) < self.WINDOWS_MAX_PATH, 'root path is too long'
# Ensure (hopefully) that the quoting done on the next line is safe.
assert '"' not in path_abs, 'path contains a quotation mark (")'
# 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(path_abs)
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
self.assertTrue(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,18 @@ class RealFileSystemTest(unittest.TestCase, GenericFileSystemTests): ...@@ -361,3 +362,18 @@ 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_path is left in the filesystem and its removal is tested during cleanup.)
# On Windows, rmtree can handle trees containing long paths as long as
# the root is not a long path.
long_path1 = self.fs.join(self.generic_test_dir, 'a' * 100, 'b' * 100 + " 'b")
long_path2 = self.fs.join(long_path1, 'c' * 100)
self.fs.maybe_make_directory(long_path2)
file1 = self.fs.join(long_path2, 'foo')
self.fs.write_text_file(file1, 'hello')
if sys.platform == 'win32':
with self.assertRaises(AssertionError):
self.fs.rmtree(long_path2, ignore_errors=False)
else:
self.fs.rmtree(long_path2, ignore_errors=False)
self.fs.rmtree(long_path1, 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