[PATCH] replace: fix replacing object with itself

Subsystems: the rest

DORMANTno replies

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

[PATCH] replace: fix replacing object with itself

From: Manzur Mukhitdinov <hidden>
Date: 2016-06-15 23:02:54

When object is replaced with itself git shows unhelpful messages like(git log):
    "fatal: replace depth too high for object <SHA1>"

Prevents user from replacing object with itself(with test for checking
this case).
---
 builtin/replace.c  | 8 +++-----
 t/t6050-replace.sh | 8 ++++++++
 2 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/builtin/replace.c b/builtin/replace.c
index 294b61b..628377a 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -157,6 +157,9 @@ static int replace_object_sha1(const char *object_ref,
 	char ref[PATH_MAX];
 	struct ref_lock *lock;
 
+	if (!hashcmp(object, repl))
+		return error("new object is the same as the old one: '%s'", sha1_to_hex(object));
+
 	obj_type = sha1_object_info(object, NULL);
 	repl_type = sha1_object_info(repl, NULL);
 	if (!force && obj_type != repl_type)
@@ -295,9 +298,6 @@ static int edit_and_replace(const char *object_ref, int force, int raw)
 
 	free(tmpfile);
 
-	if (!hashcmp(old, new))
-		return error("new object is the same as the old one: '%s'", sha1_to_hex(old));
-
 	return replace_object_sha1(object_ref, old, "replacement", new, force);
 }
 
@@ -406,8 +406,6 @@ static int create_graft(int argc, const char **argv, int force)
 
 	strbuf_release(&buf);
 
-	if (!hashcmp(old, new))
-		return error("new commit is the same as the old one: '%s'", sha1_to_hex(old));
 
 	return replace_object_sha1(old_ref, old, "replacement", new, force);
 }
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index 4d5a25e..98ac9dd 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -440,4 +440,12 @@ test_expect_success GPG '--graft on a commit with a mergetag' '
 	git replace -d $HASH10
 '
 
+test_expect_success 'replacing object with itself must fail' '
+    test_must_fail git replace $HASH1 $HASH1 &&
+    HASH8=$(git rev-parse --verify HEAD) &&
+    test_must_fail git replace HEAD $HASH8 &&
+    test_must_fail git replace --graft HEAD HEAD^ &&
+    test_must_fail env GIT_EDITOR=true git replace --edit HEAD
+'
+
 test_done
-- 
1.9.1

Re: [PATCH] replace: fix replacing object with itself

From: Jeff King <hidden>
Date: 2016-06-15 23:02:54

On Tue, Nov 11, 2014 at 12:20:56AM +0100, Manzur Mukhitdinov wrote:
When object is replaced with itself git shows unhelpful messages like(git log):
    "fatal: replace depth too high for object <SHA1>"

Prevents user from replacing object with itself(with test for checking
this case).
Thanks, this looks good to me.
+test_expect_success 'replacing object with itself must fail' '
+    test_must_fail git replace $HASH1 $HASH1 &&
+    HASH8=$(git rev-parse --verify HEAD) &&
+    test_must_fail git replace HEAD $HASH8 &&
+    test_must_fail git replace --graft HEAD HEAD^ &&
+    test_must_fail env GIT_EDITOR=true git replace --edit HEAD
+'
I think some of these overlap with earlier tests (I know that the
"--edit" case is checked already), but I think it is nice to keep the
related checks together here.

Should we maybe squash this in to drop the now redundant test (AFAICT,
that is the only one that overlaps)?
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index 98ac9dd..5f96374 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -369,9 +369,8 @@ test_expect_success '--edit with and without already replaced object' '
 	git cat-file commit "$PARA3" | grep "A fake Thor"
 '
 
-test_expect_success '--edit and change nothing or command failed' '
+test_expect_success '--edit with failed editor' '
 	git replace -d "$PARA3" &&
-	test_must_fail env GIT_EDITOR=true git replace --edit "$PARA3" &&
 	test_must_fail env GIT_EDITOR="./fakeeditor;false" git replace --edit "$PARA3" &&
 	GIT_EDITOR=./fakeeditor git replace --edit "$PARA3" &&
 	git replace -l | grep "$PARA3" &&
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help