Browse Source

Fix porcelain.add() to stage modified files when no paths specified

When called without paths, porcelain.add() now stages both untracked and
modified files from the entire repository, behaving like 'git add -A'.
Previously, only untracked files were staged, which was inconsistent
with Git's behavior.

Fixes #746
Jelmer Vernooij 2 months ago
parent
commit
a7ba57e8da
3 changed files with 107 additions and 47 deletions
  1. 5 0
      NEWS
  2. 31 17
      dulwich/porcelain.py
  3. 71 30
      tests/test_porcelain.py

+ 5 - 0
NEWS

@@ -1,5 +1,10 @@
 0.22.9	UNRELEASED
 0.22.9	UNRELEASED
 
 
+ * Fix ``porcelain.add()`` to stage both untracked and modified files when no
+   paths are specified. Previously, only untracked files were staged, inconsistent
+   with Git's behavior. Now behaves like ``git add -A`` when called without paths.
+   (Jelmer Vernooij, #746)
+
  * Fix ``porcelain.add()`` symlink handling to allow adding symlinks that point
  * Fix ``porcelain.add()`` symlink handling to allow adding symlinks that point
    outside the repository. Previously, the function would fail when trying to
    outside the repository. Previously, the function would fail when trying to
    add a symlink pointing outside the repo due to aggressive path resolution.
    add a symlink pointing outside the repo due to aggressive path resolution.

+ 31 - 17
dulwich/porcelain.py

@@ -587,7 +587,7 @@ def add(repo=".", paths=None):
 
 
     Args:
     Args:
       repo: Repository for the files
       repo: Repository for the files
-      paths: Paths to add. If None, stages all untracked files from the
+      paths: Paths to add. If None, stages all untracked and modified files from the
         current working directory (mimicking 'git add .' behavior).
         current working directory (mimicking 'git add .' behavior).
     Returns: Tuple with set of added files and ignored files
     Returns: Tuple with set of added files and ignored files
 
 
@@ -595,27 +595,23 @@ def add(repo=".", paths=None):
     contain the path to an ignored directory (with trailing slash). Individual
     contain the path to an ignored directory (with trailing slash). Individual
     files within ignored directories will not be returned.
     files within ignored directories will not be returned.
 
 
-    Note: When paths=None, this function respects the current working directory,
-    so if called from a subdirectory, it will only add files from that
-    subdirectory and below, matching Git's behavior.
+    Note: When paths=None, this function adds all untracked and modified files
+    from the entire repository, mimicking 'git add -A' behavior.
     """
     """
     ignored = set()
     ignored = set()
     with open_repo_closing(repo) as r:
     with open_repo_closing(repo) as r:
         repo_path = Path(r.path).resolve()
         repo_path = Path(r.path).resolve()
         ignore_manager = IgnoreFilterManager.from_repo(r)
         ignore_manager = IgnoreFilterManager.from_repo(r)
+
+        # Get unstaged changes once for the entire operation
+        index = r.open_index()
+        normalizer = r.get_blob_normalizer()
+        filter_callback = normalizer.checkin_normalize
+        all_unstaged_paths = list(get_unstaged_changes(index, r.path, filter_callback))
+
         if not paths:
         if not paths:
-            cwd = Path(os.getcwd()).resolve()
-            paths = list(
-                get_untracked_paths(
-                    str(cwd),
-                    str(repo_path),
-                    r.open_index(),
-                )
-            )
-            # If we're in a subdirectory, adjust paths to be relative to repo root
-            if cwd != repo_path:
-                cwd_relative_to_repo = cwd.relative_to(repo_path)
-                paths = [str(cwd_relative_to_repo / p) for p in paths]
+            # When no paths specified, add all untracked and modified files from repo root
+            paths = [str(repo_path)]
         relpaths = []
         relpaths = []
         if not isinstance(paths, list):
         if not isinstance(paths, list):
             paths = [paths]
             paths = [paths]
@@ -654,7 +650,7 @@ def add(repo=".", paths=None):
                     get_untracked_paths(
                     get_untracked_paths(
                         str(resolved_path),
                         str(resolved_path),
                         str(repo_path),
                         str(repo_path),
-                        r.open_index(),
+                        index,
                     )
                     )
                 )
                 )
                 for untracked_path in current_untracked:
                 for untracked_path in current_untracked:
@@ -666,6 +662,24 @@ def add(repo=".", paths=None):
                         relpaths.append(untracked_path)
                         relpaths.append(untracked_path)
                     else:
                     else:
                         ignored.add(untracked_path)
                         ignored.add(untracked_path)
