Re: [PATCH v1 0/4] replace: add option to edit a Git object

From: Jeff King <peff@peff.net>
Date: Apr 29, 2014, 02:36 UTC
Message-ID: <20140429023629.GB11730@sigill.intra.peff.net>
In-Reply-To: <20140426194404.21546.82305.chriscool@tuxfamily.org>

On Sat, Apr 26, 2014 at 10:00:53PM +0200, Christian Couder wrote:

> This patch series comes from what Peff sent in the following thread:
> 
> http://thread.gmane.org/gmane.comp.version-control.git/243361/focus=243528

Thanks. As I recall, these were in pretty good shape, and I just read over them again and didn't see anything wrong.

Show 8 quoted lines

> I added the following fixes:
> 
> - add "strbuf_release(&result);" in import_object(); this was suggested
>   by Eric Sunshine
> - use MODE_LIST instead of MODE_DELETE if no arguments are passed; this
>   makes the test suite pass
> - add "--no-replace-objects" when calling "git cat-file" in export_object();
>   so that we edit the original object if an object is already replaced

All sensible, I think.

> I am not happy with the fact that if the user doesn't modify the object when
> editing it, then a replace ref can still be created that points to the
> original object. I think something should be done to avoid that.

Yeah, it should be easy to just hashcmp the sha1s after calling import_object. In fact, I think we can just erase any existing replace ref in that case (the user might have started with a replace ref and converted it _back_ to the original object, for example).

> Once that is fixed, I plan to add some tests and documentation, but I wanted
> first to let you know that I am looking at this.

Great. Thanks for working on this.

-Peff