Git development
 help / color / mirror / Atom feed
* Re: [PATCH 3/7] pack-bitmap: allow aborting iteration of bitmapped objects
From: Justin Tobler @ 2026-07-09 20:19 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git
In-Reply-To: <20260709-pks-odb-for-each-object-filter-v1-3-82fe014b12b3@pks.im>

On 26/07/09 10:35AM, Patrick Steinhardt wrote:
> In a subsequent commit we'll lift iteration of bitmapped objects into
> the "packed" backend and make it accessible via `odb_for_each_object()`.
> The calling convention for that function is that the callback may return
> a non-zero exit code, and if so we'll abort iteration. This is currently
> impossible to realize though, as `for_each_bitmapped_object()` will
> ignore any return value and just churn through all objects completely.

Ok.

> This doesn't matter to the callers of `for_each_bitmapped_object()`, as
> there's only one of them in git-cat-file(1), and the callbacks we pass
> always return zero. But once we move the logic into the generic
> infrastructure it becomes a latent bug waiting to happen.
> 
> Refactor the code so that the return value of the `show_reach` callback
> is not ignored anymore. Instead, returning a non-zero value will cause
> us to abort iteration in both `show_objects_for_type()` and in
> `for_each_bitmapped_object()`.

Make sense. We want to ensure that the `show_reach` callback can
properly signal back to `for_each_bitmapped_object()` to abort.

