Bug 884587 - Part 0: Teach FileCopier how to remove unwritable files on Windows; r=glandium
authorGregory Szorc <gps@mozilla.com>
Fri, 28 Jun 2013 14:46:56 -0700
changeset 136851 92742446068906bc26150082747401600ef57d43
parent 136850 aeb89583349d19af89bd31420c67b4bcc3619e21
child 136852 b02a95fdd518633d98aec2df10bbf6c62a0c4851
push id24898
push userphilringnalda@gmail.com
push dateSat, 29 Jun 2013 13:54:45 +0000
treeherdermozilla-central@cbb24a4a96af [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersglandium
bugs884587
milestone25.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 884587 - Part 0: Teach FileCopier how to remove unwritable files on Windows; r=glandium
python/mozbuild/mozpack/copier.py
python/mozbuild/mozpack/test/test_copier.py
python/mozbuild/mozpack/test/test_files.py
--- a/python/mozbuild/mozpack/copier.py
+++ b/python/mozbuild/mozpack/copier.py
@@ -14,24 +14,30 @@ from collections import (
     namedtuple,
     OrderedDict,
 )
 
 
 def ensure_parent_dir(file):
     '''Ensures the directory parent to the given file exists'''
     dir = os.path.dirname(file)
-    if not dir or os.path.exists(dir):
+    if not dir:
         return
+
     try:
         os.makedirs(dir)
     except OSError as error:
         if error.errno != errno.EEXIST:
             raise
 
+    if not os.access(dir, os.W_OK):
+        umask = os.umask(0077)
+        os.umask(umask)
+        os.chmod(dir, 0777 & ~umask)
+
 
 class FileRegistry(object):
     '''
     Generic container to keep track of a set of BaseFile instances. It
     preserves the order under which the files are added, but doesn't keep
     track of empty directories (directories are not stored at all).
     The paths associated with the BaseFile instances are relative to an
     unspecified (virtual) root directory.
@@ -163,22 +169,34 @@ class FileCopier(FileRegistry):
         for root, dirs, files in os.walk(destination):
             for f in files:
                 actual_dest_files.add(os.path.normpath(os.path.join(root, f)))
 
         file_remove_count = 0
         directory_remove_count = 0
 
         for f in actual_dest_files - dest_files:
+            # Windows requires write access to remove files.
+            if os.name == 'nt' and not os.access(f, os.W_OK):
+                # It doesn't matter what we set the permissions to since we
+                # will remove this file shortly.
+                os.chmod(f, 0600)
+
             os.remove(f)
             file_remove_count += 1
+
         for root, dirs, files in os.walk(destination):
-            if not files and not dirs:
-                os.removedirs(root)
-                directory_remove_count += 1
+            if files or dirs:
+                continue
+
+            # Like files, permissions may not allow deletion. So, ensure write
+            # access is in place before attempting delete.
+            os.chmod(root, 0700)
+            os.removedirs(root)
+            directory_remove_count += 1
 
         return FileCopyResult(removed_files_count=file_remove_count,
             removed_directories_count=directory_remove_count)
 
 
 class FilePurger(FileCopier):
     """A variation of FileCopier that is used to purge untracked files.
 
--- a/python/mozbuild/mozpack/test/test_copier.py
+++ b/python/mozbuild/mozpack/test/test_copier.py
@@ -85,23 +85,17 @@ class TestFileRegistry(MatchTestTemplate
 
         self.assertEqual([f for f, c in self.registry], ['bar', 'foo/qux'])
         self.assertEqual(len(self.registry), 2)
 
         self.add('foo/.foo')
         self.assertTrue(self.registry.contains('foo/.foo'))
 
 
-class TestFileCopier(unittest.TestCase):
-    def setUp(self):
-        self.tmpdir = mkdtemp()
-
-    def tearDown(self):
-        shutil.rmtree(self.tmpdir)
-
+class TestFileCopier(TestWithTmpDir):
     def all_dirs(self, base):
         all_dirs = set()
         for root, dirs, files in os.walk(base):
             if not dirs:
                 all_dirs.add(mozpack.path.relpath(root, base))
         return all_dirs
 
     def all_files(self, base):
@@ -127,16 +121,37 @@ class TestFileCopier(unittest.TestCase):
                          set(['foo/deep/nested/directory', 'qux']))
 
         copier.remove('foo')
         copier.add('test', GeneratedFile('test'))
         copier.copy(self.tmpdir)
         self.assertEqual(self.all_files(self.tmpdir), set(copier.paths()))
         self.assertEqual(self.all_dirs(self.tmpdir), set(['qux']))
 
+    def test_permissions(self):
+        """Ensure files without write permission can be deleted."""
+        with open(self.tmppath('dummy'), 'a'):
+            pass
+
+        p = self.tmppath('no_perms')
+        with open(p, 'a'):
+            pass
+
+        # Make file and directory unwritable. Reminder: making a directory
+        # unwritable prevents modifications (including deletes) from the list
+        # of files in that directory.
+        os.chmod(p, 0400)
+        os.chmod(self.tmpdir, 0400)
+
+        copier = FileCopier()
+        copier.add('dummy', GeneratedFile('content'))
+        result = copier.copy(self.tmpdir)
+        self.assertEqual(result.removed_files_count, 1)
+        self.assertFalse(os.path.exists(p))
+
 
 class TestFilePurger(TestWithTmpDir):
     def test_file_purger(self):
         existing = os.path.join(self.tmpdir, 'existing')
         extra = os.path.join(self.tmpdir, 'extra')
         empty_dir = os.path.join(self.tmpdir, 'dir')
 
         with open(existing, 'a'):
--- a/python/mozbuild/mozpack/test/test_files.py
+++ b/python/mozbuild/mozpack/test/test_files.py
@@ -19,16 +19,17 @@ from mozpack.mozjar import (
 )
 from mozpack.chrome.manifest import (
     ManifestContent,
     ManifestResource,
     ManifestLocale,
     ManifestOverride,
 )
 import unittest
+import mozfile
 import mozunit
 import os
 import shutil
 import random
 import string
 import mozpack.path
 from mozpack.copier import ensure_parent_dir
 from tempfile import mkdtemp
@@ -36,17 +37,17 @@ from io import BytesIO
 from xpt import Typelib
 
 
 class TestWithTmpDir(unittest.TestCase):
     def setUp(self):
         self.tmpdir = mkdtemp()
 
     def tearDown(self):
-        shutil.rmtree(self.tmpdir)
+        mozfile.rmtree(self.tmpdir)
 
     def tmppath(self, relpath):
         return os.path.normpath(os.path.join(self.tmpdir, relpath))
 
 
 class MockDest(BytesIO, Dest):
     def __init__(self):
         BytesIO.__init__(self)