Re: [PATCH 10/31] object-store: rename and expand packed_git's sha1 member
- From
brian m. carlson <sandals@crustytoothpaste.net>
- Date
- Feb 14, 2019, 03:33 UTC
- Message-ID
- <20190214033315.GK684736@genre.crustytoothpaste.net>
- In-Reply-To
- <CAPig+cS3i2FWf6mFmpPpctyJJbx4xdHgWXrveRUkjMHct5SbJg@mail.gmail.com>
On Mon, Feb 11, 2019 at 10:32:15PM -0500, Eric Sunshine wrote:
Show 16 quoted lines
> On Mon, Feb 11, 2019 at 8:23 PM brian m. carlson
> <sandals@crustytoothpaste.net> wrote:
> > This member is used to represent the pack checksum of the pack in
> > question. Expand this member to be GIT_MAX_RAWSZ bytes in length so it
> > works with longer hashes and rename it to be "hash" instead of "sha1".
> > [...]
> > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> > ---
> > diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
> > @@ -689,7 +689,7 @@ int cmd_pack_redundant(int argc, const char **argv, const char *prefix)
> > while (pl) {
> > printf("%s\n%s\n",
> > - sha1_pack_index_name(pl->pack->sha1),
> > + sha1_pack_index_name(pl->pack->hash),
>
> I guess there is no oid_pack_index_name() function yet?Correct, although see below.
Show 8 quoted lines
> > diff --git a/http-walker.c b/http-walker.c
> > @@ -434,7 +434,7 @@ static int http_fetch_pack(struct walker *walker, struct alt_base *repo, unsigne
> > if (walker->get_verbosely) {
> > fprintf(stderr, "Getting pack %s\n",
> > - sha1_to_hex(target->sha1));
> > + sha1_to_hex(target->hash));
>
> Should this have become oid_to_hex()?No, I think what's misleading here is that the member for both is named "hash". This is a struct packed_git, not a struct object_id. The reason I didn't convert this to a struct object_id is because this isn't actually an object ID, it's a pack checksum hash.
I've tried to be diligent about separating the object IDs from other hashes because I think it makes it easier to reason about the code, but also because it may help us avoid tricky behavior when we have to deal with multiple algorithms for input and storage.
-- brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204