Re: [PATCH 1/2] archive-tar: write extended headers for file sizes >= 8GB
- From
René Scharfe <l.s.r@web.de>
- Date
- Jun 22, 2016, 05:46 UTC
- Message-ID
- <576A2625.8010706@web.de>
- In-Reply-To
- <20160621210234.GB4747@sigill.intra.peff.net>
Am 21.06.2016 um 23:02 schrieb Jeff King:
Show 43 quoted lines
> 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é