diff --git a/revup/amend.py b/revup/amend.py index 04489ae..7128a92 100644 --- a/revup/amend.py +++ b/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: \t) + # Get index entries for each staged file (format: \t). + # 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", ) diff --git a/revup/git.py b/revup/git.py index adda45c..05b954b 100644 --- a/revup/git.py +++ b/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, diff --git a/tests/test_amend.py b/tests/test_amend.py index 49132dd..d9f0572 100644 --- a/tests/test_amend.py +++ b/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()