Subject: Re: [PATCH 1/2] archive-tar: write extended headers for file sizes >= 8GB

From: René Scharfe <[email-masked]>
Date: Wed, 22 Jun 2016 07:46:13 +0200

Am 21.06.2016 um 23:02 schrieb Jeff King:
> On Tue, Jun 21, 2016 at 10:42:52PM +0200, René Scharfe wrote:
>
>>>> If we could set the limit to a lower value than 8GB for testing then we
>>>> could at least check if the extended header is written, e.g. if ustar_size()
>>>> could be convinced to return 0 every time using a hidden command line
>>>> parameter or an environment variable or something better.
>>>
>>> Yes, we could do that, though I think it loses most of the value of the
>>> test. We can check that if we hit an arbitrary value we generate the pax
>>> header, but I think what we _really_ care about is: did we generate an
>>> output that somebody else's tar implementation can handle.
>>
>> I agree with the last point, but don't see how that diminishes the
>> value of such a test.  If we provide file sizes only through extended
>> headers (the normal header field being set to 0) and we can extract
>> files with correct sizes then tar must have interpreted those header
>> as intended, right?
>
> The diminished value is:
>
>    1. This is a situation that doesn't actually happen in real life.
>
>    2. Now we're carrying extra code inside git only for the sake of
>       testing (which can have its own bugs, etc).
>
> Still, it may be better than nothing.
>
>> -- >8 --
>> Subject: archive-tar: test creation of pax extended size headers
>>
>> ---
>> The value 120 is magic; we need it to pass the tests.  That's
>> because prepare_header() is used for building extended header
>> records as well and we don't create extended headers for extended
>> headers (not sure if that would work anyway), so they simply
>> vanish when they're over the limit as their size field is set to
>> zero.
>
> Right, so this is sort of what I meant in (2). Now we have a
> tar.ustarsizemax setting shipped in git that is totally broken if you
> set it to "1".
>
> I can live with it as a tradeoff, but it is definitely a negative IMHO.

Yes, it's only useful as a debug flag, but the fact that it breaks 
highlights a (admittedly mostly theoretical) shortcoming: The code 
doesn't handle extended headers that are over the size limit as nicely 
as it could.  So the test was already worth it, even if won't land in 
master. :)

René