Re: [PATCH v2 2/2] archive-tar: huge offset and future timestamps would not work on 32-bit
On Fri, Jul 15, 2016 at 12:36 AM, Junio C Hamanowrote: > 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
Jeff Kingwrites: > 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
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