Re: [PATCH 01/14] graph: add packed graph design document

From: Stefan Beller <sbeller@google.com>
Date: Jan 26, 2018, 18:17 UTC
Message-ID: <CAGZ79kZUYW-NJYUfPco_SUDdO29Wo_cKW1-gxsFGvR0-3vV0NQ@mail.gmail.com>
In-Reply-To: <6b72406f-90cd-194e-1f5f-02a43b7b60a5@gmail.com>

Show 9 quoted lines

>>> +- A graph file is stored in a file named 'graph-<oid>.graph' in the pack
>>> +  directory.
>>
>> (guessing)
>> where every commit up to <oid> is included in the file.
>
>
> Sorry, the <oid> is the hash of the graph contents (up to its trailing bytes
> that contain <oid> in binary).

Show 6 quoted lines

>> So maybe I do not understand the <oid> after all, yet.
>
>
> Perhaps I should instead use <hash> instead of <oid>, since it is not an
> actual Git object but instead the hash of the graph contents. The
> 'graph_head' file simply stores the <oid> as plain-text hex characters.

Yes, please use hash. Oid (object id) means to me the identification of
a Git object, whereas here we only use the hash to
(1) check if that file is valid (consistent, no bit flips in that;
    --verify would do this)
(2) to have a different file name for each file such that we can have
    multiple graph files at once, e.g. build the next file in the background
    while using the old file to keep working.
(3) any other use case?

For (2) we could use `mkstemp` as well, but the property of (1) is nice to have.

When using <hash> you probably mean sha1. When writing the hash transition document, we discussed about uses of sha1 apart from object naming, such as the use here or in the new pack file format and we thought it would be ok to keep sha1 for the purpose of (1), but as we may want to lose the sha1 code in the far future, it may be easier to migrate that hash to the new hash function, too. So it is easy to confuse that hash with OIDs because it is the same hash function throughout time.

Thanks, Stefan