Re: [PATCH] replace: forbid replacing an object with one of a different type

"Philip Oakley" [off-list ref] writes:

From: "Christian Couder" <redacted>
Sent: Wednesday, August 07, 2013 5:42 AM

quoted

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 <redacted>
---
If this patch is considered useful, I will update the doc and
maybe add tests.

Acked-by: Philip Oakley <redacted>
Improved documentation would always be useful.

Will wait for a reroll with tests and doc updates, then.

quoted

@@ -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.

sha1_object_info() is coded to peek into only the early part of a
loose object or the object header part of a packed object.  Loose
ones we do mmap(), but we only inflate a tiny bit (the first 32
bytes).

quoted

+ 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"

Yes.