patchRe: [PATCH] Prevent git from rehashing 4GBi files

>Philip Oakley <philipoakley@iee.email> writes:
>
>>> This may treat non-zero multiple of 4GiB as "not racy", but has
>>> anybody double checked the concern Réne brought up earlier that a
>>> 4GiB file that was added and then got rewritten to 2GiB within the
>>> same second would suddenly start getting treated as not racy?
>> This is the pre-existing problem, that ~1in 2^31 size changes might not
>> get noticed for size change. The 0 byte / 4GiB change is an identical
>> issue, as is changing from 3 bytes to 4GiB+3 bytes, etc., so that's no
>> worse than before (well maybe twice as 'unlikely').
>
>OK, it added one more case to 2^32-1 existing cases, I guess.
>
>>> The patch (the firnal version of it anyway) needs to be accompanied
>>> by a handful of test additions to tickle corner cases like that.
>> They'd be protected by the EXPENSIVE prerequisite I would assume.
> 
> Oh, absolutely.  Thanks for spelling that out.

>> Philip Oakley <philipoakley@iee.email> writes:
>>
>>>> This may treat non-zero multiple of 4GiB as "not racy", but has
>>>> anybody double checked the concern Réne brought up earlier that a
>>>> 4GiB file that was added and then got rewritten to 2GiB within the
>>>> same second would suddenly start getting treated as not racy?
>>> This is the pre-existing problem, that ~1in 2^31 size changes might not
>>> get noticed for size change. The 0 byte / 4GiB change is an identical
>>> issue, as is changing from 3 bytes to 4GiB+3 bytes, etc., so that's no
>>> worse than before (well maybe twice as 'unlikely').
>> OK, it added one more case to 2^32-1 existing cases, I guess.
>>
>>>> The patch (the firnal version of it anyway) needs to be accompanied
>>>> by a handful of test additions to tickle corner cases like that.
>>> They'd be protected by the EXPENSIVE prerequisite I would assume.
>> Oh, absolutely.  Thanks for spelling that out.
> I have been testing out the patch a bit and have good and (mostly) bad news.
>
> What works using a munge value of 1.
>
> $ git add
> $ git status
>
> Racy seems to work.
>
> $ touch .git/index 4GiB # 4GiB is now racy
> $ git status # Git will rehash the racy file
> $ git status # Git cached the file. Second status is fast.
>
> What doesn't work.
>
> $ git checkout 4GiB
> $ fatal: packed object is corrupt!
>
> Using a munge value of 1<<31 causes even more problems. The file hash in the
> index for 4GiB files (git ls-files -s --debug) are set to the zero file hash.
>
> I looked up and down the code base and couldn't figure out how the munged
> value was leaking out of read-cache.c and breaking things. Most of the code
> I found tends to use stat and then convert that to a size_t, not using the
> munged unsigned int at all.
>
> Maybe someone else will have better luck. This seems over my head :(
>
> Thanks
> --
> Jason
>

>> Is there a problem that 1<<31, when on a 32bit long is MAX_NEG, 
>> rather than being MAX_POS? And the size would need to be positive to 
>> be an acceptable file size?
>> (The code is a bit of a mish-mash on the Windows LLP64 side, where 
>> long is only 32 bits).
>>
>> Philip
>> Apologies for the terseness.
>
> Philip
>
> I made a little test script and tried out several different
> things.
>
> tldr; It didn't make any difference.
>
> Files tested:
> 1, 2 and 4 GiB with and without LFS. Tested with 0, 1, 1<<30,
> and 1<<31 mung builds. I'm only listing the problems unless
> stated otherwise. The mung didn't appear to introduce any
> new issues with my limited tests.
>
> git 2.36.0.windows.1 release:    fails on 4GiB w/o LFS - corrupts pack 
> file
>    git status is very slow.
>    Sometimes stores zero file instead of corruption.
>
> git 2.36.0.windows.1 custom compile w/o patches:
>    fails on 4GiB w/o LFS - stores zero file
>    git status is very slow.
>
> git 2.36.0.windows.1 with 1U<<31 mung:
>    fails on 4GiB w/o LFS - stores zero file
>
> git 2.36.0.windows.1 with 1U<<30 mung:
>    fails on 4GiB w/o LFS - stores zero file
>
> git 2.36.0.windows.1 with 1 mung:
>    fails on 4GiB w/o LFS - stores zero file
>
> git 2.36.0 Ubuntu
>    unpatched works, but has the slow status issue.
>
> The test script I used is below:

>
>
> #!/bin/sh
>
> GB1=$((1 * 1024*1024*1024))
> GB2=$((2 * 1024*1024*1024))
> GB4=$((4 * 1024*1024*1024))
>
> die()
> {
>    echo "$1"
>    exit 1
> }
>
> test_file()
> {
>    echo "=== TESTING $2 ==="
>    rm -rf .git .gitattributes .gitignore .gitmodules &&
>        git init &&
>        git lfs track '*.big' &&
>        truncate --size "$1" "$2" &&
>        git add "$2" &&
>        git commit -m "$2" &&
>        git fsck &&
>        mv "$2" bak &&
>        git restore "$2" &&
>        cmp "$2" bak || die "$2"
>    git status && timeout 5 git status || die "$2 git status slow"
>    rm -rf .git .gitattributes .gitignore .gitmodules "$2" bak
> }
>
> test_file "$GB1" gb1.big
> test_file "$GB2" gb2.big
> test_file "$GB4" gb4.big
> test_file "$GB1" gb1
> test_file "$GB2" gb2
> test_file "$GB4" gb4
> echo done 

>
>
> On 7 May 2022 03:15:00 BST, Jason Hatton <jhatton@globalfinishing.com> wrote:
>
>         Philip Oakley <philipoakley@iee.email> writes:
>
>                 This may treat non-zero multiple of 4GiB as "not racy", but has
>                 anybody double checked the concern Réne brought up earlier that a
>                 4GiB file that was added and then got rewritten to 2GiB within the
>                 same second would suddenly start getting treated as not racy?
>
>             This is the pre-existing problem, that ~1in 2^31 size changes might not
>             get noticed for size change. The 0 byte / 4GiB change is an identical
>             issue, as is changing from 3 bytes to 4GiB+3 bytes, etc., so that's no
>             worse than before (well maybe twice as 'unlikely').
>
>
>         OK, it added one more case to 2^32-1 existing cases, I guess.
>
>                 The patch (the firnal version of it anyway) needs to be accompanied
>                 by a handful of test additions to tickle corner cases like that.
>
>             They'd be protected by the EXPENSIVE prerequisite I would assume.
>
>
>         Oh, absolutely. Thanks for spelling that out.
>
>
>     I have been testing out the patch a bit and have good and (mostly) bad news.
>
>     What works using a munge value of 1.
>
>     $ git add
>     $ git status
>
>     Racy seems to work.
>
>     $ touch .git/index 4GiB # 4GiB is now racy
>     $ git status # Git will rehash the racy file
>     $ git status # Git cached the file. Second status is fast.
>
>     What doesn't work.
>
>     $ git checkout 4GiB
>     $ fatal: packed object is corrupt!
>
>     Using a munge value of 1<<31 causes even more problems. The file hash in the
>     index for 4GiB files (git ls-files -s --debug) are set to the zero file hash.
>
>     I looked up and down the code base and couldn't figure out how the munged
>     value was leaking out of read-cache.c and breaking things. Most of the code
>     I found tends to use stat and then convert that to a size_t, not using the
>     munged unsigned int at all.
>
>     Maybe someone else will have better luck. This seems over my head :(
>
>     Thanks
>     --
>     Jason
>
>
> Is this on Git for Windows or a 64 bit Linux?
> There are still some issues on GfW for 2GiB+ files (long Vs long long int).