From: Philip Oakley Date: Thu, 08 Aug 2013 21:09:31 GMT Subject: Re: [PATCH] replace: forbid replacing an object with one of a different type Message-ID: In-Reply-To: <20130807044248.17464.35806.chriscool@tuxfamily.org> From: "Christian Couder" Sent: Wednesday, August 07, 2013 5:42 AM > Users replacing an object with one of a different type were not > prevented to do so, even if it was obvious, and stated in the doc, > that bad things would result from doing that. > > To avoid mistakes, it is better to just forbid that though. > > The doc will be updated in a later patch. > > Signed-off-by: Christian Couder > --- > If this patch is considered useful, I will update the doc and > maybe add tests. Acked-by: Philip Oakley Improved documentation would always be useful. > > builtin/replace.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/builtin/replace.c b/builtin/replace.c > index 59d3115..0246ab3 100644 > --- a/builtin/replace.c > +++ b/builtin/replace.c > @@ -85,6 +85,7 @@ static int replace_object(const char *object_ref, > const char *replace_ref, > int force) > { > unsigned char object[20], prev[20], repl[20]; > + enum object_type obj_type, repl_type; > char ref[PATH_MAX]; > struct ref_lock *lock; > > @@ -100,6 +101,14 @@ static int replace_object(const char *object_ref, > const char *replace_ref, > if (check_refname_format(ref, 0)) > die("'%s' is not a valid ref name.", ref); > > + obj_type = sha1_object_info(object, NULL); > + repl_type = sha1_object_info(repl, NULL); Do (very) large blobs have any issues here? As I understand it, such blobs, as with other smaller objects, need to be expanded to determine the type. Is there a heuristic (and is it worth it) to do a 'large object == blob' initial check to avoid such an expansion? Small objects shouldn't be an overhead. Just a thought. > + if (obj_type != repl_type) > + die("Object ref '%s' is of type '%s'\n" > + "while replace ref '%s' is of type '%s'.", > + object_ref, typename(obj_type), > + replace_ref, typename(repl_type)); Perhaps start with "Objects must be of the same type.\n" > + > if (read_ref(ref, prev)) > hashclr(prev); > else if (!force) > -- > 1.8.4.rc1.22.g132b1a0