Re: [PATCH v2 2/2] archive-tar: huge offset and future timestamps would not work on 32-bit

[Duy Nguyen]

On Fri, Jul 15, 2016 at 12:36 AM, Junio C Hamano  wrote:
> Jeff King  writes:
>
>> 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 0777UL 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.

If you are on a distro with multilib, building git with "CFLAGS=-m32
LDFLAGS=-m32" should do it. That's how i tested the 32-bit offset
truncation thing. I may pursue the docker thing soon. At least then
you only need to install docker and just build/test (zero docker
configuration, assuming all the relevant kernel options are enabled).
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] archive-tar: huge offset and future timestamps would not work on 32-bit

[Junio C Hamano]

Jeff King  writes:

> 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 0777UL 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.

> 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] archive-tar: huge offset and future timestamps would not work on 32-bit

[Jeff King]

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 0777UL 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.

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.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html