Repository navigation
Conversation
Co-authored-by: i80487 <222595226+i80487@users.noreply.github.com>
Co-authored-by: i80487 <222595226+i80487@users.noreply.github.com>
|
I polished these changes rather dramatically, with the result being available here: main...dscho:git:copilot/fix-zip-entry-size-truncation. Please review, in particular the commit messages. |
|
@dscho: Yes, looking back at your work in copilot/fix-zip-entry-size-truncation branch, it's starting to make sense to me: my PR was a single oversized commit that mixed too many unrelated changes. I've reviewed the changes you made, and everything looks correct to me. There are still a few harmless implicit conversions between Do you want me to also review the commit messages? |
You mean implicit casts from
@i80487 That would be lovely! |
Yes, you’re right; that’s why I used the adjective "harmless" in my previous response. The remaining occurrences are harmless and can be ignored.
To be honest, I’m probably not the best person to review and comment on your commit messages. Here are a few thoughts on the commit messages, IMHO:
I'm not sure I understand the commit title correctly. I would rather write:
From my point of view, it doesn't seem to be a regression. I would just write:
I would just say explicitly : |
|
@dscho: For your information, I deleted my clone of the Git for Windows repository, and it seems that this automatically closed this PR. However, the expected changes are still present in the |
Note: This PR was created as part of a test of GitHub Copilot agents. I believe the proposed changes are relevant and consistent, but I don’t have enough experience with the git-for-windows source code to evaluate them properly.
I was unable to verify that these changes work correctly using the test script t/t5005-archive-large-files.sh, because it appears that the GIT_TEST_LONG=true option and the SIZE_T_IS_64BIT prerequisite must be enabled, and I don’t know how to configure this in GitHub CI.
Feel free to send me your comments, modify, defer, or reject this PR.
Details:
git archivestill carriedunsigned longsize plumbing in bothzipandtarpaths, which truncates 4GiB+ entry sizes on LLP64 Windows before Zip64/PAX logic can trigger. As a result, large entries could be archived with silently wrapped sizes even though the surrounding format support already handled 64-bit lengths.Common archive callback plumbing
write_archive_entry_fn_tand thewrite_archive_entries()call chain fromunsigned longtosize_t.Zip backend: make Zip64 decisions from real sizes
size_t.uint32_t; do not conflate checksum width with payload size width.size_t-sized buffers.Tar backend: keep tar/PAX consistent with widened sizes
size_t.size_tconsistently.Regression coverage
SIZE_T_IS_64BITarchive test that creates a real 4GiB file, commits it, archives it, and verifies the reported entry size in both zip and tar output.t/meson.build.