Ver Fonte

Fix porcelain.remove() to work from any directory (#1627)

Previously, porcelain.remove() would fail when called from a directory
other than the repository root because it interpreted relative paths as
relative to the current working directory. This change makes relative
paths relative to the repository root.

Fixes #821
Jelmer Vernooij há 1 mês atrás
pai
commit
643ed8555a
3 ficheiros alterados com 78 adições e 6 exclusões
  1. 5 0
      NEWS
  2. 13 6
      dulwich/porcelain.py
  3. 60 0
      tests/test_porcelain.py

+ 5 - 0
NEWS

@@ -53,6 +53,11 @@
    behavior. Also added ``prune`` command to ``dulwich.porcelain``.
    (Jelmer Vernooij, #558)
 
+ * Fix ``porcelain.remove()`` to work correctly when called from a directory
+   other than the repository root. Relative paths are now interpreted as
+   relative to the repository root rather than the current working directory.
+   (Jelmer Vernooij, #821)
+
  * Add support for auto garbage collection, and invoke from
    some porcelain commands. (Jelmer Vernooij, #1600)
 

+ 13 - 6
dulwich/porcelain.py

@@ -779,13 +779,20 @@ def remove(repo=".", paths=None, cached=False) -> None:
 
     Args:
       repo: Repository for the files
-      paths: Paths to remove
+      paths: Paths to remove. Can be absolute or relative to the repository root.
     """
     with open_repo_closing(repo) as r:
         index = r.open_index()
         for p in paths:
-            full_path = os.fsencode(os.path.abspath(p))
-            tree_path = path_to_tree_path(r.path, p)
+            # If path is absolute, use it as-is. Otherwise, treat it as relative to repo
+            if os.path.isabs(p):
+                full_path = p
+            else:
+                # Treat relative paths as relative to the repository root
+                full_path = os.path.join(r.path, p)
+            tree_path = path_to_tree_path(r.path, full_path)
+            # Convert to bytes for file operations
+            full_path_bytes = os.fsencode(full_path)
             try:
                 index_sha = index[tree_path].sha
             except KeyError as exc:
@@ -793,12 +800,12 @@ def remove(repo=".", paths=None, cached=False) -> None:
 
             if not cached:
                 try:
-                    st = os.lstat(full_path)
+                    st = os.lstat(full_path_bytes)
                 except OSError:
                     pass
                 else:
                     try:
-                        blob = blob_from_path_and_stat(full_path, st)
+                        blob = blob_from_path_and_stat(full_path_bytes, st)
                     except OSError:
                         pass
                     else:
@@ -817,7 +824,7 @@ def remove(repo=".", paths=None, cached=False) -> None:
 
                         if index_sha != committed_sha:
                             raise Error(f"file has staged changes: {p}")
-                        os.remove(full_path)
+                        os.remove(full_path_bytes)
             del index[tree_path]
         index.write()
 

+ 60 - 0
tests/test_porcelain.py

@@ -1549,6 +1549,66 @@ class RemoveTests(PorcelainTestCase):
             os.chdir(cwd)
         self.assertFalse(os.path.exists(os.path.join(self.repo.path, "foo")))
 
+    def test_remove_from_different_directory(self) -> None:
+        # Create a subdirectory with a file
+        subdir = os.path.join(self.repo.path, "mydir")
+        os.makedirs(subdir)
+        fullpath = os.path.join(subdir, "myfile")
+        with open(fullpath, "w") as f:
+            f.write("BAR")
+
+        # Add and commit the file
+        porcelain.add(self.repo.path, paths=[fullpath])
+        porcelain.commit(
+            repo=self.repo,
+            message=b"test",
+            author=b"test <email>",
+            committer=b"test <email>",
+        )
+
+        # Change to a different directory
+        cwd = os.getcwd()
+        tempdir = tempfile.mkdtemp()
+        try:
+            os.chdir(tempdir)
+            # Remove the file using relative path from repository root
+            porcelain.remove(self.repo.path, paths=["mydir/myfile"])
+        finally:
+            os.chdir(cwd)
+            os.rmdir(tempdir)
+
+        # Verify file was removed
+        self.assertFalse(os.path.exists(fullpath))
+
+    def test_remove_with_absolute_path(self) -> None:
+        # Create a file
+        fullpath = os.path.join(self.repo.path, "foo")
+        with open(fullpath, "w") as f:
+            f.write("BAR")
+
+        # Add and commit the file
+        porcelain.add(self.repo.path, paths=[fullpath])
+        porcelain.commit(
+            repo=self.repo,
+            message=b"test",
+            author=b"test <email>",
+            committer=b"test <email>",
+        )
+
+        # Change to a different directory
+        cwd = os.getcwd()
+        tempdir = tempfile.mkdtemp()
+        try:
+            os.chdir(tempdir)
+            # Remove the file using absolute path
+            porcelain.remove(self.repo.path, paths=[fullpath])
+        finally:
+            os.chdir(cwd)
+            os.rmdir(tempdir)
+
+        # Verify file was removed
+        self.assertFalse(os.path.exists(fullpath))
+
 
 class LogTests(PorcelainTestCase):
     def test_simple(self) -> None: