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"