+
+                # Also add unstaged (modified) files within this directory
+                for unstaged_path in all_unstaged_paths:
+                    if isinstance(unstaged_path, bytes):
+                        unstaged_path = unstaged_path.decode("utf-8")
+
+                    # Check if this unstaged file is within the directory we're processing
+                    unstaged_full_path = repo_path / unstaged_path
+                    try:
+                        unstaged_full_path.relative_to(resolved_path)
+                        # File is within this directory, add it
+                        if not ignore_manager.is_ignored(unstaged_path):
+                            relpaths.append(unstaged_path)
+                        else:
+                            ignored.add(unstaged_path)
+                    except ValueError:
+                        # File is not within this directory, skip it
+                        continue
                 continue
                 continue
 
 
             # FIXME: Support patterns
             # FIXME: Support patterns

+ 71 - 30
tests/test_porcelain.py

@@ -955,7 +955,8 @@ class AddTests(PorcelainTestCase):
             os.chdir(cwd)
             os.chdir(cwd)
 
 
         index = self.repo.open_index()
         index = self.repo.open_index()
-        self.assertEqual(sorted(index), [b"foo/blie"])
+        # After fix: add() with no paths should behave like git add -A (add everything)
+        self.assertEqual(sorted(index), [b"blah", b"foo/blie"])
 
 
     def test_add_file(self) -> None:
     def test_add_file(self) -> None:
         fullpath = os.path.join(self.repo.path, "foo")
         fullpath = os.path.join(self.repo.path, "foo")
@@ -1053,19 +1054,21 @@ class AddTests(PorcelainTestCase):
         target_file = os.path.join(self.repo.path, "target.txt")
         target_file = os.path.join(self.repo.path, "target.txt")
         with open(target_file, "w") as f:
         with open(target_file, "w") as f:
             f.write("target content")
             f.write("target content")
-        
+
         # Create a symlink to the file
         # Create a symlink to the file
         symlink_path = os.path.join(self.repo.path, "link_to_target")
         symlink_path = os.path.join(self.repo.path, "link_to_target")
         os.symlink("target.txt", symlink_path)
         os.symlink("target.txt", symlink_path)
-        
+
         # Add both the target and the symlink
         # Add both the target and the symlink
-        added, ignored = porcelain.add(self.repo.path, paths=[target_file, symlink_path])
-        
+        added, ignored = porcelain.add(
+            self.repo.path, paths=[target_file, symlink_path]
+        )
+
         # Both should be added successfully
         # Both should be added successfully
         self.assertIn("target.txt", added)
         self.assertIn("target.txt", added)
         self.assertIn("link_to_target", added)
         self.assertIn("link_to_target", added)
         self.assertEqual(len(ignored), 0)
         self.assertEqual(len(ignored), 0)
-        
+
         # Verify both are in the index
         # Verify both are in the index
         index = self.repo.open_index()
         index = self.repo.open_index()
         self.assertIn(b"target.txt", index)
         self.assertIn(b"target.txt", index)
@@ -1080,20 +1083,20 @@ class AddTests(PorcelainTestCase):
             f.write("content1")
             f.write("content1")
         with open(os.path.join(target_dir, "file2.txt"), "w") as f:
         with open(os.path.join(target_dir, "file2.txt"), "w") as f:
             f.write("content2")
             f.write("content2")
-        
+
         # Create a symlink to the directory
         # Create a symlink to the directory
         symlink_path = os.path.join(self.repo.path, "link_to_dir")
         symlink_path = os.path.join(self.repo.path, "link_to_dir")
         os.symlink("target_dir", symlink_path)
         os.symlink("target_dir", symlink_path)
-        
+
         # Add the symlink
         # Add the symlink
         added, ignored = porcelain.add(self.repo.path, paths=[symlink_path])
         added, ignored = porcelain.add(self.repo.path, paths=[symlink_path])
-        
+
         # When adding a symlink to a directory, it follows the symlink and adds contents
         # When adding a symlink to a directory, it follows the symlink and adds contents
         self.assertEqual(len(added), 2)
         self.assertEqual(len(added), 2)
         self.assertIn("link_to_dir/file1.txt", added)
         self.assertIn("link_to_dir/file1.txt", added)
         self.assertIn("link_to_dir/file2.txt", added)
         self.assertIn("link_to_dir/file2.txt", added)
         self.assertEqual(len(ignored), 0)
         self.assertEqual(len(ignored), 0)
