On Tue, Oct 16, 2018 at 06:09:41PM +0200, Duy Nguyen wrote: > On Tue, Oct 16, 2018 at 6:01 PM Derrick Stolee wrote: > > > > On 10/16/2018 11:35 AM, Duy Nguyen wrote: > > > On Mon, Oct 15, 2018 at 4:23 AM brian m. carlson > > > wrote: > > >> Since the commit-graph code wants to serialize the hash algorithm into > > >> the data store, specify a version number for each supported algorithm. > > >> Note that we don't use the values of the constants themselves, as they > > >> are internal and could change in the future. > > >> > > >> Signed-off-by: brian m. carlson > > >> --- > > >> commit-graph.c | 9 ++++++++- > > >> 1 file changed, 8 insertions(+), 1 deletion(-) > > >> > > >> diff --git a/commit-graph.c b/commit-graph.c > > >> index 7a28fbb03f..e587c21bb6 100644 > > >> --- a/commit-graph.c > > >> +++ b/commit-graph.c > > >> @@ -45,7 +45,14 @@ char *get_commit_graph_filename(const char *obj_dir) > > >> > > >> static uint8_t oid_version(void) > > >> { > > >> - return 1; > > >> + switch (hash_algo_by_ptr(the_hash_algo)) { > > >> + case GIT_HASH_SHA1: > > >> + return 1; > > >> + case GIT_HASH_SHA256: > > >> + return 2; > > > Should we just increase this field to uint32_t and store format_id > > > instead? That will keep oid version unique in all data formats. > > Both the commit-graph and multi-pack-index store a single byte for the > > hash version, so that ship has sailed (without incrementing the full > > file version number in each format). > > And it's probably premature to add the oid version field when multiple > hash support has not been fully realized. Now we have different ways > of storing hash id and need separate mappings. Honestly, anything in the .git directory that is not the v3 pack indexes or the loose object file should be in exactly one hash algorithm. We could simply just leave this value at 1 all the time and ignore the field, since we already know what algorithm it will use. > I would go for incrementing file version. Otherwise maybe we just > update format_id to be one byte instead, and the way of storing hash > version in commit-graph will be used everywhere. It needs to be four bytes for pack files so that it's four-byte aligned. Otherwise accessing pack index fields will cause alignment issues if we don't massively rewrite the pack handling code. -- brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204