[PATCH RFC 05/11] builtin/history: implement "drop" subcommand

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



It is a fairly common operation to perform an interactive rebase so that
one of the commits can be dropped from history. Doing this is not very
hard in general, but still requires the user to perform multiple steps:

  1. Identify the commit in question that is to be dropped.

  2. Perform an interactive rebase on top of that commit's parent.

  3. Edit the instruction sheet to drop that commit.

This is needlessly complex for such a supposedly-trivial operation.
Furthermore, the second step doesn't account for certain edge cases like
for example dropping the root commit.

Introduce a new "drop" subcommand to make this use case significantly
simpler: all the user needs to do is to say `git history drop $COMMIT`
and they're done.

Note that for now, this command only allows users to drop a single
commit at once. It should be easy enough though to expand the command at
a later point in time to support dropping whole commit ranges.

Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
---
 Documentation/git-history.adoc |  27 +++-
 builtin/history.c              | 297 ++++++++++++++++++++++++++++++++++++++++-
 t/meson.build                  |   3 +-
 t/t3450-history-drop.sh        | 127 ++++++++++++++++++
 4 files changed, 449 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-history.adoc b/Documentation/git-history.adoc
index 9dafb8fc16..3012445ddc 100644
--- a/Documentation/git-history.adoc
+++ b/Documentation/git-history.adoc
@@ -8,7 +8,7 @@ git-history - Rewrite history of the current branch
 SYNOPSIS
 --------
 [synopsis]
-git history [<options>]
+git history drop [<options>] <revision>
 
 DESCRIPTION
 -----------
@@ -31,6 +31,31 @@ COMMANDS
 This command requires a subcommand. Several subcommands are available to
 rewrite history in different ways.
 
+drop <revision>::
+	Drop a commit from the history and reapply all children of that
+	commit on top of the commit's parent. The commit that is to be
+	dropped must be reachable from the current `HEAD` commit.
++
+Dropping the root commit converts the child of that commit into the new
+root commit. It is invalid to drop a root commit that does not have any
+child commits, as that would lead to an empty branch.
+
+EXAMPLES
+--------
+
+* Drop a commit from history.
++
+----------
+$ git log --oneline
+2d4cd6d third
+125a0f3 second
+e098c27 first
+$ git history drop HEAD~
+$ git log
+b1bc1bd third
+e098c27 first
+----------
+
 CONFIGURATION
 -------------
 
diff --git a/builtin/history.c b/builtin/history.c
index d1a40368e0..183ab9d5f7 100644
--- a/builtin/history.c
+++ b/builtin/history.c
@@ -1,20 +1,311 @@
 #include "builtin.h"
+#include "commit.h"
+#include "commit-reach.h"
+#include "config.h"
+#include "environment.h"
 #include "gettext.h"
+#include "hex.h"
+#include "object-name.h"
 #include "parse-options.h"
