Re: [PATCH v6 11/11] builtin/history: implement "split" subcommand

> It is quite a common use case that one wants to split up one commit into
> multiple commits by moving parts of the changes of the original commit
> out into a separate commit. This is quite an involved operation though:
> 
>    1. Identify the commit in question that is to be dropped.
> 
>    2. Perform an interactive rebase on top of that commit's parent.
> 
>    3. Modify the instruction sheet to "edit" the commit that is to be
>       split up.
> 
>    4. Drop the commit via "git reset HEAD~".
> 
>    5. Stage changes that should go into the first commit and commit it.
> 
>    6. Stage changes that should go into the second commit and commit it.
> 
>    7. Finalize the rebase.
> 
> This is quite complex, and overall I would claim that most people who
> are not experts in Git would struggle with this flow.

> Introduce a new "split" subcommand for git-history(1) to make this way
> easier. All the user needs to do is to say `git history split $COMMIT`.
>  From hereon, Git asks the user which parts of the commit shall be moved
> out into a separate commit and, once done, asks the user for the commit
> message. Git then creates that split-out commit and applies the original
> commit on top of it.

> [...]> +	/*
> +	* Construct the first commit. This is done by taking the original
> +	* commit parent's tree and selectively patching changes from the diff
> +	* between that parent and its child.
> +	*/
> +	repo_git_path_replace(repo, &index_file, "%s", "history-split.index");
> +
> +	read_tree_cmd.git_cmd = 1;
> +	strvec_pushf(&read_tree_cmd.env, "GIT_INDEX_FILE=%s", index_file.buf);
> +	strvec_push(&read_tree_cmd.args, "read-tree");
> +	strvec_push(&read_tree_cmd.args, oid_to_hex(&parent_tree_oid));
> +	ret = run_command(&read_tree_cmd);

> [...]
> +	/* We retain authorship of the original commit. */
> +	original_message = repo_logmsg_reencode(repo, original_commit, NULL, NULL);
> +	ptr = find_commit_header(original_message, "author", &len);
> +	if (ptr)
> +		original_author = xmemdupz(ptr, len);
> +
> +	ret = fill_commit_message(repo, &parent_tree_oid, &split_tree->object.oid,
> +				  "", "split-out", &split_message);
> +	if (ret < 0)
> +		goto out;
> +
> +	ret = commit_tree(split_message.buf, split_message.len, &split_tree->object.oid,
> +			  original_commit->parents, &out[0], original_author, NULL);
> +	if (ret < 0) {
> +		ret = error(_("failed writing split-out commit"));
> +		goto out;
> +	}

> +static int cmd_history_split(int argc,
> +			     const char **argv,
> +			     const char *prefix,
> +			     struct repository *repo)
> +{
> +	const char * const usage[] = {
> +		GIT_HISTORY_SPLIT_USAGE,
> +		NULL,
> +	};
> +	struct option options[] = {
> +		OPT_END(),
> +	};
> +	struct oidmap rewritten_commits = OIDMAP_INIT;
> +	struct commit *original_commit, *parent, *head;
> +	struct strvec commits = STRVEC_INIT;
> +	struct commit_list *from_list = NULL;
> +	struct object_id split_commits[2];
> +	struct pathspec pathspec = { 0 };
> +	int ret;
> +
> +	argc = parse_options(argc, argv, prefix, options, usage, 0);
> +	if (argc < 1) {
> +		ret = error(_("command expects a revision"));
> +		goto out;
> +	}
> +	repo_config(repo, git_default_config, NULL);
> +
> +	original_commit = lookup_commit_reference_by_name(argv[0]);
> +	if (!original_commit) {
> +		ret = error(_("commit to be split cannot be found: %s"), argv[0]);
> +		goto out;
> +	}
> +
> +	parent = original_commit->parents ? original_commit->parents->item : NULL;
> +	if (parent && repo_parse_commit(repo, parent)) {
> +		ret = error(_("unable to parse commit %s"),
> +			    oid_to_hex(&parent->object.oid));
> +		goto out;
> +	}
> +
> +	head = lookup_commit_reference_by_name("HEAD");
> +	if (!head) {
> +		ret = error(_("could not resolve HEAD to a commit"));
> +		goto out;
> +	}
> +
> +	commit_list_append(original_commit, &from_list);
> +	if (!repo_is_descendant_of(repo, head, from_list)) {
> +		ret = error(_("split commit must be reachable from current HEAD commit"));
> +		goto out;
> +	}

> diff --git a/t/t3452-history-split.sh b/t/t3452-history-split.sh
> [...]
> +test_expect_success 'refuses to work with merge commits' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	(
> +		cd repo &&
> +		test_commit base &&
> +		git branch branch &&
> +		test_commit ours &&
> +		git switch branch &&
> +		test_commit theirs &&
> +		git switch - &&
> +		git merge theirs &&
> +		test_must_fail git history split HEAD 2>err &&
> +		test_grep "cannot rearrange commit history with merges" err &&
> +		test_must_fail git history split HEAD~ 2>err &&
> +		test_grep "cannot rearrange commit history with merges" err
> +	)
> +'

> +
> +test_expect_success 'refuses to work with unrelated commits' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	(
> +		cd repo &&
> +		test_commit base &&
> +		git branch branch &&
> +		test_commit ours &&
> +		git switch branch &&
> +		test_commit theirs &&
> +		test_must_fail git history split ours 2>err &&
> +		test_grep "split commit must be reachable from current HEAD commit" err
> +	)
> +'
> +
> +test_expect_success 'can split up tip commit' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	(
> +		cd repo &&
> +		test_commit initial &&
> +		touch bar foo &&
> +		git add . &&
> +		git commit -m split-me &&
> +
> +		git symbolic-ref HEAD >expect &&
> +		set_fake_editor "split-out commit" &&
> +		git history split HEAD <<-EOF &&
> +		y
> +		n
> +		EOF
> +		git symbolic-ref HEAD >actual &&
> +		test_cmp expect actual &&
> +
> +		expect_log <<-EOF &&
> +		split-me
> +		split-out commit
> +		initial
> +		EOF
> +
> +		expect_tree_entries HEAD~ <<-EOF &&
> +		bar
> +		initial.t
> +		EOF
> +
> +		expect_tree_entries HEAD <<-EOF
> +		bar
> +		foo
> +		initial.t
> +		EOF
> +	)
> +'
> +
> +test_expect_success 'can split up root commit' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	(
> +		cd repo &&
> +		touch bar foo &&
> +		git add . &&
> +		git commit -m root &&
> +		test_commit tip &&
> +
> +		set_fake_editor "split-out commit" &&
> +		git history split HEAD~ <<-EOF &&
> +		y
> +		n
> +		EOF
> +
> +		expect_log <<-EOF &&
> +		tip
> +		root
> +		split-out commit
> +		EOF
> +
> +		expect_tree_entries HEAD~2 <<-EOF &&
> +		bar
> +		EOF
> +
> +		expect_tree_entries HEAD~ <<-EOF &&
> +		bar
> +		foo
> +		EOF
> +
> +		expect_tree_entries HEAD <<-EOF
> +		bar
> +		foo
> +		tip.t
> +		EOF
> +	)
> +'
> +
> +test_expect_success 'can split up in-between commit' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	(
> +		cd repo &&
> +		test_commit initial &&
> +		touch bar foo &&
> +		git add . &&
> +		git commit -m split-me &&
> +		test_commit tip &&
> +
> +		set_fake_editor "split-out commit" &&
> +		git history split HEAD~ <<-EOF &&
> +		y
> +		n
> +		EOF
> +
> +		expect_log <<-EOF &&
> +		tip
> +		split-me
> +		split-out commit
> +		initial
> +		EOF
> +
> +		expect_tree_entries HEAD~2 <<-EOF &&
> +		bar
> +		initial.t
> +		EOF
> +
> +		expect_tree_entries HEAD~ <<-EOF &&
> +		bar
> +		foo
> +		initial.t
> +		EOF
> +
> +		expect_tree_entries HEAD <<-EOF
> +		bar
> +		foo
> +		initial.t
> +		tip.t
> +		EOF
> +	)
> +'
> +
> +test_expect_success 'can pick multiple hunks' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	(
> +		cd repo &&
> +		touch bar baz foo qux &&
> +		git add . &&
> +		git commit -m split-me &&
> +
> +		set_fake_editor "split-out-commit" &&
> +		git history split HEAD <<-EOF &&
> +		y
> +		n
> +		y
> +		n
> +		EOF
> +
> +		expect_tree_entries HEAD~ <<-EOF &&
> +		bar
> +		foo
> +		EOF
> +
> +		expect_tree_entries HEAD <<-EOF
> +		bar
> +		baz
> +		foo
> +		qux
> +		EOF
> +	)
> +'
> +
> +
> +test_expect_success 'can use only last hunk' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	(
> +		cd repo &&
> +		touch bar foo &&
> +		git add . &&
> +		git commit -m split-me &&
> +
> +		set_fake_editor "split-out commit" &&
> +		git history split HEAD <<-EOF &&
> +		n
> +		y
> +		EOF
> +
> +		expect_log <<-EOF &&
> +		split-me
> +		split-out commit
> +		EOF
> +
> +		expect_tree_entries HEAD~ <<-EOF &&
> +		foo
> +		EOF
> +
> +		expect_tree_entries HEAD <<-EOF
> +		bar
> +		foo
> +		EOF
> +	)
> +'
> +
> +test_expect_success 'aborts with empty commit message' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	(
> +		cd repo &&
> +		touch bar foo &&
> +		git add . &&
> +		git commit -m split-me &&
> +
> +		set_fake_editor "" &&
> +		test_must_fail git history split HEAD <<-EOF 2>err &&
> +		y
> +		n
> +		EOF
> +		test_grep "Aborting commit due to empty commit message." err
> +	)
> +'
> +
> +test_expect_success 'commit message editor sees split-out changes' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	(
> +		cd repo &&
> +		touch bar foo &&
> +		git add . &&
> +		git commit -m split-me &&
> +
> +		write_script fake-editor.sh <<-\EOF &&
> +		cp "$1" . &&
> +		echo "some commit message" >>"$1"
> +		EOF
> +		test_set_editor "$(pwd)"/fake-editor.sh &&
> +
> +		git history split HEAD <<-EOF &&
> +		y
> +		n
> +		EOF
> +
> +		cat >expect <<-EOF &&
> +
> +		# Please enter the commit message for the split-out changes. Lines starting
> +		# with ${SQ}#${SQ} will be ignored.
> +		# Changes to be committed:
> +		#	new file:   bar
> +		#
> +		EOF
> +		test_cmp expect COMMIT_EDITMSG &&
> +
> +		expect_log <<-EOF
> +		split-me
> +		some commit message
> +		EOF
> +	)
> +'
> +
> +test_expect_success 'can use pathspec to limit what gets split' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	(
> +		cd repo &&
> +		touch bar foo &&
> +		git add . &&
> +		git commit -m split-me &&
> +
> +		set_fake_editor "split-out commit" &&
> +		git history split HEAD -- foo <<-EOF &&
> +		y
> +		EOF
> +
> +		expect_tree_entries HEAD~ <<-EOF &&
> +		foo
> +		EOF
> +
> +		expect_tree_entries HEAD <<-EOF
> +		bar
> +		foo
> +		EOF
> +	)
> +'
> +
> +test_expect_success 'refuses to create empty split-out commit' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	(
> +		cd repo &&
> +		test_commit base &&
> +		touch bar foo &&
> +		git add . &&
> +		git commit -m split-me &&
> +
> +		test_must_fail git history split HEAD 2>err <<-EOF &&
> +		n
> +		n
> +		EOF
> +		test_grep "split commit is empty" err
> +	)
> +'
> +
> +test_expect_success 'hooks are executed for rewritten commits' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	(
> +		cd repo &&
> +		touch bar foo &&
> +		git add . &&
> +		git commit -m split-me &&
> +		old_head=$(git rev-parse HEAD) &&
> +
> +		write_script .git/hooks/prepare-commit-msg <<-EOF &&
> +		touch "$(pwd)/hooks.log"
> +		EOF
> +		write_script .git/hooks/post-commit <<-EOF &&
> +		touch "$(pwd)/hooks.log"
> +		EOF
> +		write_script .git/hooks/post-rewrite <<-EOF &&
> +		touch "$(pwd)/hooks.log"
> +		EOF
> +
> +		set_fake_editor "split-out commit" &&
> +		git history split HEAD <<-EOF &&
> +		y
> +		n
> +		EOF
> +
> +		expect_log <<-EOF &&
> +		split-me
> +		split-out commit
> +		EOF
> +
> +		test_path_is_missing hooks.log
> +	)
> +'
> +
> +test_expect_success 'refuses to create empty original commit' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	(
> +		cd repo &&
> +		touch bar foo &&
> +		git add . &&
> +		git commit -m split-me &&
> +
> +		test_must_fail git history split HEAD 2>err <<-EOF &&
> +		y
> +		y
> +		EOF
> +		test_grep "split commit tree matches original commit" err
> +	)
> +'
> +
> +test_expect_success 'retains changes in the worktree and index' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	(
> +		cd repo &&
> +		echo a >a &&
> +		echo b >b &&
> +		git add . &&
> +		git commit -m "initial commit" &&
> +		echo a-modified >a &&
> +		echo b-modified >b &&
> +		git add b &&
> +		set_fake_editor "a-only" &&
> +		git history split HEAD <<-EOF &&
> +		y
> +		n
> +		EOF
> +
> +		expect_tree_entries HEAD~ <<-EOF &&
> +		a
> +		EOF
> +		expect_tree_entries HEAD <<-EOF &&
> +		a
> +		b
> +		EOF
> +
> +		cat >expect <<-\EOF &&
> +		 M a
> +		M  b
> +		?? actual
> +		?? expect
> +		?? fake-editor.sh
> +		EOF
> +		git status --porcelain >actual &&
> +		test_cmp expect actual
> +	)
> +'
> +
> +test_done
>