git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Junio C Hamano <gitster@pobox.com>
Date
Aug 8, 2013, 21:38 UTC
Message-ID
<7vsiyj982w.fsf@alter.siamese.dyndns.org>
In-Reply-To
<A88E12BCC58A401DBDD2EB300C1AAA2D@PhilipOakley>
"Philip Oakley" <philipoakley@iee.org> writes:
Show 17 quoted lines
> From: "Christian Couder" <chriscool@tuxfamily.org>
> 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 <chriscool@tuxfamily.org>
>> ---
>> If this patch is considered useful, I will update the doc and
>> maybe add tests.
>
> Acked-by: Philip Oakley <philipoakley@iee.org>
> Improved documentation would always be useful.
Will wait for a reroll with tests and doc updates, then.
Show 11 quoted lines
>> @@ -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).

Show 7 quoted lines
>> + 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.
Previous: Philip Oakley
Message 4 of 4 in “replace: forbid replacing an object with one of a different type”
  1. replace: forbid replacing an object with one of a different typeChristian Couder, Aug 7, 2013
  2. Thomas RastAug 7, 2013
  3. Philip OakleyAug 8, 2013
  4. Junio C HamanoAug 8, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.