+#include "refs.h"
+#include "reset.h"
+#include "revision.h"
+#include "sequencer.h"
+
+static int collect_commits(struct repository *repo,
+			   struct commit *old_commit,
+			   struct commit *new_commit,
+			   struct strvec *out)
+{
+	struct setup_revision_opt revision_opts = {
+		.assume_dashdash = 1,
+	};
+	struct strvec revisions = STRVEC_INIT;
+	struct commit_list *from_list = NULL;
+	struct commit *child;
+	struct rev_info rev = { 0 };
+	int ret;
+
+	/*
+	 * Check that the old actually is an ancestor of HEAD. If not
+	 * the whole request becomes nonsensical.
+	*/
+	if (old_commit) {
+		commit_list_insert(old_commit, &from_list);
+		if (!repo_is_descendant_of(repo, new_commit, from_list)) {
+			ret = error(_("commit must be reachable from current HEAD commit"));
+			goto out;
+		}
+	}
+
+	repo_init_revisions(repo, &rev, NULL);
+	strvec_push(&revisions, "");
+	strvec_push(&revisions, oid_to_hex(&new_commit->object.oid));
+	if (old_commit)
+		strvec_pushf(&revisions, "^%s", oid_to_hex(&old_commit->object.oid));
+	if (setup_revisions(revisions.nr, revisions.v, &rev, &revision_opts) != 1 ||
+	    prepare_revision_walk(&rev)) {
+		ret = error(_("revision walk setup failed"));
+		goto out;
+	}
+
+	while ((child = get_revision(&rev))) {
+		if (old_commit && !child->parents)
+			BUG("revision walk did not find child commit");
+		if (child->parents && child->parents->next) {
+			ret = error(_("cannot rearrange commit history with merges"));
+			goto out;
+		}
+
+		strvec_push(out, oid_to_hex(&child->object.oid));
+
+		if (child->parents && old_commit &&
+		    commit_list_contains(old_commit, child->parents))
+			break;
+	}
+
+	/*
+	 * Revisions are in newest-order-first. We have to reverse the
+	 * array though so that we pick the oldest commits first. Note
+	 * that we keep the first string untouched, as it is the
+	 * equivalent of `argv[0]` to `setup_revisions()`.
+	 */
+	for (size_t i = 0, j = out->nr - 1; i < j; i++, j--)
+		SWAP(out->v[i], out->v[j]);
+
+	ret = 0;
+
+out:
+	free_commit_list(from_list);
+	strvec_clear(&revisions);
+	release_revisions(&rev);
+	reset_revision_walk();
+	return ret;
+}
+
+static int apply_commits(struct repository *repo,
+			 const struct strvec *commits,
+			 struct commit *head,
+			 struct commit *base,
+			 const char *action)
+{
+	struct setup_revision_opt revision_opts = {
+		.assume_dashdash = 1,
+	};
+	struct replay_opts replay_opts = REPLAY_OPTS_INIT;
+	struct reset_head_opts reset_opts = { 0 };
+	struct object_id root_commit;
+	struct strvec args = STRVEC_INIT;
+	struct strbuf buf = STRBUF_INIT;
+	char hex[GIT_MAX_HEXSZ + 1];
+	int ref_flags, ret;
+
+	/*
+	 * We have performed all safety checks, so we now prepare
+	 * replaying the commits.
+	*/
+	replay_opts.action = REPLAY_PICK;
+	sequencer_init_config(&replay_opts);
+	replay_opts.quiet = 1;
+	replay_opts.skip_commit_summary = 1;
+	if (!replay_opts.strategy && replay_opts.default_strategy) {
+		replay_opts.strategy = replay_opts.default_strategy;
+		replay_opts.default_strategy = NULL;
+	}
+
+	strvec_push(&args, "");
+	strvec_pushv(&args, commits->v);
+
+	replay_opts.revs = xmalloc(sizeof(*replay_opts.revs));
+	repo_init_revisions(repo, replay_opts.revs, NULL);
+	replay_opts.revs->no_walk = 1;
+	replay_opts.revs->unsorted_input = 1;
+	if (setup_revisions(args.nr, args.v, replay_opts.revs,
+			    &revision_opts) != 1) {
+		ret = error(_("setting up revisions failed"));
+		goto out;
+	}
+
+	/*
+	 * If we're dropping the root commit we first need to create
+	 * a new empty root. We then instruct the seqencer machinery to
+	 * squash that root commit with the first commit we're picking
+	 * onto it.
+	 */
+	if (!base) {
+		if (commit_tree("", 0, repo->hash_algo->empty_tree, NULL,
+				&root_commit, NULL, NULL) < 0) {
+			ret = error(_("Could not create new root commit"));
+			goto out;
+		}
+
+		replay_opts.squash_onto = root_commit;
+		replay_opts.have_squash_onto = 1;
+		reset_opts.oid = &root_commit;
+	} else {
+		reset_opts.oid = &base->object.oid;
+	}
+
+	replay_opts.restore_head_target =
+		xstrdup_or_null(refs_resolve_ref_unsafe(get_main_ref_store(repo),
+							"HEAD", 0, NULL, &ref_flags));
+	if (!(ref_flags & REF_ISSYMREF))
+		FREE_AND_NULL(replay_opts.restore_head_target);
+
+	/*
+	 * Perform a hard-reset to the parent of our commit that is to
+	 * be dropped. This is the new base onto which we'll pick all
+	 * the descendants.
+	 */
+	strbuf_addf(&buf, "%s (start): checkout %s", action,
+		    oid_to_hex_r(hex, reset_opts.oid));
+	reset_opts.orig_head = &head->object.oid;
+	reset_opts.flags = RESET_HEAD_DETACH | RESET_ORIG_HEAD;
+	reset_opts.head_msg = buf.buf;
+	reset_opts.default_reflog_action = action;
+	if (reset_head(repo, &reset_opts) < 0) {
+		ret = error(_("could not switch to %s"), oid_to_hex(reset_opts.oid));
+		goto out;
+	}
+
+	ret = sequencer_pick_revisions(repo, &replay_opts);
+	if (ret < 0) {
+		ret = error(_("could not pick commits"));
+		goto out;
+	} else if (ret > 0) {
+		/*
+		 * A positive return value indicates we've got a merge
+		 * conflict. Bail out, but don't print a message as
+		 * `sequencer_pick_revisions()` already printed enough
+		 * information.
+		 */
+		ret = -1;
+		goto out;
+	}
+
+	ret = 0;
+
+out:
+	replay_opts_release(&replay_opts);
+	strbuf_release(&buf);
+	strvec_clear(&args);
+	return ret;
+}
+
+static int cmd_history_drop(int argc,
+			    const char **argv,
+			    const char *prefix,
+			    struct repository *repo)
+{
+	const char * const usage[] = {
+		N_("git history drop [<options>] <revision>"),
+		NULL,
+	};
+	struct option options[] = {
+		OPT_END(),
+	};
+	struct commit *commit_to_drop, *head;
+	struct strvec commits = STRVEC_INIT;
+	struct strbuf buf = STRBUF_INIT;
+	int ret;
+
+	argc = parse_options(argc, argv, prefix, options, usage, 0);
+	if (argc != 1) {
+		ret = error(_("command expects a single revision"));
+		goto out;
+	}
+	repo_config(repo, git_default_config, NULL);
+
+	commit_to_drop = lookup_commit_reference_by_name(argv[0]);
+	if (!commit_to_drop) {
+		ret = error(_("commit to be dropped cannot be found: %s"), argv[0]);
+		goto out;
+	}
+	if (commit_to_drop->parents && commit_to_drop->parents->next) {
+		ret = error(_("commit to be dropped must not be a merge commit"));
+		goto out;
+	}
+
+	head = lookup_commit_reference_by_name("HEAD");
+	if (!head) {
+		ret = error(_("could not resolve HEAD to a commit"));
+		goto out;
+	}
+
+	if (oideq(&commit_to_drop->object.oid, &head->object.oid)) {
+		/*
+		 * If we want to drop the tip of the current branch we don't
+		 * have to perform any rebase at all. Instead, we simply
+		 * perform a hard reset to the parent commit.
+		 */
+		struct reset_head_opts reset_opts = {
+			.orig_head = &head->object.oid,
+			.flags = RESET_ORIG_HEAD,
+			.default_reflog_action = "drop",
+		};
+		char hex[GIT_MAX_HEXSZ + 1];
+
+		if (!commit_to_drop->parents) {
+			ret = error(_("cannot drop the only commit on this branch"));
+			goto out;
+		}
+
+		oid_to_hex_r(hex, &commit_to_drop->parents->item->object.oid);
+		strbuf_addf(&buf, "drop (start): checkout %s", hex);
+		reset_opts.oid = &commit_to_drop->parents->item->object.oid;
+		reset_opts.head_msg = buf.buf;
+
+		if (reset_head(repo, &reset_opts) < 0) {
+			ret = error(_("could not switch to %s"), hex);
+			goto out;
+		}
+	} else {
+		/*
+		 * Prepare a revision walk from old commit to the commit that is
+		 * about to be dropped. This serves three purposes:
+		 *
+		 *   - We verify that the history doesn't contain any merges.
+		 *     For now, merges aren't yet handled by us.
+		 *
+		 *   - We need to find the child of the commit-to-be-dropped.
+		 *     This child is what will be adopted by the parent of the
+		 *     commit that we are about to drop.
+		 *
+		 *   - We compute the list of commits-to-be-picked.
+		 */
+		ret = collect_commits(repo, commit_to_drop, head, &commits);
+		if (ret < 0)
+			goto out;
+
+		ret = apply_commits(repo, &commits, head, commit_to_drop->parents ?
+				    commit_to_drop->parents->item : NULL, "drop");
+		if (ret < 0)
+			goto out;
+	}
+
+	ret = 0;
+
+out:
+	strvec_clear(&commits);
+	strbuf_release(&buf);
+	return ret;
+}
 
 int cmd_history(int argc,
 		const char **argv,
 		const char *prefix,
-		struct repository *repo UNUSED)
+		struct repository *repo)
 {
 	const char * const usage[] = {
-		N_("git history [<options>]"),
+		N_("git history drop [<options>] <revision>"),
 		NULL,
 	};
+	parse_opt_subcommand_fn *fn = NULL;
 	struct option options[] = {
+		OPT_SUBCOMMAND("drop", &fn, cmd_history_drop),
 		OPT_END(),
 	};
 
 	argc = parse_options(argc, argv, prefix, options, usage, 0);
-	return 0;
+	return fn(argc, argv, prefix, repo);
 }