> Note though that there's a second user of `show_objects_for_type()` with
> `traverse_bitmap_commit_list()`, and that function does indeed invoke
> callbacks that may return non-zero. This non-zero return value never had
> any effect at all though, and the callbacks that return non-zero values
> are only ever invoked via `traverse_bitmap_commit_list()`. Consequently,
> we adapt them to always return 0.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  builtin/pack-objects.c |  2 +-
>  builtin/rev-list.c     |  2 +-
>  pack-bitmap.c          | 31 +++++++++++++++++++++----------
>  pack-bitmap.h          |  3 ++-
>  4 files changed, 25 insertions(+), 13 deletions(-)
> 
> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> index ea5eab4cf8..8ff92c5272 100644
> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c
> @@ -1909,7 +1909,7 @@ static int add_object_entry_from_bitmap(const struct object_id *oid,
>  		return 0;
>  
>  	create_object_entry(oid, type, name_hash, 0, 0, pack, offset);
> -	return 1;
> +	return 0;

I wonder why this was even returning 1 to begin with? As you mentioned,
the return value appears to be ignored anyways. I'm assuming it was
signal that an object entry was created?

>  }
>  
>  struct pbase_tree_cache {
> diff --git a/builtin/rev-list.c b/builtin/rev-list.c
> index 8f63003709..02818b81c6 100644
> --- a/builtin/rev-list.c
> +++ b/builtin/rev-list.c
> @@ -486,7 +486,7 @@ static int show_object_fast(
>  	void *payload UNUSED)
>  {
>  	fprintf(stdout, "%s\n", oid_to_hex(oid));
> -	return 1;
> +	return 0;

Also curious about this one too. It probably doesn't matter though.

>  }
>  
>  static void print_disk_usage(off_t size)
> diff --git a/pack-bitmap.c b/pack-bitmap.c
> index a47c231632..eda38a5433 100644
> --- a/pack-bitmap.c
> +++ b/pack-bitmap.c
> @@ -1695,7 +1695,7 @@ static void init_type_iterator(struct ewah_or_iterator *it,
>  	}
>  }
>  
> -static void show_objects_for_type(
> +static int show_objects_for_type(
>  	struct bitmap_index *bitmap_git,
>  	struct bitmap *objects,
>  	enum object_type object_type,
> @@ -1704,6 +1704,7 @@ static void show_objects_for_type(
>  {
>  	size_t i = 0;
>  	uint32_t offset;
> +	int ret;
>  
>  	struct ewah_or_iterator it;
>  	eword_t filter;
> @@ -1749,11 +1750,17 @@ static void show_objects_for_type(
>  
>  			hash = bitmap_name_hash(bitmap_git, index_pos);
>  
> -			show_reach(&oid, object_type, 0, hash, pack, ofs, payload);
> +			ret = show_reach(&oid, object_type, 0, hash, pack, ofs, payload);
> +			if (ret)
> +				goto out;

The show_reach callback now wires back its return code.

>  		}
>  	}
>  
> +	ret = 0;
> +
> +out:
>  	ewah_or_iterator_release(&it);
> +	return ret;
>  }
>  
>  static int in_bitmapped_pack(struct bitmap_index *bitmap_git,
> @@ -2062,6 +2069,12 @@ int for_each_bitmapped_object(struct bitmap_index *bitmap_git,
>  			      show_reachable_fn show_reach,
>  			      void *payload)
>  {
> +	const enum object_type types[] = {
> +		OBJ_COMMIT,
> +		OBJ_TREE,
> +		OBJ_BLOB,
> +		OBJ_TAG,
> +	};
>  	struct bitmap *filtered_bitmap = NULL;
>  	uint32_t objects_nr;
>  	size_t full_word_count;
> @@ -2086,14 +2099,12 @@ int for_each_bitmapped_object(struct bitmap_index *bitmap_git,
>  		goto out;
>  	}
>  
> -	show_objects_for_type(bitmap_git, filtered_bitmap,
> -			      OBJ_COMMIT, show_reach, payload);
> -	show_objects_for_type(bitmap_git, filtered_bitmap,
> -			      OBJ_TREE, show_reach, payload);
> -	show_objects_for_type(bitmap_git, filtered_bitmap,
> -			      OBJ_BLOB, show_reach, payload);
> -	show_objects_for_type(bitmap_git, filtered_bitmap,
> -			      OBJ_TAG, show_reach, payload);
> +	for (size_t i = 0; i < ARRAY_SIZE(types); i++) {
> +		ret = show_objects_for_type(bitmap_git, filtered_bitmap,
> +					    types[i], show_reach, payload);
> +		if (ret)
> +			goto out;
> +	}

`for_each_bitmapped_object()` now has access to the underlying return
code and can abort. Looks good.

-Justin

^ permalink raw reply

* Re: [PATCH 11/11] shallow: fix NULL dereference
From: Junio C Hamano @ 2026-07-09 20:10 UTC (permalink / raw)
  To: Johannes Schindelin via GitGitGadget; +Cc: git, Johannes Schindelin
In-Reply-To: <9f3a23948475eaa382e9507543fe08d933a4a461.1783590159.git.gitgitgadget@gmail.com>

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> diff --git a/shallow.c b/shallow.c
> index 07cae44ae5..3d2230351e 100644
> --- a/shallow.c
> +++ b/shallow.c
> @@ -371,7 +371,7 @@ static int write_one_shallow(const struct commit_graft *graft, void *cb_data)
>  		if (!c || !(c->object.flags & SEEN)) {
>  			if (data->flags & VERBOSE)
>  				printf("Removing %s from .git/shallow\n",
> -				       oid_to_hex(&c->object.oid));
> +				       oid_to_hex(&graft->oid));
>  			return 0;

Haha.  We come into this block and emit this message when we may not
even have a valid 'c', yet we use c->object.oid there.  It makes
perfect sense to use graft->oid here instead, as your patch does.

However, its hexadecimal representation has already been computed in
the local variable 'hex', and the "happy path" code after this
section seems to assume that 'hex' is still valid (even though
oid_to_hex() uses rotating 4-element buffer, which makes the
assumption a risky one).

We should use "hex" here instead of oid_to_hex(&graft->oid), which
does not add to the existing risk.  In addition, if we add something
like:

                struct write_shallow_data *data = cb_data;
        -	const char *hex = oid_to_hex(&graft->oid);
        +	char hex[GIT_MAX_HEXSZ + 1];
        +
        +       oid_to_hex_r(hex, &graft->oid);
                if (graft->nr_parent != -1)
                        return 0;

to the beginning of the function, we can get rid of existing
riskiness entirely.

^ permalink raw reply

* Re: [PATCH 1/7] odb/source-packed: improve lookup when enumerating objects
From: Justin Tobler @ 2026-07-09 19:54 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git
In-Reply-To: <20260709-pks-odb-for-each-object-filter-v1-1-82fe014b12b3@pks.im>

On 26/07/09 10:35AM, Patrick Steinhardt wrote:
> When iterating through packed objects via `odb_for_each_object()` we
> do so via two different mechanisms:
> 
>   - When a multi-pack index is available we use that one to efficiently
>     loop through all objects.
> 
>   - We then loop through all packfiles that aren't covered by a
>     multi-pack index.

To be specific, we are talking only about the for_each_object callback
for the packed source `odb_source_packed_for_each_object()` correct?
Also, this appears to only matter when we are enumerating OIDs with a
specific prefix.

> Regardless of which mechanism we use, we then iterate through all the
> objects indexed by the respective data structure. Curiously though,
> while we use the indices for enumerating the objects, we completely
> ignore it for the actual object lookup. Instead, we call into the
> generic `odb_source_read_object_info()` function, which will itself
> consult the indices to figure out where the object in question even
> lives.
> 
> This has two consequences:
> 
>   - It's inefficient, as we basically have to figure out the position of
>     the object a second time.

Since we already have the position from the index, there is no need to
start over. Makes sense.

>   - It's subtly wrong, as it may now happen that a specific object will
>     be looked up via a different pack in case it exists multiple times.

Naive question: Is there any real harm in reading the same object, but
from a different packfile here?

Regardless I do think it's a good idea to just reuse the same packfile
to get the same object here.

> Fix the issue by using `packed_object_info()` directly. While at it,
> rename the `store` variable to `source`.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  odb/source-packed.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/odb/source-packed.c b/odb/source-packed.c
> index 0edea5356d..9cfa02b7a2 100644
> --- a/odb/source-packed.c
> +++ b/odb/source-packed.c
> @@ -143,7 +143,7 @@ static bool should_exclude_pack(struct packed_git *p, enum odb_for_each_object_f
>  }
>  
>  static int for_each_prefixed_object_in_midx(
> -	struct odb_source_packed *store,
> +	struct odb_source_packed *source,
>  	struct multi_pack_index *m,
>  	const struct odb_for_each_object_options *opts,
>  	struct odb_source_packed_for_each_object_wrapper_data *data)
> @@ -170,6 +170,7 @@ static int for_each_prefixed_object_in_midx(
>  		 */
>  		for (i = first; i < num; i++) {
>  			const struct object_id *current = NULL;
> +			struct packed_git *pack;
>  			struct object_id oid;
>  
>  			current = nth_midxed_object_oid(&oid, m, i);
> @@ -177,9 +178,8 @@ static int for_each_prefixed_object_in_midx(
>  			if (!match_hash(len, opts->prefix->hash, current->hash))
>  				break;
>  
> -			if (opts->flags) {
> +			if (opts->flags || data->request) {

I'm not sure I follow why the above condition needed to change.

>  				uint32_t pack_id = nth_midxed_pack_int_id(m, i);
> -				struct packed_git *pack;
>  
>  				if (prepare_midx_pack(m, pack_id)) {
>  					pack_errors = true;
> @@ -193,9 +193,9 @@ static int for_each_prefixed_object_in_midx(
>  
>  			if (data->request) {
>  				struct object_info oi = *data->request;
> +				off_t offset = nth_midxed_offset(m, i);
>  
> -				ret = odb_source_read_object_info(&store->base, current,
> -								  &oi, 0);
> +				ret = packed_object_info(source, pack, offset, &oi);

We not longer use the generic function to read object info. This ensures
the exact same object is read.

>  				if (ret)
>  					goto out;
>  
> @@ -219,7 +219,7 @@ static int for_each_prefixed_object_in_midx(
>  }
>  
>  static int for_each_prefixed_object_in_pack(
> -	struct odb_source_packed *store,
> +	struct odb_source_packed *source,
>  	struct packed_git *p,
>  	const struct odb_for_each_object_options *opts,
>  	struct odb_source_packed_for_each_object_wrapper_data *data)
> @@ -246,8 +246,9 @@ static int for_each_prefixed_object_in_pack(
>  
>  		if (data->request) {
>  			struct object_info oi = *data->request;
> +			off_t offset = nth_packed_object_offset(p, i);
>  
> -			ret = odb_source_read_object_info(&store->base, &oid, &oi, 0);
> +			ret = packed_object_info(source, p, offset, &oi);

And we do the same thing here when reading the object from a packfile.

-Justin

^ permalink raw reply

* [PATCH] builtin/add.c: replace run_command() with direct apply_all_patches() call
From: Gatla Vishweshwar Reddy @ 2026-07-09 19:26 UTC (permalink / raw)
  To: git; +Cc: Gatla Vishweshwar Reddy

When the user runs "git add -e", the diff of the working tree changes
is written to a temporary file, opened in an editor, and then applied
back to the index. The application step was done by spawning a child
process running "git apply --recount --cached <file>", which is an
unnecessary subprocess since the apply machinery is available as a
native C API.

Replace the run_command() call with a direct call to apply_all_patches()
using an initialized apply_state with the cached and recount options set
appropriately. This avoids the overhead of forking a subprocess, keeps
the operation within the same process, and makes the intent of the code
clearer to the reader.

Remove the now-unused includes of "run-command.h" and "strvec.h" since
no other code in this file requires them after this change.

Signed-off-by: Gatla Vishweshwar Reddy <gatlavishweshwarreddy26@gmail.com>
---
 builtin/add.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/builtin/add.c b/builtin/add.c
index c859f66519..8172c0c935 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -13,7 +13,6 @@
 #include "dir.h"
 #include "gettext.h"
 #include "pathspec.h"
-#include "run-command.h"
 #include "object-file.h"
 #include "odb.h"
 #include "odb/transaction.h"
@@ -23,9 +22,9 @@
 #include "diff.h"
 #include "read-cache.h"
 #include "revision.h"
-#include "strvec.h"
 #include "submodule.h"
 #include "add-interactive.h"
+#include "apply.h"
 
 static const char * const builtin_add_usage[] = {
 	N_("git add [<options>] [--] <pathspec>..."),
@@ -187,7 +186,6 @@ static int edit_patch(struct repository *repo,
 		      const char *prefix)
 {
 	char *file = repo_git_path(repo, "ADD_EDIT.patch");
-	struct child_process child = CHILD_PROCESS_INIT;
 	struct rev_info rev;
 	int out;
 	struct stat st;
@@ -217,11 +215,15 @@ static int edit_patch(struct repository *repo,
 	if (!st.st_size)
 		die(_("empty patch. aborted"));
 
-	child.git_cmd = 1;
-	strvec_pushl(&child.args, "apply", "--recount", "--cached", file,
-		     NULL);
-	if (run_command(&child))
+	struct apply_state state;
+	const char *apply_argv[] = { file, NULL };
+
+	if (init_apply_state(&state, repo, prefix))
+		die(_("could not initialize apply state"));
+	state.cached = 1;
+	if (apply_all_patches(&state, 1, apply_argv, APPLY_OPT_RECOUNT))
 		die(_("could not apply '%s'"), file);
+	clear_apply_state(&state);
 
 	unlink(file);
 	free(file);
-- 
2.54.0


^ permalink raw reply related

* Re: [PATCH 7/7] builtin/cat-file: filter objects via object database
From: Junio C Hamano @ 2026-07-09 18:59 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git
In-Reply-To: <20260709-pks-odb-for-each-object-filter-v1-7-82fe014b12b3@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> Refactor git-cat-file(1) to use the new object filter option when
> batching all objects. This significantly simplifies the logic and
> ensures that we don't have to reach into internals of the "files" source
> anymore.

This would become more convincing if you spent a few lines before
presenting the solution to give an observation of what the current
code does, e.g.,

    When batching all objects, git-cat-file(1) reaches into the
    internals of the object database and manually manages bitmaps to
    apply object filters. This creates coupling between the command
    and ODB backend internals.

to highlight the perceived problem in it.  That would flow naturally
to the description of your solution.

> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  builtin/cat-file.c | 76 +++++-------------------------------------------------
>  1 file changed, 7 insertions(+), 69 deletions(-)

Very nice.

> diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> index b4b99a73da..1458dd76d6 100644
> --- a/builtin/cat-file.c
> +++ b/builtin/cat-file.c
> @@ -20,7 +20,6 @@
>  #include "userdiff.h"
>  #include "oid-array.h"
>  #include "packfile.h"
> -#include "pack-bitmap.h"
>  #include "object-file.h"
>  #include "object-name.h"
>  #include "odb.h"
> @@ -844,28 +843,6 @@ static int batch_one_object_oi(const struct object_id *oid,
>  	return payload->callback(oid, NULL, 0, payload->payload);
>  }
>  
> -static int batch_one_object_packed(const struct object_id *oid,
> -				   struct packed_git *pack,
> -				   uint32_t pos,
> -				   void *_payload)
> -{
> -	struct for_each_object_payload *payload = _payload;
> -	return payload->callback(oid, pack, nth_packed_object_offset(pack, pos),
> -				 payload->payload);
> -}
> -
> -static int batch_one_object_bitmapped(const struct object_id *oid,
> -				      enum object_type type UNUSED,
> -				      int flags UNUSED,
> -				      uint32_t hash UNUSED,
> -				      struct packed_git *pack,
> -				      off_t offset,
> -				      void *_payload)
> -{
> -	struct for_each_object_payload *payload = _payload;
> -	return payload->callback(oid, pack, offset, payload->payload);
> -}
> -
>  static void batch_each_object(struct batch_options *opt,
>  			      for_each_object_fn callback,
>  			      unsigned flags,
> @@ -875,56 +852,17 @@ static void batch_each_object(struct batch_options *opt,
>  		.callback = callback,
>  		.payload = _payload,
>  	};
> +	struct odb_source_info source_info;
> +	struct object_info oi = {
> +		.source_infop = &source_info,
> +	};
>  	struct odb_for_each_object_options opts = {
>  		.flags = flags,
> +		.filter = &opt->objects_filter,
>  	};
> -	struct bitmap_index *bitmap = NULL;
> -	struct odb_source *source;
> -
> -	/*
> -	 * TODO: we still need to tap into implementation details of the object
> -	 * database sources. Ideally, we should extend `odb_for_each_object()`
> -	 * to handle object filters itself so that we can move the filtering
> -	 * logic into the individual sources.
> -	 */
> -	odb_prepare_alternates(the_repository->objects);
> -	for (source = the_repository->objects->sources; source; source = source->next) {
> -		struct odb_source_files *files = odb_source_files_downcast(source);
> -		int ret = odb_source_for_each_object(&files->loose->base, NULL, batch_one_object_oi,
> -						     &payload, &opts);
> -		if (ret)
> -			break;
> -	}
> -
> -	if (opt->objects_filter.choice != LOFC_DISABLED &&
> -	    (bitmap = prepare_bitmap_git(the_repository)) &&
> -	    !for_each_bitmapped_object(bitmap, &opt->objects_filter,
> -				       batch_one_object_bitmapped, &payload)) {
> -		struct packed_git *pack;
> -
> -		repo_for_each_pack(the_repository, pack) {
> -			if (bitmap_index_contains_pack(bitmap, pack) ||
> -			    open_pack_index(pack))
> -				continue;
> -			for_each_object_in_pack(pack, batch_one_object_packed,
> -						&payload, flags);
> -		}
> -	} else {
> -		struct odb_source_info source_info;
> -		struct object_info oi = {
> -			.source_infop = &source_info,
> -		};
> -
> -		for (source = the_repository->objects->sources; source; source = source->next) {
> -			struct odb_source_files *files = odb_source_files_downcast(source);
> -			int ret = odb_source_for_each_object(&files->packed->base, &oi,
> -							     batch_one_object_oi, &payload, &opts);
> -			if (ret)
> -				break;
> -		}
> -	}
>  
> -	free_bitmap_index(bitmap);
> +	odb_for_each_object_ext(the_repository->objects, &oi,
> +				batch_one_object_oi, &payload, &opts);
>  }
>  
>  static int batch_objects(struct batch_options *opt)

^ permalink raw reply

* Re: [PATCH 1/7] refs/packed: de-globalize handling of "core.packedRefsTimeout"
From: Junio C Hamano @ 2026-07-09 18:52 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git
In-Reply-To: <20260709-pks-refs-wo-the-repository-v1-1-1ad6f27529c9@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> diff --git a/refs/packed-backend.c b/refs/packed-backend.c
> index 499cb55dfa..5c49c06493 100644
> --- a/refs/packed-backend.c
> +++ b/refs/packed-backend.c
> @@ -162,6 +162,13 @@ struct packed_ref_store {
>  	 * `packed_ref_store`) must not be freed.
>  	 */
>  	struct tempfile *tempfile;
> +
> +	/*
> +	 * Timeout when taking the "packed-refs.lock" file. configurable via
> +	 * "core.packedRefsTimeout".
> +	 */
> +	bool timeout_configured;
> +	int timeout_value;
>  };
>  
>  /*
> @@ -1233,12 +1240,10 @@ int packed_refs_lock(struct ref_store *ref_store, int flags, struct strbuf *err)
>  	struct packed_ref_store *refs =
>  		packed_downcast(ref_store, REF_STORE_WRITE | REF_STORE_MAIN,
>  				"packed_refs_lock");
> -	static int timeout_configured = 0;
> -	static int timeout_value = 1000;
>  
> -	if (!timeout_configured) {
> -		repo_config_get_int(the_repository, "core.packedrefstimeout", &timeout_value);
> -		timeout_configured = 1;

In the original code, when core.packedrefstimeout is not configured,
our call to repo_config_get_int() does not touch timeout_value.  As
a result, we get the static 1000 and flip the "configured" flag to
prevent this _value from further getting updated.

> +	if (!refs->timeout_configured) {
> +		repo_config_get_int(ref_store->repo, "core.packedrefstimeout", &refs->timeout_value);
> +		refs->timeout_configured = true;

But what happens in the new code when core.packedrefstimeout is not
configured?  It is up to whoever initialised refs->timeout_value.

If I am not mistaken, packed_ref_store_init() does xcalloc(), lets
base_ref_store_init() initialise some members, initialises a few
members itself (such as .store_flags and .path), and leaves other
members, including .timeout_configured and .timeout_value,
NUL-filled.  .timeout_configured starting as false is perfectly
fine, but shouldn't we initialise .timeout_value to 1000 as before?

Thanks.

> @@ -1249,7 +1254,7 @@ int packed_refs_lock(struct ref_store *ref_store, int flags, struct strbuf *err)
>  	if (hold_lock_file_for_update_timeout(
>  			    &refs->lock,
>  			    refs->path,
> -			    flags, timeout_value) < 0) {
> +			    flags, refs->timeout_value) < 0) {
>  		unable_to_lock_message(refs->path, errno, err);
>  		return -1;
>  	}

^ permalink raw reply

* Re: [PATCH 2/3] t/lib-httpd: make http-429 first-request check atomic
From: Michael Montalbo @ 2026-07-09 18:10 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Michael Montalbo via GitGitGadget, git
In-Reply-To: <xmqqcxwxtfkp.fsf@gitster.g>

On Wed, Jul 8, 2026 at 1:02 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Michael Montalbo via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > -# Check if this is the first call (no state file exists)
> > -if test -f "$state_file"
> > +# Apache can run this CGI for concurrent requests, so the script decides
> > +# whether this is the first call with a single atomic "mkdir": it succeeds for
> > +# exactly one of any racing requests and fails for the rest. "permanent"
> > +# always rate-limits and records no state.
> > +if test "$retry_after" != permanent && ! mkdir "$state" 2>/dev/null
>
> I think the last sentence in the above comment was meant to explain
> why the new code checks the value of "$retry_after", but it is not
> clear if it is needed for correctness (in other words, the original
> was wrong to do "test -f && touch" but also was wrong to do so even
> when "$retry_after" is set to "permanent), or if it is a mere
> "optimization opportunity" you are taking advantage of.  In either
> case, it would be nice to see it explained in the proposed commit
> log message.
>

It is needed for correctness, and I agree it is not very clear from the log
message / comment. I will spell out the reasoning for the change more
clearly in both.

Thanks for taking a look at this!

^ permalink raw reply

* Re: [PATCH 1/3] t/lib-httpd: fix apply-one-time-script race under concurrent requests
From: Michael Montalbo @ 2026-07-09 17:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Michael Montalbo via GitGitGadget, git
In-Reply-To: <xmqqpl0xtfyz.fsf@gitster.g>

On Wed, Jul 8, 2026 at 12:54 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Michael Montalbo via GitGitGadget" <gitgitgadget@gmail.com> writes:
> >
> > +#
> > +# Apache can run this CGI for concurrent requests (for example a partial fetch
> > +# that lazily fetches a missing object while the first response is still in
> > +# flight), so the helper claims the marker atomically with a rename, and only
> > +# once it has decided to modify the response. A request that loses the race
> > +# finds the marker already gone and serves its response unchanged; no request
> > +# is left emitting an empty body, which the server would report as HTTP 500.
> > +# Scratch files are per-request ($$) so concurrent requests do not clobber each
> > +# other.
> > +
> > +test -f one-time-script || exec "$GIT_EXEC_PATH/git-http-backend"
> >
> > -     "$GIT_EXEC_PATH/git-http-backend" >out
> > -     ./one-time-script out >out_modified
> > +LC_ALL=C
> > +export LC_ALL
>
> The original was somehow inconsistent in that it forced C locale
> only when one-time-script munged the output, and otherwise the
> backend was run in the original locale.  I am not sure if that
> matters very much.
>

I think it's still the same after the rewrite, though I could be
mistaken. If the
first `test -f` fails git-http-backend executes with inherited locale
(analogous to
the else branch execution in the original), and if `test -f` succeeds the locale
is forced to C and the one-time-script / git-http-backend run with the forced
locale. That being said, I think forcing the locale to C consistently would
make more sense. Depending on what you think, I can integrate that into the
series or leave for a future cleanup.

>
> Ah, we assume running one-time-script itself multiple times is safe
> and does not cause issues.  Our objective is to avoid returning
> modified output twice.  So while the first instance of us
> successfully renames one-time-script to one-time-script.$$ and emits
> the modified result, even if the second instance raced and managed
> to run the script again, it will fail to rename with "mv", and
> discard the modified output, and instead show the unmodified output
> generated by the backend.
>
> OK.  It is a bit tricky.  It may help future readers if we said
> something about this in the proposed log message (i.e., we consider
> that it is perfectly fine to run one-time-script more than once; we
> only want to avoid letting the second invocation's output used).
>

Yes that is a good call, I will add some detail about this subtlety in the
log message and helper comment.

^ permalink raw reply

* [PATCH 12/12] git-zlib: widen `git_deflate_bound()` to `size_t`
From: Johannes Schindelin via GitGitGadget @ 2026-07-09 16:49 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Johannes Schindelin
In-Reply-To: <pull.2175.git.1783615780.gitgitgadget@gmail.com>

From: Johannes Schindelin <johannes.schindelin@gmx.de>

All four `unsigned long`/`int`/`ssize_t` receivers across archive-zip,
diff, http-push and t/helper/test-pack-deltas were widened to `size_t`
in the prior commits, and remote-curl and fast-import were already
there. With every caller prepared, both the parameter and the return
type can now move without introducing any silent narrowing.

For inputs above zlib's `uLong` range (i.e. >4 GiB on platforms where
`uLong` is 32-bit, notably 64-bit Windows), defer to zlib's stored-block
formula (the same fallback it would itself use for an unknown stream
state) plus the worst-case wrapper overhead. The existing path through
`deflateBound()` is unchanged for inputs that fit.

Assisted-by: Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 git-zlib.c | 16 ++++++++++++++--
 git-zlib.h |  2 +-
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/git-zlib.c b/git-zlib.c
index d21adb3bf5..ebbbcc6d1a 100644
--- a/git-zlib.c
+++ b/git-zlib.c
@@ -167,9 +167,21 @@ int git_inflate(git_zstream *strm, int flush)
 	return status;
 }
 
-unsigned long git_deflate_bound(git_zstream *strm, unsigned long size)
+size_t git_deflate_bound(git_zstream *strm, size_t size)
 {
-	return deflateBound(&strm->z, size);
+#if SIZE_MAX > ULONG_MAX
+	if (size > maximum_unsigned_value_of_type(uLong))
+		/*
+		 * deflateBound() takes uLong, which is 32-bit on
+		 * Windows. For inputs above that range, return zlib's
+		 * stored-block formula (the conservative path it would
+		 * itself use for an unknown stream state) plus the
+		 * worst-case wrapper overhead.
+		 */
+		return size + (size >> 5) + (size >> 7) + (size >> 11)
+			+ 7 + 18;
+#endif
+	return deflateBound(&strm->z, (uLong)size);
 }
 
 void git_deflate_init(git_zstream *strm, int level)
diff --git a/git-zlib.h b/git-zlib.h
index 0b24b15bd0..9248d11ca9 100644
--- a/git-zlib.h
+++ b/git-zlib.h
@@ -25,6 +25,6 @@ void git_deflate_end(git_zstream *);
 int git_deflate_abort(git_zstream *);
 int git_deflate_end_gently(git_zstream *);
 int git_deflate(git_zstream *, int flush);
-unsigned long git_deflate_bound(git_zstream *, unsigned long);
+size_t git_deflate_bound(git_zstream *, size_t);
 
 #endif /* GIT_ZLIB_H */
-- 
gitgitgadget

^ permalink raw reply related

* [PATCH 11/12] t/helper/test-pack-deltas: widen `do_compress()`'s maxsize local to `size_t`
From: Johannes Schindelin via GitGitGadget @ 2026-07-09 16:49 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Johannes Schindelin
In-Reply-To: <pull.2175.git.1783615780.gitgitgadget@gmail.com>

From: Johannes Schindelin <johannes.schindelin@gmx.de>

Prep for the upcoming `git_deflate_bound()` widening to `size_t`. The
local is only ever the return value of `git_deflate_bound()` and the
`xmalloc()`/`stream.avail_out` sizes derived from it; widening it has no
semantic effect today.

Assisted-by: Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/helper/test-pack-deltas.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/helper/test-pack-deltas.c b/t/helper/test-pack-deltas.c
index 5e0f726842..959705feca 100644
--- a/t/helper/test-pack-deltas.c
+++ b/t/helper/test-pack-deltas.c
@@ -22,7 +22,7 @@ static unsigned long do_compress(void **pptr, unsigned long size)
 {
 	git_zstream stream;
 	void *in, *out;
-	unsigned long maxsize;
+	size_t maxsize;
 
 	git_deflate_init(&stream, 1);
 	maxsize = git_deflate_bound(&stream, size);
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH 10/12] http-push: widen `start_put()`'s size local from `ssize_t` to `size_t`
From: Johannes Schindelin via GitGitGadget @ 2026-07-09 16:49 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Johannes Schindelin
In-Reply-To: <pull.2175.git.1783615780.gitgitgadget@gmail.com>

From: Johannes Schindelin <johannes.schindelin@gmx.de>

The local is initialised from `git_deflate_bound()` (an unsigned upper
bound on the deflated output, never negative) and used in exactly three
places: the initialising assignment, `strbuf_grow(buf, size)` whose
parameter is already `size_t`, and `stream.avail_out` which became
`size_t` in the prior commit. There is no comparison against zero or a
negative value, no subtraction, no arithmetic that depends on
signedness, and no path that would assign a signed quantity to it.

The original `ssize_t` was the wrong type to begin with: a
`git_deflate_bound()` result above `SSIZE_MAX` would have wrapped
negative on assignment and then implicitly re-extended to a huge
`size_t` at `strbuf_grow()`/`stream.avail_out`, requesting an absurd
allocation. That is not a real-world concern for the object sizes
http-push pushes today, but it is also the reason the type needs to move
to `size_t` before `git_deflate_bound()` itself is widened.

Assisted-by: Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 http-push.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/http-push.c b/http-push.c
index 3c23cbba27..2a07d14259 100644
--- a/http-push.c
+++ b/http-push.c
@@ -367,7 +367,7 @@ static void start_put(struct transfer_request *request)
 	void *unpacked;
 	size_t len;
 	int hdrlen;
-	ssize_t size;
+	size_t size;
 	git_zstream stream;
 	struct repo_config_values *cfg = repo_config_values(the_repository);
 
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH 09/12] diff: widen `deflate_it()`'s bound local from int to `size_t`
From: Johannes Schindelin via GitGitGadget @ 2026-07-09 16:49 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Johannes Schindelin
In-Reply-To: <pull.2175.git.1783615780.gitgitgadget@gmail.com>

From: Johannes Schindelin <johannes.schindelin@gmx.de>

Fixes a pre-existing silent narrowing from `git_deflate_bound()`'s
`unsigned long` return into an `int` local: anything past 2 GiB has
always wrapped negative here and then been re-extended to `size_t`
inside `xmalloc()`. Also prep for the upcoming `git_deflate_bound()`
widening to `size_t`, which would extend the narrowing further if
`bound` stayed `int`.

Assisted-by: Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 diff.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/diff.c b/diff.c
index 69eb2f76a4..c14f69719b 100644
--- a/diff.c
+++ b/diff.c
@@ -3609,7 +3609,7 @@ static unsigned char *deflate_it(char *data,
 				 unsigned long size,
 				 unsigned long *result_size)
 {
-	int bound;
+	size_t bound;
 	unsigned char *deflated;
 	git_zstream stream;
 	struct repo_config_values *cfg = repo_config_values(the_repository);
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH 08/12] archive-zip: widen `zlib_deflate_raw()`'s maxsize local to `size_t`
From: Johannes Schindelin via GitGitGadget @ 2026-07-09 16:49 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Johannes Schindelin
In-Reply-To: <pull.2175.git.1783615780.gitgitgadget@gmail.com>

From: Johannes Schindelin <johannes.schindelin@gmx.de>

Prep for the upcoming `git_deflate_bound()` widening to `size_t`: the
local that catches its return needs to be `size_t` too, otherwise the
widening would introduce a silent Windows narrowing here. No semantic
effect with the current unsigned-long-returning `git_deflate_bound()`
(`size_t == unsigned long` on this caller's platforms today).

Assisted-by: Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 archive-zip.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/archive-zip.c b/archive-zip.c
index 97ea8d60d6..a487d4c041 100644
--- a/archive-zip.c
+++ b/archive-zip.c
@@ -206,7 +206,7 @@ static void *zlib_deflate_raw(void *data, unsigned long size,
 			      unsigned long *compressed_size)
 {
 	git_zstream stream;
-	unsigned long maxsize;
+	size_t maxsize;
 	void *buffer;
 	int result;
 
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH 07/12] packfile, git-zlib: widen `use_pack()` and zstream avail fields to `size_t`
From: Johannes Schindelin via GitGitGadget @ 2026-07-09 16:49 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Johannes Schindelin
In-Reply-To: <pull.2175.git.1783615780.gitgitgadget@gmail.com>

From: Johannes Schindelin <johannes.schindelin@gmx.de>

Bundling the two widenings: four call sites pass `&stream.avail_in`
directly to `use_pack()`, and widening either type fencepost alone would
force a bridge variable at each. Doing both together is the simpler end
state and is the prerequisite for the `do_compress()` widening in the
next commit, which is what lets `write_no_reuse_object()` lose its last
`cast_size_t_to_ulong()` shim.

The unsigned-long locals widened at the other `use_pack()` callers
(avail / remaining / left) hold pack-window sizes bounded by
`core.packedGitWindowSize`, so the change is type consistency rather
than a new >4GB capability. `git_zstream.avail_in`/`avail_out` likewise
reach zlib's `uInt` fields only after `zlib_buf_cap()`'s 1 GiB cap, so
the wrapper already accepted `size_t`-shaped inputs in practice.

Assisted-by: Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin/pack-objects.c | 8 ++++----
 git-zlib.h             | 4 ++--
 pack-check.c           | 4 ++--
 packfile.c             | 4 ++--
 packfile.h             | 3 ++-
 5 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 315ea0ed7e..cedda6ba9c 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -488,7 +488,7 @@ static void copy_pack_data(struct hashfile *f,
 		off_t len)
 {
 	unsigned char *in;
-	unsigned long avail;
+	size_t avail;
 
 	while (len) {
 		in = use_pack(p, w_curs, offset, &avail);
@@ -2261,7 +2261,7 @@ static void check_object(struct object_entry *entry, uint32_t object_index)
 		struct object_id base_ref;
 		struct object_entry *base_entry;
 		unsigned long used, used_0;
-		unsigned long avail;
+		size_t avail;
 		off_t ofs;
 		unsigned char *buf, c;
 		enum object_type type;
@@ -2773,8 +2773,8 @@ size_t oe_get_size_slow(struct packing_data *pack,
 	struct pack_window *w_curs;
 	unsigned char *buf;
 	enum object_type type;
-	unsigned long used, avail;
-	size_t size;
+	unsigned long used;
+	size_t avail, size;
 
 	if (e->type_ != OBJ_OFS_DELTA && e->type_ != OBJ_REF_DELTA) {
 		size_t sz;
diff --git a/git-zlib.h b/git-zlib.h
index 44380e8ad3..0b24b15bd0 100644
--- a/git-zlib.h
+++ b/git-zlib.h
@@ -5,8 +5,8 @@
 
 typedef struct git_zstream {
 	struct z_stream_s z;
-	unsigned long avail_in;
-	unsigned long avail_out;
+	size_t avail_in;
+	size_t avail_out;
 	size_t total_in;
 	size_t total_out;
 	unsigned char *next_in;
diff --git a/pack-check.c b/pack-check.c
index 5adfb3f272..befb860472 100644
--- a/pack-check.c
+++ b/pack-check.c
@@ -34,7 +34,7 @@ int check_pack_crc(struct packed_git *p, struct pack_window **w_curs,
 	uint32_t data_crc = crc32(0, NULL, 0);
 
 	do {
-		unsigned long avail;
+		size_t avail;
 		void *data = use_pack(p, w_curs, offset, &avail);
 		if (avail > len)
 			avail = len;
@@ -71,7 +71,7 @@ static int verify_packfile(struct repository *r,
 
 	r->hash_algo->init_fn(&ctx);
 	do {
-		unsigned long remaining;
+		size_t remaining;
 		unsigned char *in = use_pack(p, w_curs, offset, &remaining);
 		offset += remaining;
 		if (!pack_sig_ofs)
diff --git a/packfile.c b/packfile.c
index 1d1b23b6cc..629fe46a6a 100644
--- a/packfile.c
+++ b/packfile.c
@@ -620,7 +620,7 @@ static int in_window(struct repository *r, struct pack_window *win,
 unsigned char *use_pack(struct packed_git *p,
 		struct pack_window **w_cursor,
 		off_t offset,
-		unsigned long *left)
+		size_t *left)
 {
 	struct pack_window *win = *w_cursor;
 
@@ -960,7 +960,7 @@ int unpack_object_header(struct packed_git *p,
 			 size_t *sizep)
 {
 	unsigned char *base;
-	unsigned long left;
+	size_t left;
 	unsigned long used;
 	enum object_type type;
 
diff --git a/packfile.h b/packfile.h
index 2329a69701..3cff8bdcb9 100644
--- a/packfile.h
+++ b/packfile.h
@@ -240,7 +240,8 @@ uint32_t get_pack_fanout(struct packed_git *p, uint32_t value);
 
 struct object_database;
 
-unsigned char *use_pack(struct packed_git *, struct pack_window **, off_t, unsigned long *);
+unsigned char *use_pack(struct packed_git *, struct pack_window **, off_t,
+			size_t *);
 void close_pack_windows(struct packed_git *);
 void close_pack(struct packed_git *);
 void unuse_pack(struct pack_window **);
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH 06/12] delta: widen `create_delta()` and `diff_delta()` to `size_t`
From: Johannes Schindelin via GitGitGadget @ 2026-07-09 16:49 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Johannes Schindelin
In-Reply-To: <pull.2175.git.1783615780.gitgitgadget@gmail.com>

From: Johannes Schindelin <johannes.schindelin@gmx.de>

Last stop in the delta-encoding API widening for >4 GiB blobs on
Windows: with `create_delta_index()` done in the prior commit and
`create_delta()`/`diff_delta()` finished here, every byte count that
crosses delta.h is now `size_t`. The struct fields they store into have
been `size_t` since the diff-delta struct widening.

The API change must move with all callers in the same commit (the build
only passes when every `&delta_size` matches the new `size_t*`). Caller
updates are kept minimal:

  * builtin/pack-objects.c `get_delta()` and `try_delta()`: widen only
    the local `delta_size` variable; the surrounding unsigned-long
    locals and their `cast_size_t_to_ulong()` shims are out of scope
    here and will be cleaned up in their own commits.

  * builtin/fast-import.c, diff.c, t/helper/test-pack-deltas.c:
    keep the local unsigned-long delta size (each feeds a still-
    unsigned-long downstream consumer: zlib's `avail_in`,
    `deflate_it()`, the test helper's own `do_compress()`), and bridge
    via a temporary `size_t` plus `cast_size_t_to_ulong()`. The new
    casts are paid back in later topics that widen those consumers.

  * t/helper/test-delta.c: widen the local outright (no downstream
    consumer beyond the test's own `out_size`, which is already
    `size_t`).

Note that GCC struggles a bit to figure out that `deltalen` is always
initialized before it is used; To help it along, we initialize it to 0.
This work-around will go away in a later patch series when `deltalen`
can be widened to `size_t`.

Assisted-by: Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin/fast-import.c       |  6 ++++--
 builtin/pack-objects.c      |  6 ++++--
 delta.h                     | 10 +++++-----
 diff-delta.c                |  4 ++--
 diff.c                      |  4 +++-
 t/helper/test-delta.c       |  2 +-
 t/helper/test-pack-deltas.c |  5 +++--
 7 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/builtin/fast-import.c b/builtin/fast-import.c
index aa656c5195..1c6e5366c2 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -962,7 +962,7 @@ static int store_object(
 	struct object_entry *e;
 	unsigned char hdr[96];
 	struct object_id oid;
-	unsigned long hdrlen, deltalen;
+	unsigned long hdrlen, deltalen = 0;
 	struct git_hash_ctx c;
 	git_zstream s;
 	struct repo_config_values *cfg = repo_config_values(the_repository);
@@ -998,11 +998,13 @@ static int store_object(
 
 	if (last && last->data.len && last->data.buf && last->depth < max_depth
 		&& dat->len > the_hash_algo->rawsz) {
+		size_t deltalen_st;
 
 		delta_count_attempts_by_type[type]++;
 		delta = diff_delta(last->data.buf, last->data.len,
 			dat->buf, dat->len,
-			&deltalen, dat->len - the_hash_algo->rawsz);
+			&deltalen_st, dat->len - the_hash_algo->rawsz);
+		deltalen = cast_size_t_to_ulong(deltalen_st);
 	} else
 		delta = NULL;
 
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 63ceeb736f..315ea0ed7e 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -353,7 +353,8 @@ static void index_commit_for_bitmap(struct commit *commit)
 
 static void *get_delta(struct object_entry *entry)
 {
-	unsigned long size, base_size, delta_size;
+	unsigned long size, base_size;
+	size_t delta_size;
 	void *buf, *base_buf, *delta_buf;
 	enum object_type type;
 	size_t size_st = 0, base_size_st = 0;
@@ -2808,7 +2809,8 @@ static int try_delta(struct unpacked *trg, struct unpacked *src,
 {
 	struct object_entry *trg_entry = trg->entry;
 	struct object_entry *src_entry = src->entry;
-	unsigned long trg_size, src_size, delta_size, sizediff, max_size, sz;
+	unsigned long trg_size, src_size, sizediff, max_size, sz;
+	size_t delta_size;
 	unsigned ref_depth;
 	enum object_type type;
 	void *delta_buf;
diff --git a/delta.h b/delta.h
index a19586d789..59ccaaa0e0 100644
--- a/delta.h
+++ b/delta.h
@@ -42,8 +42,8 @@ unsigned long sizeof_delta_index(struct delta_index *index);
  */
 void *
 create_delta(const struct delta_index *index,
-	     const void *buf, unsigned long bufsize,
-	     unsigned long *delta_size, unsigned long max_delta_size);
+	     const void *buf, size_t bufsize,
+	     size_t *delta_size, size_t max_delta_size);
 
 /*
  * diff_delta: create a delta from source buffer to target buffer
@@ -54,9 +54,9 @@ create_delta(const struct delta_index *index,
  * updated with its size.  The returned buffer must be freed by the caller.
  */
 static inline void *
-diff_delta(const void *src_buf, unsigned long src_bufsize,
-	   const void *trg_buf, unsigned long trg_bufsize,
-	   unsigned long *delta_size, unsigned long max_delta_size)
+diff_delta(const void *src_buf, size_t src_bufsize,
+	   const void *trg_buf, size_t trg_bufsize,
+	   size_t *delta_size, size_t max_delta_size)
 {
 	struct delta_index *index = create_delta_index(src_buf, src_bufsize);
 	if (index) {
diff --git a/diff-delta.c b/diff-delta.c
index c93ac42594..15210e8381 100644
--- a/diff-delta.c
+++ b/diff-delta.c
@@ -318,8 +318,8 @@ unsigned long sizeof_delta_index(struct delta_index *index)
 
 void *
 create_delta(const struct delta_index *index,
-	     const void *trg_buf, unsigned long trg_size,
-	     unsigned long *delta_size, unsigned long max_size)
+	     const void *trg_buf, size_t trg_size,
+	     size_t *delta_size, size_t max_size)
 {
 	unsigned int i, val;
 	off_t outpos, moff;
diff --git a/diff.c b/diff.c
index 2a9d0d8687..69eb2f76a4 100644
--- a/diff.c
+++ b/diff.c
@@ -3647,9 +3647,11 @@ static void emit_binary_diff_body(struct diff_options *o,
 	delta = NULL;
 	deflated = deflate_it(two->ptr, two->size, &deflate_size);
 	if (one->size && two->size) {
+		size_t delta_size_st = 0;
 		delta = diff_delta(one->ptr, one->size,
 				   two->ptr, two->size,
-				   &delta_size, deflate_size);
+				   &delta_size_st, deflate_size);
+		delta_size = cast_size_t_to_ulong(delta_size_st);
 		if (delta) {
 			void *to_free = delta;
 			orig_size = delta_size;
diff --git a/t/helper/test-delta.c b/t/helper/test-delta.c
index 8223a60229..d807afef75 100644
--- a/t/helper/test-delta.c
+++ b/t/helper/test-delta.c
@@ -32,7 +32,7 @@ int cmd__delta(int argc, const char **argv)
 		die_errno("unable to read '%s'", argv[3]);
 
 	if (argv[1][1] == 'd') {
-		unsigned long delta_size;
+		size_t delta_size;
 		out_buf = diff_delta(from.buf, from.len,
 				     data.buf, data.len,
 				     &delta_size, 0);
diff --git a/t/helper/test-pack-deltas.c b/t/helper/test-pack-deltas.c
index 840797cf0d..5e0f726842 100644
--- a/t/helper/test-pack-deltas.c
+++ b/t/helper/test-pack-deltas.c
@@ -49,7 +49,7 @@ static void write_ref_delta(struct hashfile *f,
 {
 	unsigned char header[MAX_PACK_OBJECT_HEADER];
 	unsigned long delta_size, compressed_size, hdrlen;
-	size_t size, base_size;
+	size_t size, base_size, delta_size_st = 0;
 	enum object_type type;
 	void *base_buf, *delta_buf;
 	void *buf = odb_read_object(the_repository->objects,
@@ -65,7 +65,8 @@ static void write_ref_delta(struct hashfile *f,
 		die("unable to read %s", oid_to_hex(base));
 
 	delta_buf = diff_delta(base_buf, base_size,
-			       buf, size, &delta_size, 0);
+			       buf, size, &delta_size_st, 0);
+	delta_size = cast_size_t_to_ulong(delta_size_st);
 
 	compressed_size = do_compress(&delta_buf, delta_size);
 
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH 05/12] pack-objects: widen `mem_usage` and `try_delta()`'s out-param to `size_t`
From: Johannes Schindelin via GitGitGadget @ 2026-07-09 16:49 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Johannes Schindelin
In-Reply-To: <pull.2175.git.1783615780.gitgitgadget@gmail.com>

From: Johannes Schindelin <johannes.schindelin@gmx.de>

The pair must move together because `find_deltas()` passes `&mem_usage`
to `try_delta()`: widening either alone breaks the type match.

`mem_usage` accumulates per-object byte counts already computed in
`size_t` (`SIZE()` and `sizeof_delta_index()` reach here through
`free_unpacked()`, now `size_t`), and was the last 32-bit-on-Windows
narrowing point in the delta-window memory accounting chain. With this
commit, that chain uses `size_t` consistently except for
`sizeof_delta_index()`'s still-narrow return, whose value is bounded by
`create_delta_index()`'s entries cap.

`window_memory_limit` (config-driven via `git_config_ulong()`) stays
`unsigned long`: it is only compared against `mem_usage` and promotes.

Assisted-by: Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin/pack-objects.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 4737a6a32c..63ceeb736f 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2804,7 +2804,7 @@ size_t oe_get_size_slow(struct packing_data *pack,
 }
 
 static int try_delta(struct unpacked *trg, struct unpacked *src,
-		     unsigned max_depth, unsigned long *mem_usage)
+		     unsigned max_depth, size_t *mem_usage)
 {
 	struct object_entry *trg_entry = trg->entry;
 	struct object_entry *src_entry = src->entry;
@@ -2991,7 +2991,7 @@ static void find_deltas(struct object_entry **list, unsigned *list_size,
 {
 	uint32_t i, idx = 0, count = 0;
 	struct unpacked *array;
-	unsigned long mem_usage = 0;
+	size_t mem_usage = 0;
 
 	CALLOC_ARRAY(array, window);
 
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH 04/12] pack-objects: widen `free_unpacked()` return to `size_t`
From: Johannes Schindelin via GitGitGadget @ 2026-07-09 16:49 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Johannes Schindelin
In-Reply-To: <pull.2175.git.1783615780.gitgitgadget@gmail.com>

From: Johannes Schindelin <johannes.schindelin@gmx.de>

`free_unpacked()` sums two byte counts: `sizeof_delta_index()` and
`SIZE(n->entry)`. The latter has been `size_t` since the prior topic
"More work supporting objects larger than 4GB on Windows" widened
`SIZE()`/`oe_size()` to `size_t`, so accumulating it into an `unsigned
long` return was a silent Windows-only truncation on a packing run with
many large objects.

The sole caller, `find_deltas()`, still holds its own `mem_usage` in an
`unsigned long` for now, and therefore still truncates silently.

Assisted-by: Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin/pack-objects.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index f89628a760..4737a6a32c 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2972,9 +2972,9 @@ static unsigned int check_delta_limit(struct object_entry *me, unsigned int n)
 	return m;
 }
 
-static unsigned long free_unpacked(struct unpacked *n)
+static size_t free_unpacked(struct unpacked *n)
 {
-	unsigned long freed_mem = sizeof_delta_index(n->index);
+	size_t freed_mem = sizeof_delta_index(n->index);
 	free_delta_index(n->index);
 	n->index = NULL;
 	if (n->data) {
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH 03/12] pack-objects: widen delta-cache accounting to `size_t`
From: Johannes Schindelin via GitGitGadget @ 2026-07-09 16:49 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Johannes Schindelin
In-Reply-To: <pull.2175.git.1783615780.gitgitgadget@gmail.com>

From: Johannes Schindelin <johannes.schindelin@gmx.de>

These three are a single accounting tuple (the globals tracking
cumulative cached-delta bytes, plus the helper that compares them
against an incoming delta size) and are latently 32-bit on Windows where
`unsigned long` != `size_t`: a pack with many large cached deltas could
wrap silently.

The widening is internally consistent on its own: the additions and
subtractions against delta_cache_size already come from `size_t` sources
(`DELTA_SIZE()` returns `size_t`), and `delta_cacheable()`'s sole caller
in `try_delta()` still passes `unsigned long`, which promotes.

Prerequisite for dropping `try_delta()`'s `cast_size_t_to_ulong()`
shims, which becomes possible once 1create_delta()` and `diff_delta()`
are widened in a later commit.

Assisted-by: Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin/pack-objects.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index e3760b3492..f89628a760 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -260,8 +260,8 @@ static int exclude_promisor_objects_best_effort;
 
 static int use_delta_islands;
 
-static unsigned long delta_cache_size = 0;
-static unsigned long max_delta_cache_size = DEFAULT_DELTA_CACHE_SIZE;
+static size_t delta_cache_size = 0;
+static size_t max_delta_cache_size = DEFAULT_DELTA_CACHE_SIZE;
 static unsigned long cache_max_small_delta_size = 1000;
 
 static unsigned long window_memory_limit = 0;
@@ -2688,8 +2688,8 @@ struct unpacked {
 	unsigned depth;
 };
 
-static int delta_cacheable(unsigned long src_size, unsigned long trg_size,
-			   unsigned long delta_size)
+static int delta_cacheable(size_t src_size, size_t trg_size,
+			   size_t delta_size)
 {
 	if (max_delta_cache_size && delta_cache_size + delta_size > max_delta_cache_size)
 		return 0;
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH 02/12] delta: widen `create_delta_index()` parameter to `size_t`
From: Johannes Schindelin via GitGitGadget @ 2026-07-09 16:49 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Johannes Schindelin
In-Reply-To: <pull.2175.git.1783615780.gitgitgadget@gmail.com>

From: Johannes Schindelin <johannes.schindelin@gmx.de>

The sole caller (`try_delta()` in builtin/pack-objects.c) passes an
`unsigned long`, which promotes safely, so no caller fixups are needed.
Splitting it out keeps the `diff_delta()`/`create_delta()` widening,
which does ripple to several callers, in its own commit.

Assisted-by: Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 delta.h      | 2 +-
 diff-delta.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/delta.h b/delta.h
index eb5c6d2fdb..a19586d789 100644
--- a/delta.h
+++ b/delta.h
@@ -14,7 +14,7 @@ struct delta_index;
  * using free_delta_index().
  */
 struct delta_index *
-create_delta_index(const void *buf, unsigned long bufsize);
+create_delta_index(const void *buf, size_t bufsize);
 
 /*
  * free_delta_index: free the index created by create_delta_index()
diff --git a/diff-delta.c b/diff-delta.c
index b6b65d7607..c93ac42594 100644
--- a/diff-delta.c
+++ b/diff-delta.c
@@ -132,7 +132,7 @@ struct delta_index {
 	struct index_entry *hash[FLEX_ARRAY];
 };
 
-struct delta_index * create_delta_index(const void *buf, unsigned long bufsize)
+struct delta_index * create_delta_index(const void *buf, size_t bufsize)
 {
 	unsigned int i, hsize, hmask, entries, prev_val, *hash_count;
 	const unsigned char *data, *buffer = buf;
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH 01/12] diff-delta: widen `struct delta_index`' size fields to `size_t`
From: Johannes Schindelin via GitGitGadget @ 2026-07-09 16:49 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Johannes Schindelin
In-Reply-To: <pull.2175.git.1783615780.gitgitgadget@gmail.com>

From: Johannes Schindelin <johannes.schindelin@gmx.de>

Preparation for widening the delta-encoding API to `size_t` in
subsequent commits, which is what lets pack-objects drop the
`cast_size_t_to_ulong()` shims that 606c192380 (odb, packfile: use
size_t for streaming object sizes, 2026-05-08) had to leave behind in
`get_delta()` and `try_delta()` because their downstream consumers were
still narrow.

The struct is private to diff-delta.c, so widening its fields in
isolation is a no-op at runtime: the values stored continue to fit in 32
bits on Windows because the public API around it still truncates.
Splitting it out keeps the API-change commit focused on caller updates.

Assisted-by: Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 diff-delta.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/diff-delta.c b/diff-delta.c
index 43c339f010..b6b65d7607 100644
--- a/diff-delta.c
+++ b/diff-delta.c
@@ -125,9 +125,9 @@ struct unpacked_index_entry {
 };
 
 struct delta_index {
-	unsigned long memsize;
+	size_t memsize;
 	const void *src_buf;
-	unsigned long src_size;
+	size_t src_size;
 	unsigned int hash_mask;
 	struct index_entry *hash[FLEX_ARRAY];
 };
@@ -140,7 +140,7 @@ struct delta_index * create_delta_index(const void *buf, unsigned long bufsize)
 	struct unpacked_index_entry *entry, **hash;
 	struct index_entry *packed_entry, **packed_hash;
 	void *mem;
-	unsigned long memsize;
+	size_t memsize;
 
 	if (!buf || !bufsize)
 		return NULL;
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH 00/12] Next size_t stop: pack-objects/delta
From: Johannes Schindelin via GitGitGadget @ 2026-07-09 16:49 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin

This patch series continues the effort to stop using unsigned long where
size_t should have been used in the first place. This makes a difference on
64-bit Windows, where unsigned long is 32-bit.

With these fixes, the pack-objects machinery works as intended on 64-bit
Windows (and any other 64-bit platform where unsigned long isn't 64-bit).

Johannes Schindelin (12):
  diff-delta: widen `struct delta_index`' size fields to `size_t`
  delta: widen `create_delta_index()` parameter to `size_t`
  pack-objects: widen delta-cache accounting to `size_t`
  pack-objects: widen `free_unpacked()` return to `size_t`
  pack-objects: widen `mem_usage` and `try_delta()`'s out-param to
    `size_t`
  delta: widen `create_delta()` and `diff_delta()` to `size_t`
  packfile, git-zlib: widen `use_pack()` and zstream avail fields to
    `size_t`
  archive-zip: widen `zlib_deflate_raw()`'s maxsize local to `size_t`
  diff: widen `deflate_it()`'s bound local from int to `size_t`
  http-push: widen `start_put()`'s size local from `ssize_t` to `size_t`
  t/helper/test-pack-deltas: widen `do_compress()`'s maxsize local to
    `size_t`
  git-zlib: widen `git_deflate_bound()` to `size_t`

 archive-zip.c               |  2 +-
 builtin/fast-import.c       |  6 ++++--
 builtin/pack-objects.c      | 30 ++++++++++++++++--------------
 delta.h                     | 12 ++++++------
 diff-delta.c                | 12 ++++++------
 diff.c                      |  6 ++++--
 git-zlib.c                  | 16 ++++++++++++++--
 git-zlib.h                  |  6 +++---
 http-push.c                 |  2 +-
 pack-check.c                |  4 ++--
 packfile.c                  |  4 ++--
 packfile.h                  |  3 ++-
 t/helper/test-delta.c       |  2 +-
 t/helper/test-pack-deltas.c |  7 ++++---
 14 files changed, 66 insertions(+), 46 deletions(-)


base-commit: f85a7e662054a7b0d9070e432508831afa214b47
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2175%2Fdscho%2Fsize-t%2Fpack-objects-delta-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2175/dscho/size-t/pack-objects-delta-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2175
-- 
gitgitgadget

^ permalink raw reply

* Re: [PATCH v8 4/9] environment: move pager_program into repo_config_values
From: Junio C Hamano @ 2026-07-09 16:41 UTC (permalink / raw)
  To: Tian Yuchen
  Cc: git, cirnovskyv, szeder.dev, Christian Couder, Ayush Chandekar,
	Olamide Caleb Bello
In-Reply-To: <0da20189-4e5b-4af0-b504-e99ac16d40af@malon.dev>

Tian Yuchen <cat@malon.dev> writes:

>>>   	if (!strcmp(var, "core.pager"))
>>> -		return git_config_string(&pager_program, var, value);
>>> +		return git_config_string(&repo_config_values(r)->pager_program, var, value);
>> 
>> Isn't this still overwriting what was in the .pager_program member
>> of the config values struct?  In check_pager_config() below, there
>> is a free() to avoid such a leak, but wouldn't this have the same
>> issue?
>> 
>>> @@ -91,10 +94,10 @@ const char *git_pager(struct repository *r, int stdout_is_tty)
>>>   
>>>   	pager = getenv("GIT_PAGER");
>>>   	if (!pager) {
>>> -		if (!pager_program)
>>> +		if (!repo_config_values(r)->pager_program)
>>>   			read_early_config(r,
>>> -					  core_pager_config, NULL);
>>> -		pager = pager_program;
>>> +					  core_pager_config, r);
>>> +		pager = repo_config_values(r)->pager_program;
>>>   	}
>>>   	if (!pager)
>>>   		pager = getenv("PAGER");
>>> @@ -302,7 +305,9 @@ int check_pager_config(struct repository *r, const char *cmd)
>>>   
>>>   	read_early_config(r, pager_command_config, &data);
>>>   
>>> -	if (data.value)
>>> -		pager_program = data.value;
>>> +	if (data.value) {
>>> +		free(repo_config_values(r)->pager_program);
>>> +		repo_config_values(r)->pager_program = data.value;
>>> +	}
>>>   	return data.want;
>>>   }
>
> Nice catch, sorry for missing that!

You do not have to be or say sorry.  This is a team effort, and I am
reasonably sure that I did not catch _all_ similar bugs in this
iteration.  So before you send an updated version, please make sure
that you just do not fix this one only and be content with it.
Instead try to see if there are other similar issues and fix them,
too.

Thanks.

^ permalink raw reply

* Re: [PATCH v4] t1410-reflog.sh: avoid suppressing git's exit code in pipelines
From: Junio C Hamano @ 2026-07-09 16:38 UTC (permalink / raw)
  To: Gatla Vishweshwar Reddy; +Cc: git
In-Reply-To: <20260709051229.40363-1-gatlavishweshwarreddy26@gmail.com>

Gatla Vishweshwar Reddy <gatlavishweshwarreddy26@gmail.com> writes:

> Piping git commands directly to wc -l suppresses the exit code of
> git, hiding potential failures from the test suite. Use
> test_stdout_line_count instead, which handles exit code preservation
> internally while keeping the test logic clean and readable.
>
> Signed-off-by: Gatla Vishweshwar Reddy <gatlavishweshwarreddy26@gmail.com>
> ---
>
> Changes in v4:
> - Restored blank line between test_expect_success blocks that was
>   accidentally removed in v2
> - Updated commit message to accurately describe the solution

This version looks good to me.

Will queue and mark the topic for 'next'.

Thanks.

>
> Thank you for the detailed review!
>
>  t/t1410-reflog.sh | 26 +++++++++-----------------
>  1 file changed, 9 insertions(+), 17 deletions(-)
>
> diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh
> index ce71f9a30a..5a40a62ba2 100755
> --- a/t/t1410-reflog.sh
> +++ b/t/t1410-reflog.sh
> @@ -244,30 +244,22 @@ test_expect_success 'delete' '
>  	test_tick &&
>  	git commit -m tiger C &&
>
> -	HEAD_entry_count=$(git reflog | wc -l) &&
> -	main_entry_count=$(git reflog show main | wc -l) &&
> -
> -	test $HEAD_entry_count = 5 &&
> -	test $main_entry_count = 5 &&
> -
> +	test_stdout_line_count = 5 git reflog &&
> +	test_stdout_line_count = 5 git reflog show main &&
>
>  	git reflog delete main@{1} &&
> +	test_stdout_line_count = 4 git reflog show main &&
> +	test_stdout_line_count = 5 git reflog &&
>  	git reflog show main > output &&
> -	test_line_count = $(($main_entry_count - 1)) output &&
> -	test $HEAD_entry_count = $(git reflog | wc -l) &&
>  	! grep ox < output &&
>
> -	main_entry_count=$(wc -l < output) &&
> -
>  	git reflog delete HEAD@{1} &&
> -	test $(($HEAD_entry_count -1)) = $(git reflog | wc -l) &&
> -	test $main_entry_count = $(git reflog show main | wc -l) &&
> -
> -	HEAD_entry_count=$(git reflog | wc -l) &&
> +	test_stdout_line_count = 4 git reflog &&
> +	test_stdout_line_count = 4 git reflog show main &&
>
>  	git reflog delete main@{07.04.2005.15:15:00.-0700} &&
> +	test_stdout_line_count = 3 git reflog show main &&
>  	git reflog show main > output &&
> -	test_line_count = $(($main_entry_count - 1)) output &&
>  	! grep dragon < output
>
>  '
> @@ -321,11 +313,11 @@ test_expect_success 'git reflog expire unknown reference' '
>  '
>
>  test_expect_success 'checkout should not delete log for packed ref' '
> -	test $(git reflog main | wc -l) = 4 &&
> +	test_stdout_line_count = 4 git reflog main &&
>  	git branch foo &&
>  	git pack-refs --all &&
>  	git checkout foo &&
> -	test $(git reflog main | wc -l) = 4
> +	test_stdout_line_count = 4 git reflog main
>  '
>
>  test_expect_success 'stale dirs do not cause d/f conflicts (reflogs on)' '

^ permalink raw reply

* Re: [PATCH v8 4/9] environment: move pager_program into repo_config_values
From: Tian Yuchen @ 2026-07-09 16:12 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, cirnovskyv, szeder.dev, Christian Couder, Ayush Chandekar,
	Olamide Caleb Bello
In-Reply-To: <xmqqy0fkq0nw.fsf@gitster.g>

On 7/9/26 11:53, Junio C Hamano wrote:
> Tian Yuchen <cat@malon.dev> writes:
> 
>> On top of that, fix a memory leak in pager.c while we are at it.
> 
> Hmph.
> 
>> @@ -75,10 +76,12 @@ static void wait_for_pager_signal(int signo)
>>   
>>   static int core_pager_config(const char *var, const char *value,
>>   			     const struct config_context *ctx UNUSED,
>> -			     void *data UNUSED)
>> +			     void *data)
>>   {
>> +	struct repository *r = data;
>> +
>>   	if (!strcmp(var, "core.pager"))
>> -		return git_config_string(&pager_program, var, value);
>> +		return git_config_string(&repo_config_values(r)->pager_program, var, value);
> 
> Isn't this still overwriting what was in the .pager_program member
> of the config values struct?  In check_pager_config() below, there
> is a free() to avoid such a leak, but wouldn't this have the same
> issue?
> 
>> @@ -91,10 +94,10 @@ const char *git_pager(struct repository *r, int stdout_is_tty)
>>   
>>   	pager = getenv("GIT_PAGER");
>>   	if (!pager) {
>> -		if (!pager_program)
>> +		if (!repo_config_values(r)->pager_program)
>>   			read_early_config(r,
>> -					  core_pager_config, NULL);
>> -		pager = pager_program;
>> +					  core_pager_config, r);
>> +		pager = repo_config_values(r)->pager_program;
>>   	}
>>   	if (!pager)
>>   		pager = getenv("PAGER");
>> @@ -302,7 +305,9 @@ int check_pager_config(struct repository *r, const char *cmd)
>>   
>>   	read_early_config(r, pager_command_config, &data);
>>   
>> -	if (data.value)
>> -		pager_program = data.value;
>> +	if (data.value) {
>> +		free(repo_config_values(r)->pager_program);
>> +		repo_config_values(r)->pager_program = data.value;
>> +	}
>>   	return data.want;
>>   }

Nice catch, sorry for missing that!

Regards, yuchen

^ permalink raw reply

* [PATCH v9 9/9] environment: move object_creation_mode into repo_config_values
From: Tian Yuchen @ 2026-07-09 16:11 UTC (permalink / raw)
  To: git
  Cc: cirnovskyv, szeder.dev, Tian Yuchen, Christian Couder,
	Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <20260709161145.13349-1-cat@malon.dev>

The global variable 'object_creation_mode' controls how Git creates
object files, specifically determining whether to use hardlinks or
renames when moving temporary files into the object database. Move
it into 'struct repo_config_values' to continue the libification
effort.

Move the 'enum object_creation_mode' definition higher up in
'environment.h' to ensure it is visible to the structure. Initialize
the per-repository value to its default macro value
OBJECT_CREATION_MODE inside 'repo_config_values_init()'.

Update configuration parsing in 'git_default_core_config()' to write
directly to the repository-specific configuration structure.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
 environment.c |  6 +++---
 environment.h | 12 ++++++------
 object-file.c |  2 +-
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/environment.c b/environment.c
index 7701aa3bc0..e50beda918 100644
--- a/environment.c
+++ b/environment.c
@@ -61,7 +61,6 @@ char *check_roundtrip_encoding;
 #ifndef OBJECT_CREATION_MODE
 #define OBJECT_CREATION_MODE OBJECT_CREATION_USES_HARDLINKS
 #endif
-enum object_creation_mode object_creation_mode = OBJECT_CREATION_MODE;
 int grafts_keep_true_parents;
 unsigned long pack_size_limit_cfg;
 
@@ -511,9 +510,9 @@ int git_default_core_config(const char *var, const char *value,
 		if (!value)
 			return config_error_nonbool(var);
 		if (!strcmp(value, "rename"))
-			object_creation_mode = OBJECT_CREATION_USES_RENAMES;
+			cfg->object_creation_mode = OBJECT_CREATION_USES_RENAMES;
 		else if (!strcmp(value, "link"))
-			object_creation_mode = OBJECT_CREATION_USES_HARDLINKS;
+			cfg->object_creation_mode = OBJECT_CREATION_USES_HARDLINKS;
 		else
 			die(_("invalid mode for object creation: %s"), value);
 		return 0;
@@ -727,6 +726,7 @@ void repo_config_values_init(struct repo_config_values *cfg)
 	cfg->apply_default_ignorewhitespace = NULL;
 	cfg->push_default = PUSH_DEFAULT_UNSPECIFIED;
 	cfg->autorebase = AUTOREBASE_NEVER;
+	cfg->object_creation_mode = OBJECT_CREATION_MODE;
 	cfg->apply_sparse_checkout = 0;
 	cfg->branch_track = BRANCH_TRACK_REMOTE;
 	cfg->trust_ctime = 1;
diff --git a/environment.h b/environment.h
index 464ff73136..eaa0aba7bc 100644
--- a/environment.h
+++ b/environment.h
@@ -109,6 +109,11 @@ enum rebase_setup_type {
 	AUTOREBASE_ALWAYS
 };
 
+enum object_creation_mode {
+	OBJECT_CREATION_USES_HARDLINKS = 0,
+	OBJECT_CREATION_USES_RENAMES = 1
+};
+
 struct repo_config_values {
 	/* section "core" config values */
 	char *attributes_file;
@@ -120,6 +125,7 @@ struct repo_config_values {
 	char *apply_default_ignorewhitespace;
 	enum push_default_type push_default;
 	enum rebase_setup_type autorebase;
+	enum object_creation_mode object_creation_mode;
 	int apply_sparse_checkout;
 	int trust_ctime;
 	int check_stat;
@@ -213,12 +219,6 @@ extern unsigned long pack_size_limit_cfg;
 extern int protect_hfs;
 extern int protect_ntfs;
 
-enum object_creation_mode {
-	OBJECT_CREATION_USES_HARDLINKS = 0,
-	OBJECT_CREATION_USES_RENAMES = 1
-};
-extern enum object_creation_mode object_creation_mode;
-
 extern int grafts_keep_true_parents;
 
 const char *get_log_output_encoding(void);
diff --git a/object-file.c b/object-file.c
index 9afa842da2..cbbfc8f1dc 100644
--- a/object-file.c
+++ b/object-file.c
@@ -415,7 +415,7 @@ int finalize_object_file_flags(struct repository *repo,
 retry:
 	ret = 0;
 
-	if (object_creation_mode == OBJECT_CREATION_USES_RENAMES)
+	if (repo_config_values(repo)->object_creation_mode == OBJECT_CREATION_USES_RENAMES)
 		goto try_rename;
 	else if (link(tmpfile, filename))
 		ret = errno;
-- 
2.43.0


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox