-
Notifications
You must be signed in to change notification settings - Fork 188
More work supporting objects larger than 4GB on Windows #2137
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
de9fc5c
1fd7646
ddb7532
bdebc36
68750ba
460d733
f3aeae9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3232,7 +3232,7 @@ static int apply_binary_fragment(struct apply_state *state, | |
| struct patch *patch) | ||
| { | ||
| struct fragment *fragment = patch->fragments; | ||
| unsigned long len; | ||
| size_t len; | ||
| void *dst; | ||
|
|
||
| if (!fragment) | ||
|
|
@@ -3321,7 +3321,7 @@ static int apply_binary(struct apply_state *state, | |
| if (odb_has_object(the_repository->objects, &oid, 0)) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Patrick Steinhardt wrote on the Git mailing list (how to reply to this email): On Thu, Jun 04, 2026 at 10:51:12AM +0000, Johannes Schindelin via GitGitGadget wrote:
> diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> index fa45f774d7..fa6e396ddc 100644
> --- a/builtin/cat-file.c
> +++ b/builtin/cat-file.c
> @@ -120,7 +120,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
> struct object_id oid;
> enum object_type type;
> char *buf;
> - unsigned long size;
> + size_t size;
> struct object_context obj_context = {0};
> struct object_info oi = OBJECT_INFO_INIT;
> unsigned flags = OBJECT_INFO_LOOKUP_REPLACE;
> @@ -166,7 +166,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
> if (use_mailmap && (type == OBJ_COMMIT || type == OBJ_TAG)) {
> size_t s = size;
> buf = replace_idents_using_mailmap(buf, &s);
> - size = cast_size_t_to_ulong(s);
> + size = s;
> }
>
> printf("%"PRIuMAX"\n", (uintmax_t)size);
Can't we drop this local variable completely and instead supply `&size`
directly?
> @@ -219,7 +225,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
> if (use_mailmap) {
> size_t s = size;
> buf = replace_idents_using_mailmap(buf, &s);
> - size = cast_size_t_to_ulong(s);
> + size = s;
> }
>
> /* otherwise just spit out the data */
> @@ -266,7 +272,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
> if (use_mailmap) {
> size_t s = size;
> buf = replace_idents_using_mailmap(buf, &s);
> - size = cast_size_t_to_ulong(s);
> + size = s;
> }
> break;
> }
> @@ -446,7 +455,7 @@ static void print_object_or_die(struct batch_options *opt, struct expand_data *d
> if (use_mailmap) {
> size_t s = size;
> contents = replace_idents_using_mailmap(contents, &s);
> - size = cast_size_t_to_ulong(s);
> + size = s;
> }
>
> if (type != data->type)
Likewise for these three instances.
> @@ -555,7 +564,7 @@ static void batch_object_write(const char *obj_name,
> if (!buf)
> die(_("unable to read %s"), oid_to_hex(&data->oid));
> buf = replace_idents_using_mailmap(buf, &s);
> - data->size = cast_size_t_to_ulong(s);
> + data->size = s;
>
> free(buf);
> }
And I think this site here can be adapted, as well.
> diff --git a/diff.c b/diff.c
> index 5a584fa1d5..816b89dc6c 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -4594,8 +4594,9 @@ int diff_populate_filespec(struct repository *r,
> }
> }
> else {
> + size_t size_st = 0;
> struct object_info info = {
> - .sizep = &s->size
> + .sizep = &size_st
> };
>
> if (!(size_only || check_binary))
> @@ -4617,6 +4618,7 @@ int diff_populate_filespec(struct repository *r,
> die("unable to read %s", oid_to_hex(&s->oid));
>
> object_read:
> + s->size = cast_size_t_to_ulong(size_st);
> if (size_only || check_binary) {
> if (size_only)
> return 0;
> @@ -4631,6 +4633,7 @@ object_read:
> if (odb_read_object_info_extended(r->objects, &s->oid, &info,
> OBJECT_INFO_LOOKUP_REPLACE))
> die("unable to read %s", oid_to_hex(&s->oid));
> + s->size = cast_size_t_to_ulong(size_st);
> }
> s->should_free = 1;
> }
The flow in this function is quite weird if you ask me, but that's a
preexisting issue. This does look correct to me, even if it's awkward.
Patrick |
||
| /* We already have the postimage */ | ||
| enum object_type type; | ||
| unsigned long size; | ||
| size_t size; | ||
| char *result; | ||
|
|
||
| result = odb_read_object(the_repository->objects, &oid, | ||
|
|
@@ -3384,7 +3384,7 @@ static int read_blob_object(struct strbuf *buf, const struct object_id *oid, uns | |
| strbuf_addf(buf, "Subproject commit %s\n", oid_to_hex(oid)); | ||
| } else { | ||
| enum object_type type; | ||
| unsigned long sz; | ||
| size_t sz; | ||
| char *result; | ||
|
|
||
| result = odb_read_object(the_repository->objects, oid, | ||
|
|
@@ -3611,7 +3611,7 @@ static int load_preimage(struct apply_state *state, | |
|
|
||
| static int resolve_to(struct image *image, const struct object_id *result_id) | ||
| { | ||
| unsigned long size; | ||
| size_t size; | ||
| enum object_type type; | ||
| char *data; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1239,6 +1239,8 @@ static void *gfi_unpack_entry( | |
| unsigned long *sizep) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Patrick Steinhardt wrote on the Git mailing list (how to reply to this email): On Thu, Jun 04, 2026 at 10:51:09AM +0000, Johannes Schindelin via GitGitGadget wrote:
> diff --git a/builtin/fast-import.c b/builtin/fast-import.c
> index 82bc6dcc00..3dff898c43 100644
> --- a/builtin/fast-import.c
> +++ b/builtin/fast-import.c
> @@ -1239,6 +1239,8 @@ static void *gfi_unpack_entry(
> unsigned long *sizep)
> {
> enum object_type type;
> + size_t size_st = 0;
> + void *data;
> struct packed_git *p = all_packs[oe->pack_id];
> if (p == pack_data && p->pack_size < (pack_size + the_hash_algo->rawsz)) {
> /* The object is stored in the packfile we are writing to
> @@ -1260,7 +1262,10 @@ static void *gfi_unpack_entry(
> */
> p->pack_size = pack_size + the_hash_algo->rawsz;
> }
> - return unpack_entry(the_repository, p, oe->idx.offset, &type, sizep);
> + data = unpack_entry(the_repository, p, oe->idx.offset, &type, &size_st);
> + if (sizep)
> + *sizep = cast_size_t_to_ulong(size_st);
> + return data;
> }
Nit, please feel free to ignore: do we want to add a NEEDSWORK comment
here?
Patrick |
||
| { | ||
| enum object_type type; | ||
| size_t size_st = 0; | ||
| void *data; | ||
| struct packed_git *p = all_packs[oe->pack_id]; | ||
| if (p == pack_data && p->pack_size < (pack_size + the_hash_algo->rawsz)) { | ||
| /* The object is stored in the packfile we are writing to | ||
|
|
@@ -1260,7 +1262,10 @@ static void *gfi_unpack_entry( | |
| */ | ||
| p->pack_size = pack_size + the_hash_algo->rawsz; | ||
| } | ||
| return unpack_entry(the_repository, p, oe->idx.offset, &type, sizep); | ||
| data = unpack_entry(the_repository, p, oe->idx.offset, &type, &size_st); | ||
| if (sizep) | ||
| *sizep = cast_size_t_to_ulong(size_st); | ||
| return data; | ||
| } | ||
|
|
||
| static void load_tree(struct tree_entry *root) | ||
|
|
@@ -1286,7 +1291,10 @@ static void load_tree(struct tree_entry *root) | |
| die(_("can't load tree %s"), oid_to_hex(oid)); | ||
| } else { | ||
| enum object_type type; | ||
| buf = odb_read_object(the_repository->objects, oid, &type, &size); | ||
| size_t size_st = 0; | ||
| buf = odb_read_object(the_repository->objects, oid, &type, | ||
| &size_st); | ||
| size = cast_size_t_to_ulong(size_st); | ||
| if (!buf || type != OBJ_TREE) | ||
| die(_("can't load tree %s"), oid_to_hex(oid)); | ||
| } | ||
|
|
@@ -2555,7 +2563,7 @@ static void note_change_n(const char *p, struct branch *b, unsigned char *old_fa | |
| die(_("mark :%" PRIuMAX " not a commit"), commit_mark); | ||
| oidcpy(&commit_oid, &commit_oe->idx.oid); | ||
| } else if (!repo_get_oid(the_repository, p, &commit_oid)) { | ||
| unsigned long size; | ||
| size_t size; | ||
| char *buf = odb_read_object_peeled(the_repository->objects, | ||
| &commit_oid, OBJ_COMMIT, &size, | ||
| &commit_oid); | ||
|
|
@@ -2622,10 +2630,12 @@ static void parse_from_existing(struct branch *b) | |
| oidclr(&b->branch_tree.versions[1].oid, the_repository->hash_algo); | ||
| } else { | ||
| unsigned long size; | ||
| size_t size_st = 0; | ||
| char *buf; | ||
|
|
||
| buf = odb_read_object_peeled(the_repository->objects, &b->oid, | ||
| OBJ_COMMIT, &size, &b->oid); | ||
| OBJ_COMMIT, &size_st, &b->oid); | ||
| size = cast_size_t_to_ulong(size_st); | ||
| parse_from_commit(b, buf, size); | ||
| free(buf); | ||
| } | ||
|
|
@@ -2717,7 +2727,7 @@ static struct hash_list *parse_merge(unsigned int *count) | |
| die(_("mark :%" PRIuMAX " not a commit"), idnum); | ||
| oidcpy(&n->oid, &oe->idx.oid); | ||
| } else if (!repo_get_oid(the_repository, from, &n->oid)) { | ||
| unsigned long size; | ||
| size_t size; | ||
| char *buf = odb_read_object_peeled(the_repository->objects, | ||
| &n->oid, OBJ_COMMIT, | ||
| &size, &n->oid); | ||
|
|
@@ -3325,7 +3335,10 @@ static void cat_blob(struct object_entry *oe, struct object_id *oid) | |
| char *buf; | ||
|
|
||
| if (!oe || oe->pack_id == MAX_PACK_ID) { | ||
| buf = odb_read_object(the_repository->objects, oid, &type, &size); | ||
| size_t size_st = 0; | ||
| buf = odb_read_object(the_repository->objects, oid, &type, | ||
| &size_st); | ||
| size = cast_size_t_to_ulong(size_st); | ||
| } else { | ||
| type = oe->type; | ||
| buf = gfi_unpack_entry(oe, &size); | ||
|
|
@@ -3433,8 +3446,10 @@ static struct object_entry *dereference(struct object_entry *oe, | |
| buf = gfi_unpack_entry(oe, &size); | ||
| } else { | ||
| enum object_type unused; | ||
| size_t size_st = 0; | ||
| buf = odb_read_object(the_repository->objects, oid, | ||
| &unused, &size); | ||
| &unused, &size_st); | ||
| size = cast_size_t_to_ulong(size_st); | ||
| } | ||
| if (!buf) | ||
| die(_("can't load object %s"), oid_to_hex(oid)); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Patrick Steinhardt wrote on the Git mailing list (how to reply to this email):