Re: [PATCH 02/14] builtin/repack: replace hard-coded constant
- From
brian m. carlson <sandals@crustytoothpaste.net>
- Date
- Oct 9, 2018, 23:00 UTC
- Message-ID
- <20181009230038.GU432229@genre.crustytoothpaste.net>
- In-Reply-To
- <CAPig+cS5VdYq1ZOrP9oPwpXZ3ZmSV3_QdQAHiz5fV1Tc1uEF9A@mail.gmail.com>
On Mon, Oct 08, 2018 at 07:01:27PM -0400, Eric Sunshine wrote:
Show 20 quoted lines
> On Mon, Oct 8, 2018 at 6:27 PM Stefan Beller <sbeller@google.com> wrote:
> > On Mon, Oct 8, 2018 at 2:57 PM brian m. carlson
> > <sandals@crustytoothpaste.net> wrote:
> > > - if (line.len != 40)
> > > - die("repack: Expecting 40 character sha1 lines only from pack-objects.");
> > > + if (line.len != the_hash_algo->hexsz)
> > > + die("repack: Expecting full hex object ID lines only from pack-objects.");
> >
> > This is untranslated as it is plumbing? If so, maybe
> >
> > if (is_sha1(the_hash_algo)
> > die("repack: Expecting 40 character sh...
> > else
> > die(repack: Expecting full hex object ID in %s, of length %d",
> > the_hash_algo->name,
> > the_hash_algo->hexsz);
>
> Special-casing for SHA-1 seems overkill for an error message. A script
> expecting this particular error condition and this particular error
> message would be fragile indeed.Yeah, I don't think a special case is needed here. Moreover, since we just invoked pack-objects ourselves and are now reading the output of it, seeing this error message means that someone has broken Git in a significant way. The end user should never see this message.
-- brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204