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

Re: [PATCH v2 01/10] Define a structure for object IDs.

From
Duy Nguyen <pclouds@gmail.com>
Date
Mar 12, 2015, 11:16 UTC
Message-ID
<CACsJy8AL0jGwzTfWDSszkvyqzNtPA7o7vQRT5NFaBYrVU02nOA@mail.gmail.com>
In-Reply-To
<20150312104651.GF46326@vauxhall.crustytoothpaste.net>

On Thu, Mar 12, 2015 at 5:46 PM, brian m. carlson <sandals@crustytoothpaste.net> wrote:

Show 47 quoted lines
> On Thu, Mar 12, 2015 at 11:28:10AM +0100, Michael Haggerty wrote:
>>
>> On 03/12/2015 01:26 AM, Junio C Hamano wrote:
>>>
>>> And that would break the abstraction effort if you start calling the
>>> field with a name that is specific to the underlying hash function.
>>> The caller has to change o->sha1 to o->sha256 instead of keeping
>>> that as o->oid and letting the callee handle the implementation
>>> details when calling
>>>
>>>         if (!hashcmp(o1->oid, o2->oid))
>>>                 ; /* they are the same */
>>>         else
>>>                 ; /* they are different */
>>> [...]
>>
>>
>> Hmm, I guess you imagine that we might sometimes pack SHA-1s, sometimes
>> SHA-256s (or whatever) in the "oid" field, which would be dimensioned
>> large enough for either one (with, say, SHA-1s padded with zeros).
>>
>> I was imagining that this would evolve into a union (or maybe struct) of
>> different hash types, like
>>
>>    struct object_id {
>>        unsigned char hash_type;
>>        union {
>>            unsigned char sha1[GIT_SHA1_RAWSZ];
>>            unsigned char sha256[GIT_SHA256_RAWSZ];
>>        } hash;
>>    };
>>
>> BTW in either case, any hopes of mapping object_id objects directly on
>> top of buffer memory would disappear.
>
>
> What I think might be more beneficial is to make the hash function
> specific to a particular repository, and therefore you could maintain
> something like this:
>
>  struct object_id {
>      unsigned char hash[GIT_MAX_RAWSZ];
>  };
>
> and make hash_type (or hash_length) a global[0].  I don't think it's
> very worthwhile to try to mix two different hash functions in the same
> repository,

This may or may not fall into the "mix different hash functions" category. In pack files version 4, trees are encoded to point to other trees or blobs by a (pack, offset) tuple. It would be great if the new object_id could support carrying this kind of object id around because it could help reduce object lookup cost a lot. (pack, offset) can be converted back to SHA-1 so no info is lost and hashcmp() can compare (pack, tuple) against an SHA-1 just fine.

Show 16 quoted lines
> so we could still map directly onto buffer memory if we
> decide that's portable enough.  I expect the cases where we need to do
> that will be relatively limited.
>
> Regardless, it seems that this solution has the most support (including
> Junio's) and it's more self-documenting than my current set of patches,
> so I'm going to go with it for now.  It should be easy to change if the
> consensus goes back the other way.
>
> [0] I personally think globals are a bit gross, but they don't seem to
> have the problems that they would if git were a shared library.
>
> --
> brian m. carlson / brian with sandals: Houston, Texas, US
> +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
> OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187
-- 
Duy
Previous: brian m. carlsonNext: Junio C Hamano
Message 28 of 36 in “Use a structure for object IDs.”
  1. 00/10 Use a structure for object IDs.brian m. carlson, Mar 7, 2015
  2. 01/10 Define a structure for object IDs.brian m. carlson, Mar 7, 2015
  3. 02/10 Define utility functions for object IDs.brian m. carlson, Mar 7, 2015
  4. Duy NguyenMar 8, 2015
  5. brian m. carlsonMar 8, 2015
  6. Michael HaggertyMar 11, 2015
  7. 03/10 bisect.c: convert leaf functions to use struct object_idbrian m. carlson, Mar 7, 2015
  8. 04/10 archive.c: convert to use struct object_idbrian m. carlson, Mar 7, 2015
  9. Michael HaggertyMar 11, 2015
  10. brian m. carlsonMar 11, 2015
  11. 05/10 zip: use GIT_SHA1_HEXSZ for trailersbrian m. carlson, Mar 7, 2015
  12. 06/10 bulk-checkin.c: convert to use struct object_idbrian m. carlson, Mar 7, 2015
  13. 07/10 diff: convert struct combine_diff_path to object_idbrian m. carlson, Mar 7, 2015
  14. 08/10 commit: convert parts to struct object_idbrian m. carlson, Mar 7, 2015
  15. Michael HaggertyMar 11, 2015
  16. 09/10 patch-id: convert to use struct object_idbrian m. carlson, Mar 7, 2015
  17. 10/10 apply: convert threeway_stage to object_idbrian m. carlson, Mar 7, 2015
  18. Junio C HamanoMar 8, 2015
  19. Kyle J. McKayMar 11, 2015
  20. Michael HaggertyMar 11, 2015
  21. Junio C HamanoMar 11, 2015
  22. brian m. carlsonMar 13, 2015
  23. brian m. carlsonMar 11, 2015
  24. Junio C HamanoMar 12, 2015
  25. brian m. carlsonMar 12, 2015
  26. Michael HaggertyMar 12, 2015
  27. brian m. carlsonMar 12, 2015
  28. Duy NguyenMar 12, 2015
  29. Junio C HamanoMar 12, 2015
  30. Junio C HamanoMar 12, 2015
  31. Duy NguyenMar 13, 2015
  32. Junio C HamanoMar 13, 2015
  33. Duy NguyenMar 14, 2015
  34. Junio C HamanoMar 14, 2015
  35. Duy NguyenMar 15, 2015
  36. Junio C HamanoMar 15, 2015

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.