-        
+
         # Verify files are added through the symlink path
         # Verify files are added through the symlink path
         index = self.repo.open_index()
         index = self.repo.open_index()
         self.assertIn(b"link_to_dir/file1.txt", index)
         self.assertIn(b"link_to_dir/file1.txt", index)
@@ -1108,24 +1111,26 @@ class AddTests(PorcelainTestCase):
         target_file = os.path.join(self.repo.path, "original.txt")
         target_file = os.path.join(self.repo.path, "original.txt")
         with open(target_file, "w") as f:
         with open(target_file, "w") as f:
             f.write("original content")
             f.write("original content")
-        
+
         # Create first symlink
         # Create first symlink
         first_link = os.path.join(self.repo.path, "link1")
         first_link = os.path.join(self.repo.path, "link1")
         os.symlink("original.txt", first_link)
         os.symlink("original.txt", first_link)
-        
+
         # Create second symlink pointing to first
         # Create second symlink pointing to first
         second_link = os.path.join(self.repo.path, "link2")
         second_link = os.path.join(self.repo.path, "link2")
         os.symlink("link1", second_link)
         os.symlink("link1", second_link)
-        
+
         # Add all files
         # Add all files
-        added, ignored = porcelain.add(self.repo.path, paths=[target_file, first_link, second_link])
-        
+        added, ignored = porcelain.add(
+            self.repo.path, paths=[target_file, first_link, second_link]
+        )
+
         # All should be added
         # All should be added
         self.assertEqual(len(added), 3)
         self.assertEqual(len(added), 3)
         self.assertIn("original.txt", added)
         self.assertIn("original.txt", added)
         self.assertIn("link1", added)
         self.assertIn("link1", added)
         self.assertIn("link2", added)
         self.assertIn("link2", added)
-        
+
         # Verify all are in the index
         # Verify all are in the index
         index = self.repo.open_index()
         index = self.repo.open_index()
         self.assertIn(b"original.txt", index)
         self.assertIn(b"original.txt", index)
@@ -1137,14 +1142,14 @@ class AddTests(PorcelainTestCase):
         # Create a symlink to a non-existent file
         # Create a symlink to a non-existent file
         broken_link = os.path.join(self.repo.path, "broken_link")
         broken_link = os.path.join(self.repo.path, "broken_link")
         os.symlink("does_not_exist.txt", broken_link)
         os.symlink("does_not_exist.txt", broken_link)
-        
+
         # Add the broken symlink
         # Add the broken symlink
         added, ignored = porcelain.add(self.repo.path, paths=[broken_link])
         added, ignored = porcelain.add(self.repo.path, paths=[broken_link])
-        
+
         # Should be added successfully (Git tracks the symlink, not its target)
         # Should be added successfully (Git tracks the symlink, not its target)
         self.assertIn("broken_link", added)
         self.assertIn("broken_link", added)
         self.assertEqual(len(ignored), 0)
         self.assertEqual(len(ignored), 0)
-        
+
         # Verify it's in the index
         # Verify it's in the index
         index = self.repo.open_index()
         index = self.repo.open_index()
         self.assertIn(b"broken_link", index)
         self.assertIn(b"broken_link", index)
@@ -1155,18 +1160,18 @@ class AddTests(PorcelainTestCase):
         outside_file = os.path.join(self.test_dir, "outside.txt")
         outside_file = os.path.join(self.test_dir, "outside.txt")
         with open(outside_file, "w") as f:
         with open(outside_file, "w") as f:
             f.write("outside content")
             f.write("outside content")
-        
+
         # Create a symlink using relative path to go outside
         # Create a symlink using relative path to go outside
         symlink_path = os.path.join(self.repo.path, "link_outside")
         symlink_path = os.path.join(self.repo.path, "link_outside")
         os.symlink("../outside.txt", symlink_path)
         os.symlink("../outside.txt", symlink_path)
-        
+
         # Add the symlink
         # Add the symlink
         added, ignored = porcelain.add(self.repo.path, paths=[symlink_path])
         added, ignored = porcelain.add(self.repo.path, paths=[symlink_path])
-        
+
         # Should be added successfully
         # Should be added successfully
         self.assertIn("link_outside", added)
         self.assertIn("link_outside", added)
         self.assertEqual(len(ignored), 0)
         self.assertEqual(len(ignored), 0)
-        
+
         # Verify it's in the index
         # Verify it's in the index
         index = self.repo.open_index()
         index = self.repo.open_index()
         self.assertIn(b"link_outside", index)
         self.assertIn(b"link_outside", index)
