Git development
 help / color / mirror / Atom feed
* [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

* [PATCH v9 8/9] environment: move autorebase 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 'autorebase' dictates whether a newly created
branch should be configured to automatically rebase by default.
Move it into 'struct repo_config_values' to continue the
libification effort.

The 'enum rebase_setup_type' definition is moved higher up in
'environment.h' so that it is visible to the repository-specific
structure. The default state AUTOREBASE_NEVER is now correctly
initialized in 'repo_config_values_init()'.

Configuration parsing in 'git_default_branch_config()' is updated to
write directly to the repository's configuration instance.

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>
---
 branch.c      |  2 +-
 environment.c | 10 +++++-----
 environment.h | 16 ++++++++--------
 3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/branch.c b/branch.c
index 243db7d0fc..e1c1f8c89d 100644
--- a/branch.c
+++ b/branch.c
@@ -61,7 +61,7 @@ static int find_tracked_branch(struct remote *remote, void *priv)
 
 static int should_setup_rebase(const char *origin)
 {
-	switch (autorebase) {
+	switch (repo_config_values(the_repository)->autorebase) {
 	case AUTOREBASE_NEVER:
 		return 0;
 	case AUTOREBASE_LOCAL:
diff --git a/environment.c b/environment.c
index 09de2fee87..7701aa3bc0 100644
--- a/environment.c
+++ b/environment.c
@@ -58,7 +58,6 @@ enum auto_crlf auto_crlf = AUTO_CRLF_FALSE;
 enum eol core_eol = EOL_UNSET;
 int global_conv_flags_eol = CONV_EOL_RNDTRP_WARN;
 char *check_roundtrip_encoding;
-enum rebase_setup_type autorebase = AUTOREBASE_NEVER;
 #ifndef OBJECT_CREATION_MODE
 #define OBJECT_CREATION_MODE OBJECT_CREATION_USES_HARDLINKS
 #endif
@@ -600,13 +599,13 @@ static int git_default_branch_config(const char *var, const char *value)
 		if (!value)
 			return config_error_nonbool(var);
 		else if (!strcmp(value, "never"))
-			autorebase = AUTOREBASE_NEVER;
+			cfg->autorebase = AUTOREBASE_NEVER;
 		else if (!strcmp(value, "local"))
-			autorebase = AUTOREBASE_LOCAL;
+			cfg->autorebase = AUTOREBASE_LOCAL;
 		else if (!strcmp(value, "remote"))
-			autorebase = AUTOREBASE_REMOTE;
+			cfg->autorebase = AUTOREBASE_REMOTE;
 		else if (!strcmp(value, "always"))
-			autorebase = AUTOREBASE_ALWAYS;
+			cfg->autorebase = AUTOREBASE_ALWAYS;
 		else
 			return error(_("malformed value for %s"), var);
 		return 0;
@@ -727,6 +726,7 @@ void repo_config_values_init(struct repo_config_values *cfg)
 	cfg->apply_default_whitespace = NULL;
 	cfg->apply_default_ignorewhitespace = NULL;
 	cfg->push_default = PUSH_DEFAULT_UNSPECIFIED;
+	cfg->autorebase = AUTOREBASE_NEVER;
 	cfg->apply_sparse_checkout = 0;
 	cfg->branch_track = BRANCH_TRACK_REMOTE;
 	cfg->trust_ctime = 1;
diff --git a/environment.h b/environment.h
index 72859b5d76..464ff73136 100644
--- a/environment.h
+++ b/environment.h
@@ -102,6 +102,13 @@ enum push_default_type {
 	PUSH_DEFAULT_UNSPECIFIED
 };
 
+enum rebase_setup_type {
+	AUTOREBASE_NEVER = 0,
+	AUTOREBASE_LOCAL,
+	AUTOREBASE_REMOTE,
+	AUTOREBASE_ALWAYS
+};
+
 struct repo_config_values {
 	/* section "core" config values */
 	char *attributes_file;
@@ -112,6 +119,7 @@ struct repo_config_values {
 	char *apply_default_whitespace;
 	char *apply_default_ignorewhitespace;
 	enum push_default_type push_default;
+	enum rebase_setup_type autorebase;
 	int apply_sparse_checkout;
 	int trust_ctime;
 	int check_stat;
@@ -205,14 +213,6 @@ extern unsigned long pack_size_limit_cfg;
 extern int protect_hfs;
 extern int protect_ntfs;
 
-enum rebase_setup_type {
-	AUTOREBASE_NEVER = 0,
-	AUTOREBASE_LOCAL,
-	AUTOREBASE_REMOTE,
-	AUTOREBASE_ALWAYS
-};
-extern enum rebase_setup_type autorebase;
-
 enum object_creation_mode {
 	OBJECT_CREATION_USES_HARDLINKS = 0,
 	OBJECT_CREATION_USES_RENAMES = 1
-- 
2.43.0


^ permalink raw reply related

* [PATCH v9 7/9] environment: move push_default 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 'push_default' specifies the default behavior of
'git push' when no explicit refspec is provided. Move 'push_default'
into 'struct repo_config_values' to continue the libification effort.

While 'enum push_default_type' ideally belongs in 'remote.h', moving it
there introduces a circular dependency chain:

  remote.h -> hash.h -> repository.h -> environment.h.

Therefore, the enum definition is kept in 'environment.h' just above
'struct repo_config_values' with a NEEDSWORK comment for future cleanup.

Modify the configuration parsing in environment.c to update the
per-repository structure directly, and update caller across the
codebase to access the value via 'repo_config_values()'.

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>
---
 builtin/push.c |  8 ++++----
 environment.c  | 16 +++++++++-------
 environment.h  | 26 ++++++++++++++++----------
 remote.c       |  2 +-
 4 files changed, 30 insertions(+), 22 deletions(-)

diff --git a/builtin/push.c b/builtin/push.c
index 6021b71d66..6dc3224b60 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -88,7 +88,7 @@ static void refspec_append_mapped(struct refspec *refspec, const char *ref,
 		}
 	}
 
-	if (push_default == PUSH_DEFAULT_UPSTREAM &&
+	if (repo_config_values(the_repository)->push_default == PUSH_DEFAULT_UPSTREAM &&
 	    skip_prefix(matched->name, "refs/heads/", &branch_name)) {
 		struct branch *branch = branch_get(branch_name);
 		if (branch->merge_nr == 1 && branch->merge[0]->src) {
@@ -160,7 +160,7 @@ static NORETURN void die_push_simple(struct branch *branch,
 	 * Don't show advice for people who explicitly set
 	 * push.default.
 	 */
-	if (push_default == PUSH_DEFAULT_UNSPECIFIED)
+	if (cfg->push_default == PUSH_DEFAULT_UNSPECIFIED)
 		advice_pushdefault_maybe = _("\n"
 				 "To choose either option permanently, "
 				 "see push.default in 'git help config'.\n");
@@ -232,7 +232,7 @@ static void setup_default_push_refspecs(int *flags, struct remote *remote)
 	const char *dst;
 	int same_remote;
 
-	switch (push_default) {
+	switch (repo_config_values(the_repository)->push_default) {
 	case PUSH_DEFAULT_MATCHING:
 		refspec_append(&rs, ":");
 		return;
@@ -252,7 +252,7 @@ static void setup_default_push_refspecs(int *flags, struct remote *remote)
 	dst = branch->refname;
 	same_remote = !strcmp(remote->name, remote_for_branch(branch, NULL));
 
-	switch (push_default) {
+	switch (repo_config_values(the_repository)->push_default) {
 	default:
 	case PUSH_DEFAULT_UNSPECIFIED:
 	case PUSH_DEFAULT_SIMPLE:
diff --git a/environment.c b/environment.c
index 8744790219..09de2fee87 100644
--- a/environment.c
+++ b/environment.c
@@ -59,7 +59,6 @@ enum eol core_eol = EOL_UNSET;
 int global_conv_flags_eol = CONV_EOL_RNDTRP_WARN;
 char *check_roundtrip_encoding;
 enum rebase_setup_type autorebase = AUTOREBASE_NEVER;
-enum push_default_type push_default = PUSH_DEFAULT_UNSPECIFIED;
 #ifndef OBJECT_CREATION_MODE
 #define OBJECT_CREATION_MODE OBJECT_CREATION_USES_HARDLINKS
 #endif
@@ -619,21 +618,23 @@ static int git_default_branch_config(const char *var, const char *value)
 
 static int git_default_push_config(const char *var, const char *value)
 {
+	struct repo_config_values *cfg = repo_config_values(the_repository);
+
 	if (!strcmp(var, "push.default")) {
 		if (!value)
 			return config_error_nonbool(var);
 		else if (!strcmp(value, "nothing"))
-			push_default = PUSH_DEFAULT_NOTHING;
+			cfg->push_default = PUSH_DEFAULT_NOTHING;
 		else if (!strcmp(value, "matching"))
-			push_default = PUSH_DEFAULT_MATCHING;
+			cfg->push_default = PUSH_DEFAULT_MATCHING;
 		else if (!strcmp(value, "simple"))
-			push_default = PUSH_DEFAULT_SIMPLE;
+			cfg->push_default = PUSH_DEFAULT_SIMPLE;
 		else if (!strcmp(value, "upstream"))
-			push_default = PUSH_DEFAULT_UPSTREAM;
+			cfg->push_default = PUSH_DEFAULT_UPSTREAM;
 		else if (!strcmp(value, "tracking")) /* deprecated */
-			push_default = PUSH_DEFAULT_UPSTREAM;
+			cfg->push_default = PUSH_DEFAULT_UPSTREAM;
 		else if (!strcmp(value, "current"))
-			push_default = PUSH_DEFAULT_CURRENT;
+			cfg->push_default = PUSH_DEFAULT_CURRENT;
 		else {
 			error(_("malformed value for %s: %s"), var, value);
 			return error(_("must be one of nothing, matching, simple, "
@@ -725,6 +726,7 @@ void repo_config_values_init(struct repo_config_values *cfg)
 	cfg->askpass_program = NULL;
 	cfg->apply_default_whitespace = NULL;
 	cfg->apply_default_ignorewhitespace = NULL;
+	cfg->push_default = PUSH_DEFAULT_UNSPECIFIED;
 	cfg->apply_sparse_checkout = 0;
 	cfg->branch_track = BRANCH_TRACK_REMOTE;
 	cfg->trust_ctime = 1;
diff --git a/environment.h b/environment.h
index 9aecd64152..72859b5d76 100644
--- a/environment.h
+++ b/environment.h
@@ -87,6 +87,21 @@ extern const char * const local_repo_env[];
 struct strvec;
 
 struct repository;
+
+/*
+ * NEEDSWORK: It would be better if these definitions could be moved to
+ * other more specific files, but care is needed to avoid circular
+ * inclusion issues.
+ */
+enum push_default_type {
+	PUSH_DEFAULT_NOTHING = 0,
+	PUSH_DEFAULT_MATCHING,
+	PUSH_DEFAULT_SIMPLE,
+	PUSH_DEFAULT_UPSTREAM,
+	PUSH_DEFAULT_CURRENT,
+	PUSH_DEFAULT_UNSPECIFIED
+};
+
 struct repo_config_values {
 	/* section "core" config values */
 	char *attributes_file;
@@ -96,6 +111,7 @@ struct repo_config_values {
 	char *askpass_program;
 	char *apply_default_whitespace;
 	char *apply_default_ignorewhitespace;
+	enum push_default_type push_default;
 	int apply_sparse_checkout;
 	int trust_ctime;
 	int check_stat;
@@ -197,16 +213,6 @@ enum rebase_setup_type {
 };
 extern enum rebase_setup_type autorebase;
 
-enum push_default_type {
-	PUSH_DEFAULT_NOTHING = 0,
-	PUSH_DEFAULT_MATCHING,
-	PUSH_DEFAULT_SIMPLE,
-	PUSH_DEFAULT_UPSTREAM,
-	PUSH_DEFAULT_CURRENT,
-	PUSH_DEFAULT_UNSPECIFIED
-};
-extern enum push_default_type push_default;
-
 enum object_creation_mode {
 	OBJECT_CREATION_USES_HARDLINKS = 0,
 	OBJECT_CREATION_USES_RENAMES = 1
diff --git a/remote.c b/remote.c
index 00723b385e..d48c01d375 100644
--- a/remote.c
+++ b/remote.c
@@ -1933,7 +1933,7 @@ static char *branch_get_push_1(struct repository *repo,
 	if (remote->mirror)
 		return tracking_for_push_dest(remote, branch->refname, err);
 
-	switch (push_default) {
+	switch (repo_config_values(repo)->push_default) {
 	case PUSH_DEFAULT_NOTHING:
 		return error_buf(err, _("push has no destination (push.default is 'nothing')"));
 
-- 
2.43.0


^ permalink raw reply related

* [PATCH v9 6/9] environment: migrate apply_default_whitespace and apply_default_ignorewhitespace
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 variables 'apply_default_whitespace' and
'apply_default_ignorewhitespace' are used to store the default
whitespace configuration for 'git apply'. Move these variables
into 'struct repo_config_values' to continue the libification
effort.

Dynamically allocated strings fetched via 'repo_config_get_string()'
are now tracked per-repository and safely freed in
'repo_config_values_clear()'.

As part of this transition, update 'git_apply_config()' to accept a
'struct repository *' argument rather than relying on the
'the_repository' global.

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>
---
 apply.c       | 20 ++++++++++++--------
 environment.c |  6 ++++--
 environment.h |  4 ++--
 3 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/apply.c b/apply.c
index 249248d4f2..66db9b7678 100644
--- a/apply.c
+++ b/apply.c
@@ -47,11 +47,13 @@ struct gitdiff_data {
 	int p_value;
 };
 
-static void git_apply_config(void)
+static void git_apply_config(struct repository *repo)
 {
-	repo_config_get_string(the_repository, "apply.whitespace", &apply_default_whitespace);
-	repo_config_get_string(the_repository, "apply.ignorewhitespace", &apply_default_ignorewhitespace);
-	repo_config(the_repository, git_xmerge_config, NULL);
+	repo_config_get_string(repo, "apply.whitespace",
+			       &repo_config_values(repo)->apply_default_whitespace);
+	repo_config_get_string(repo, "apply.ignorewhitespace",
+			       &repo_config_values(repo)->apply_default_ignorewhitespace);
+	repo_config(repo, git_xmerge_config, NULL);
 }
 
 static int parse_whitespace_option(struct apply_state *state, const char *option)
@@ -126,10 +128,12 @@ int init_apply_state(struct apply_state *state,
 	strset_init(&state->kept_symlinks);
 	strbuf_init(&state->root, 0);
 
-	git_apply_config();
-	if (apply_default_whitespace && parse_whitespace_option(state, apply_default_whitespace))
+	git_apply_config(repo);
+	if (repo_config_values(repo)->apply_default_whitespace &&
+	    parse_whitespace_option(state, repo_config_values(repo)->apply_default_whitespace))
 		return -1;
-	if (apply_default_ignorewhitespace && parse_ignorewhitespace_option(state, apply_default_ignorewhitespace))
+	if (repo_config_values(repo)->apply_default_ignorewhitespace &&
+	    parse_ignorewhitespace_option(state, repo_config_values(repo)->apply_default_ignorewhitespace))
 		return -1;
 	return 0;
 }
@@ -192,7 +196,7 @@ int check_apply_state(struct apply_state *state, int force_apply)
 
 static void set_default_whitespace_mode(struct apply_state *state)
 {
-	if (!state->whitespace_option && !apply_default_whitespace)
+	if (!state->whitespace_option && !repo_config_values(state->repo)->apply_default_whitespace)
 		state->ws_error_action = (state->apply ? warn_on_ws_error : nowarn_ws_error);
 }
 
diff --git a/environment.c b/environment.c
index 3782bf68aa..8744790219 100644
--- a/environment.c
+++ b/environment.c
@@ -49,8 +49,6 @@ int assume_unchanged;
 int is_bare_repository_cfg = -1; /* unspecified */
 char *git_commit_encoding;
 char *git_log_output_encoding;
-char *apply_default_whitespace;
-char *apply_default_ignorewhitespace;
 int fsync_object_files = -1;
 int use_fsync = -1;
 enum fsync_method fsync_method = FSYNC_METHOD_DEFAULT;
@@ -725,6 +723,8 @@ void repo_config_values_init(struct repo_config_values *cfg)
 	cfg->editor_program = NULL;
 	cfg->pager_program = NULL;
 	cfg->askpass_program = NULL;
+	cfg->apply_default_whitespace = NULL;
+	cfg->apply_default_ignorewhitespace = NULL;
 	cfg->apply_sparse_checkout = 0;
 	cfg->branch_track = BRANCH_TRACK_REMOTE;
 	cfg->trust_ctime = 1;
@@ -758,4 +758,6 @@ void repo_config_values_clear(struct repository *repo)
 	FREE_AND_NULL(cfg->editor_program);
 	FREE_AND_NULL(cfg->pager_program);
 	FREE_AND_NULL(cfg->askpass_program);
+	FREE_AND_NULL(cfg->apply_default_whitespace);
+	FREE_AND_NULL(cfg->apply_default_ignorewhitespace);
 }
diff --git a/environment.h b/environment.h
index d55b1ba073..9aecd64152 100644
--- a/environment.h
+++ b/environment.h
@@ -94,6 +94,8 @@ struct repo_config_values {
 	char *editor_program;
 	char *pager_program;
 	char *askpass_program;
+	char *apply_default_whitespace;
+	char *apply_default_ignorewhitespace;
 	int apply_sparse_checkout;
 	int trust_ctime;
 	int check_stat;
@@ -182,8 +184,6 @@ extern int has_symlinks;
 extern int minimum_abbrev, default_abbrev;
 extern int ignore_case;
 extern int assume_unchanged;
-extern char *apply_default_whitespace;
-extern char *apply_default_ignorewhitespace;
 extern unsigned long pack_size_limit_cfg;
 
 extern int protect_hfs;
-- 
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