diff --git a/t/meson.build b/t/meson.build
index bbeba1a8d5..859c388987 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -376,6 +376,7 @@ integration_tests = [
   't3436-rebase-more-options.sh',
   't3437-rebase-fixup-options.sh',
   't3438-rebase-broken-files.sh',
+  't3450-history-drop.sh',
   't3500-cherry.sh',
   't3501-revert-cherry-pick.sh',
   't3502-cherry-pick-merge.sh',
@@ -1214,4 +1215,4 @@ if perl.found() and time.found()
       timeout: 0,
     )
   endforeach
-endif
\ No newline at end of file
+endif
diff --git a/t/t3450-history-drop.sh b/t/t3450-history-drop.sh
new file mode 100755
index 0000000000..4782144da0
--- /dev/null
+++ b/t/t3450-history-drop.sh
@@ -0,0 +1,127 @@
+#!/bin/sh
+
+test_description='tests for git-history drop subcommand'
+
+. ./test-lib.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 drop HEAD 2>err &&
+		test_grep "commit to be dropped must not be a merge commit" err &&
+		test_must_fail git history drop HEAD~ 2>err &&
+		test_grep "cannot rearrange commit history with merges" err
+	)
+'
+
+test_expect_success 'refuses to work when history becomes empty' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		test_commit base &&
+		test_must_fail git history drop HEAD 2>err &&
+		test_grep "cannot drop the only commit on this branch" err
+	)
+'
+
+test_expect_success 'can drop tip of a branch' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		test_commit first &&
+		test_commit second &&
+		test_commit third &&
+
+		git symbolic-ref HEAD >expect &&
+		git history drop HEAD &&
+		git symbolic-ref HEAD >actual &&
+		test_cmp expect actual &&
+
+		cat >expect <<-EOF &&
+		second
+		first
+		EOF
+		git log --format=%s >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'can drop commit in the middle' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		test_commit first &&
+		test_commit second &&
+		test_commit third &&
+		test_commit fourth &&
+		test_commit fifth &&
+
+		git symbolic-ref HEAD >expect &&
+		git history drop HEAD~2 &&
+		git symbolic-ref HEAD >actual &&
+		test_cmp expect actual &&
+
+		cat >expect <<-EOF &&
+		fifth
+		fourth
+		second
+		first
+		EOF
+		git log --format=%s >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'correct order is retained' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		test_commit first &&
+		test_commit second &&
+		test_commit third &&
+		test_commit fourth &&
+		test_commit fifth &&
+		git history drop HEAD~3 &&
+		cat >expect <<-EOF &&
+		fifth
+		fourth
+		third
+		first
+		EOF
+		git log --format=%s >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'can drop root commit' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		test_commit first &&
+		test_commit second &&
+		test_commit third &&
+		git history drop HEAD~2 &&
+		cat >expect <<-EOF &&
+		third
+		second
+		EOF
+		git log --format=%s >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_done

-- 
2.51.0.261.g7ce5a0a67e.dirty





[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux