amend: Fix last-touched when deleting files

revup/amend.py

@@ -181,7 +181,9 @@ async def rebuild_stack_last_touched(
     if not files_to_amend:
         return GitCommitHash("")
 
-    # Get index entries for each staged file (format: <mode> <blob> <stage>\t<path>)
+    # Get index entries for each staged file (format: <mode> <blob> <stage>\t<path>).
+    # A staged deletion has no entry here; its path stays out of staged_entries so
+    # the overlay omits it, which the pre-image merge base below turns into a removal.
     staged_entries: Dict[str, str] = {}
     ls_output = await git_ctx.git_stdout("ls-files", "--stage", "--", *files_to_amend)
     for line in ls_output.split("\n"):
@@ -199,11 +201,21 @@ async def rebuild_stack_last_touched(
         if i not in commit_to_files:
             continue
 
-        # Build a tree of just this commit's staged files and overlay it onto the
-        # cherry-picked commit. The merge base is the empty tree, so --theirs will
-        # always take the incoming version and will never conflict.
+        # Overlay the staged version of this commit's files onto the cherry-picked
+        # commit via a merge with --theirs, which takes the staged (incoming) side.
+        # Modified/added files resolve against an empty base as add/add. Deletions
+        # can't be expressed by a tree, so the base carries the pre-image of only
+        # the deleted files: absent from the overlay, present in base == ours, they
+        # merge to a removal without conflict. With no deletions the base is empty
+        # and no extra git calls are made.
+        deleted = [f for f in commit_to_files[i] if f not in staged_entries]
+        base_tree = (
+            await git_ctx.make_tree_from_paths(await git_ctx.to_tree(new_commit), deleted)
+            if deleted
+            else await git_ctx.empty_tree()
+        )
         overlay = await git_ctx.make_tree_from_index_entries(
-            [staged_entries[f] for f in commit_to_files[i]]
+            [staged_entries[f] for f in commit_to_files[i] if f in staged_entries]
         )
         amended = CommitHeader(GitTreeHash(""), [rebuilt_parent])
         amended.author_name = commit_obj.author_name
@@ -218,7 +230,7 @@ async def rebuild_stack_last_touched(
             new_commit,
             GitCommitHash(overlay),
             amended,
-            GitCommitHash(await git_ctx.empty_tree()),
+            GitCommitHash(base_tree),
             "theirs",
         )
 

revup/git.py

@@ -617,10 +617,29 @@ async def build(node: Dict[str, Any]) -> GitTreeHash:
                     continue
                 subtree = await build(child)
                 lines.append(f"040000 tree {subtree}\t{name}")
-            return GitTreeHash(await self.git_stdout("mktree", input_str="\n".join(lines) + "\n"))
+            # An empty input produces the empty tree; a lone blank line is an error.
+            input_str = "\n".join(lines) + "\n" if lines else ""
+            return GitTreeHash(await self.git_stdout("mktree", input_str=input_str))
 
         return await build(root)
 
+    async def make_tree_from_paths(self, tree: GitTreeHash, paths: List[str]) -> GitTreeHash:
+        """
+        Build a tree containing only `paths`, each with the content it has in
+        `tree`. Returns the empty tree if `paths` is empty.
+        """
+        if not paths:
+            return await self.empty_tree()
+        ls_output = await self.git_stdout("ls-tree", "-r", tree, "--", *paths)
+        entries = []
+        for line in ls_output.split("\n"):
+            if not line:
+                continue
+            meta, path = line.split("\t", 1)
+            mode, _type, blob = meta.split(" ")
+            entries.append(f"{mode} {blob} 0\t{path}")
+        return await self.make_tree_from_index_entries(entries)
+
     @lru_cache(maxsize=None)
     async def merge_tree(
         self,

tests/test_amend.py

@@ -794,6 +794,7 @@ async def test_amends_file_into_last_commit_that_touched_it(self):
             assert await env.get_file_at_commit("a.txt", "HEAD") == "v3"
             # b.txt should be unchanged
             assert await env.get_file_at_commit("b.txt", "HEAD") == "v1"
+            assert not await env.has_staged_changes()
 
     @async_test
     async def test_multiple_files_to_different_commits(self):
@@ -813,6 +814,7 @@ async def test_multiple_files_to_different_commits(self):
             assert await env.get_file_at_commit("a.txt", "HEAD~2") == "a2"
             assert await env.get_file_at_commit("b.txt", "HEAD~1") == "b2"
             assert await env.get_file_at_commit("c.txt", "HEAD") == "c2"
+            assert not await env.has_staged_changes()
 
     @async_test
     async def test_amended_change_propagates_forward(self):
@@ -846,10 +848,8 @@ async def test_file_not_in_stack_remains_staged(self):
             args = make_amend_args(last_touched=True, parse_topics=True)
             await amend.main(args, env.git_ctx)
 
-            # new.txt should still be staged
-            assert await env.has_staged_changes()
-            staged = await env.get_staged_files()
-            assert "new.txt" in staged
+            # new.txt should still be staged, and be the only thing left staged
+            assert await env.get_staged_files() == ["new.txt"]
 
     @async_test
     async def test_uses_most_recent_commit_for_file(self):
@@ -868,6 +868,7 @@ async def test_uses_most_recent_commit_for_file(self):
             assert await env.get_file_at_commit("a.txt", "HEAD") == "v3"
             # "first" keeps its original content
             assert await env.get_file_at_commit("a.txt", "HEAD~1") == "v1"
+            assert not await env.has_staged_changes()
 
     @async_test
     async def test_preserves_commit_messages(self):
@@ -885,6 +886,7 @@ async def test_preserves_commit_messages(self):
             msg2 = await env.get_commit_message("HEAD")
             assert "first msg" in msg1
             assert "second msg" in msg2
+            assert not await env.has_staged_changes()
 
     @async_test
     async def test_noop_when_no_staged_files(self):
@@ -898,6 +900,7 @@ async def test_noop_when_no_staged_files(self):
             await amend.main(args, env.git_ctx)
 
             assert await env.get_commit_hash() == original_hash
+            assert not await env.has_staged_changes()
 
     @async_test
     async def test_mutually_exclusive_with_ref_or_topic(self):
@@ -955,6 +958,7 @@ async def test_works_with_subdirectories(self):
             assert await env.get_file_at_commit("src/lib/a.txt", "HEAD~1") == "a2"
             assert await env.get_file_at_commit("src/lib/c.txt", "HEAD~1") == "c2"
             assert await env.get_file_at_commit("src/lib/b.txt", "HEAD") == "b1"
+            assert not await env.has_staged_changes()
 
     @async_test
     async def test_with_all_flag_stages_unstaged_changes(self):
@@ -969,3 +973,26 @@ async def test_with_all_flag_stages_unstaged_changes(self):
             await amend.main(args, env.git_ctx)
 
             assert await env.get_file_at_commit("a.txt", "HEAD") == "a2"
+            assert not await env.has_staged_changes()
+
+    @async_test
+    async def test_staged_deletion_amended_into_last_commit(self):
+        async with GitTestEnvironment() as env:
+            await env.commit("root", {"root.txt": "r"})
+            await env.git_ctx.git("branch", "origin/main", "HEAD")
+            # a.txt is added in "first", b.txt in "second".
+            await env.commit("first\n\nTopic: alpha", {"a.txt": "v1", "keep.txt": "k"})
+            await env.commit("second\n\nTopic: beta", {"b.txt": "b1"})
+
+            # Stage a deletion of a.txt; it should be removed from "first" (the
+            # commit that added it) and stay gone through the rebuilt "second".
+            await env.git_ctx.git("rm", "a.txt")
+            args = make_amend_args(last_touched=True, parse_topics=True)
+            await amend.main(args, env.git_ctx)
+
+            assert await env.git_ctx.git_return_code("cat-file", "-e", "HEAD~1:a.txt") != 0
+            assert await env.git_ctx.git_return_code("cat-file", "-e", "HEAD:a.txt") != 0
+            # Sibling file in the same commit is untouched.
+            assert await env.get_file_at_commit("keep.txt", "HEAD~1") == "k"
+            assert await env.get_file_at_commit("b.txt", "HEAD") == "b1"
+            assert not await env.has_staged_changes()