Do re-download corrupt objects with GVFS #782
Conversation
I am letting GitHub Actions build a test version for that PR as I am writing this. Signed-off-by: Johannes Schindelin <[email protected]>
Let's let it run on the `tmp` branch. Signed-off-by: Johannes Schindelin <[email protected]>
I am letting GitHub Actions build a test version for that PR as I am writing this. Signed-off-by: Johannes Schindelin <[email protected]>
Hmm. Valid point. The sad news is that the VFS for Git test case was introduced in microsoft/VFSForGit@2db0c03, which is one of those absurdly big and lamentably under-documented commits that I frequently use as examples why code changes really need to be crafted with skill. But I realize that I even failed to target the correct commit with the |
If the `read-object` hook is not found, the code currently would fail with the rather obscure message: fatal: Out of memory, strdup failed The reason is because without a check whether the hook was found, eventually `strvec_push()` would try to call `xstrdup()` with `cmd` (which is `NULL` if the hook was not found) which would fail and print that rather misleading message. Signed-off-by: Johannes Schindelin <[email protected]>
dscho
force-pushed
the
do-re-download-corrupt-objects-with-gvfs
branch
from
fd34fe0 to
cda6dd6
Compare
|
@derrickstolee thank you for helping me be a better engineer. I thought that I had a really good reproducer, but I hadn't; My initial draft of a test case reused the This experience of trying to find a way to trigger the same code path that does require the fix, however, kind of makes me think that 9e59b38, despite the claims in its commit message, is doing an incomplete job of reporting corrupted objects. Oh well. Maybe some day, when I have some free time... In the meantime, @derrickstolee, could I solicit another review, please? |
|
Ach, darn it. macOS' |
As of 9e59b38 (object-file: emit corruption errors when detected, 2022-12-14), Git will loudly complain about corrupt objects. That is fine, as long as the idea isn't to re-download locally-corrupted objects. But that's exactly what we want to do in VFS for Git via the `read-object` hook, as per the `GitCorruptObjectTests` code added in microsoft/VFSForGit@2db0c030eb25 (New features: [...] - GVFS can now recover from corrupted git object files [...] , 2018-02-16). So let's support precisely that, and add a regression test that ensures that re-downloading corrupt objects via the `read-object` hook works. While at it, avoid the XOR operator to flip the bits, when we actually want to make sure that they are turned off: Use the AND-NOT operator for that purpose. Helped-by: Matthew John Cheetham <[email protected]> Helped-by: Derrick Stolee <[email protected]> Signed-off-by: Johannes Schindelin <[email protected]>
dscho
force-pushed
the
do-re-download-corrupt-objects-with-gvfs
branch
from
cda6dd6 to
db6f8f4
Compare
As of 9e59b38, Git will loudly complain about corrupt objects. That is fine, as long as the idea isn't to re-download locally-corrupted objects. But that's exactly what we want to do in VFS for Git. This is even tested for in the functional tests of VFS for Git, which has been identified as a regression in microsoft/VFSForGit#1853.
As of 9e59b38, Git will loudly complain about corrupt objects. That is fine, as long as the idea isn't to re-download locally-corrupted objects. But that's exactly what we want to do in VFS for Git. This is even tested for in the functional tests of VFS for Git, which has been identified as a regression in microsoft/VFSForGit#1853.
As of 9e59b38, Git will loudly complain about corrupt objects. That is fine, as long as the idea isn't to re-download locally-corrupted objects. But that's exactly what we want to do in VFS for Git. This is even tested for in the functional tests of VFS for Git, which has been identified as a regression in microsoft/VFSForGit#1853.
As of 9e59b38, Git will loudly complain about corrupt objects. That is fine, as long as the idea isn't to re-download locally-corrupted objects. But that's exactly what we want to do in VFS for Git. This is even tested for in the functional tests of VFS for Git, which has been identified as a regression in microsoft/VFSForGit#1853.
2 participants
As of 9e59b38, Git will loudly complain about corrupt objects.
That is fine, as long as the idea isn't to re-download locally-corrupted objects. But that's exactly what we want to do in VFS for Git. This is even tested for in the functional tests of VFS for Git, which has been identified as a regression in microsoft/VFSForGit#1853.