Re: [PATCH v3 31/39] bundle: add new version for use with SHA-256
- From
brian m. carlson <sandals@crustytoothpaste.net>
- Date
- Jul 24, 2020, 01:11 UTC
- Message-ID
- <20200724011127.GC1758454@crustytoothpaste.net>
- In-Reply-To
- <CAPig+cRWptYRJUpT71jW6_O9UZ1RK=FCbZj-+OkR_5kGqSWScg@mail.gmail.com>
On 2020-07-23 at 05:31:15, Eric Sunshine wrote:
> Rather than having to worry about updating this each time the format > changes, perhaps just say: > > A Git bundle consists of several parts.
Seems prudent. The reader probably doesn't care exactly how many parts there are, and if they do, they can count for themselves.
Show 23 quoted lines
> I wonder if it would be simpler and cleaner to do something like this instead: > > cat >expect <<\-EOF && > # v3 git bundle > @object-format=sha256 > -[OID] message > [OID] refs/head/master > > EOF > head -n 5 "$D"/bundle1 | sanitize_oid >actual && > test_cmp expect actual > > where sanitize_oid() is a function which converts a hex OID into the > literal string "[OID]" (or whatever). I believe I've seen you employ > such sanitization functions already in these series in cases when you > want to verify that an OID is present in some output but don't care > about the actual value. > > Or perhaps this approach is overkill? > > Reading further in this patch, I see that you actually do employ this > technique in a new test you add to t5607, though you don't bother with > OID sanitation in that test.
Sure, I can do that.
Show 9 quoted lines
> I worry about passing binary bundle data through 'sed' like this. > Historically, some 'sed' implementations would drop the last part of a > file if it didn't end with a newline. Even today, not all 'sed' > implementations necessarily pass the binary data through unmolested. > For instance, on Mac OS, 'sed' adds a newline at the end of the file > if the binary bundle blob didn't end with a newline. Perhaps a more > reliable approach would be to use Perl to read the entire bundle in as > a blob, use s/// to munge the @object-format= line into the @unknown= > line, and write it out.
Yeah, I was slightly worried about that, but as you mentioned in the followup email, it's probably fine to just parse the header and verify we reject that.
-- brian m. carlson: Houston, Texas, US