git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[RFC/PATCH 1/2] reset: learn to reset to tree

From
Martin von Zweigbergk <martinvonz@gmail.com>
Date
Nov 29, 2012, 18:32 UTC
Message-ID
<1354213975-17866-2-git-send-email-martinvonz@gmail.com>
In-Reply-To
<1354213975-17866-1-git-send-email-martinvonz@gmail.com>

In cases where HEAD is not supposed to be updated, there is no reason that "git reset" should require a commit, a tree should be enough. So make "git reset $rev^{tree}" work just like "git reset $rev", except that the former will not update HEAD (since there is no commit to point it to).

Disallow --soft with trees, since that is about updating only HEAD.

By not updating HEAD, "git reset $rev^{tree}" behaves quite like "git reset $rev .". One might therefore consider requiring a path when using reset with a tree to make that similarity more obvious. However, a future commit will make "git reset" work on an unborn branch by interpreting it as "git reset $empty_tree" and it would seem unintuitive to the user to say "git reset ." on an unborn branch. Requiring a path would also make "git reset --hard $tree" disallowed.

This commit effectively undoes some of commit 13243c2 (reset: the command takes committish, 2012-07-03). The command line argument is now required to be an unambiguous treeish.

---

My implementation of lookup_commit_or_tree looks a little clunky. I'm not very familiar with the API. Suggestions welcome.

Why is the "HEAD is now at ..." message printed only for --hard reset? After all, HEAD is updated for all types of reset not involving paths.

 builtin/reset.c                     | 67 +++++++++++++++++++++----------------
 t/t1512-rev-parse-disambiguation.sh |  4 ---
 t/t7102-reset.sh                    | 26 ++++++++++++++
 3 files changed, 65 insertions(+), 32 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 915cc9f..cec9874 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -225,6 +225,21 @@ static void die_if_unmerged_cache(int reset_type)
 
 }
 
