Actually remove files from disk in ``dulwich.porcelain.remove``. Fixes

NEWS

@@ -17,6 +17,9 @@
   * Properly deal with submodules in 'porcelain.status'.
     (Jelmer Vernooij, #517)
 
+  * ``dulwich.porcelain.remove`` now actually removes files from
+    disk, not just from the index. (Jelmer Vernooij, #488)
+
  IMPROVEMENTS
 
   * Add basic support for reading ignore files in ``dulwich.ignore``.

dulwich/porcelain.py

@@ -81,7 +81,13 @@ from dulwich.errors import (
     UpdateRefsError,
     )
 from dulwich.ignore import IgnoreFilterManager
-from dulwich.index import get_unstaged_changes
+from dulwich.index import (
+    blob_from_path_and_stat,
+    get_unstaged_changes,
+    )
+from dulwich.object_store import (
+    tree_lookup_path,
+    )
 from dulwich.objects import (
     Commit,
     Tag,
@@ -353,7 +359,7 @@ def add(repo=".", paths=None):
     return (relpaths, ignored)
 
 
-def rm(repo=".", paths=None):
+def remove(repo=".", paths=None, cached=False):
     """Remove files from the staging area.
 
     :param repo: Repository for the files
@@ -362,11 +368,46 @@ def rm(repo=".", paths=None):
     with open_repo_closing(repo) as r:
         index = r.open_index()
         for p in paths:
-            p = path_to_tree_path(r, p)
-            del index[p]
+            full_path = os.path.abspath(p).encode(sys.getfilesystemencoding())
+            index_path = path_to_tree_path(r, p)
+            try:
+                index_sha = index[index_path].sha
+            except KeyError:
+                raise Exception('%s did not match any files' % p)
+
+            if not cached:
+                try:
+                    st = os.lstat(full_path)
+                except OSError:
+                    pass
+                else:
+                    try:
+                        blob = blob_from_path_and_stat(full_path, st)
+                    except IOError:
+                        pass
+                    else:
+                        try:
+                            committed_sha = tree_lookup_path(
+                                r.__getitem__, r[r.head()].tree, p)[1]
+                        except KeyError:
+                            committed_sha = None
+
+                        if blob.id != index_sha and index_sha != committed_sha:
+                            raise Exception(
+                                'file has staged content differing '
+                                'from both the file and head: %s' % p)
+
+                        if index_sha != committed_sha:
+                            raise Exception(
+                                'file has staged changes: %s' % p)
+                        os.remove(full_path)
+            del index[index_path]
         index.write()
 
 
+rm = remove
+
+
 def commit_decode(commit, contents, default_encoding=DEFAULT_ENCODING):
     if commit.encoding is not None:
         return contents.decode(commit.encoding, "replace")

dulwich/tests/test_ignore.py

@@ -218,7 +218,7 @@ class IgnoreFilterManagerTests(TestCase):
         with open(p, 'wb') as f:
             f.write(b'/excluded\n')
         m = IgnoreFilterManager.from_repo(repo)
-        self.assertTrue(m.is_ignored(os.path.join('dir', 'blie')))
+        self.assertTrue(m.is_ignored('dir/blie'))
         self.assertIs(None,
                       m.is_ignored(os.path.join(repo.path, 'dir', 'bloe')))
         self.assertIs(None, m.is_ignored(os.path.join(repo.path, 'dir')))

dulwich/tests/test_porcelain.py

@@ -302,7 +302,28 @@ class RemoveTests(PorcelainTestCase):
         with open(os.path.join(self.repo.path, 'foo'), 'w') as f:
             f.write("BAR")
         porcelain.add(self.repo.path, paths=["foo"])
-        porcelain.rm(self.repo.path, paths=["foo"])
+        porcelain.commit(repo=self.repo, message=b'test', author=b'test',
+                         committer=b'test')
+        self.assertTrue(os.path.exists(os.path.join(self.repo.path, 'foo')))
+        cwd = os.getcwd()
+        try:
+            os.chdir(self.repo.path)
+            porcelain.remove(self.repo.path, paths=["foo"])
+        finally:
+            os.chdir(cwd)
+        self.assertFalse(os.path.exists(os.path.join(self.repo.path, 'foo')))
+
+    def test_remove_file_staged(self):
+        with open(os.path.join(self.repo.path, 'foo'), 'w') as f:
+            f.write("BAR")
+        cwd = os.getcwd()
+        try:
+            os.chdir(self.repo.path)
+            porcelain.add(self.repo.path, paths=["foo"])
+            self.assertRaises(Exception, porcelain.rm, self.repo.path,
+                              paths=["foo"])
+        finally:
+            os.chdir(cwd)
 
 
 class LogTests(PorcelainTestCase):
@@ -780,7 +801,12 @@ class StatusTests(PorcelainTestCase):
         porcelain.add(repo=self.repo.path, paths=filename)
         porcelain.commit(repo=self.repo.path, message=b'test status',
                          author=b'', committer=b'')
-        porcelain.rm(repo=self.repo.path, paths=[filename])
+        cwd = os.getcwd()
+        try:
+            os.chdir(self.repo.path)
+            porcelain.remove(repo=self.repo.path, paths=[filename])
+        finally:
+            os.chdir(cwd)
         changes = porcelain.get_tree_changes(self.repo.path)
 
         self.assertEqual(changes['delete'][0], filename.encode('ascii'))