From 513dac977c09a1f061a6ec964fa687ef05d5cb00 Mon Sep 17 00:00:00 2001 From: Jerry Zhang Date: Thu, 7 May 2026 13:11:27 -0700 Subject: [PATCH] amend: Add --last-touched flag to amend files into their most recent commit Each staged file is amended into the last commit in the local stack that touched it. Files not touched by any stack commit remain staged. --- revup/amend.py | 153 +++++++++++++++++++++++++++++++++++---- revup/revup.py | 24 +++++-- tests/test_amend.py | 171 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 328 insertions(+), 20 deletions(-) diff --git a/revup/amend.py b/revup/amend.py index 0aedadf..035271e 100644 --- a/revup/amend.py +++ b/revup/amend.py @@ -4,10 +4,13 @@ import re import shlex import subprocess +from collections import defaultdict +from typing import Dict, List, Set from revup import git, topic_stack from revup.types import ( CommitHeader, + GitCommitHash, GitConflictException, GitTreeHash, RevupConflictException, @@ -24,7 +27,12 @@ async def invoke_editor_for_commit_msg( - git_ctx: git.Git, editor: str, topic_summary: str, commit_msg: str, cache_stat: str, stat: str + git_ctx: git.Git, + editor: str, + topic_summary: str, + commit_msg: str, + cache_stat: str, + stat: str, ) -> str: """ Allow the user to modify the given commit msg by opening an editor. @@ -129,6 +137,89 @@ async def parse_ref_or_topic( raise RevupUsageException("Can't have both --no-parse-refs and --no-parse-topics!") +async def replay_cherry_pick( + git_ctx: git.Git, commit_obj: CommitHeader, new_parent: GitCommitHash +) -> GitCommitHash: + try: + return await git_ctx.synthetic_cherry_pick_from_commit(commit_obj, new_parent) + except GitConflictException as exc: + await git_ctx.dump_conflict(exc) + raise RevupConflictException( + commit_obj, + new_parent, + "You may need to `git rebase -i` to resolve these conflicts!", + ) from exc + + +async def rebuild_stack_last_touched( + git_ctx: git.Git, stack: List[CommitHeader], staged_files: Set[str] +) -> GitCommitHash: + """ + Rebuild the stack, amending each staged file into the most recent commit that touched it. + Returns the new HEAD commit. + """ + # Map each staged file to its most recent commit in the stack (ordered oldest-first) + file_to_commit: Dict[str, int] = {} + for i, commit_obj in enumerate(stack): + touched = await git_ctx.git_stdout( + "diff-tree", + "--no-commit-id", + "--name-only", + "-r", + "--no-renames", + commit_obj.commit_id, + ) + for f in touched.split("\n"): + if f and f in staged_files: + file_to_commit[f] = i + + commit_to_files: Dict[int, Set[str]] = defaultdict(set) + for f, idx in file_to_commit.items(): + commit_to_files[idx].add(f) + + files_to_amend = set(file_to_commit.keys()) + if not files_to_amend: + return GitCommitHash("") + + # Get index entries for each staged file (format: \t) + 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"): + if not line: + continue + _, path = line.split("\t", 1) + staged_entries[path] = line + + tmp_index = git_ctx.get_scratch_dir() + "/last_touched_index" + + new_commit = GitCommitHash(stack[0].parents[0]) if stack[0].parents else GitCommitHash("") + for i, commit_obj in enumerate(stack): + if i in commit_to_files: + idx_env = {"GIT_INDEX_FILE": tmp_index} + await git_ctx.git("read-tree", commit_obj.tree, env=idx_env) + update_lines = [staged_entries[f] for f in commit_to_files[i]] + await git_ctx.git( + "update-index", + "--index-info", + env=idx_env, + input_str="\n".join(update_lines) + "\n", + ) + new_tree = GitTreeHash(await git_ctx.git_stdout("write-tree", env=idx_env)) + amended = CommitHeader(new_tree, [new_commit]) + amended.author_name = commit_obj.author_name + amended.author_email = commit_obj.author_email + amended.author_date = commit_obj.author_date + amended.committer_name = commit_obj.committer_name + amended.committer_email = commit_obj.committer_email + amended.committer_date = commit_obj.committer_date + amended.commit_msg = commit_obj.commit_msg + new_commit = await git_ctx.commit_tree(amended) + else: + new_commit = await replay_cherry_pick(git_ctx, commit_obj, new_commit) + + return new_commit + + async def main(args: argparse.Namespace, git_ctx: git.Git) -> int: """ Amend the given commit and recreate the history on top of that commit to make @@ -147,12 +238,23 @@ async def get_has_unstaged() -> bool: args.edit = args.edit or args.insert has_diff = has_staged or has_unstaged or args.drop - if not has_diff and not args.edit: + if not has_diff and not args.edit and not args.last_touched: return 0 if args.drop and args.insert: raise RevupUsageException("Doesn't make sense to drop and insert") + if args.last_touched and (args.insert or args.drop): + raise RevupUsageException("--last-touched is mutually exclusive with --insert and --drop") + + if args.last_touched and args.ref_or_topic: + raise RevupUsageException( + "--last-touched is mutually exclusive with providing a ref_or_topic" + ) + + if args.last_touched and not args.parse_topics: + raise RevupUsageException("--last-touched requires --parse-topics") + if has_unstaged: await git_ctx.git("add", "--update") @@ -163,6 +265,35 @@ async def get_has_unstaged() -> bool: None, None, ) + + if args.last_touched: + staged_files_output = await git_ctx.git_stdout( + "diff", "--cached", "--name-only", "--no-renames" + ) + if not staged_files_output: + return 0 + + await topics.populate_topics() + if not topics.commits: + return 0 + + fork = await git_ctx.fork_point("HEAD", topics.relative_branch) + stack = git.parse_rev_list( + await git_ctx.rev_list( + "HEAD", fork, header=True, first_parent=True, exclude_first_parent=True + ) + ) + if not stack: + return 0 + + staged_files = set(staged_files_output.split("\n")) + new_commit = await rebuild_stack_last_touched(git_ctx, stack, staged_files) + if not new_commit: + return 0 + + await git_ctx.soft_reset(new_commit, {"GIT_REFLOG_ACTION": "revup amend --last-touched"}) + return 0 + if args.ref_or_topic: commit = await parse_ref_or_topic(args.ref_or_topic, args, git_ctx, topics) @@ -180,7 +311,11 @@ async def get_has_unstaged() -> bool: stack = git.parse_rev_list( await git_ctx.rev_list( - "HEAD", f"{commit}~", header=True, first_parent=True, exclude_first_parent=True + "HEAD", + f"{commit}~", + header=True, + first_parent=True, + exclude_first_parent=True, ) ) if len(stack) == 0: @@ -258,17 +393,7 @@ async def get_has_unstaged() -> bool: # don't actually have to apply a patch. new_commit = await git_ctx.cherry_pick_from_tree(commit_obj, new_commit) else: - try: - new_commit = await git_ctx.synthetic_cherry_pick_from_commit( - commit_obj, new_commit - ) - except GitConflictException as exc: - await git_ctx.dump_conflict(exc) - raise RevupConflictException( - commit_obj, - new_commit, - "You may need to `git rebase -i` to resolve these conflicts!", - ) from exc + new_commit = await replay_cherry_pick(git_ctx, commit_obj, new_commit) else: # If there's no diff (only text changed), its much faster to use the same trees new_commit = stack[0].parents[0] diff --git a/revup/revup.py b/revup/revup.py index 3585413..11d2a86 100755 --- a/revup/revup.py +++ b/revup/revup.py @@ -71,7 +71,8 @@ def make_toplevel_parser() -> RevupArgParser: def get_config_path() -> str: home_path = os.path.expanduser("~") return os.environ.get( - os.path.expanduser(REVUP_CONFIG_ENV_VAR), os.path.join(home_path, CONFIG_FILE_NAME) + os.path.expanduser(REVUP_CONFIG_ENV_VAR), + os.path.join(home_path, CONFIG_FILE_NAME), ) @@ -184,7 +185,9 @@ def build_parser() -> Tuple[RevupArgParser, List[RevupArgParser]]: ) upload_parser.add_argument("--uploader") upload_parser.add_argument( - "--branch-format", choices=["user+branch", "user", "branch", "none"], default="user+branch" + "--branch-format", + choices=["user+branch", "user", "branch", "none"], + default="user+branch", ) upload_parser.add_argument("--pre-upload", "-p") upload_parser.add_argument("--relative-chain", "-c", action="store_true") @@ -199,6 +202,7 @@ def build_parser() -> Tuple[RevupArgParser, List[RevupArgParser]]: amend_parser.add_argument("--edit", "-s", default=True, action="store_true") amend_parser.add_argument("--insert", "-i", action="store_true") amend_parser.add_argument("--drop", "-d", action="store_true") + amend_parser.add_argument("--last-touched", "-l", action="store_true") amend_parser.add_argument("--all", "-a", action="store_true") amend_parser.add_argument("--parse-topics", default=True, action="store_true") @@ -222,7 +226,10 @@ def build_parser() -> Tuple[RevupArgParser, List[RevupArgParser]]: "detect-branch", description="Detect the base branch of the current branch." ) detect_branch.add_argument( - "--show-all", "-s", action="store_true", help="Show all candidates, not just the best one" + "--show-all", + "-s", + action="store_true", + help="Show all candidates, not just the best one", ) detect_branch.add_argument( "--no-limit", "-n", action="store_true", help="Don't limit to release branches" @@ -249,17 +256,22 @@ def build_parser() -> Tuple[RevupArgParser, List[RevupArgParser]]: ) toolkit_fork_point.add_argument("branches", nargs=2, help="Branches to compare") toolkit_closest_branch = toolkit_subparsers.add_parser( - "closest-branch", description="Find the nearest base branch to the given commit." + "closest-branch", + description="Find the nearest base branch to the given commit.", ) toolkit_closest_branch.add_argument("branch", nargs=1, help="Commit/branch") toolkit_closest_branch.add_argument( - "--allow-self", action="store_true", help='Allow the branch itself to be a valid "closest"' + "--allow-self", + action="store_true", + help='Allow the branch itself to be a valid "closest"', ) toolkit_list_topics = toolkit_subparsers.add_parser( "list-topics", description="List all topics and their commits" ) toolkit_list_topics.add_argument( - "--base-branch", "-b", help="Use the given branch as the base instead of autodetecting." + "--base-branch", + "-b", + help="Use the given branch as the base instead of autodetecting.", ) toolkit_list_topics.add_argument( "--relative-branch", "-e", help="Use the given relative branch." diff --git a/tests/test_amend.py b/tests/test_amend.py index 816d476..62da8ea 100644 --- a/tests/test_amend.py +++ b/tests/test_amend.py @@ -19,6 +19,7 @@ def make_amend_args(**kwargs): "edit": False, "insert": False, "drop": False, + "last_touched": False, "all": False, "base_branch": None, "relative_branch": None, @@ -693,3 +694,173 @@ async def test_non_ancestor_commit_raises(self): args = make_amend_args(ref_or_topic=side_hash, edit=False) with pytest.raises(RevupUsageException, match="not a first parent ancestor"): await amend.main(args, env.git_ctx) + + +class TestAmendLastTouched: + @async_test + async def test_amends_file_into_last_commit_that_touched_it(self): + async with GitTestEnvironment() as env: + await env.commit("root", {"root.txt": "r"}) + await env.git_ctx.git("branch", "origin/main", "HEAD") + await env.commit("first\n\nTopic: alpha", {"a.txt": "v1"}) + await env.commit("second\n\nTopic: beta", {"b.txt": "v1"}) + + await env.stage_file("a.txt", "v2") + args = make_amend_args(last_touched=True, parse_topics=True) + await amend.main(args, env.git_ctx) + + # a.txt should be amended into "first" (HEAD~1), not HEAD + content = await env.get_file_at_commit("a.txt", "HEAD~1") + assert content == "v2" + # b.txt should be unchanged + content_b = await env.get_file_at_commit("b.txt", "HEAD") + assert content_b == "v1" + + @async_test + async def test_multiple_files_to_different_commits(self): + async with GitTestEnvironment() as env: + await env.commit("root", {"root.txt": "r"}) + await env.git_ctx.git("branch", "origin/main", "HEAD") + await env.commit("first\n\nTopic: alpha", {"a.txt": "a1"}) + await env.commit("second\n\nTopic: beta", {"b.txt": "b1"}) + await env.commit("third\n\nTopic: gamma", {"c.txt": "c1"}) + + await env.stage_file("a.txt", "a2") + await env.stage_file("b.txt", "b2") + await env.stage_file("c.txt", "c2") + args = make_amend_args(last_touched=True, parse_topics=True) + await amend.main(args, env.git_ctx) + + 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" + + @async_test + async def test_file_not_in_stack_remains_staged(self): + async with GitTestEnvironment() as env: + await env.commit("root", {"root.txt": "r"}) + await env.git_ctx.git("branch", "origin/main", "HEAD") + await env.commit("first\n\nTopic: alpha", {"a.txt": "a1"}) + + # Stage a file that no commit in the stack touched + await env.stage_file("new.txt", "new content") + 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 + + @async_test + async def test_uses_most_recent_commit_for_file(self): + """If multiple commits touch the same file, amend into the most recent one.""" + async with GitTestEnvironment() as env: + await env.commit("root", {"root.txt": "r"}) + await env.git_ctx.git("branch", "origin/main", "HEAD") + await env.commit("first\n\nTopic: alpha", {"a.txt": "v1"}) + await env.commit("second\n\nTopic: beta", {"a.txt": "v2"}) + + await env.stage_file("a.txt", "v3") + args = make_amend_args(last_touched=True, parse_topics=True) + await amend.main(args, env.git_ctx) + + # Should go into "second" (HEAD), the most recent to touch a.txt + 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" + + @async_test + async def test_preserves_commit_messages(self): + async with GitTestEnvironment() as env: + await env.commit("root", {"root.txt": "r"}) + await env.git_ctx.git("branch", "origin/main", "HEAD") + await env.commit("first msg\n\nTopic: alpha", {"a.txt": "a1"}) + await env.commit("second msg\n\nTopic: beta", {"b.txt": "b1"}) + + await env.stage_file("a.txt", "a2") + args = make_amend_args(last_touched=True, parse_topics=True) + await amend.main(args, env.git_ctx) + + msg1 = await env.get_commit_message("HEAD~1") + msg2 = await env.get_commit_message("HEAD") + assert "first msg" in msg1 + assert "second msg" in msg2 + + @async_test + async def test_noop_when_no_staged_files(self): + async with GitTestEnvironment() as env: + await env.commit("root", {"root.txt": "r"}) + await env.git_ctx.git("branch", "origin/main", "HEAD") + await env.commit("first\n\nTopic: alpha", {"a.txt": "a1"}) + + original_hash = await env.get_commit_hash() + args = make_amend_args(last_touched=True, parse_topics=True) + await amend.main(args, env.git_ctx) + + assert await env.get_commit_hash() == original_hash + + @async_test + async def test_mutually_exclusive_with_ref_or_topic(self): + async with GitTestEnvironment() as env: + await env.commit("root", {"root.txt": "r"}) + await env.git_ctx.git("branch", "origin/main", "HEAD") + await env.commit("first\n\nTopic: alpha", {"a.txt": "a1"}) + + await env.stage_file("a.txt", "a2") + args = make_amend_args(last_touched=True, parse_topics=True, ref_or_topic="HEAD") + with pytest.raises(RevupUsageException, match="mutually exclusive"): + await amend.main(args, env.git_ctx) + + @async_test + async def test_mutually_exclusive_with_drop(self): + async with GitTestEnvironment() as env: + await env.commit("root", {"root.txt": "r"}) + await env.git_ctx.git("branch", "origin/main", "HEAD") + await env.commit("first\n\nTopic: alpha", {"a.txt": "a1"}) + + await env.stage_file("a.txt", "a2") + args = make_amend_args(last_touched=True, parse_topics=True, drop=True) + with pytest.raises(RevupUsageException, match="mutually exclusive"): + await amend.main(args, env.git_ctx) + + @async_test + async def test_requires_parse_topics(self): + async with GitTestEnvironment() as env: + await env.commit("root", {"root.txt": "r"}) + await env.git_ctx.git("branch", "origin/main", "HEAD") + await env.commit("first\n\nTopic: alpha", {"a.txt": "a1"}) + + await env.stage_file("a.txt", "a2") + args = make_amend_args(last_touched=True, parse_topics=False) + with pytest.raises(RevupUsageException, match="requires --parse-topics"): + await amend.main(args, env.git_ctx) + + @async_test + async def test_works_with_subdirectories(self): + async with GitTestEnvironment() as env: + await env.commit("root", {"root.txt": "r"}) + await env.git_ctx.git("branch", "origin/main", "HEAD") + await env.commit("first\n\nTopic: alpha", {"src/lib/a.txt": "a1"}) + await env.commit("second\n\nTopic: beta", {"src/lib/b.txt": "b1"}) + + await env.stage_file("src/lib/a.txt", "a2") + args = make_amend_args(last_touched=True, parse_topics=True) + await amend.main(args, env.git_ctx) + + assert await env.get_file_at_commit("src/lib/a.txt", "HEAD~1") == "a2" + assert await env.get_file_at_commit("src/lib/b.txt", "HEAD") == "b1" + + @async_test + async def test_with_all_flag_stages_unstaged_changes(self): + async with GitTestEnvironment() as env: + await env.commit("root", {"root.txt": "r"}) + await env.git_ctx.git("branch", "origin/main", "HEAD") + await env.commit("first\n\nTopic: alpha", {"a.txt": "a1"}) + + # Modify without staging + await env.write_file("a.txt", "a2") + args = make_amend_args(last_touched=True, parse_topics=True, **{"all": True}) + await amend.main(args, env.git_ctx) + + assert await env.get_file_at_commit("a.txt", "HEAD") == "a2"