+static struct object *lookup_commit_or_tree(const char *rev) {
+	unsigned char sha1[20];
+	struct commit *commit;
+	struct tree *tree;
+	if (get_sha1_treeish(rev, sha1))
+		die(_("Failed to resolve '%s' as a valid ref."), rev);
+	commit = lookup_commit_reference_gently(sha1, 1);
+	if (commit)
+		return (struct object *) commit;
+	tree = parse_tree_indirect(sha1);
+	if (tree)
+		return (struct object *) tree;
+	die(_("Could not parse object '%s'."), rev);
+}
+
 int cmd_reset(int argc, const char **argv, const char *prefix)
 {
 	int i = 0, reset_type = NONE, update_ref_status = 0, quiet = 0;
@@ -232,7 +247,8 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 	const char *rev = "HEAD";
 	unsigned char sha1[20], *orig = NULL, sha1_orig[20],
 				*old_orig = NULL, sha1_old_orig[20];
-	struct commit *commit;
+	struct object *object;
+	struct commit *commit = NULL;
 	struct strbuf msg = STRBUF_INIT;
 	const struct option options[] = {
 		OPT__QUIET(&quiet, N_("be quiet, only report errors")),
@@ -276,7 +292,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 		 * Otherwise, argv[i] could be either <rev> or <paths> and
 		 * has to be unambiguous.
 		 */
-		else if (!get_sha1_committish(argv[i], sha1)) {
+		else if (!get_sha1_treeish(argv[i], sha1)) {
 			/*
 			 * Ok, argv[i] looks like a rev; it should not
 			 * be a filename.
@@ -289,19 +305,12 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 		}
 	}
 
-	if (get_sha1_committish(rev, sha1))
-		die(_("Failed to resolve '%s' as a valid ref."), rev);
-
-	/*
-	 * NOTE: As "git reset $treeish -- $path" should be usable on
-	 * any tree-ish, this is not strictly correct. We are not
-	 * moving the HEAD to any commit; we are merely resetting the
-	 * entries in the index to that of a treeish.
-	 */
-	commit = lookup_commit_reference(sha1);
-	if (!commit)
-		die(_("Could not parse object '%s'."), rev);
-	hashcpy(sha1, commit->object.sha1);
+	object = lookup_commit_or_tree(rev);
+	if (object->type == OBJ_COMMIT)
+		commit = (struct commit*) object;
+	else if (reset_type == SOFT)
+		die(_("--soft requires a commit, which '%s' is not"), rev);
+	hashcpy(sha1, object->sha1);
 
 	if (patch_mode) {
 		if (reset_type != NONE)
@@ -347,23 +356,25 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 			die(_("Could not reset index file to revision '%s'."), rev);
 	}
 
-	/* Any resets update HEAD to the head being switched to,
-	 * saving the previous head in ORIG_HEAD before. */
-	if (!get_sha1("ORIG_HEAD", sha1_old_orig))
-		old_orig = sha1_old_orig;
-	if (!get_sha1("HEAD", sha1_orig)) {
-		orig = sha1_orig;
-		set_reflog_message(&msg, "updating ORIG_HEAD", NULL);
-		update_ref(msg.buf, "ORIG_HEAD", orig, old_orig, 0, MSG_ON_ERR);
+	if (commit) {
+		/* Any resets update HEAD to the head being switched to,
+		 * saving the previous head in ORIG_HEAD before. */
+		if (!get_sha1("ORIG_HEAD", sha1_old_orig))
+			old_orig = sha1_old_orig;
+		if (!get_sha1("HEAD", sha1_orig)) {
+			orig = sha1_orig;
+			set_reflog_message(&msg, "updating ORIG_HEAD", NULL);
+			update_ref(msg.buf, "ORIG_HEAD", orig, old_orig, 0, MSG_ON_ERR);
+		}
+		else if (old_orig)
+			delete_ref("ORIG_HEAD", old_orig, 0);
+		set_reflog_message(&msg, "updating HEAD", rev);
+		update_ref_status = update_ref(msg.buf, "HEAD", sha1, orig, 0, MSG_ON_ERR);
 	}
-	else if (old_orig)
-		delete_ref("ORIG_HEAD", old_orig, 0);
-	set_reflog_message(&msg, "updating HEAD", rev);
-	update_ref_status = update_ref(msg.buf, "HEAD", sha1, orig, 0, MSG_ON_ERR);
 
 	switch (reset_type) {
 	case HARD:
-		if (!update_ref_status && !quiet)
+		if (commit && !update_ref_status && !quiet)
 			print_new_head_line(commit);
 		break;
 	case SOFT: /* Nothing else to do. */
diff --git a/t/t1512-rev-parse-disambiguation.sh b/t/t1512-rev-parse-disambiguation.sh
index 6b3d797..bc1e40c 100755
--- a/t/t1512-rev-parse-disambiguation.sh
+++ b/t/t1512-rev-parse-disambiguation.sh
@@ -121,10 +121,6 @@ test_expect_success 'git log takes only commit-ish' '
 	git log 000000000
 '
 
-test_expect_success 'git reset takes only commit-ish' '
-	git reset 000000000
-'
-
 test_expect_success 'first tag' '
 	# create one tag 0000000000f8f
 	git tag -a -m j7cp83um v1.0.0
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index b096dc8..d723ef5 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -491,4 +491,30 @@ test_expect_success 'disambiguation (4)' '
 	test ! -f secondfile
 '
 
+test_expect_success 'reset to tree does not update HEAD' '
+	git reset --hard HEAD &&
+	rev_before=$(git rev-parse HEAD) &&
+	git reset HEAD^^{tree} &&
+	test $(git rev-parse HEAD) == $rev_before
+'
+
+test_expect_success 'reset to tree' '
+	# for simpler tests, drop last commit containing added files
+	git reset --hard HEAD^ &&
+	git reset HEAD^^{tree} &&
+	git diff --cached HEAD^ --exit-code &&
+	git diff HEAD --exit-code
+'
+
+test_expect_success 'reset --hard to tree' '
+	git reset --hard &&
+	git reset --hard HEAD^^{tree} &&
+	git diff --cached HEAD^ --exit-code &&
+	git diff HEAD^ --exit-code
+'
+
+test_expect_success 'reset to tree not allowed with --soft}' '
+	test_must_fail git reset --soft HEAD^^{tree}
+'
+
 test_done

-- 
1.8.0.1.240.ge8a1f5a
Previous: Martin von ZweigbergkNext: Junio C Hamano
Message 7 of 18 in “Operations on unborn branch”
  1. Martin von ZweigbergkNov 27, 2012
  2. Junio C HamanoNov 27, 2012
  3. Martin von ZweigbergkNov 27, 2012
  4. Junio C HamanoNov 28, 2012
  5. Martin von ZweigbergkNov 30, 2012
  6. 0/2 Fix "git reset" on unborn branchMartin von Zweigbergk, Nov 29, 2012
  7. 1/2 reset: learn to reset to treeMartin von Zweigbergk, Nov 29, 2012
  8. Junio C HamanoNov 29, 2012
  9. Martin von ZweigbergkNov 29, 2012
  10. Junio C HamanoNov 29, 2012
  11. Junio C HamanoNov 29, 2012
  12. Martin von ZweigbergkNov 29, 2012
  13. Martin von ZweigbergkNov 30, 2012
  14. Junio C HamanoDec 1, 2012
  15. Martin von ZweigbergkDec 5, 2012
  16. Junio C HamanoDec 5, 2012
  17. Martin von ZweigbergkDec 5, 2012
  18. 2/2 reset: learn to reset on unborn branchMartin von Zweigbergk, Nov 29, 2012

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.