t5608, t7508: require LONG_IS_64BIT for >4GB tests#2129
Open
spkrka wants to merge 1 commit into
Open
Conversation
The >4GB clone tests in t5608 (tests 5-6) fail on Windows because the server-side pack-objects still uses `unsigned long` for object sizes. On Windows x86_64 (LLP64), `unsigned long` is 32 bits, so pack-objects dies in cast_size_t_to_ulong() when it encounters a 4294967297-byte object: fatal: object too large to read on this platform: 4294967297 is cut off to 1 The streaming/odb/index-pack side was widened to size_t in js/objects-larger-than-4gb-on-windows, but pack-objects was deliberately left as a stop-gap with cast_size_t_to_ulong() at the type boundaries. Until the pack-objects path is also widened, gate these tests on LONG_IS_64BIT so they skip on Windows rather than failing. Similarly, t7508's "status does not re-read unchanged 4 or 8 GiB file" test fails on Windows because ftruncate is implemented via _chsize which takes a `long` parameter, overflowing at 2 GiB. The subsequent git-add and git-diff-index also route through object_info.sizep (unsigned long), which would truncate. Add LONG_IS_64BIT here too. Note: LONG_IS_64BIT is a stop-gap, not the right long-term fix. Once the remaining `unsigned long` code paths (pack-objects, object_info.sizep, and the MSVC ftruncate compat shim) are widened to use size_t, these tests should work on all 64-bit platforms and the LONG_IS_64BIT prerequisite can be dropped. Signed-off-by: Kristofer Karlsson <krka@spotify.com>
spkrka
force-pushed
the
fix-4gb-test-prereqs
branch
from
279b211 to
79334a2
Compare
Member
|
I am still hoping that my side-track comes to fruition, where the failures are fixed, so that we don't have to suppress them. |
dscho
added a commit
to dscho/git
that referenced
this pull request
Jun 1, 2026
The >4GB tests called out by PR 2129 (gitgitgadget#2129), namely t5608 5-7 ("set up repo with >4GB object" and the two clone flavours) and t7508 msysgit#126 ("status does not re-read unchanged 4 or 8 GiB file"), are tagged `EXPENSIVE`. The `EXPENSIVE` prerequisite evaluates true only when `GIT_TEST_LONG` is set, which `ci/lib.sh` gates on pull-request events and on pushes to the integration branches (main, master, next, maint). A push of a topic branch matches none of those. Force `GIT_TEST_LONG=YesPlease` unconditionally for the duration of this verification cycle so the CI build of this topic actually runs the tests the series exists to make pass. Drop this commit before squashing the topic for upstream submission; the existing gating is the correct upstream behaviour. Assisted-by: Opus 4.7 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Author
|
Yeah, I have not submitted this yet, I was thinking the same, not sure how far away a better fix is. |
dscho
added a commit
to dscho/git
that referenced
this pull request
Jun 1, 2026
PR 2102 (`js/objects-larger-than-4gb-on-windows`) widened the streaming, index-pack and unpack-objects paths to `size_t` but deliberately stopped at the in-memory `unpack_entry()` cascade, which still hands back the unpacked size through `unsigned long *`. On Windows that boundary truncates above 4 GiB, and is one of the two narrow API boundaries PR 2129 [1] proposes to skip past by gating `t5608` 5/6 and `t7508` 126 behind `LONG_IS_64BIT`. Widen the cascade. `packed_object_info_with_index_pos()` cannot yet pass `oi->sizep` directly because the field is still `unsigned long *`; bridge with a `size_t` temporary that narrows back, and let a later commit drop the bridge once the field is wide too. `gfi_unpack_entry()` keeps its narrow signature because fast-import tracks sizes through `unsigned long` everywhere it crosses subsystem boundaries; one shim inside the wrapper localises the narrowing. [1] gitgitgadget#2129 Assisted-by: Opus 4.7 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
dscho
added a commit
to dscho/git
that referenced
this pull request
Jun 1, 2026
PR 2102 (`js/objects-larger-than-4gb-on-windows`) widened the
streaming, index-pack and unpack-objects code paths but left the
public ODB API still typed in `unsigned long`. In particular
`struct object_info::sizep` and the four wrappers built on top of
it (`odb_read_object`, `odb_read_object_peeled`,
`odb_read_object_info`, `odb_pretend_object`) still return the
unpacked size through `unsigned long *`, so on Windows
`cat-file -s` and the `git add` / `git status` paths for a >4 GiB
blob silently cap at 4 GiB. That is the other half of what
PR 2129 [1] proposes to skip behind `LONG_IS_64BIT`, alongside
`t7508` 126 ("status does not re-read unchanged 4 or 8 GiB file").
Widen the field and the four wrappers. The previous commits
already widened the `unpack_entry()` cascade and pack-objects'
in-core size accessors, so most of the cascade arrives here with
no further work: the temporary shims in
`packed_object_info_with_index_pos()` and in `unpack_entry()`'s
delta-base recovery path go away, the two
`SET_SIZE(entry, cast_size_t_to_ulong(canonical_size))` calls in
`check_object()` and the matching one in `drop_reused_delta()`
collapse to plain `SET_SIZE`, and `oe_get_size_slow()`'s tail
`cast_size_t_to_ulong()` is gone too.
What remains narrow are the boundaries this series does not
intend to touch: `diff_filespec::size`, `blame_scoreboard::
final_buf_size`, the `mmfile_t::size` used by xdiff (and hence by
`textconv_object()`), `tree_desc`'s `unsigned long`, the
`pbase_tree_cache::tree_size` field, and `fast-import`'s
`unsigned long size` locals that feed `mark_pack_data` and
`parse_from_commit`. At each of those boundaries this commit
adds a localised `size_t` temporary followed by an explicit
`cast_size_t_to_ulong()`, so the narrowing is loud and traceable.
`merge_blobs()` (only caller `builtin/merge-tree.c::show_diff()`)
and the file-static `archive.c::object_file_to_archive()` are
widened in passing because they have a single caller each;
neither cascades further.
[1] gitgitgadget#2129
Assisted-by: Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
dscho
added a commit
to dscho/git
that referenced
this pull request
Jun 1, 2026
The >4GB tests called out by PR 2129 (gitgitgadget#2129), namely t5608 5-7 ("set up repo with >4GB object" and the two clone flavours) and t7508 msysgit#126 ("status does not re-read unchanged 4 or 8 GiB file"), are tagged `EXPENSIVE`. The `EXPENSIVE` prerequisite evaluates true only when `GIT_TEST_LONG` is set, which `ci/lib.sh` gates on pull-request events and on pushes to the integration branches (main, master, next, maint). A push of a topic branch matches none of those. Force `GIT_TEST_LONG=YesPlease` unconditionally for the duration of this verification cycle so the CI build of this topic actually runs the tests the series exists to make pass. Drop this commit before squashing the topic for upstream submission; the existing gating is the correct upstream behaviour. Assisted-by: Opus 4.7 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The >4GB clone tests in t5608 and the large-file status test in t7508
fail on Windows x86_64 where
unsigned longis 32 bits.t5608 (tests 5-6): The server-side pack-objects still uses
unsigned longfor object sizes, socast_size_t_to_ulong()dieswhen it encounters a 4294967297-byte object. The streaming/odb side
was widened to
size_tin js/objects-larger-than-4gb-on-windows, butpack-objects was deliberately left as a stop-gap.
t7508 (test 126):
ftruncateis implemented via_chsizeonMSVC, which takes a
longparameter — overflowing at 2 GiB. Thesubsequent
git add/git diff-indexalso route throughobject_info.sizep(unsigned long), which would truncate.Add
LONG_IS_64BITprerequisite so these tests skip on Windowsuntil the remaining
unsigned long→size_twidening lands.cc: @dscho