Re: [PATCH v2 2/2] archive-tar: huge offset and future timestamps would not work on 32-bit
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 14, 2016, 22:36 UTC
- Message-ID
- <xmqqlh13onik.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <20160714222047.GA21868@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 10 quoted lines
> On Thu, Jul 14, 2016 at 01:43:57PM -0700, Junio C Hamano wrote: > >> As we are not yet moving everything to size_t but still using ulong >> internally when talking about the size of object, platforms with >> 32-bit long will not be able to produce tar archive with 4GB+ file, >> and cannot grok 077777777777UL as a constant. Disable the extended >> header feature and do not test it on them. > > I tried testing this in a VM with 32-bit Debian. It fixes the build > problems, but t5000 still fails.
Thanks for testing. I need to find some time hunting for (or building) an environment to do that myself.
Show 29 quoted lines
> I think you need to add the prereq to one more test:
>
> diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
> index 699355b..80b2387 100755
> --- a/t/t5000-tar-tree.sh
> +++ b/t/t5000-tar-tree.sh
> @@ -347,7 +347,7 @@ test_lazy_prereq TAR_HUGE '
> test_cmp expect actual
> '
>
> -test_expect_success 'set up repository with huge blob' '
> +test_expect_success LONG_IS_64BIT 'set up repository with huge blob' '
> obj_d=19 &&
> obj_f=f9c8273ec45a8938e6999cb59b3ff66739902a &&
> obj=${obj_d}${obj_f} &&
>
> We shouldn't be accessing the blob in update-index, but I think "git
> commit" does so for the diff (and then after seeing the size says
> "whoops, that's binary", but even the size check fails on 32-bit
> systems).
>
> So another solution would be to use "commit -q" at the end of that test.
> I don't think there's much point, though; it's just setting up a state
> for other tests that need LONG_IS_64BIT.
>
> As an aside, it is inadvertently testing that our diff code does not
> bother to read the whole blob in such a case. Which maybe argues for
> using "commit -q", just because that is not a thing we are intending to
> test here.Thanks. Let's add a prereq there.