@@ -1176,11 +1181,11 @@ class AddTests(PorcelainTestCase):
         # Create a symlink to a system directory
         # Create a symlink to a system directory
         symlink_path = os.path.join(self.repo.path, "link_to_tmp")
         symlink_path = os.path.join(self.repo.path, "link_to_tmp")
         os.symlink("/tmp", symlink_path)
         os.symlink("/tmp", symlink_path)
-        
+
         # Adding a symlink to a directory outside the repo should raise ValueError
         # Adding a symlink to a directory outside the repo should raise ValueError
         with self.assertRaises(ValueError) as cm:
         with self.assertRaises(ValueError) as cm:
             porcelain.add(self.repo.path, paths=[symlink_path])
             porcelain.add(self.repo.path, paths=[symlink_path])
-        
+
         # Check that the error indicates the path is outside the repository
         # Check that the error indicates the path is outside the repository
         self.assertIn("is not in the subpath of", str(cm.exception))
         self.assertIn("is not in the subpath of", str(cm.exception))
 
 
@@ -1192,21 +1197,21 @@ class AddTests(PorcelainTestCase):
         real_file = os.path.join(real_dir, "file.txt")
         real_file = os.path.join(real_dir, "file.txt")
         with open(real_file, "w") as f:
         with open(real_file, "w") as f:
             f.write("content")
             f.write("content")
-        
+
         # Create a symlink to the directory
         # Create a symlink to the directory
         link_dir = os.path.join(self.repo.path, "link_dir")
         link_dir = os.path.join(self.repo.path, "link_dir")
         os.symlink("real_dir", link_dir)
         os.symlink("real_dir", link_dir)
-        
+
         # Try to add the file through the symlink path
         # Try to add the file through the symlink path
         symlink_file_path = os.path.join(link_dir, "file.txt")
         symlink_file_path = os.path.join(link_dir, "file.txt")
-        
+
         # This should add the real file, not create a new entry
         # This should add the real file, not create a new entry
         added, ignored = porcelain.add(self.repo.path, paths=[symlink_file_path])
         added, ignored = porcelain.add(self.repo.path, paths=[symlink_file_path])
-        
+
         # The real file should be added
         # The real file should be added
         self.assertIn("real_dir/file.txt", added)
         self.assertIn("real_dir/file.txt", added)
         self.assertEqual(len(added), 1)
         self.assertEqual(len(added), 1)
-        
+
         # Verify correct path in index
         # Verify correct path in index
         index = self.repo.open_index()
         index = self.repo.open_index()
         self.assertIn(b"real_dir/file.txt", index)
         self.assertIn(b"real_dir/file.txt", index)
@@ -1406,6 +1411,42 @@ class AddTests(PorcelainTestCase):
         index = self.repo.open_index()
         index = self.repo.open_index()
         self.assertEqual(len(index), 6)
         self.assertEqual(len(index), 6)
 
 
+    def test_add_default_paths_includes_modified_files(self) -> None:
+        """Test that add() with no paths includes both untracked and modified files."""
+        # Create and commit initial file
+        initial_file = os.path.join(self.repo.path, "existing.txt")
+        with open(initial_file, "w") as f:
+            f.write("initial content\n")
+        porcelain.add(repo=self.repo.path, paths=[initial_file])
+        porcelain.commit(
+            repo=self.repo.path,
+            message=b"initial commit",
+            author=b"test <email>",
+            committer=b"test <email>",
+        )
+
+        # Modify the existing file (this creates an unstaged change)
+        with open(initial_file, "w") as f:
+            f.write("modified content\n")
+
+        # Create a new untracked file
+        new_file = os.path.join(self.repo.path, "new.txt")
+        with open(new_file, "w") as f:
+            f.write("new file content\n")
+
+        # Call add() with no paths - should stage both modified and untracked files
+        added_files, ignored_files = porcelain.add(repo=self.repo.path)
+
+        # Verify both files were added
+        self.assertIn("existing.txt", added_files)
+        self.assertIn("new.txt", added_files)
+        self.assertEqual(len(ignored_files), 0)
+
+        # Verify both files are now staged
+        index = self.repo.open_index()
+        self.assertIn(b"existing.txt", index)
+        self.assertIn(b"new.txt", index)
+
 
 
 class RemoveTests(PorcelainTestCase):
 class RemoveTests(PorcelainTestCase):
     def test_remove_file(self) -> None:
     def test_remove_file(self) -> None: