Bug 1230060 - Allow to restrict what unaccounted files to remove when copying from a FileCopier. r=gps
☠☠ backed out by 006d8b1fd2ff ☠ ☠
authorMike Hommey <mh+mozilla@glandium.org>
Thu, 03 Dec 2015 15:02:50 +0900
changeset 309851 0dcea48cbf382f68b8c74ed454184413ff83602a
parent 309850 611771d8de5857df525ab3dbfd44804bd38984c8
child 309852 9d1f7f04a68283d91ea243ffbf4d37d9b5e2c419
push id5513
push userraliiev@mozilla.com
push dateMon, 25 Jan 2016 13:55:34 +0000
treeherdermozilla-beta@5ee97dd05b5c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgps
bugs1230060
milestone45.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 1230060 - Allow to restrict what unaccounted files to remove when copying from a FileCopier. r=gps The default behavior for a FileCopier's copy is to remove all the files and directories in the destination that aren't in its registry. The remove_unaccounted argument can be passed as False to disable this behavior. This change adds another possibility, where remove_unaccounted may be a FileRegistry, in which case only the files in that registry are removed. This allows to e.g. only remove files that were copied from a previous FileCopier.copy, leaving aside files that were in the destination for some other reason.
python/mozbuild/mozpack/copier.py
python/mozbuild/mozpack/test/test_copier.py
--- a/python/mozbuild/mozpack/copier.py
+++ b/python/mozbuild/mozpack/copier.py
@@ -326,51 +326,60 @@ class FileCopier(FileRegistry):
                     os.remove(d)
                     os.mkdir(d)
 
             if not os.access(d, os.W_OK):
                 umask = os.umask(0077)
                 os.umask(umask)
                 os.chmod(d, 0777 & ~umask)
 
-        # While we have remove_unaccounted, it doesn't apply to empty
-        # directories because it wouldn't make sense: an empty directory
-        # is empty, so removing it should have no effect.
-        existing_dirs = set()
-        existing_files = set()
-        for root, dirs, files in os.walk(destination):
-            # We need to perform the same symlink detection as above. os.walk()
-            # doesn't follow symlinks into directories by default, so we need
-            # to check dirs (we can't wait for root).
-            if have_symlinks:
-                filtered = []
+        if isinstance(remove_unaccounted, FileRegistry):
+            existing_files = set(os.path.normpath(os.path.join(destination, p))
+                                 for p in remove_unaccounted.paths())
+            existing_dirs = set(os.path.normpath(os.path.join(destination, p))
+                                for p in remove_unaccounted
+                                         .required_directories())
+            existing_dirs |= {os.path.normpath(destination)}
+        else:
+            # While we have remove_unaccounted, it doesn't apply to empty
+            # directories because it wouldn't make sense: an empty directory
+            # is empty, so removing it should have no effect.
+            existing_dirs = set()
+            existing_files = set()
+            for root, dirs, files in os.walk(destination):
+                # We need to perform the same symlink detection as above.
+                # os.walk() doesn't follow symlinks into directories by
+                # default, so we need to check dirs (we can't wait for root).
+                if have_symlinks:
+                    filtered = []
+                    for d in dirs:
+                        full = os.path.join(root, d)
+                        st = os.lstat(full)
+                        if stat.S_ISLNK(st.st_mode):
+                            # This directory symlink is not a required
+                            # directory: any such symlink would have been
+                            # removed and a directory created above.
+                            if remove_all_directory_symlinks:
+                                os.remove(full)
+                                result.removed_files.add(
+                                    os.path.normpath(full))
+                            else:
+                                existing_files.add(os.path.normpath(full))
+                        else:
+                            filtered.append(d)
+
+                    dirs[:] = filtered
+
+                existing_dirs.add(os.path.normpath(root))
+
                 for d in dirs:
-                    full = os.path.join(root, d)
-                    st = os.lstat(full)
-                    if stat.S_ISLNK(st.st_mode):
-                        # This directory symlink is not a required
-                        # directory: any such symlink would have been
-                        # removed and a directory created above.
-                        if remove_all_directory_symlinks:
-                            os.remove(full)
-                            result.removed_files.add(os.path.normpath(full))
-                        else:
-                            existing_files.add(os.path.normpath(full))
-                    else:
-                        filtered.append(d)
+                    existing_dirs.add(os.path.normpath(os.path.join(root, d)))
 
-                dirs[:] = filtered
-
-            existing_dirs.add(os.path.normpath(root))
-
-            for d in dirs:
-                existing_dirs.add(os.path.normpath(os.path.join(root, d)))
-
-            for f in files:
-                existing_files.add(os.path.normpath(os.path.join(root, f)))
+                for f in files:
+                    existing_files.add(os.path.normpath(os.path.join(root, f)))
 
         # Now we reconcile the state of the world against what we want.
 
         # Install files.
         for p, f in self:
             destfile = os.path.normpath(os.path.join(destination, p))
             if f.copy(destfile, skip_if_older):
                 result.updated_files.add(destfile)
@@ -415,20 +424,35 @@ class FileCopier(FileRegistry):
                         break
 
                     previous = parent
 
                 remove_dirs -= parents
 
         # Remove empty directories that aren't required.
         for d in sorted(remove_dirs, key=len, reverse=True):
-            # Permissions may not allow deletion. So ensure write access is
-            # in place before attempting delete.
-            os.chmod(d, 0700)
-            os.rmdir(d)
+            try:
+                try:
+                    os.rmdir(d)
+                except OSError as e:
+                    if e.errno in (errno.EPERM, errno.EACCES):
+                        # Permissions may not allow deletion. So ensure write
+                        # access is in place before attempting to rmdir again.
+                        os.chmod(d, 0700)
+                        os.rmdir(d)
+                    else:
+                        raise
+            except OSError as e:
+                # If remove_unaccounted is a # FileRegistry, then we have a
+                # list of directories that may not be empty, so ignore rmdir
+                # ENOTEMPTY errors for them.
+                if (isinstance(remove_unaccounted, FileRegistry) and
+                        e.errno == errno.ENOTEMPTY):
+                    continue
+                raise
             result.removed_directories.add(d)
 
         return result
 
 
 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
@@ -416,16 +416,60 @@ class TestFileCopier(TestWithTmpDir):
 
         # What's worse, we have no record that dest was created.
         self.assertEquals(len(result.updated_files), 0)
 
         # But we do have an erroneous record of an optional file
         # existing when it does not.
         self.assertIn(self.tmppath('dest/foo/bar'), result.existing_files)
 
+    def test_remove_unaccounted_file_registry(self):
+        """Test FileCopier.copy(remove_unaccounted=FileRegistry())"""
+
+        dest = self.tmppath('dest')
+
+        copier = FileCopier()
+        copier.add('foo/bar/baz', GeneratedFile('foobarbaz'))
+        copier.add('foo/bar/qux', GeneratedFile('foobarqux'))
+        copier.add('foo/hoge/fuga', GeneratedFile('foohogefuga'))
+        copier.add('foo/toto/tata', GeneratedFile('footototata'))
+
+        os.makedirs(os.path.join(dest, 'bar'))
+        with open(os.path.join(dest, 'bar', 'bar'), 'w') as fh:
+            fh.write('barbar');
+        os.makedirs(os.path.join(dest, 'foo', 'toto'))
+        with open(os.path.join(dest, 'foo', 'toto', 'toto'), 'w') as fh:
+            fh.write('foototototo');
+
+        result = copier.copy(dest, remove_unaccounted=False)
+
+        self.assertEqual(self.all_files(dest),
+                         set(copier.paths()) | { 'foo/toto/toto', 'bar/bar'})
+        self.assertEqual(self.all_dirs(dest),
+                         {'foo/bar', 'foo/hoge', 'foo/toto', 'bar'})
+
+        copier2 = FileCopier()
+        copier2.add('foo/hoge/fuga', GeneratedFile('foohogefuga'))
+
+        # We expect only files copied from the first copier to be removed,
+        # not the extra file that was there beforehand.
+        result = copier2.copy(dest, remove_unaccounted=copier)
+
+        self.assertEqual(self.all_files(dest),
+                         set(copier2.paths()) | { 'foo/toto/toto', 'bar/bar'})
+        self.assertEqual(self.all_dirs(dest),
+                         {'foo/hoge', 'foo/toto', 'bar'})
+        self.assertEqual(result.updated_files,
+                         {self.tmppath('dest/foo/hoge/fuga')})
+        self.assertEqual(result.existing_files, set())
+        self.assertEqual(result.removed_files, {self.tmppath(p) for p in
+            ('dest/foo/bar/baz', 'dest/foo/bar/qux', 'dest/foo/toto/tata')})
+        self.assertEqual(result.removed_directories,
+                         {self.tmppath('dest/foo/bar')})
+
 
 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'):