Re: [PATCH v2 05/10] replace: make sure --edit results in a different object

[Jeff King]

On Sat, May 17, 2014 at 08:41:27AM +0200, Christian Couder wrote:

> It's a bad idea to create a replace ref for an object
> that points to the original object itself.
> 
> That's why we have to check if the result from editing
> the original object is a different object and error out
> if it isn't.

I think that's reasonable.

Another option, which I think I mentioned earlier, would be to delete
any existing replacement ref, and return success. So you could use that
to "revert" a replaced object back to its original non-replaced form.

On a similar note, we might want to consider what happens when you
"--edit" an object which already has a replacement. Right now you end up
editing the _original_ object. I wonder if it would make sense to start
the editor with the contents of the replacement object (in which case
you might even "revert" without realizing it).

But those can easily come later if somebody feels like working on them.
Erroring out is not that bad an outcome, since the user can then "git
replace -d" themselves if they want to.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 05/10] replace: make sure --edit results in a different object

[Christian Couder]

It's a bad idea to create a replace ref for an object
that points to the original object itself.

That's why we have to check if the result from editing
the original object is a different object and error out
if it isn't.

Signed-off-by: Christian Couder 
---
 builtin/replace.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/builtin/replace.c b/builtin/replace.c
index 3da1bae..0751804 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -275,6 +275,9 @@ static int edit_and_replace(const char *object_ref, int 
force)
 
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);
 }
 
-- 
1.9.rc0.17.g651113e


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html