* Re: [PATCH v3 0/6] refs: remove use of `the_repository`
From: Christian Couder @ 2026-07-16 6:53 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Junio C Hamano, Toon Claes
In-Reply-To: <20260716-pks-refs-wo-the-repository-v3-0-db0a804e0224@pks.im>
On Thu, Jul 16, 2026 at 7:33 AM Patrick Steinhardt <ps@pks.im> wrote:
>
> Hi,
>
> this patch series refactors the ref subsystem to drop uses of
> `the_repository`. These patches were part of a discarded attempt to
> make the initialization of the refdb eager. I guess they make sense by
> themselves though, so here we go.
>
> Note that these patches contain a slight tangent to also adapt
> "worktree.c". This is one of the subsystems that caused problems with
> eager refdb initialization because of `has_worktrees()`, so I refactored
> this subsystem while at it.
The changes in this series look good to me too.
Thanks.
^ permalink raw reply
* Re: [PATCH v1] repository: move fetch_if_missing into struct repository
From: Tian Yuchen @ 2026-07-16 7:06 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: git, five231003, hariom18599, Christian Couder, Ayush Chandekar,
Olamide Caleb Bello
In-Reply-To: <alcqQp0lkwRIIE1t@pks.im>
On 7/15/26 14:35, Patrick Steinhardt wrote:
> On Wed, Jul 15, 2026 at 09:18:50AM +0800, Tian Yuchen wrote:
>> The global variable 'fetch_if_missing' controls whether a missing
>> object check should prompt a lazy fetch from a promisor remote.
>> In order to continue the libification effort, move it into
>> 'struct repository' and initialize it to 1 by default to keep the
>> previous behavior.
>
> Right. I was also thinking about moving this into a non-global scope
> multiple times. I was approaching this a bit differently though: it's
> ultimately a property of the object database whether or not we want to
> accept missing objects, so I moved it in there instead.
>
> I don't really think there's a downside with your version, though. Quite
> on the contrary: we can really only perform the backfill fetches with a
> whole repository at hand anyway. So conceptually your version might even
> be more sensible.
>
>> Subsystems that already pass around a repository pointer, are
>> updated to read this flag directly from their respective 'repo'
>> instances. For the rest, we access 'the_repository'.
>>
>> Note that in builtin/fsck.c and builtin/index-pack.c, when running
>> related commands with the '-h' parameter, the 'repo' pointer is not
>> passed in. To prevent null pointer dereferences, we defer
>> operations on the repo in until after parameter parsing is complete.
>
> s/on the repo in/on the repo/
>
>> diff --git a/builtin/index-pack.c b/builtin/index-pack.c
>> index 0793dc595c..721d576938 100644
>> --- a/builtin/index-pack.c
>> +++ b/builtin/index-pack.c
>> @@ -1898,15 +1898,16 @@ int cmd_index_pack(int argc,
>> int report_end_of_input = 0;
>> int hash_algo = 0;
>>
>> + show_usage_if_asked(argc, argv, index_pack_usage);
>> +
>> /*
>> * index-pack never needs to fetch missing objects except when
>> * REF_DELTA bases are missing (which are explicitly handled). It only
>> * accesses the repo to do hash collision checks and to check which
>> * REF_DELTA bases need to be fetched.
>> */
>> - fetch_if_missing = 0;
>> -
>> - show_usage_if_asked(argc, argv, index_pack_usage);
>> + if (repo)
>> + repo->fetch_if_missing = 0;
>>
>> disable_replace_refs();
>>
>
> Okay. This command can run without a repository, in which case we'll end
> up just indexing the pack. My assumption is that we'll probably end up
> using `the_repository` if so, as we still use `the_repository` in this
> file. So could this here cause a change in behaviour?
Hummm...
>
> If the answer is "maybe" I'd propose that we simply continue to use
> `the_repository` here.
>
>> diff --git a/revision.c b/revision.c
>> index e91d7e1f11..bb645654c3 100644
>> --- a/revision.c
>> +++ b/revision.c
>> @@ -2714,7 +2714,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
>> revs->ignore_missing = 1;
>> } else if (opt && opt->allow_exclude_promisor_objects &&
>> !strcmp(arg, "--exclude-promisor-objects")) {
>> - if (fetch_if_missing)
>> + if (revs->repo->fetch_if_missing)
>> BUG("exclude_promisor_objects can only be used when fetch_if_missing is 0");
>> revs->exclude_promisor_objects = 1;
>> } else {
>
> This one here also makes me wonder whether it could cause weird
> interactions in case a caller passes a repository other than
> `the_repository`. It ideally _shouldn't_, but it's hard to tell because
> we still use `the_repository` in lots of places here.
>
This makes sense to me. Let's use the_repository then.
> Thanks!
>
> Patrick
Regards, yuchen
^ permalink raw reply
* Re: [PATCH] t7614: avoid hiding git's exit code in a pipe
From: Shlok Kulshreshtha @ 2026-07-16 7:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Shlok Kulshreshtha
In-Reply-To: <xmqq1pd4m4ea.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> All look trivially correct.
Thanks for the review.
> * This "cat-file the commit object, and strip away the object
> header with sed" pattern appears quite often throughout the test
> suite.
> [...]
> or something like that.
>
> But again, these are clearly outside the scope of this patch.
Agreed on keeping them out of this patch. I'd like to take up the
commit_body() helper as a separate follow-up once this lands, and
convert the existing "cat-file ... | sed" call sites (including the
unnecessary backslash before the dollar sign) over to it.
Thanks,
Shlok
^ permalink raw reply
* [PATCH v2] repository: move fetch_if_missing into struct repository
From: Tian Yuchen @ 2026-07-16 7:29 UTC (permalink / raw)
To: git
Cc: ps, five231003, hariom18599, Tian Yuchen, Christian Couder,
Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <20260715011850.3181131-1-cat@malon.dev>
The global variable 'fetch_if_missing' controls whether a missing
object check should prompt a lazy fetch from a promisor remote.
In order to continue the libification effort, move it into
'struct repository' and initialize it to 1 by default to keep the
previous behavior.
Note that in builtin/fsck.c and builtin/index-pack.c, when running
related commands with the '-h' parameter, the 'repo' pointer is not
passed in. To prevent null pointer dereferences, we defer
operations on the repo until after parameter parsing is complete.
Additionally, update the partial clone documentation to reflect
that this is now a per-repository flag.
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>
---
Change since V1:
- Following Patrick's advice, use the_repository whenever possible
without re-introducing #define USE_THE_REPOSITORY_VARIABLE.
Documentation/technical/partial-clone.adoc | 2 +-
builtin/fetch-pack.c | 2 +-
builtin/fsck.c | 6 +++---
builtin/index-pack.c | 7 ++++---
builtin/pack-objects.c | 14 +++++++-------
builtin/prune.c | 2 +-
builtin/rev-list.c | 10 +++++-----
git.c | 2 +-
midx-write.c | 2 +-
odb.c | 4 +---
odb.h | 8 --------
repository.c | 1 +
repository.h | 6 ++++++
revision.c | 2 +-
setup.c | 2 +-
15 files changed, 34 insertions(+), 36 deletions(-)
diff --git a/Documentation/technical/partial-clone.adoc b/Documentation/technical/partial-clone.adoc
index e513e391ea..18718a3840 100644
--- a/Documentation/technical/partial-clone.adoc
+++ b/Documentation/technical/partial-clone.adoc
@@ -159,7 +159,7 @@ and prefetch those objects in bulk.
- `repack` in GC has been updated to not touch promisor packfiles at all,
and to only repack other objects.
-- The global variable "fetch_if_missing" is used to control whether an
+- The per-repository flag "fetch_if_missing" is used to control whether an
object lookup will attempt to dynamically fetch a missing object or
report an error.
+
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 316badd969..c5edd7b80f 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -67,7 +67,7 @@ int cmd_fetch_pack(int argc,
struct packet_reader reader;
enum protocol_version version;
- fetch_if_missing = 0;
+ the_repository->fetch_if_missing = 0;
packet_trace_identity("fetch-pack");
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 248f8ff5a0..aa31c69486 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -1017,15 +1017,15 @@ int cmd_fsck(int argc,
.ref = NULL
};
- /* fsck knows how to handle missing promisor objects */
- fetch_if_missing = 0;
-
errors_found = 0;
disable_replace_refs();
save_commit_buffer = 0;
argc = parse_options(argc, argv, prefix, fsck_opts, fsck_usage, 0);
+ /* fsck knows how to handle missing promisor objects */
+ repo->fetch_if_missing = 0;
+
fsck_options_init(&fsck_walk_options, repo, FSCK_OPTIONS_DEFAULT);
fsck_walk_options.walk = mark_object;
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 0793dc595c..74f9694662 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -1898,15 +1898,16 @@ int cmd_index_pack(int argc,
int report_end_of_input = 0;
int hash_algo = 0;
+ show_usage_if_asked(argc, argv, index_pack_usage);
+
/*
* index-pack never needs to fetch missing objects except when
* REF_DELTA bases are missing (which are explicitly handled). It only
* accesses the repo to do hash collision checks and to check which
* REF_DELTA bases need to be fetched.
*/
- fetch_if_missing = 0;
-
- show_usage_if_asked(argc, argv, index_pack_usage);
+ if (repo)
+ the_repository->fetch_if_missing = 0;
disable_replace_refs();
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 8a1709a1ab..c6536b1f65 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -4059,7 +4059,7 @@ static void add_unreachable_loose_objects(struct rev_info *revs);
static void read_stdin_packs(enum stdin_packs_mode mode, int rev_list_unpacked)
{
- int prev_fetch_if_missing = fetch_if_missing;
+ int prev_fetch_if_missing = the_repository->fetch_if_missing;
struct rev_info revs;
/*
@@ -4067,7 +4067,7 @@ static void read_stdin_packs(enum stdin_packs_mode mode, int rev_list_unpacked)
* walk is best-effort though we don't want to perform backfill fetches
* for them.
*/
- fetch_if_missing = 0;
+ the_repository->fetch_if_missing = 0;
repo_init_revisions(the_repository, &revs, NULL);
/*
@@ -4115,7 +4115,7 @@ static void read_stdin_packs(enum stdin_packs_mode mode, int rev_list_unpacked)
trace2_data_intmax("pack-objects", the_repository, "stdin_packs_hints",
stdin_packs_hints_nr);
- fetch_if_missing = prev_fetch_if_missing;
+ the_repository->fetch_if_missing = prev_fetch_if_missing;
}
static void add_cruft_object_entry(const struct object_id *oid, enum object_type type,
@@ -4451,14 +4451,14 @@ static int option_parse_missing_action(const struct option *opt UNUSED,
if (!strcmp(arg, "allow-any")) {
arg_missing_action = MA_ALLOW_ANY;
- fetch_if_missing = 0;
+ the_repository->fetch_if_missing = 0;
fn_show_object = show_object__ma_allow_any;
return 0;
}
if (!strcmp(arg, "allow-promisor")) {
arg_missing_action = MA_ALLOW_PROMISOR;
- fetch_if_missing = 0;
+ the_repository->fetch_if_missing = 0;
fn_show_object = show_object__ma_allow_promisor;
return 0;
}
@@ -5247,7 +5247,7 @@ int cmd_pack_objects(int argc,
exclude_promisor_objects_best_effort,
"--exclude-promisor-objects-best-effort");
if (exclude_promisor_objects) {
- fetch_if_missing = 0;
+ the_repository->fetch_if_missing = 0;
/* --stdin-packs handles promisor objects separately. */
if (!stdin_packs) {
@@ -5256,7 +5256,7 @@ int cmd_pack_objects(int argc,
}
} else if (exclude_promisor_objects_best_effort) {
use_internal_rev_list = 1;
- fetch_if_missing = 0;
+ the_repository->fetch_if_missing = 0;
option_parse_missing_action(NULL, "allow-any", 0);
/* revs configured below */
}
diff --git a/builtin/prune.c b/builtin/prune.c
index 55635a891f..a7e4678d11 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -194,7 +194,7 @@ int cmd_prune(int argc,
if (show_progress == -1)
show_progress = isatty(2);
if (exclude_promisor_objects) {
- fetch_if_missing = 0;
+ repo->fetch_if_missing = 0;
revs.exclude_promisor_objects = 1;
}
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 8f63003709..a6a0c5559e 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -509,25 +509,25 @@ static inline int parse_missing_action_value(const char *value)
if (!strcmp(value, "allow-any")) {
arg_missing_action = MA_ALLOW_ANY;
- fetch_if_missing = 0;
+ the_repository->fetch_if_missing = 0;
return 1;
}
if (!strcmp(value, "print")) {
arg_missing_action = MA_PRINT;
- fetch_if_missing = 0;
+ the_repository->fetch_if_missing = 0;
return 1;
}
if (!strcmp(value, "print-info")) {
arg_missing_action = MA_PRINT_INFO;
- fetch_if_missing = 0;
+ the_repository->fetch_if_missing = 0;
return 1;
}
if (!strcmp(value, "allow-promisor")) {
arg_missing_action = MA_ALLOW_PROMISOR;
- fetch_if_missing = 0;
+ the_repository->fetch_if_missing = 0;
return 1;
}
@@ -745,7 +745,7 @@ int cmd_rev_list(int argc,
for (i = 1; i < argc; i++) {
const char *arg = argv[i];
if (!strcmp(arg, "--exclude-promisor-objects")) {
- fetch_if_missing = 0;
+ the_repository->fetch_if_missing = 0;
revs.exclude_promisor_objects = 1;
} else if (skip_prefix(arg, "--missing=", &arg)) {
parse_missing_action_value(arg);
diff --git a/git.c b/git.c
index 36f08891ef..315d2e160e 100644
--- a/git.c
+++ b/git.c
@@ -202,7 +202,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
if (envchanged)
*envchanged = 1;
} else if (!strcmp(cmd, "--no-lazy-fetch")) {
- fetch_if_missing = 0;
+ the_repository->fetch_if_missing = 0;
setenv(NO_LAZY_FETCH_ENVIRONMENT, "1", 1);
if (envchanged)
*envchanged = 1;
diff --git a/midx-write.c b/midx-write.c
index 19e1cd10b7..e7313c9d2c 100644
--- a/midx-write.c
+++ b/midx-write.c
@@ -865,7 +865,7 @@ static void find_commits_for_midx_bitmap(struct commit_stack *commits,
* complain later that we don't have reachability closure (and fail
* appropriately).
*/
- fetch_if_missing = 0;
+ ctx->repo->fetch_if_missing = 0;
revs.exclude_promisor_objects = 1;
if (prepare_revision_walk(&revs))
diff --git a/odb.c b/odb.c
index 965ef68e4e..664256e1a4 100644
--- a/odb.c
+++ b/odb.c
@@ -528,8 +528,6 @@ void disable_obj_read_lock(void)
pthread_mutex_destroy(&obj_read_mutex);
}
-int fetch_if_missing = 1;
-
static int register_all_submodule_sources(struct object_database *odb)
{
int ret = odb->submodule_source_paths.nr;
@@ -595,7 +593,7 @@ static int do_oid_object_info_extended(struct object_database *odb,
continue;
/* Check if it is a missing object */
- if (fetch_if_missing && repo_has_promisor_remote(odb->repo) &&
+ if (odb->repo->fetch_if_missing && repo_has_promisor_remote(odb->repo) &&
!already_retried &&
!(flags & OBJECT_INFO_SKIP_FETCH_OBJECT)) {
promisor_remote_get_direct(odb->repo, real, 1);
diff --git a/odb.h b/odb.h
index 0030467a52..1dca583fcb 100644
--- a/odb.h
+++ b/odb.h
@@ -14,14 +14,6 @@ struct repository;
struct strbuf;
struct strvec;
-/*
- * Set this to 0 to prevent odb_read_object_info_extended() from fetching missing
- * blobs. This has a difference only if extensions.partialClone is set.
- *
- * Its default value is 1.
- */
-extern int fetch_if_missing;
-
/*
* Compute the exact path an alternate is at and returns it. In case of
* error NULL is returned and the human readable error is added to `err`
diff --git a/repository.c b/repository.c
index 187dd471c4..b959f7a028 100644
--- a/repository.c
+++ b/repository.c
@@ -73,6 +73,7 @@ void initialize_repository(struct repository *repo)
ALLOC_ARRAY(repo->index, 1);
index_state_init(repo->index, repo);
repo->check_deprecated_config = true;
+ repo->fetch_if_missing = 1;
repo_config_values_init(&repo->config_values_private_);
/*
diff --git a/repository.h b/repository.h
index 36e2db2633..e8bd6ef0e7 100644
--- a/repository.h
+++ b/repository.h
@@ -169,6 +169,12 @@ struct repository {
/* True if commit-graph has been disabled within this process. */
int commit_graph_disabled;
+ /*
+ * Controls whether the repository should lazily fetch missing
+ * objects from promisor remotes. Defaults to 1.
+ */
+ int fetch_if_missing;
+
/*
* Lazily-populated cache mapping hook event names to configured hooks.
* NULL until first hook use.
diff --git a/revision.c b/revision.c
index e91d7e1f11..5f70aa81e6 100644
--- a/revision.c
+++ b/revision.c
@@ -2714,7 +2714,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
revs->ignore_missing = 1;
} else if (opt && opt->allow_exclude_promisor_objects &&
!strcmp(arg, "--exclude-promisor-objects")) {
- if (fetch_if_missing)
+ if (the_repository->fetch_if_missing)
BUG("exclude_promisor_objects can only be used when fetch_if_missing is 0");
revs->exclude_promisor_objects = 1;
} else {
diff --git a/setup.c b/setup.c
index b4652651df..ce2a80ac31 100644
--- a/setup.c
+++ b/setup.c
@@ -1064,7 +1064,7 @@ static void setup_git_env_internal(struct repository *repo,
set_alternate_shallow_file(repo, shallow_file, 0);
if (git_env_bool(NO_LAZY_FETCH_ENVIRONMENT, 0))
- fetch_if_missing = 0;
+ the_repository->fetch_if_missing = 0;
}
static void set_git_dir_1(struct repository *repo, const char *path)
--
2.43.0
^ permalink raw reply related
* [PATCH 1/5] compat/posix: introduce writev(3p) wrapper
From: Patrick Steinhardt @ 2026-07-16 7:52 UTC (permalink / raw)
To: git
Cc: Ben Knoble, Junio C Hamano, Jeff King, brian m. carlson,
Randall S. Becker, Phillip Wood, Johannes Schindelin
In-Reply-To: <20260716-pks-reintroduce-writev-v1-0-ea9038c884bc@pks.im>
In a subsequent commit we're going to add the first caller to
writev(3p). Introduce a compatibility wrapper for this syscall that we
can use on systems that don't have this syscall.
The syscall exists on modern Unixes like Linux and macOS, and seemingly
even for NonStop according to [1]. It doesn't seem to exist on Windows
though.
[1]: http://nonstoptools.com/manuals/OSS-SystemCalls.pdf
[2]: https://www.gnu.org/software/gnulib/manual/html_node/writev.html
Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Makefile | 4 ++++
compat/posix.h | 14 ++++++++++++
compat/writev.c | 44 +++++++++++++++++++++++++++++++++++++
config.mak.uname | 2 ++
contrib/buildsystems/CMakeLists.txt | 6 ++++-
meson.build | 1 +
6 files changed, 70 insertions(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index 1f3f099f5c..eda5ecc5b4 100644
--- a/Makefile
+++ b/Makefile
@@ -2033,6 +2033,10 @@ ifdef NO_PREAD
COMPAT_CFLAGS += -DNO_PREAD
COMPAT_OBJS += compat/pread.o
endif
+ifdef NO_WRITEV
+ COMPAT_CFLAGS += -DNO_WRITEV
+ COMPAT_OBJS += compat/writev.o
+endif
ifdef NO_FAST_WORKING_DIRECTORY
BASIC_CFLAGS += -DNO_FAST_WORKING_DIRECTORY
endif
diff --git a/compat/posix.h b/compat/posix.h
index e2e794cad7..71cc731620 100644
--- a/compat/posix.h
+++ b/compat/posix.h
@@ -148,6 +148,9 @@
#include <sys/socket.h>
#include <sys/ioctl.h>
#include <sys/statvfs.h>
+#ifndef NO_WRITEV
+#include <sys/uio.h>
+#endif
#include <termios.h>
#ifndef NO_SYS_SELECT_H
#include <sys/select.h>
@@ -334,6 +337,17 @@ int git_lstat(const char *, struct stat *);
ssize_t git_pread(int fd, void *buf, size_t count, off_t offset);
#endif
+#ifdef NO_WRITEV
+#define writev git_writev
+#define iovec git_iovec
+struct git_iovec {
+ void *iov_base;
+ size_t iov_len;
+};
+
+ssize_t git_writev(int fd, const struct iovec *iov, int iovcnt);
+#endif
+
#ifdef NO_SETENV
#define setenv gitsetenv
int gitsetenv(const char *, const char *, int);
diff --git a/compat/writev.c b/compat/writev.c
new file mode 100644
index 0000000000..ab2e223634
--- /dev/null
+++ b/compat/writev.c
@@ -0,0 +1,44 @@
+#include "../git-compat-util.h"
+#include "../wrapper.h"
+
+ssize_t git_writev(int fd, const struct iovec *iov, int iovcnt)
+{
+ size_t total_written = 0;
+ size_t sum = 0;
+
+ /*
+ * According to writev(3p), the syscall shall error with EINVAL in case
+ * the sum of `iov_len` overflows `ssize_t`.
+ */
+ for (int i = 0; i < iovcnt; i++) {
+ if (iov[i].iov_len > maximum_signed_value_of_type(ssize_t) ||
+ iov[i].iov_len + sum > maximum_signed_value_of_type(ssize_t)) {
+ errno = EINVAL;
+ return -1;
+ }
+
+ sum += iov[i].iov_len;
+ }
+
+ for (int i = 0; i < iovcnt; i++) {
+ const char *bytes = iov[i].iov_base;
+ size_t iovec_written = 0;
+
+ while (iovec_written < iov[i].iov_len) {
+ ssize_t bytes_written = xwrite(fd, bytes + iovec_written,
+ iov[i].iov_len - iovec_written);
+ if (bytes_written < 0) {
+ if (total_written)
+ goto out;
+ return bytes_written;
+ }
+ if (!bytes_written)
+ goto out;
+ iovec_written += bytes_written;
+ total_written += bytes_written;
+ }
+ }
+
+out:
+ return (ssize_t) total_written;
+}
diff --git a/config.mak.uname b/config.mak.uname
index 9ebd240378..95ef6e64dc 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -483,6 +483,7 @@ ifeq ($(uname_S),Windows)
SANE_TOOL_PATH ?= $(msvc_bin_dir_msys)
HAVE_ALLOCA_H = YesPlease
NO_PREAD = YesPlease
+ NO_WRITEV = YesPlease
NEEDS_CRYPTO_WITH_SSL = YesPlease
NO_LIBGEN_H = YesPlease
NO_POLL = YesPlease
@@ -697,6 +698,7 @@ ifeq ($(uname_S),MINGW)
pathsep = ;
HAVE_ALLOCA_H = YesPlease
NO_PREAD = YesPlease
+ NO_WRITEV = YesPlease
NEEDS_CRYPTO_WITH_SSL = YesPlease
NO_LIBGEN_H = YesPlease
NO_POLL = YesPlease
diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
index a57c4b464f..8f56203f34 100644
--- a/contrib/buildsystems/CMakeLists.txt
+++ b/contrib/buildsystems/CMakeLists.txt
@@ -378,7 +378,7 @@ endif()
#function checks
set(function_checks
strcasestr memmem strlcpy strtoimax strtoumax strtoull
- setenv mkdtemp poll pread memmem)
+ setenv mkdtemp poll pread memmem writev)
#unsetenv,hstrerror are incompatible with windows build
if(NOT WIN32)
@@ -423,6 +423,10 @@ if(NOT HAVE_MEMMEM)
list(APPEND compat_SOURCES compat/memmem.c)
endif()
+if(NOT HAVE_WRITEV)
+ list(APPEND compat_SOURCES compat/writev.c)
+endif()
+
if(NOT WIN32)
if(NOT HAVE_UNSETENV)
list(APPEND compat_SOURCES compat/unsetenv.c)
diff --git a/meson.build b/meson.build
index ca235801cf..613828ff25 100644
--- a/meson.build
+++ b/meson.build
@@ -1446,6 +1446,7 @@ checkfuncs = {
'initgroups' : [],
'strtoumax' : ['strtoumax.c', 'strtoimax.c'],
'pread' : ['pread.c'],
+ 'writev' : ['writev.c'],
}
if host_machine.system() == 'windows'
--
2.55.0.313.g8d093f411d.dirty
^ permalink raw reply related
* [PATCH 0/5] Reintroduce writev(3p)
From: Patrick Steinhardt @ 2026-07-16 7:52 UTC (permalink / raw)
To: git
Cc: Ben Knoble, Junio C Hamano, Jeff King, brian m. carlson,
Randall S. Becker, Phillip Wood, Johannes Schindelin
Hi,
this patch series reintroduces the writev(3p) wrapper. This wrapper was
originally introduced as part of Git 2.54 [1], but was ejected due to
issues on NonStop [2].
This patch series here revives the effort with a couple of fixes on top:
- It picks Dscho's fix for CMake [3].
- It picks a fix for NonStop [4] and polishes it a bit.
- It adapts one more site to demonstrate that its usefulness is not
limited to a single callsite, only.
Furthermore, I have included benchmarks now that demonstrate the
benefits to make this series a bit more appealing. Ultimately, I'd be
fine if we say we rather don't want to go this way though. I merely
wanted to tie some loose ends that I left dangling.
That, and it's nice to not work on pluggable object databases once in a
while.
Thanks!
Patrick
[1]: <20260227-pks-upload-pack-write-contention-v1-0-7166fe255704@pks.im>
[2]: <028901dcc859$d2419470$76c4bd50$@nexbridge.com>
[3]: <pull.2078.git.1775206502134.gitgitgadget@gmail.com>
[4]: <20260409-b4-pks-writev-max-io-size-v1-1-81730e8f35df@pks.im>
---
Patrick Steinhardt (5):
compat/posix: introduce writev(3p) wrapper
wrapper: introduce writev(3p) wrappers
wrapper: properly handle MAX_IO_SIZE in writev(3p)
sideband: use writev(3p) to send pktlines
fast-import: use writev(3p) to send cat-blob responses
Makefile | 4 ++
builtin/fast-import.c | 18 +++++++--
compat/posix.h | 14 +++++++
compat/writev.c | 44 +++++++++++++++++++++
config.mak.uname | 2 +
contrib/buildsystems/CMakeLists.txt | 6 ++-
meson.build | 1 +
sideband.c | 14 +++++--
wrapper.c | 78 +++++++++++++++++++++++++++++++++++++
wrapper.h | 10 +++++
write-or-die.c | 8 ++++
write-or-die.h | 1 +
12 files changed, 193 insertions(+), 7 deletions(-)
---
base-commit: 55526a18268bbc1ddaf8a6b7850c33d984eac9e9
change-id: 20260714-pks-reintroduce-writev-2d8f7e52eee9
^ permalink raw reply
* [PATCH 2/5] wrapper: introduce writev(3p) wrappers
From: Patrick Steinhardt @ 2026-07-16 7:52 UTC (permalink / raw)
To: git
Cc: Ben Knoble, Junio C Hamano, Jeff King, brian m. carlson,
Randall S. Becker, Phillip Wood, Johannes Schindelin
In-Reply-To: <20260716-pks-reintroduce-writev-v1-0-ea9038c884bc@pks.im>
In the preceding commit we have added a compatibility wrapper for the
writev(3p) syscall. Introduce some generic wrappers for this function
that we nowadays take for granted in the Git codebase.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
wrapper.c | 41 +++++++++++++++++++++++++++++++++++++++++
wrapper.h | 9 +++++++++
write-or-die.c | 8 ++++++++
write-or-die.h | 1 +
4 files changed, 59 insertions(+)
diff --git a/wrapper.c b/wrapper.c
index 16f5a63fbb..be8fa575e6 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -323,6 +323,47 @@ ssize_t write_in_full(int fd, const void *buf, size_t count)
return total;
}
+ssize_t writev_in_full(int fd, struct iovec *iov, int iovcnt)
+{
+ ssize_t total_written = 0;
+
+ while (iovcnt) {
+ ssize_t bytes_written = writev(fd, iov, iovcnt);
+ if (bytes_written < 0) {
+ if (errno == EINTR || errno == EAGAIN)
+ continue;
+ return -1;
+ }
+ if (!bytes_written) {
+ errno = ENOSPC;
+ return -1;
+ }
+
+ total_written += bytes_written;
+
+ /*
+ * We first need to discard any iovec entities that have been
+ * fully written.
+ */
+ while (iovcnt && (size_t)bytes_written >= iov->iov_len) {
+ bytes_written -= iov->iov_len;
+ iov++;
+ iovcnt--;
+ }
+
+ /*
+ * Finally, we need to adjust the last iovec in case we have
+ * performed a partial write.
+ */
+ if (iovcnt && bytes_written) {
+ iov->iov_base = (char *) iov->iov_base + bytes_written;
+ iov->iov_len -= bytes_written;
+ }
+ }
+
+ return total_written;
+}
+
ssize_t pread_in_full(int fd, void *buf, size_t count, off_t offset)
{
char *p = buf;
diff --git a/wrapper.h b/wrapper.h
index 15ac3bab6e..27519b32d1 100644
--- a/wrapper.h
+++ b/wrapper.h
@@ -47,6 +47,15 @@ ssize_t read_in_full(int fd, void *buf, size_t count);
ssize_t write_in_full(int fd, const void *buf, size_t count);
ssize_t pread_in_full(int fd, void *buf, size_t count, off_t offset);
+/*
+ * Try to write all iovecs. Returns -1 in case an error occurred with a proper
+ * errno set, the number of bytes written otherwise.
+ *
+ * Note that the iovec will be modified as a result of this call to adjust for
+ * partial writes!
+ */
+ssize_t writev_in_full(int fd, struct iovec *iov, int iovcnt);
+
static inline ssize_t write_str_in_full(int fd, const char *str)
{
return write_in_full(fd, str, strlen(str));
diff --git a/write-or-die.c b/write-or-die.c
index 01a9a51fa2..5f522fb728 100644
--- a/write-or-die.c
+++ b/write-or-die.c
@@ -96,6 +96,14 @@ void write_or_die(int fd, const void *buf, size_t count)
}
}
+void writev_or_die(int fd, struct iovec *iov, int iovlen)
+{
+ if (writev_in_full(fd, iov, iovlen) < 0) {
+ check_pipe(errno);
+ die_errno("writev error");
+ }
+}
+
void fwrite_or_die(FILE *f, const void *buf, size_t count)
{
if (fwrite(buf, 1, count, f) != count)
diff --git a/write-or-die.h b/write-or-die.h
index ff0408bd84..a045bdfaef 100644
--- a/write-or-die.h
+++ b/write-or-die.h
@@ -7,6 +7,7 @@ void fprintf_or_die(FILE *, const char *fmt, ...);
void fwrite_or_die(FILE *f, const void *buf, size_t count);
void fflush_or_die(FILE *f);
void write_or_die(int fd, const void *buf, size_t count);
+void writev_or_die(int fd, struct iovec *iov, int iovlen);
/*
* These values are used to help identify parts of a repository to fsync.
--
2.55.0.313.g8d093f411d.dirty
^ permalink raw reply related
* [PATCH 3/5] wrapper: properly handle MAX_IO_SIZE in writev(3p)
From: Patrick Steinhardt @ 2026-07-16 7:52 UTC (permalink / raw)
To: git
Cc: Ben Knoble, Junio C Hamano, Jeff King, brian m. carlson,
Randall S. Becker, Phillip Wood, Johannes Schindelin
In-Reply-To: <20260716-pks-reintroduce-writev-v1-0-ea9038c884bc@pks.im>
Some systems like NonStop set a comparatively small `MAX_IO_SIZE`, which
limits the maximum number of bytes we're allowed to write in a single
call. We already handle this limit properly in `xwrite()`, but we have
recently introduced wrappers for writev(3p) where we don't. This will
cause the syscall to return EINVAL in case somebody passes an iovec
entry to writev(3p) that is larger than `MAX_IO_SIZE`.
Introduce a new function `xwritev()` that is similar to `xwrite()` in
that it handles such platform-specific nuances:
- We only pass the leading iovec entries to writev(3p) that fit into
`MAX_IO_SIZE`, pretending that the underlying syscall performed a
short write. This mirrors how `xwrite()` chomps overly large
requests before handing them to write(3p). As a consequence, callers
will never see writev(3p)'s EINVAL error for requests whose summed
length would overflow an ssize_t, but observe a short write instead.
- If already the first iovec entry exceeds the limit we instead punt
to `xwrite()`, which knows to handle this case for us.
- We restart the underlying syscall on EINTR and EAGAIN, just like
`xwrite()` does for write(3p).
Adapt `writev_in_full()` to use this new wrapper. With the retry logic
now living in `xwritev()`, the calling loop becomes the exact mirror
image of `write_in_full()`, which also retains the responsibility of
translating a zero-length write into ENOSPC.
Reported-by: Randall Becker <randall.becker@nexbridge.ca>
Helped-by: Jeff King <peff@peff.net>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
wrapper.c | 47 ++++++++++++++++++++++++++++++++++++++++++-----
wrapper.h | 1 +
2 files changed, 43 insertions(+), 5 deletions(-)
diff --git a/wrapper.c b/wrapper.c
index be8fa575e6..561f9ee9c9 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -323,17 +323,54 @@ ssize_t write_in_full(int fd, const void *buf, size_t count)
return total;
}
+ssize_t xwritev(int fd, struct iovec *iov, int iovcnt)
+{
+ size_t allowed = MAX_IO_SIZE;
+ int i;
+
+ /*
+ * Some platforms define a comparatively small `MAX_IO_SIZE` that
+ * limits how many bytes can be written with a single call to
+ * write(3p) or writev(3p); exceeding that limit causes the syscall to
+ * fail with EINVAL. Just like xwrite() chomps overly large requests
+ * for write(3p), pretend that the underlying writev(3p) performed a
+ * short write by only passing along the leading iovec entries that
+ * fit into that limit.
+ */
+ for (i = 0; i < iovcnt; i++) {
+ if (iov[i].iov_len > allowed) {
+ /*
+ * If the first buffer is larger than MAX_IO_SIZE,
+ * let xwrite() deal with it.
+ */
+ if (!i)
+ return xwrite(fd, iov->iov_base, iov->iov_len);
+ break;
+ }
+ allowed -= iov[i].iov_len;
+ }
+
+ while (1) {
+ ssize_t bytes_written = writev(fd, iov, i);
+ if (bytes_written < 0) {
+ if (errno == EINTR)
+ continue;
+ if (handle_nonblock(fd, POLLOUT, errno))
+ continue;
+ }
+
+ return bytes_written;
+ }
+}
+
ssize_t writev_in_full(int fd, struct iovec *iov, int iovcnt)
{
ssize_t total_written = 0;
while (iovcnt) {
- ssize_t bytes_written = writev(fd, iov, iovcnt);
- if (bytes_written < 0) {
- if (errno == EINTR || errno == EAGAIN)
- continue;
+ ssize_t bytes_written = xwritev(fd, iov, iovcnt);
+ if (bytes_written < 0)
return -1;
- }
if (!bytes_written) {
errno = ENOSPC;
return -1;
diff --git a/wrapper.h b/wrapper.h
index 27519b32d1..a6287d7f4d 100644
--- a/wrapper.h
+++ b/wrapper.h
@@ -16,6 +16,7 @@ void *xmmap_gently(void *start, size_t length, int prot, int flags, int fd, off_
int xopen(const char *path, int flags, ...);
ssize_t xread(int fd, void *buf, size_t len);
ssize_t xwrite(int fd, const void *buf, size_t len);
+ssize_t xwritev(int fd, struct iovec *iov, int iovcnt);
ssize_t xpread(int fd, void *buf, size_t len, off_t offset);
int xdup(int fd);
FILE *xfopen(const char *path, const char *mode);
--
2.55.0.313.g8d093f411d.dirty
^ permalink raw reply related
* [PATCH 4/5] sideband: use writev(3p) to send pktlines
From: Patrick Steinhardt @ 2026-07-16 7:52 UTC (permalink / raw)
To: git
Cc: Ben Knoble, Junio C Hamano, Jeff King, brian m. carlson,
Randall S. Becker, Phillip Wood, Johannes Schindelin
In-Reply-To: <20260716-pks-reintroduce-writev-v1-0-ea9038c884bc@pks.im>
Every pktline that we send out via `send_sideband()` currently requires
two syscalls: one to write the pktline's length, and one to send its
data. This typically isn't all that much of a problem, but under extreme
load the syscalls may cause contention in the kernel.
Refactor the code to instead use the newly introduced writev(3p) infra
so that we can send out the data with a single syscall. This reduces the
number of syscalls from around 133,000 calls to write(3p) to around
67,000 calls to writev(3p).
This change leads to a performance improvement for git-upload-pack(1),
but we have to cheat a bit to really make it measurable. Usually, the
time is strongly dominated by generating the packfile itself. But if we
precompute the pack and serve it via the pack-objects hook then we can
essentially eliminate that overhead. The following setup is executed in
the Git repository:
$ cat >request <<-EOF
0048want 5ce91c059e41090e7d2cffad39c04af8acf98dc1 side-band no-progress
00000009done
EOF
$ echo 5ce91c059e41090e7d2cffad39c04af8acf98dc1 | git pack-objects --revs --stdout >pack
$ cat >hook <<-EOF
#!/bin/sh
cat >/dev/null
cat "$(pwd)"/pack
EOF
$ chmod u+x hook
$ git -c uploadpack.packObjectsHook="$(pwd)"/hook upload-pack . <request
Benchmarking the last command leads to the following results:
Benchmark 1: HEAD~
Time (mean ± σ): 192.9 ms ± 0.6 ms [User: 106.5 ms, System: 95.3 ms]
Range (min … max): 191.7 ms … 194.1 ms 50 runs
Benchmark 2: HEAD
Time (mean ± σ): 141.1 ms ± 0.7 ms [User: 63.2 ms, System: 86.6 ms]
Range (min … max): 139.8 ms … 142.7 ms 50 runs
Summary
HEAD ran
1.37 ± 0.01 times faster than HEAD~
This might not be impressive in absolute numbers when you also take into
account the time it takes to generate the packfile itself. But GitLab
(and supposedly other forges) have caching mechanisms in place that work
exactly like the above setup, where repeated incoming requests can be
served from the same cached packfile. And in those cases, the impact is
sizeable.
More importantly though, as hinted at above, GitLab has observed in the
past that with enough cache hits we eventually start to saturate a
semaphore in the Linux kernel itself in the pipe write path. This
bottleneck is being moved a bit by having to do less syscalls.
Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
sideband.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/sideband.c b/sideband.c
index 1523a53e1d..94e5b56172 100644
--- a/sideband.c
+++ b/sideband.c
@@ -441,6 +441,7 @@ void send_sideband(int fd, int band, const char *data, ssize_t sz, int packet_ma
const char *p = data;
while (sz) {
+ struct iovec iov[2];
unsigned n;
char hdr[5];
@@ -450,12 +451,19 @@ void send_sideband(int fd, int band, const char *data, ssize_t sz, int packet_ma
if (0 <= band) {
xsnprintf(hdr, sizeof(hdr), "%04x", n + 5);
hdr[4] = band;
- write_or_die(fd, hdr, 5);
+ iov[0].iov_base = hdr;
+ iov[0].iov_len = 5;
} else {
xsnprintf(hdr, sizeof(hdr), "%04x", n + 4);
- write_or_die(fd, hdr, 4);
+ iov[0].iov_base = hdr;
+ iov[0].iov_len = 4;
}
- write_or_die(fd, p, n);
+
+ iov[1].iov_base = (void *) p;
+ iov[1].iov_len = n;
+
+ writev_or_die(fd, iov, ARRAY_SIZE(iov));
+
p += n;
sz -= n;
}
--
2.55.0.313.g8d093f411d.dirty
^ permalink raw reply related
* [PATCH 5/5] fast-import: use writev(3p) to send cat-blob responses
From: Patrick Steinhardt @ 2026-07-16 7:52 UTC (permalink / raw)
To: git
Cc: Ben Knoble, Junio C Hamano, Jeff King, brian m. carlson,
Randall S. Becker, Phillip Wood, Johannes Schindelin
In-Reply-To: <20260716-pks-reintroduce-writev-v1-0-ea9038c884bc@pks.im>
When answering a `cat-blob` command, `cat_blob()` issues three separate
calls to write(3p) on the cat-blob fd: one for the header line, one for
the full blob payload, and one for the trailing newline. Frontends like
git-filter-repo issue these commands in bulk, once per rewritten blob,
so the syscall overhead adds up.
Use `writev_in_full()` to send all three parts with a single syscall.
This can be benchmarked with the following setup:
$ git cat-file --unordered --filter=object:type=blob
--batch-check='cat-blob %(objectname)' --batch-all-objects >request
$ git fast-import --cat-blob-fd=3 <request
Executing this with 100,000 objects in linux.git:
Benchmark 1: HEAD~
Time (mean ± σ): 1.320 s ± 0.003 s [User: 1.154 s, System: 0.161 s]
Range (min … max): 1.314 s … 1.324 s 10 runs
Benchmark 2: HEAD
Time (mean ± σ): 1.270 s ± 0.022 s [User: 1.133 s, System: 0.132 s]
Range (min … max): 1.209 s … 1.282 s 10 runs
Summary
HEAD ran
1.04 ± 0.02 times faster than HEAD~
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/fast-import.c | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/builtin/fast-import.c b/builtin/fast-import.c
index aa656c5195..48fda01c94 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -3332,6 +3332,7 @@ static void cat_blob_write(const char *buf, unsigned long size)
static void cat_blob(struct object_entry *oe, struct object_id *oid)
{
struct strbuf line = STRBUF_INIT;
+ struct iovec iov[3];
unsigned long size;
enum object_type type = 0;
char *buf;
@@ -3365,10 +3366,21 @@ static void cat_blob(struct object_entry *oe, struct object_id *oid)
strbuf_reset(&line);
strbuf_addf(&line, "%s %s %"PRIuMAX"\n", oid_to_hex(oid),
type_name(type), (uintmax_t)size);
- cat_blob_write(line.buf, line.len);
+
+ /*
+ * Write the header, the payload and the trailing newline with a
+ * single writev(3p) call instead of three separate write(3p) calls.
+ */
+ iov[0].iov_base = line.buf;
+ iov[0].iov_len = line.len;
+ iov[1].iov_base = buf;
+ iov[1].iov_len = size;
+ iov[2].iov_base = (void *) "\n";
+ iov[2].iov_len = 1;
+
+ if (writev_in_full(cat_blob_fd, iov, ARRAY_SIZE(iov)) < 0)
+ die_errno(_("write to frontend failed"));
strbuf_release(&line);
- cat_blob_write(buf, size);
- cat_blob_write("\n", 1);
if (oe && oe->pack_id == pack_id) {
last_blob.offset = oe->idx.offset;
strbuf_attach(&last_blob.data, buf, size, size + 1);
--
2.55.0.313.g8d093f411d.dirty
^ permalink raw reply related
* [PATCH] stash: add 'rename' subcommand
From: Emin Özata via GitGitGadget @ 2026-07-16 8:31 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Greg Hewgill, Micheil Smith, Michael Haggerty,
Ævar Arnfjörð Bjarmason, Emin Özata,
Emin Özata
From: =?UTF-8?q?Emin=20=C3=96zata?= <eminozata@proton.me>
There is no way to change the message of a stash entry after the
fact. The only option is dropping the entry and re-storing it by
hand, which moves it to the top of the stash list and gets fiddly
for deeper entries.
Add 'git stash rename <message> [<stash>]', defaulting to the
latest entry like the other subcommands do. It reads the object id
and reflog message of the target entry and of the entries above it,
drops them all like 'git stash drop' would, and stores them back in
the same order, with the new message going to the target. Position,
contents and the reflog chain stay as they were.
The command checks every entry it is about to rewrite and refuses
to start if one of them does not look like a stash commit, which
can only happen when refs/stash was written to by hand. Finding
that out halfway through the sequence would lose entries. Should a
write-back fail anyway, the entry's object id is reported so it can
be recovered with 'git stash store', and the command only reports
success when the reflog ended up in the requested state.
This was proposed before: in 2010, as a "git reflog update" command
that edited reflog entries in place [1]. When it came up again in
2013 [2], Junio rejected it on the grounds that reflogs are
append-only recovery logs, and that whoever really cares about a
stash message can pop and re-stash [3]. Michael Haggerty pointed
out in that thread that refs/stash does not fit the description:
its reflog is the primary data store for stash entries, and 'git
stash drop' rewrites it all the time [4]. So this patch stays away
from the reflog machinery entirely and does the suggested
pop-and-re-stash workaround mechanically, without the detour
through the working tree.
The sequence only works if entry positions hold still while it
runs, so the command takes index-based selectors (stash@{1}) and
rejects time-based ones. It also refreshes the reflog timestamps
of the rewritten entries, and renaming stash@{n} costs n+1 reflog
deletions and ref updates.
[1] https://lore.kernel.org/git/20100620093142.GF24805@occam.hewgill.net/
[2] https://lore.kernel.org/git/loom.20130104T192132-16@post.gmane.org/
[3] https://lore.kernel.org/git/7vbod4tynt.fsf@alter.siamese.dyndns.org/
[4] https://lore.kernel.org/git/50ED2C78.1030300@alum.mit.edu/
Signed-off-by: Emin Özata <eminozata@proton.me>
---
stash: add 'rename' subcommand
eo/stash-rename
"git stash rename" learned to change the message of an existing stash
entry without changing its position or its contents.
This came up in 2010 and again in 2013, and was rejected back then
because the proposed implementation rewrote reflog entries in place.
This version doesn't: it does the drop-and-re-store dance that was
suggested as the manual workaround, with the machinery stash already
uses for drop and store, and touches nothing but refs/stash. Details and
links to the old threads are in the commit message.
Costs, so nobody has to dig for them: rewritten entries get fresh reflog
timestamps (hence index-only selectors), and renaming stash@{n} does n+1
reflog deletions, each of them a locked rewrite of the whole reflog. The
sequence is not atomic either: a failure halfway is handled by writing
the collected entries back best-effort, whatever cannot be written back
is reported with its object id so "git stash store" can recover it, and
a process killed between the drop and store phases loses the collected
entries (git fsck still finds them). A single refs_reflog_expire() pass
would cut both the I/O and that window down, and closing the window for
real needs a new refs API operation; I'd rather do either as a follow-up
if the feature is wanted at all.
I picked a positional <message> over -m <message> ("stash store" style);
no strong opinion, happy to switch.
t3903 passes with GIT_TEST_DEFAULT_REF_FORMAT=files and reftable.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2180%2Fozemin%2Fstash-rename-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2180/ozemin/stash-rename-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2180
Documentation/git-stash.adoc | 11 +-
builtin/stash.c | 197 +++++++++++++++++++++++--
contrib/completion/git-completion.bash | 4 +-
t/t3903-stash.sh | 79 ++++++++++
4 files changed, 276 insertions(+), 15 deletions(-)
diff --git a/Documentation/git-stash.adoc b/Documentation/git-stash.adoc
index 50bb89f483..03f2e03096 100644
--- a/Documentation/git-stash.adoc
+++ b/Documentation/git-stash.adoc
@@ -25,6 +25,7 @@ git stash create [<message>]
git stash store [(-m | --message) <message>] [-q | --quiet] <commit>
git stash export (--print | --to-ref <ref>) [<stash>...]
git stash import <commit>
+git stash rename [-q | --quiet] <message> [<stash>]
DESCRIPTION
-----------
@@ -163,6 +164,12 @@ with no conflicts.
created by `export`, and add them to the list of stashes. To replace the
existing stashes, use `clear` first.
+`rename [-q | --quiet] <message> [<stash>]`::
+ Change the message of a single stash entry. The entry keeps its
+ position and its contents. _<stash>_ must name an entry by
+ index (e.g. `stash@{1}`); renaming refreshes the reflog
+ timestamps of the entry and of the entries above it.
+
OPTIONS
-------
`-a`::
@@ -258,7 +265,7 @@ literally (including newlines and quotes).
`-q`::
`--quiet`::
This option is only valid for `apply`, `drop`, `pop`, `push`,
- `save`, `store` commands.
+ `rename`, `save`, `store` commands.
+
Quiet, suppress feedback messages.
@@ -292,7 +299,7 @@ For more details, see the 'pathspec' entry in linkgit:gitglossary[7].
_<stash>_::
This option is only valid for `apply`, `branch`, `drop`, `pop`,
- `show`, and `export` commands.
+ `show`, `export`, and `rename` commands.
+
A reference of the form `stash@{<revision>}`. When no _<stash>_ is
given, the latest stash is assumed (that is, `stash@{0}`).
diff --git a/builtin/stash.c b/builtin/stash.c
index c4809f299a..94e66d6074 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -63,6 +63,8 @@
N_("git stash export (--print | --to-ref <ref>) [<stash>...]")
#define BUILTIN_STASH_IMPORT_USAGE \
N_("git stash import <commit>")
+#define BUILTIN_STASH_RENAME_USAGE \
+ N_("git stash rename [-q | --quiet] <message> [<stash>]")
#define BUILTIN_STASH_CLEAR_USAGE \
"git stash clear"
@@ -80,6 +82,7 @@ static const char * const git_stash_usage[] = {
BUILTIN_STASH_STORE_USAGE,
BUILTIN_STASH_EXPORT_USAGE,
BUILTIN_STASH_IMPORT_USAGE,
+ BUILTIN_STASH_RENAME_USAGE,
NULL
};
@@ -143,6 +146,11 @@ static const char * const git_stash_import_usage[] = {
NULL
};
+static const char * const git_stash_rename_usage[] = {
+ BUILTIN_STASH_RENAME_USAGE,
+ NULL
+};
+
static const char ref_stash[] = "refs/stash";
static struct strbuf stash_index_path = STRBUF_INIT;
@@ -820,18 +828,12 @@ static int reflog_is_empty(const char *refname)
refname, reject_reflog_ent, NULL);
}
-static int do_drop_stash(struct stash_info *info, int quiet)
+static int drop_reflog_entry(const char *revision)
{
- if (!reflog_delete(info->revision.buf,
- EXPIRE_REFLOGS_REWRITE | EXPIRE_REFLOGS_UPDATE_REF,
- 0)) {
- if (!quiet)
- printf_ln(_("Dropped %s (%s)"), info->revision.buf,
- oid_to_hex(&info->w_commit));
- } else {
- return error(_("%s: Could not drop stash entry"),
- info->revision.buf);
- }
+ if (reflog_delete(revision,
+ EXPIRE_REFLOGS_REWRITE | EXPIRE_REFLOGS_UPDATE_REF,
+ 0))
+ return error(_("%s: Could not drop stash entry"), revision);
if (reflog_is_empty(ref_stash))
do_clear_stash();
@@ -839,6 +841,18 @@ static int do_drop_stash(struct stash_info *info, int quiet)
return 0;
}
+static int do_drop_stash(struct stash_info *info, int quiet)
+{
+ if (drop_reflog_entry(info->revision.buf))
+ return -1;
+
+ if (!quiet)
+ printf_ln(_("Dropped %s (%s)"), info->revision.buf,
+ oid_to_hex(&info->w_commit));
+
+ return 0;
+}
+
static int get_stash_info_assert(struct stash_info *info, int argc,
const char **argv)
{
@@ -1190,6 +1204,166 @@ out:
return ret;
}
+struct rename_entry {
+ struct object_id oid;
+ char *msg;
+};
+
+struct rename_data {
+ struct rename_entry *entries;
+ size_t nr, alloc;
+ size_t want;
+};
+
+static int collect_rename_entries(const char *refname UNUSED,
+ struct object_id *old_oid UNUSED,
+ struct object_id *new_oid,
+ const char *committer UNUSED,
+ timestamp_t timestamp UNUSED,
+ int tz UNUSED, const char *msg,
+ void *cb_data)
+{
+ struct rename_data *data = cb_data;
+ const char *eol = strchrnul(msg, '\n');
+
+ ALLOC_GROW(data->entries, data->nr + 1, data->alloc);
+ oidcpy(&data->entries[data->nr].oid, new_oid);
+ data->entries[data->nr].msg = xstrndup(msg, eol - msg);
+ data->nr++;
+
+ return data->nr >= data->want;
+}
+
+static int parse_stash_index(const char *revision, size_t *idx)
+{
+ const char *num = strstr(revision, "@{");
+ char *end;
+
+ if (!num || !isdigit(num[2]))
+ return -1;
+ *idx = strtoumax(num + 2, &end, 10);
+ if (*end != '}' || end[1])
+ return -1;
+
+ return 0;
+}
+
+static int store_rename_entry(struct rename_entry *entry, const char *msg)
+{
+ if (!do_store_stash(&entry->oid, msg, 1))
+ return 0;
+ warning(_("could not restore stash entry %s; "
+ "recover it with 'git stash store %s'"),
+ oid_to_hex(&entry->oid), oid_to_hex(&entry->oid));
+ return -1;
+}
+
+static int do_rename_stash(struct stash_info *info, size_t idx,
+ const char *msg, int quiet)
+{
+ struct rename_data data = { .want = idx + 1 };
+ size_t i, missing = 0;
+ int ret = -1;
+
+ refs_for_each_reflog_ent_reverse(get_main_ref_store(the_repository),
+ ref_stash, collect_rename_entries,
+ &data);
+ if (data.nr <= idx) {
+ error(_("%s does not exist"), info->revision.buf);
+ goto cleanup;
+ }
+
+ if (!oideq(&info->w_commit, &data.entries[idx].oid)) {
+ error(_("%s changed concurrently; try again"),
+ info->revision.buf);
+ goto cleanup;
+ }
+
+ /* refuse up front; do_store_stash() would die halfway through */
+ for (i = 0; i < data.nr; i++) {
+ struct commit *stash = lookup_commit_reference(the_repository,
+ &data.entries[i].oid);
+
+ if (!stash || check_stash_topology(the_repository, stash)) {
+ error(_("%s does not look like a stash commit"),
+ oid_to_hex(&data.entries[i].oid));
+ goto cleanup;
+ }
+ }
+
+ while (missing <= idx) {
+ if (drop_reflog_entry("stash@{0}"))
+ goto restore;
+ missing++;
+ }
+
+ ret = 0;
+ while (missing) {
+ i = missing - 1;
+ if (store_rename_entry(&data.entries[i],
+ i == idx ? msg : data.entries[i].msg))
+ ret = -1;
+ missing--;
+ }
+
+ if (!ret && !quiet)
+ printf_ln(_("Renamed %s (%s)"), info->revision.buf,
+ oid_to_hex(&data.entries[idx].oid));
+ goto cleanup;
+
+restore:
+ /* dropping failed midway; put the dropped entries back */
+ while (missing) {
+ store_rename_entry(&data.entries[missing - 1],
+ data.entries[missing - 1].msg);
+ missing--;
+ }
+cleanup:
+ for (i = 0; i < data.nr; i++)
+ free(data.entries[i].msg);
+ free(data.entries);
+ return ret;
+}
+
+static int rename_stash(int argc, const char **argv, const char *prefix,
+ struct repository *repo UNUSED)
+{
+ int ret = -1;
+ int quiet = 0;
+ size_t idx;
+ struct stash_info info = STASH_INFO_INIT;
+ struct option options[] = {
+ OPT__QUIET(&quiet, N_("be quiet, only report errors")),
+ OPT_END()
+ };
+
+ argc = parse_options(argc, argv, prefix, options,
+ git_stash_rename_usage, 0);
+
+ if (!argc)
+ usage_with_options(git_stash_rename_usage, options);
+
+ if (!argv[0][strspn(argv[0], " \t\r\n")]) {
+ ret = error(_("stash message cannot be empty"));
+ goto cleanup;
+ }
+
+ if (get_stash_info_assert(&info, argc - 1, argv + 1))
+ goto cleanup;
+
+ /* positions must stay stable across the drop-and-store sequence */
+ if (parse_stash_index(info.revision.buf, &idx)) {
+ error(_("cannot rename '%s': name the entry by index, "
+ "like 'stash@{1}'"), info.revision.buf);
+ goto cleanup;
+ }
+
+ ret = do_rename_stash(&info, idx, argv[0], quiet);
+cleanup:
+ free_stash_info(&info);
+ return ret;
+}
+
static void add_pathspecs(struct strvec *args,
const struct pathspec *ps) {
int i;
@@ -2472,6 +2646,7 @@ int cmd_stash(int argc,
OPT_SUBCOMMAND("push", &fn, push_stash_unassumed),
OPT_SUBCOMMAND("export", &fn, export_stash),
OPT_SUBCOMMAND("import", &fn, import_stash),
+ OPT_SUBCOMMAND("rename", &fn, rename_stash),
OPT_SUBCOMMAND_F("save", &fn, save_stash, PARSE_OPT_NOCOMPLETE),
OPT_END()
};
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index e875787710..08c53cea49 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -3465,7 +3465,7 @@ _git_sparse_checkout ()
_git_stash ()
{
- local subcommands='push list show apply clear drop pop create branch import export'
+ local subcommands='push list show apply clear drop pop create branch import export rename'
local subcommand="$(__git_find_on_cmdline "$subcommands save")"
if [ -z "$subcommand" ]; then
@@ -3508,7 +3508,7 @@ _git_stash ()
import,*)
__git_complete_refs
;;
- show,*|apply,*|drop,*|pop,*|export,*)
+ show,*|apply,*|drop,*|pop,*|export,*|rename,*)
__gitcomp_nl "$(__git stash list \
| sed -n -e 's/:.*//p')"
;;
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index ecc35aae82..f175302c1a 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -1831,4 +1831,83 @@ test_expect_success 'stash show --include-untracked includes untracked files' '
test_grep "untracked" actual
'
+test_expect_success 'rename a stash entry' '
+ git stash clear &&
+ >file-to-rename &&
+ git add file-to-rename &&
+ git stash push -m "original message" &&
+ git stash rename "new message" stash@{0} >out &&
+ test_grep "Renamed stash@{0}" out &&
+ git stash list >list &&
+ test_grep "stash@{0}: new message" list &&
+ test_grep ! "original message" list
+'
+
+test_expect_success 'rename defaults to the latest stash entry' '
+ git stash rename "default target" >out &&
+ test_grep "Renamed refs/stash@{0}" out &&
+ git stash list >list &&
+ test_grep "stash@{0}: default target" list
+'
+
+test_expect_success 'rename a deeper stash entry keeps positions and states' '
+ git stash clear &&
+ for i in 1 2 3
+ do
+ >file$i &&
+ git add file$i &&
+ git stash push -m "message $i" || return 1
+ done &&
+ git rev-parse stash@{0} stash@{1} stash@{2} >expect &&
+ git stash rename "renamed middle" stash@{1} &&
+ git rev-parse stash@{0} stash@{1} stash@{2} >actual &&
+ test_cmp expect actual &&
+ git stash list >list &&
+ test_grep "stash@{0}: On.*message 3" list &&
+ test_grep "stash@{1}: renamed middle" list &&
+ test_grep "stash@{2}: On.*message 1" list
+'
+
+test_expect_success 'rename the deepest stash entry' '
+ git rev-parse stash@{0} stash@{1} stash@{2} >expect &&
+ git stash rename "renamed deepest" stash@{2} &&
+ git rev-parse stash@{0} stash@{1} stash@{2} >actual &&
+ test_cmp expect actual &&
+ git stash list >list &&
+ test_grep "stash@{2}: renamed deepest" list
+'
+
+test_expect_success 'rename accepts a bare index and honors --quiet' '
+ git stash rename -q "quietly renamed" 1 >out &&
+ test_must_be_empty out &&
+ git stash list >list &&
+ test_grep "stash@{1}: quietly renamed" list
+'
+
+test_expect_success 'rename rejects bad arguments' '
+ test_must_fail git stash rename "no such entry" stash@{99} &&
+ test_must_fail git stash rename "" &&
+ test_must_fail git stash rename " " &&
+ test_must_fail git stash rename "not a stash" HEAD &&
+ test_must_fail git stash rename "not an index" "stash@{now}" &&
+ test_expect_code 129 git stash rename &&
+ git stash list >list &&
+ test_grep "stash@{1}: quietly renamed" list
+'
+
+test_expect_success 'rename refuses to rewrite a non-stash reflog entry' '
+ git stash clear &&
+ >real-a &&
+ git add real-a &&
+ git stash push -m "real A" &&
+ git update-ref -m junk --create-reflog refs/stash HEAD &&
+ >real-b &&
+ git add real-b &&
+ git stash push -m "real B" &&
+ git stash list >expect &&
+ test_must_fail git stash rename "renamed A" stash@{2} &&
+ git stash list >actual &&
+ test_cmp expect actual
+'
+
test_done
base-commit: 55526a18268bbc1ddaf8a6b7850c33d984eac9e9
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH 1/5] compat/posix: introduce writev(3p) wrapper
From: Simon Richter @ 2026-07-16 8:47 UTC (permalink / raw)
To: Patrick Steinhardt, git
Cc: Ben Knoble, Junio C Hamano, Jeff King, brian m. carlson,
Randall S. Becker, Phillip Wood, Johannes Schindelin
In-Reply-To: <20260716-pks-reintroduce-writev-v1-1-ea9038c884bc@pks.im>
Hi,
> + if (iov[i].iov_len > maximum_signed_value_of_type(ssize_t) ||
> + iov[i].iov_len + sum > maximum_signed_value_of_type(ssize_t)) {
That feels like it could overflow.
Simon
^ permalink raw reply
* [PATCH v6 0/4] environment: migrate 'trust_executable_bit' and 'has_symlinks' into 'repo_config_values'
From: Tian Yuchen @ 2026-07-16 8:49 UTC (permalink / raw)
To: git; +Cc: ps, Tian Yuchen
In-Reply-To: <20260715035501.48271-1-cat@malon.dev>
This series moves 'trust_executable_bit' and 'has_symlinks' into
'struct repo_config_values' to tie them to the specific repository
instance they were read from. Eager parsing is maintained because
these two flags are heavily consulted in hot paths.
Note: 'repo_config_values()' still does not support any struct
repository other than the_repository due to how deeply these flags
are accessed. In other words, this series of patches is laying
the groundwork for the eventual elimination of the_repository.
Previous related work:
[PATCH 2/6] config: add trust_executable_bit to global config [1]
[PATCH] Refactor 'trust_executable_bit' to repository-scoped setting [2]
(This previous attempt was unsuccessful because the target location
selected was 'struct repo_settings', which our analysis indicated
was not the optimal choice. For further details, please see: [3])
[PATCH 5/6] config: move has_symlinks [4]
RFC:
- Is the locations of the newly introduced definitions/macros
appropriate?
Change since V5:
- do not intruduce new global variable to deal with compat/mingw.c.
Make use of macro preprocessing to allow platforms to override
platform_has_symlinks().
Thanks!
[1] https://lore.kernel.org/git/837b5360b40f992351f489a0ae05fedf49884c6e.1685716420.git.gitgitgadget@gmail.com/
[2] https://lore.kernel.org/git/20260301190017.53539-1-dronarajgyawali@gmail.com/
[3] https://lore.kernel.org/git/xmqq1pht6nyx.fsf@gitster.g/
[4] https://lore.kernel.org/git/a154008619790f7a60f2bba91db7b0fe29e67e1a.1685716420.git.gitgitgadget@gmail.com/
[5] https://lore.kernel.org/git/xmqq7bokebct.fsf@gitster.g/
Tian Yuchen (4):
read-cache: remove redundant extern declarations
read-cache: move 'ce_mode_from_stat()' to 'read-cache.c'
environment: move trust_executable_bit into repo_config_values
environment: move has_symlinks into repo_config_values
apply.c | 4 ++--
combine-diff.c | 2 +-
compat/mingw.c | 17 +++++++++++++----
compat/mingw.h | 3 +++
entry.c | 2 +-
environment.c | 27 +++++++++++++++++++++++----
environment.h | 8 ++++++--
git-compat-util.h | 4 ++++
read-cache.c | 33 ++++++++++++++++++++++++++-------
read-cache.h | 16 ++--------------
10 files changed, 81 insertions(+), 35 deletions(-)
--
2.43.0
^ permalink raw reply
* [PATCH v6 1/4] read-cache: remove redundant extern declarations
From: Tian Yuchen @ 2026-07-16 8:49 UTC (permalink / raw)
To: git; +Cc: ps, Tian Yuchen, Christian Couder, Ayush Chandekar,
Olamide Caleb Bello
In-Reply-To: <20260716084941.1101918-1-cat@malon.dev>
The 'read-cache.c' file already includes 'environment.h', which provides
the extern declarations for variables like 'trust_executable_bit' and
'has_symlinks'.
Remove the redundant extern declarations inside 'st_mode_from_ce()' to
clean up the code.
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>
---
read-cache.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index 38a04b8de3..c44e4d128f 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -204,8 +204,6 @@ void fill_stat_cache_info(struct index_state *istate, struct cache_entry *ce, st
static unsigned int st_mode_from_ce(const struct cache_entry *ce)
{
- extern int trust_executable_bit, has_symlinks;
-
switch (ce->ce_mode & S_IFMT) {
case S_IFLNK:
return has_symlinks ? S_IFLNK : (S_IFREG | 0644);
--
2.43.0
^ permalink raw reply related
* [PATCH v6 2/4] read-cache: move 'ce_mode_from_stat()' to 'read-cache.c'
From: Tian Yuchen @ 2026-07-16 8:49 UTC (permalink / raw)
To: git; +Cc: ps, Tian Yuchen, Christian Couder, Ayush Chandekar,
Olamide Caleb Bello
In-Reply-To: <20260716084941.1101918-1-cat@malon.dev>
The ce_mode_from_stat() function is declared as a static inline function
in 'read-cache.h'. As we want to migrate configuration variables, this
helper function will need access to corresponding repository-specific
configuration logic. Move the implementation to 'read-cache.c' to
cleanly encapsulate its dependencies.
Note that the 'extern int trust_executable_bit, has_symlinks;' line is
discarded because it's not necessary when the function lives in
"read-cache.c".
At present, this change has no visible impact, but it is crucial
for our future plans to pass in the repo context. Comment
has been added whilst we are at it.
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>
---
read-cache.c | 20 ++++++++++++++++++++
read-cache.h | 16 ++--------------
2 files changed, 22 insertions(+), 14 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index c44e4d128f..cb4f4878c8 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -202,6 +202,26 @@ void fill_stat_cache_info(struct index_state *istate, struct cache_entry *ce, st
}
}
+/*
+ * Determine the appropriate index mode for a file based on its stat()
+ * information and the existing cache entry (if any).
+ *
+ * This function handles degradation for filesystems that lack
+ * symlink support or reliable executable bits.
+ */
+unsigned int ce_mode_from_stat(const struct cache_entry *ce, unsigned int mode)
+{
+ if (!has_symlinks && S_ISREG(mode) &&
+ ce && S_ISLNK(ce->ce_mode))
+ return ce->ce_mode;
+ if (!trust_executable_bit && S_ISREG(mode)) {
+ if (ce && S_ISREG(ce->ce_mode))
+ return ce->ce_mode;
+ return create_ce_mode(0666);
+ }
+ return create_ce_mode(mode);
+}
+
static unsigned int st_mode_from_ce(const struct cache_entry *ce)
{
switch (ce->ce_mode & S_IFMT) {
diff --git a/read-cache.h b/read-cache.h
index 043da1f1aa..3c4af2faeb 100644
--- a/read-cache.h
+++ b/read-cache.h
@@ -5,20 +5,8 @@
#include "object.h"
#include "pathspec.h"
-static inline unsigned int ce_mode_from_stat(const struct cache_entry *ce,
- unsigned int mode)
-{
- extern int trust_executable_bit, has_symlinks;
- if (!has_symlinks && S_ISREG(mode) &&
- ce && S_ISLNK(ce->ce_mode))
- return ce->ce_mode;
- if (!trust_executable_bit && S_ISREG(mode)) {
- if (ce && S_ISREG(ce->ce_mode))
- return ce->ce_mode;
- return create_ce_mode(0666);
- }
- return create_ce_mode(mode);
-}
+unsigned int ce_mode_from_stat(const struct cache_entry *ce,
+ unsigned int mode);
static inline int ce_to_dtype(const struct cache_entry *ce)
{
--
2.43.0
^ permalink raw reply related
* [PATCH v6 3/4] environment: move trust_executable_bit into repo_config_values
From: Tian Yuchen @ 2026-07-16 8:49 UTC (permalink / raw)
To: git; +Cc: ps, Tian Yuchen, Christian Couder, Ayush Chandekar,
Olamide Caleb Bello
In-Reply-To: <20260716084941.1101918-1-cat@malon.dev>
Move the global 'trust_executable_bit' configuration
into the repository-specific 'repo_config_values'
struct.
To ensure code readability, the getter function
'repo_trust_executable_bit()' has been introduced.
Callers access this configuration by passing in 'repo'
when possible, and explicitly fall back to 'the_repository'
the rest of time.
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 | 2 +-
environment.c | 11 +++++++++--
environment.h | 4 +++-
read-cache.c | 8 ++++----
4 files changed, 17 insertions(+), 8 deletions(-)
diff --git a/apply.c b/apply.c
index 249248d4f2..47b6ae5904 100644
--- a/apply.c
+++ b/apply.c
@@ -3893,7 +3893,7 @@ static int check_preimage(struct apply_state *state,
if (*ce && !(*ce)->ce_mode)
BUG("ce_mode == 0 for path '%s'", old_name);
- if (trust_executable_bit || !S_ISREG(st->st_mode))
+ if (repo_trust_executable_bit(state->repo) || !S_ISREG(st->st_mode))
st_mode = ce_mode_from_stat(*ce, st->st_mode);
else if (*ce)
st_mode = (*ce)->ce_mode;
diff --git a/environment.c b/environment.c
index fc3ed8bb1c..75069a884d 100644
--- a/environment.c
+++ b/environment.c
@@ -41,7 +41,6 @@
static int pack_compression_seen;
static int zlib_compression_seen;
-int trust_executable_bit = 1;
int trust_ctime = 1;
int check_stat = 1;
int has_symlinks = 1;
@@ -142,6 +141,13 @@ int is_bare_repository(void)
return is_bare_repository_cfg && !repo_get_work_tree(the_repository);
}
+int repo_trust_executable_bit(struct repository *repo)
+{
+ return repo->gitdir?
+ repo_config_values(repo)->trust_executable_bit :
+ 1;
+}
+
int have_git_dir(void)
{
return startup_info->have_repository
@@ -305,7 +311,7 @@ int git_default_core_config(const char *var, const char *value,
/* This needs a better name */
if (!strcmp(var, "core.filemode")) {
- trust_executable_bit = git_config_bool(var, value);
+ cfg->trust_executable_bit = git_config_bool(var, value);
return 0;
}
if (!strcmp(var, "core.trustctime")) {
@@ -720,5 +726,6 @@ void repo_config_values_init(struct repo_config_values *cfg)
{
cfg->attributes_file = NULL;
cfg->apply_sparse_checkout = 0;
+ cfg->trust_executable_bit = 1;
cfg->branch_track = BRANCH_TRACK_REMOTE;
}
diff --git a/environment.h b/environment.h
index 123a71cdc8..72b59fd89c 100644
--- a/environment.h
+++ b/environment.h
@@ -91,6 +91,7 @@ struct repo_config_values {
/* section "core" config values */
char *attributes_file;
int apply_sparse_checkout;
+ int trust_executable_bit;
/* section "branch" config values */
enum branch_track branch_track;
@@ -123,6 +124,8 @@ int git_default_config(const char *, const char *,
int git_default_core_config(const char *var, const char *value,
const struct config_context *ctx, void *cb);
+int repo_trust_executable_bit(struct repository *repo);
+
void repo_config_values_init(struct repo_config_values *cfg);
/*
@@ -160,7 +163,6 @@ int is_bare_repository(void);
extern char *git_work_tree_cfg;
/* Environment bits from configuration mechanism */
-extern int trust_executable_bit;
extern int trust_ctime;
extern int check_stat;
extern int has_symlinks;
diff --git a/read-cache.c b/read-cache.c
index cb4f4878c8..a9c11a3346 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -214,7 +214,7 @@ unsigned int ce_mode_from_stat(const struct cache_entry *ce, unsigned int mode)
if (!has_symlinks && S_ISREG(mode) &&
ce && S_ISLNK(ce->ce_mode))
return ce->ce_mode;
- if (!trust_executable_bit && S_ISREG(mode)) {
+ if (!repo_trust_executable_bit(the_repository) && S_ISREG(mode)) {
if (ce && S_ISREG(ce->ce_mode))
return ce->ce_mode;
return create_ce_mode(0666);
@@ -228,7 +228,7 @@ static unsigned int st_mode_from_ce(const struct cache_entry *ce)
case S_IFLNK:
return has_symlinks ? S_IFLNK : (S_IFREG | 0644);
case S_IFREG:
- return (ce->ce_mode & (trust_executable_bit ? 0755 : 0644)) | S_IFREG;
+ return (ce->ce_mode & (repo_trust_executable_bit(the_repository) ? 0755 : 0644)) | S_IFREG;
case S_IFGITLINK:
return S_IFDIR | 0755;
case S_IFDIR:
@@ -338,7 +338,7 @@ static int ce_match_stat_basic(const struct cache_entry *ce, struct stat *st)
/* We consider only the owner x bit to be relevant for
* "mode changes"
*/
- if (trust_executable_bit &&
+ if (repo_trust_executable_bit(the_repository) &&
(0100 & (ce->ce_mode ^ st->st_mode)))
changed |= MODE_CHANGED;
break;
@@ -759,7 +759,7 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
ce->ce_flags |= CE_INTENT_TO_ADD;
- if (trust_executable_bit && has_symlinks) {
+ if (repo_trust_executable_bit(istate->repo) && has_symlinks) {
ce->ce_mode = create_ce_mode(st_mode);
} else {
/* If there is an existing entry, pick the mode bits and type
--
2.43.0
^ permalink raw reply related
* [PATCH v6 4/4] environment: move has_symlinks into repo_config_values
From: Tian Yuchen @ 2026-07-16 8:49 UTC (permalink / raw)
To: git; +Cc: ps, Tian Yuchen, Christian Couder, Ayush Chandekar,
Olamide Caleb Bello
In-Reply-To: <20260716084941.1101918-1-cat@malon.dev>
Move the global 'has_symlinks' configuration into the
repository-specific 'repo_config_values' struct.
To ensure code readability, the getter function
'repo_has_symlinks()' has been introduced. Callers access
this configuration by passing in 'repo' when possible,
and explicitly fall back to 'the_repository' the rest
of the time.
Note:
To support platform-specific overrides (MinGW) before
repository initialization, the 'platform_has_symlinks()'
macro is introduced in git-compat-util.h. Platforms can
override this in their respective headers.
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 | 2 +-
combine-diff.c | 2 +-
compat/mingw.c | 17 +++++++++++++----
compat/mingw.h | 3 +++
entry.c | 2 +-
environment.c | 16 ++++++++++++++--
environment.h | 4 +++-
git-compat-util.h | 4 ++++
read-cache.c | 9 +++++----
9 files changed, 45 insertions(+), 14 deletions(-)
diff --git a/apply.c b/apply.c
index 47b6ae5904..4ce4160b48 100644
--- a/apply.c
+++ b/apply.c
@@ -4511,7 +4511,7 @@ static int try_create_file(struct apply_state *state, const char *path,
return !!mkdir(path, 0777);
}
- if (has_symlinks && S_ISLNK(mode))
+ if (repo_has_symlinks(state->repo) && S_ISLNK(mode))
/* Although buf:size is counted string, it also is NUL
* terminated.
*/
diff --git a/combine-diff.c b/combine-diff.c
index b799862068..80e5c46e9b 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -1078,7 +1078,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
/* if symlinks don't work, assume symlink if all parents
* are symlinks
*/
- is_file = has_symlinks;
+ is_file = repo_has_symlinks(rev->repo);
for (i = 0; !is_file && i < num_parent; i++)
is_file = !S_ISLNK(elem->parent[i].mode);
if (!is_file)
diff --git a/compat/mingw.c b/compat/mingw.c
index aa7525f419..4781911929 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -7,6 +7,7 @@
#include "config.h"
#include "dir.h"
#include "environment.h"
+#include "repository.h"
#include "gettext.h"
#include "run-command.h"
#include "strbuf.h"
@@ -1043,7 +1044,7 @@ int mingw_chdir(const char *dirname)
if (xutftowcs_path(wdirname, dirname) < 0)
return -1;
- if (has_symlinks) {
+ if (repo_has_symlinks(the_repository)) {
HANDLE hnd = CreateFileW(wdirname, 0,
FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, NULL,
OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL);
@@ -2903,7 +2904,7 @@ int symlink(const char *target, const char *link)
int len;
/* fail if symlinks are disabled or API is not supported (WinXP) */
- if (!has_symlinks) {
+ if (!repo_has_symlinks(the_repository)) {
errno = ENOSYS;
return -1;
}
@@ -3173,15 +3174,23 @@ static void setup_windows_environment(void)
if (!tmp && (tmp = getenv("USERPROFILE")))
setenv("HOME", tmp, 1);
}
+}
+int mingw_platform_has_symlinks(void)
+{
+ static int has_symlinks = -1;
/*
* Change 'core.symlinks' default to false, unless native symlinks are
* enabled in MSys2 (via 'MSYS=winsymlinks:nativestrict'). Thus we can
* run the test suite (which doesn't obey config files) with or without
* symlink support.
*/
- if (!(tmp = getenv("MSYS")) || !strstr(tmp, "winsymlinks:nativestrict"))
- has_symlinks = 0;
+ if (has_symlinks < 0) {
+ const char *tmp = getenv("MSYS");
+ has_symlinks = (tmp && strstr(tmp, "winsymlinks:nativestrict")) ? 1 : 0;
+ }
+
+ return has_symlinks;
}
static void get_current_user_sid(PSID *sid, HANDLE *linked_token)
diff --git a/compat/mingw.h b/compat/mingw.h
index 444daedfa5..df02aeb632 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -208,6 +208,9 @@ void open_in_gdb(void);
*/
int err_win_to_posix(DWORD winerr);
+int mingw_platform_has_symlinks(void);
+#define platform_has_symlinks() mingw_platform_has_symlinks()
+
#ifndef NO_UNIX_SOCKETS
int mingw_have_unix_sockets(void);
#undef have_unix_sockets
diff --git a/entry.c b/entry.c
index 7817aee362..f2854b4cd8 100644
--- a/entry.c
+++ b/entry.c
@@ -321,7 +321,7 @@ static int write_entry(struct cache_entry *ce, char *path, struct conv_attrs *ca
* We can't make a real symlink; write out a regular file entry
* with the symlink destination as its contents.
*/
- if (!has_symlinks || to_tempfile)
+ if (!repo_has_symlinks(state->istate ? state->istate->repo : NULL) || to_tempfile)
goto write_file_entry;
ret = symlink(new_blob, path);
diff --git a/environment.c b/environment.c
index 75069a884d..760689d6e7 100644
--- a/environment.c
+++ b/environment.c
@@ -43,7 +43,6 @@ static int zlib_compression_seen;
int trust_ctime = 1;
int check_stat = 1;
-int has_symlinks = 1;
int minimum_abbrev = 4, default_abbrev = -1;
int ignore_case;
int assume_unchanged;
@@ -148,6 +147,17 @@ int repo_trust_executable_bit(struct repository *repo)
1;
}
+int repo_has_symlinks(struct repository *repo)
+{
+ if (!repo)
+ repo = the_repository;
+
+ if (!repo->gitdir)
+ return platform_has_symlinks();
+
+ return repo_config_values(repo)->has_symlinks;
+}
+
int have_git_dir(void)
{
return startup_info->have_repository
@@ -336,7 +346,8 @@ int git_default_core_config(const char *var, const char *value,
}
if (!strcmp(var, "core.symlinks")) {
- has_symlinks = git_config_bool(var, value);
+ struct repo_config_values *cfg = repo_config_values(the_repository);
+ cfg->has_symlinks = git_config_bool(var, value);
return 0;
}
@@ -727,5 +738,6 @@ void repo_config_values_init(struct repo_config_values *cfg)
cfg->attributes_file = NULL;
cfg->apply_sparse_checkout = 0;
cfg->trust_executable_bit = 1;
+ cfg->has_symlinks = platform_has_symlinks();
cfg->branch_track = BRANCH_TRACK_REMOTE;
}
diff --git a/environment.h b/environment.h
index 72b59fd89c..ef64a783b0 100644
--- a/environment.h
+++ b/environment.h
@@ -92,6 +92,7 @@ struct repo_config_values {
char *attributes_file;
int apply_sparse_checkout;
int trust_executable_bit;
+ int has_symlinks;
/* section "branch" config values */
enum branch_track branch_track;
@@ -126,6 +127,8 @@ int git_default_core_config(const char *var, const char *value,
int repo_trust_executable_bit(struct repository *repo);
+int repo_has_symlinks(struct repository *repo);
+
void repo_config_values_init(struct repo_config_values *cfg);
/*
@@ -165,7 +168,6 @@ extern char *git_work_tree_cfg;
/* Environment bits from configuration mechanism */
extern int trust_ctime;
extern int check_stat;
-extern int has_symlinks;
extern int minimum_abbrev, default_abbrev;
extern int ignore_case;
extern int assume_unchanged;
diff --git a/git-compat-util.h b/git-compat-util.h
index 5024814bd4..333a5acf33 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -245,6 +245,10 @@ static inline int git_is_dir_sep(int c)
#define is_dir_sep git_is_dir_sep
#endif
+#ifndef platform_has_symlinks
+#define platform_has_symlinks() 1
+#endif
+
#ifndef offset_1st_component
static inline int git_offset_1st_component(const char *path)
{
diff --git a/read-cache.c b/read-cache.c
index a9c11a3346..5a40ffa061 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -211,7 +211,7 @@ void fill_stat_cache_info(struct index_state *istate, struct cache_entry *ce, st
*/
unsigned int ce_mode_from_stat(const struct cache_entry *ce, unsigned int mode)
{
- if (!has_symlinks && S_ISREG(mode) &&
+ if (!repo_has_symlinks(the_repository) && S_ISREG(mode) &&
ce && S_ISLNK(ce->ce_mode))
return ce->ce_mode;
if (!repo_trust_executable_bit(the_repository) && S_ISREG(mode)) {
@@ -226,7 +226,7 @@ static unsigned int st_mode_from_ce(const struct cache_entry *ce)
{
switch (ce->ce_mode & S_IFMT) {
case S_IFLNK:
- return has_symlinks ? S_IFLNK : (S_IFREG | 0644);
+ return repo_has_symlinks(the_repository) ? S_IFLNK : (S_IFREG | 0644);
case S_IFREG:
return (ce->ce_mode & (repo_trust_executable_bit(the_repository) ? 0755 : 0644)) | S_IFREG;
case S_IFGITLINK:
@@ -344,7 +344,7 @@ static int ce_match_stat_basic(const struct cache_entry *ce, struct stat *st)
break;
case S_IFLNK:
if (!S_ISLNK(st->st_mode) &&
- (has_symlinks || !S_ISREG(st->st_mode)))
+ (repo_has_symlinks(the_repository) || !S_ISREG(st->st_mode)))
changed |= TYPE_CHANGED;
break;
case S_IFGITLINK:
@@ -759,7 +759,8 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
ce->ce_flags |= CE_INTENT_TO_ADD;
- if (repo_trust_executable_bit(istate->repo) && has_symlinks) {
+ if (repo_trust_executable_bit(istate->repo) &&
+ repo_has_symlinks(istate->repo)) {
ce->ce_mode = create_ce_mode(st_mode);
} else {
/* If there is an existing entry, pick the mode bits and type
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v4 1/3] commit-reach: reject cycles in contains walk
From: Jeff King @ 2026-07-16 9:05 UTC (permalink / raw)
To: Tamir Duberstein
Cc: git, Karthik Nayak, Junio C Hamano, Victoria Dye, Derrick Stolee,
Elijah Newren, Kristofer Karlsson
In-Reply-To: <20260612-ref-filter-memoized-contains-v4-1-5ed39fd001dd@gmail.com>
On Fri, Jun 12, 2026 at 05:49:12PM -0400, Tamir Duberstein wrote:
> @@ -708,7 +708,8 @@ static int in_commit_list(const struct commit_list *want, struct commit *c)
>
> /*
> * Test whether the candidate is contained in the list.
> - * Do not recurse to find out, though, but return -1 if inconclusive.
> + * Do not recurse to find out, though, but return CONTAINS_UNKNOWN if
> + * inconclusive.
> */
> static enum contains_result contains_test(struct commit *candidate,
> const struct commit_list *want,
This hunk is a good cleanup, but unrelated to the patch at hand.
We used to return a bare -1, then that became CONTAINS_UNKNOWN in
a0262c51d0 (ref-filter: use contains_result enum consistently,
2017-03-09). And then that value changed to 0 in a91aca44bf (ref-filter:
use separate cache for contains_tag_algo, 2017-03-09) when we started
using a slab.
So the code is correct and the comment is wrong, and it is worth
updating. I was just surprised to find it here.
> @@ -765,6 +766,7 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
> if (result != CONTAINS_UNKNOWN)
> return result;
>
> + *contains_cache_at(cache, candidate) = CONTAINS_IN_PROGRESS;
> push_to_contains_stack(candidate, &contains_stack);
> while (contains_stack.nr) {
> struct contains_stack_entry *entry = &contains_stack.contains_stack[contains_stack.nr - 1];
> @@ -776,8 +778,8 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
> contains_stack.nr--;
> }
> /*
> - * If we just popped the stack, parents->item has been marked,
> - * therefore contains_test will return a meaningful yes/no.
> + * A parent may have just been popped and marked, or may still
> + * be active when replacement refs create a cycle.
> */
> else switch (contains_test(parents->item, want, cache, cutoff)) {
> case CONTAINS_YES:
> @@ -787,7 +789,11 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
> case CONTAINS_NO:
> entry->parents = parents->next;
> break;
> + case CONTAINS_IN_PROGRESS:
> + die(_("commit ancestry contains a cycle"));
> case CONTAINS_UNKNOWN:
> + *contains_cache_at(cache, parents->item) =
> + CONTAINS_IN_PROGRESS;
> push_to_contains_stack(parents->item, &contains_stack);
> break;
> }
Nice, this looks cleanly done.
> +test_expect_success 'tag --contains rejects cyclic replacement histories' '
> + first=$(git rev-parse HEAD~2) &&
> + second=$(git rev-parse HEAD~) &&
> + third=$(git rev-parse HEAD) &&
> + test_when_finished "
> + git replace -d $first &&
> + git replace -d $third &&
> + git tag -d cycle-a cycle-b
> + " &&
> + git tag cycle-a "$first" &&
> + git tag cycle-b "$third" &&
> + git replace --graft "$first" "$third" "$second" &&
> + git replace --graft "$third" "$first" &&
> + test_must_fail git tag --contains="$second" --list "cycle-*" \
> + >/dev/null 2>err &&
> + test_grep "fatal: commit ancestry contains a cycle" err
> +'
Likewise the test looks good.
-Peff
^ permalink raw reply
* [GIT PULL] git-gui: larger commit msg field, Bulgarian translation, silent make -s
From: Johannes Sixt @ 2026-07-16 9:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
The following changes since commit 1b2c2a2edbaa1638becef4c3755b3e0633b9c304:
Merge branch 'ml/repo-discovery' (2026-06-12 11:05:28 +0200)
are available in the Git repository at:
https://github.com/j6t/git-gui.git master
for you to fetch changes up to 5dcb97869546d600a114ef422a135e2e909c923c:
Merge branch 'master' of github.com:alshopov/git-gui (2026-07-16 11:05:03 +0200)
----------------------------------------------------------------
Alexander Shopov (2):
git-gui i18n: Update Bulgarian translation (562t)
git-gui: allow larger width for the commit message field
Harald Nordgren (1):
git-gui: drop msgfmt --statistics output
Johannes Sixt (4):
Merge branch 'master' of github.com:alshopov/git-gui
git-gui: reduce complexity of the quiet msgfmt rule
Merge branch 'hn/silence-make-s'
Merge branch 'master' of github.com:alshopov/git-gui
Makefile | 5 ++---
lib/option.tcl | 2 +-
po/bg.po | 37 ++++++++++++++++++++++++++-----------
3 files changed, 29 insertions(+), 15 deletions(-)
^ permalink raw reply
* Re: [PATCH v4 3/3] commit-reach: die on contains walk errors
From: Jeff King @ 2026-07-16 9:18 UTC (permalink / raw)
To: Tamir Duberstein
Cc: git, Karthik Nayak, Junio C Hamano, Victoria Dye, Derrick Stolee,
Elijah Newren, Kristofer Karlsson
In-Reply-To: <20260612-ref-filter-memoized-contains-v4-3-5ed39fd001dd@gmail.com>
On Fri, Jun 12, 2026 at 05:49:14PM -0400, Tamir Duberstein wrote:
> int commit_contains(struct ref_filter *filter, struct commit *commit,
> struct commit_list *list, struct contains_cache *cache)
> {
> + int result;
> +
> if (filter->with_commit_tag_algo ||
> generation_numbers_enabled(the_repository))
> return contains_tag_algo(commit, list, cache) == CONTAINS_YES;
> - return repo_is_descendant_of(the_repository, commit, list);
> +
> + result = repo_is_descendant_of(the_repository, commit, list);
> + if (result < 0)
> + die(_("failed to check reachability"));
> + return result;
Makes sense. And we can see from the test that repo_is_descendant_of()
will already have printed the real reason for the error.
-Peff
^ permalink raw reply
* Re: [PATCH v4 0/3] Reuse --contains traversal results
From: Jeff King @ 2026-07-16 9:19 UTC (permalink / raw)
To: Junio C Hamano
Cc: Tamir Duberstein, git, Karthik Nayak, Victoria Dye,
Derrick Stolee, Elijah Newren, Kristofer Karlsson
In-Reply-To: <xmqqqzlpulkp.fsf@gitster.g>
On Mon, Jun 29, 2026 at 01:40:38PM -0700, Junio C Hamano wrote:
> Tamir Duberstein <tamird@gmail.com> writes:
>
> > git tag uses a memoized traversal for --contains, while git branch
> > and git for-each-ref repeat a reachability walk for each ref. Reuse
> > the memoized traversal when generation numbers can bound the walk.
> >
> > The first patch makes the memoized traversal reject cyclic replacement
> > histories. The last makes the non-memoized path report reachability
> > errors.
>
> This unfortunately hasn't heard any responses since June 12th. Are
> there remaining issues with it? Or do people fundamentally have
> objections against this change? Or things are too busy in general
> that there are more patches than there are folks willing to review
> them?
The last one. ;)
I think the direction is good and the patches themselves look fine. The
only nit I had was that there's an unrelated (but good) comment cleanup
in patch 1. That could be split into its own patch, but I am also fine
to declare victory on v4.
-Peff
^ permalink raw reply
* Re: git-last-modified(1) slower than git-log(1)?
From: Toon Claes @ 2026-07-16 9:26 UTC (permalink / raw)
To: Gusted, git, Jeff King
In-Reply-To: <17f356ff-7bfb-47f5-b714-62a95cc8b821@codeberg.org>
Gusted <gusted@codeberg.org> writes:
> Hi,
>
> I'm working at switching Forgejo's implementation of getting the last
> modified commits in a directory to git-last-modified(1). I'd expected
> equal or better performance than the current implementation, but have
> not yet been able to get this and I'm a bit puzzled as to why.
>
> The current implementation of Forgejo (inherited from Gitea) works
> roughly like this:
> 1. Run `git log --name-status -c --format=commit%x00%H %P%x00" --parents
> --no-renames -t -z $OID -- :(literal)some/path`, the output of this is
> quite complex and possible outputs more information than necessary.
> 2. The output of this is piped to some code to a parser and reconstructs
> what commit ID last modified each file in the directory.
> 3. Via `git cat-file --batch` get each unique commits information.
>
> With git-last-modified(1) (-z --show-trees --max-depth=0) this replaces
> step 1-2, but is slower. I've isolated the degraded performance to the
> fact that git-last-changed(1) takes more time to finish. So from my
> perspective it does not seem worth it to replace the current
> implementation with git-last-modified(1), and I would like to know if
> I'm missing something here or if git-last-modified(1) possibly could see
> a speedup?
>
> The repository I'm currently using to evaluate the performance is
> https://codeberg.org/ziglang/zig
>
> Reproduction steps:
> 1. `git clone https://codeberg.org/ziglang/zig $(mktemp -d)`
> 2. cd to tmp directory.
> 3. `git commit-graph write --changed-paths`. As git-last-modified(1)
> makes good use of the bloom filters.
> 4. `hyperfine 'git last-modified -z -t --max-depth=0
> 80d06578ac66bce3aa0a21e9610cdb782b9a0593 -- doc/langref/' 'git log
> --name-status -c "--format=commit%x00%H %P%x00" --parents --no-renames
> -t -z 80d06578ac66bce3aa0a21e9610cdb782b9a0593 -- ":(literal)doc/langref"'`
>
> With as output:
> Benchmark 1: git last-modified -z -t --max-depth=0
> 80d06578ac66bce3aa0a21e9610cdb782b9a0593 -- doc/langref/
> Time (mean ± σ): 66.5 ms ± 0.6 ms [User: 60.6 ms, System: 5.2 ms]
> Range (min … max): 65.3 ms … 67.7 ms 44 runs
>
> Benchmark 2: git log --name-status -c "--format=commit%x00%H %P%x00"
> --parents --no-renames -t -z 80d06578ac66bce3aa0a21e9610cdb782b9a0593 --
> ":(literal)doc/langref"
> Time (mean ± σ): 26.2 ms ± 1.0 ms [User: 17.3 ms, System: 8.4 ms]
> Range (min … max): 24.3 ms … 30.1 ms 110 runs
>
> Summary
> git log --name-status -c "--format=commit%x00%H %P%x00" --parents
> --no-renames -t -z 80d06578ac66bce3aa0a21e9610cdb782b9a0593 --
> ":(literal)doc/langref" ran
> 2.54 ± 0.10 times faster than git last-modified -z -t --max-depth=0
> 80d06578ac66bce3aa0a21e9610cdb782b9a0593 -- doc/langref/
Hi Gusted,
Thanks for reaching out.
You're actually not the first to notice this, and I've been aware of
this.
The thing is, you're testing the difference on a single file. For us at
GitLab, it wasn't very useful to optimize that use-case, because usually
we want to see the last commit for a bunch of files at once.
So the use-case for git-last-modified(1) for us has been to replace
(pseudo code):
$ FILES=$(git ls-tree $COMMIT $PATH)
$ foreach $FILE in $FILES; do git log -1 $COMMIT -- $FILE; end
GitLab is batching files 25 at once, and in my benchmarking, it was
shown git-last-modified(1) is faster:
$ git last-modified $COMMIT -- <files
(I did this benchmarking in our Gitaly component to have a real-world
experience and you can visit the results at:
https://gitlab.com/gitlab-org/gitaly/-/merge_requests/7999#note_2850505479
)
So we left the door open for future improvement, although I never have
gotten to it. At some point I was trying to chase down when git-log(1)
was doing differently, but I never figured it out.
But this email challenged me already. And with some help of AI, I
managed to work on some improvements. You can expect a patch series
soon.
(Right before sending out this mail I noticed Peff sent out some changes
as well. I'll coordinate how to combine.)
--
Cheers,
Toon
^ permalink raw reply
* Re: [PATCH] rebase: mention --abort alongside --continue
From: Phillip Wood @ 2026-07-16 9:37 UTC (permalink / raw)
To: Harald Nordgren, Junio C Hamano; +Cc: Harald Nordgren via GitGitGadget, git
In-Reply-To: <CAHwyqnVy=4oHBTmtDJ6jX38Kh1aLYYXHR-_12DdiiUxpXZ5kNg@mail.gmail.com>
Hi Harald
On 16/07/2026 07:02, Harald Nordgren wrote:
> I'll revive this discussion because the 'git rebase --keep-base -x'
> case still bothers me.
>
> When getting stuck in the middle of an operation, it just makes sense
> to offer a way forward and a way back, why be more obtuse than we need
> to?
I'm still not clear why you'd want to abort after a failed exec. In the
example you gave earlier in the thread where the exec command was trying
to run a command that did not exist isn't the solution to edit the todo
list to fix that, or if just this exec command is wrong, continue the
rebase?
In the latter case it would be useful is to teach "git rebase --skip"
to skip a failed exec command that has been rescheduled by
"--reschedule-failed-exec" and provide a hint to the user that they can
skip the rescheduled command. We could potentially add a hint to suggest
that if the failure was due to a bad command then the user should edit
the todo list.
To me aborting a rebase because an exec command failed is almost never a
sensible route forward and we should not be encouraging users to abort
after a failed test - surely the sensible thing to do in that case is
fix the problem with "git commit --amend" and continue the rebase.
Thanks
Phillip
^ permalink raw reply
* [GIT PULL] gitk: Bulgarian+Spanish translations, silent make -s
From: Johannes Sixt @ 2026-07-16 8:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
The following changes since commit bad83ada0ebf9e293d570e6e7ca4f1cd7877f482:
Merge branch 'horizontal-scroll' of github.com:ramcdona/gitk (2026-06-12 11:30:22 +0200)
are available in the Git repository at:
https://github.com/j6t/gitk.git master
for you to fetch changes up to f1de86371cb85dd09d55070d139e5fcdc595f026:
Merge branch 'spanish_pr_bis' of github.com:basuradeluis/gitkbis (2026-07-16 10:53:01 +0200)
----------------------------------------------------------------
Alexander Shopov (1):
gitk i18n: Update Bulgarian translation (329t)
Harald Nordgren (1):
gitk: make "make -s" silent
Johannes Sixt (2):
Merge branch 'master' of github.com:alshopov/gitk
Merge branch 'spanish_pr_bis' of github.com:basuradeluis/gitkbis
basuradeluis (1):
gitk: spanish translations
Makefile | 6 +-
po/bg.po | 45 ++++--
po/es.po | 488 +++++++++++++++++++++++++++++++++++++--------------------------
3 files changed, 321 insertions(+), 218 deletions(-)
^ permalink raw reply
* [PATCH] copy: drop dependency on `the_repository`
From: Patrick Steinhardt @ 2026-07-16 9:56 UTC (permalink / raw)
To: git
When copying a file we need to potentially adapt permissions of the new
file based on whether or not "core.shared" is enabled. Parsing this
configuration makes us implicitly depend on `the_repository`.
Refactor the code to instead require the caller to pass in a repository
so that we can remove `USE_THE_REPOSITORY_VARIABLE`.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Hi,
I guess the title says it all: this small patch removes the dependency
on `the_repository` in "copy.c". Thanks!
Patrick
---
builtin/clone.c | 2 +-
builtin/difftool.c | 4 ++--
builtin/worktree.c | 4 ++--
bundle-uri.c | 2 +-
copy.c | 12 ++++++------
copy.h | 8 ++++++--
refs/files-backend.c | 2 +-
rerere.c | 2 +-
sequencer.c | 6 +++---
setup.c | 2 +-
10 files changed, 24 insertions(+), 20 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index d60d1b60bc..18603dd4ce 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -335,7 +335,7 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,
die_errno(_("failed to create link '%s'"), dest->buf);
option_no_hardlinks = 1;
}
- if (copy_file_with_time(dest->buf, src->buf, 0666))
+ if (copy_file_with_time(the_repository, dest->buf, src->buf, 0666))
die_errno(_("failed to copy file to '%s'"), dest->buf);
}
diff --git a/builtin/difftool.c b/builtin/difftool.c
index 26778f8515..5e7777fbe4 100644
--- a/builtin/difftool.c
+++ b/builtin/difftool.c
@@ -552,7 +552,7 @@ static int run_dir_diff(struct repository *repo,
struct stat st;
if (stat(wtdir.buf, &st))
st.st_mode = 0644;
- if (copy_file(rdir.buf, wtdir.buf,
+ if (copy_file(repo, rdir.buf, wtdir.buf,
st.st_mode)) {
ret = error("could not copy '%s' to '%s'", wtdir.buf, rdir.buf);
goto finish;
@@ -658,7 +658,7 @@ static int run_dir_diff(struct repository *repo,
warning("%s", "");
err = 1;
} else if (unlink(wtdir.buf) ||
- copy_file(wtdir.buf, rdir.buf, st.st_mode))
+ copy_file(repo, wtdir.buf, rdir.buf, st.st_mode))
warning_errno(_("could not copy '%s' to '%s'"),
rdir.buf, wtdir.buf);
}
diff --git a/builtin/worktree.c b/builtin/worktree.c
index d21c43fde3..84b01960fb 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -349,7 +349,7 @@ static void copy_sparse_checkout(const char *worktree_git_dir)
if (file_exists(from_file)) {
if (safe_create_leading_directories(the_repository, to_file) ||
- copy_file(to_file, from_file, 0666))
+ copy_file(the_repository, to_file, from_file, 0666))
error(_("failed to copy '%s' to '%s'; sparse-checkout may not work correctly"),
from_file, to_file);
}
@@ -368,7 +368,7 @@ static void copy_filtered_worktree_config(const char *worktree_git_dir)
int bare;
if (safe_create_leading_directories(the_repository, to_file) ||
- copy_file(to_file, from_file, 0666)) {
+ copy_file(the_repository, to_file, from_file, 0666)) {
error(_("failed to copy worktree config from '%s' to '%s'"),
from_file, to_file);
goto worktree_copy_cleanup;
diff --git a/bundle-uri.c b/bundle-uri.c
index 3b2e347288..ef37aebf30 100644
--- a/bundle-uri.c
+++ b/bundle-uri.c
@@ -396,7 +396,7 @@ static int copy_uri_to_file(const char *filename, const char *uri)
uri = out;
/* Copy as a file */
- return copy_file(filename, uri, 0);
+ return copy_file(the_repository, filename, uri, 0);
}
static int unbundle_from_file(struct repository *r, const char *file)
diff --git a/copy.c b/copy.c
index b668209b6c..6074132050 100644
--- a/copy.c
+++ b/copy.c
@@ -1,5 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
-
#include "git-compat-util.h"
#include "copy.h"
#include "path.h"
@@ -35,7 +33,8 @@ static int copy_times(const char *dst, const char *src)
return 0;
}
-int copy_file(const char *dst, const char *src, int mode)
+int copy_file(struct repository *repo,
+ const char *dst, const char *src, int mode)
{
int fdi, fdo, status;
@@ -59,15 +58,16 @@ int copy_file(const char *dst, const char *src, int mode)
if (close(fdo) != 0)
return error_errno("%s: close error", dst);
- if (!status && adjust_shared_perm(the_repository, dst))
+ if (!status && adjust_shared_perm(repo, dst))
return -1;
return status;
}
-int copy_file_with_time(const char *dst, const char *src, int mode)
+int copy_file_with_time(struct repository *repo,
+ const char *dst, const char *src, int mode)
{
- int status = copy_file(dst, src, mode);
+ int status = copy_file(repo, dst, src, mode);
if (!status)
return copy_times(dst, src);
return status;
diff --git a/copy.h b/copy.h
index 2af77cba86..1059b118d6 100644
--- a/copy.h
+++ b/copy.h
@@ -1,10 +1,14 @@
#ifndef COPY_H
#define COPY_H
+struct repository;
+
#define COPY_READ_ERROR (-2)
#define COPY_WRITE_ERROR (-3)
int copy_fd(int ifd, int ofd);
-int copy_file(const char *dst, const char *src, int mode);
-int copy_file_with_time(const char *dst, const char *src, int mode);
+int copy_file(struct repository *repo,
+ const char *dst, const char *src, int mode);
+int copy_file_with_time(struct repository *repo,
+ const char *dst, const char *src, int mode);
#endif /* COPY_H */
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 3df56c25c8..442c98414e 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1736,7 +1736,7 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,
goto out;
}
- if (copy && log && copy_file(tmp_renamed_log.buf, sb_oldref.buf, 0644)) {
+ if (copy && log && copy_file(refs->base.repo, tmp_renamed_log.buf, sb_oldref.buf, 0644)) {
ret = error("unable to copy logfile logs/%s to logs/"TMP_RENAMED_LOG": %s",
oldrefname, strerror(errno));
goto out;
diff --git a/rerere.c b/rerere.c
index 8232542585..bf5cfc6e51 100644
--- a/rerere.c
+++ b/rerere.c
@@ -756,7 +756,7 @@ static void do_rerere_one_path(struct index_state *istate,
/* Has the user resolved it already? */
if (variant >= 0) {
if (!handle_file(istate, path, NULL, NULL)) {
- copy_file(rerere_path(&buf, id, "postimage"), path, 0666);
+ copy_file(the_repository, rerere_path(&buf, id, "postimage"), path, 0666);
id->collection->status[variant] |= RR_HAS_POSTIMAGE;
fprintf_ln(stderr, _("Recorded resolution for '%s'."), path);
free_rerere_id(rr_item);
diff --git a/sequencer.c b/sequencer.c
index 1355a99a09..c9ede9c02d 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2419,7 +2419,7 @@ static int do_pick_commit(struct repository *r,
} else {
const char *dest = git_path_squash_msg(r);
unlink(dest);
- if (copy_file(dest, rebase_path_squash_msg(), 0666)) {
+ if (copy_file(the_repository, dest, rebase_path_squash_msg(), 0666)) {
res = error(_("could not copy '%s' to '%s'"),
rebase_path_squash_msg(), dest);
goto leave;
@@ -3864,11 +3864,11 @@ static int error_failed_squash(struct repository *r,
int subject_len,
const char *subject)
{
- if (copy_file(rebase_path_message(), rebase_path_squash_msg(), 0666))
+ if (copy_file(the_repository, rebase_path_message(), rebase_path_squash_msg(), 0666))
return error(_("could not copy '%s' to '%s'"),
rebase_path_squash_msg(), rebase_path_message());
unlink(git_path_merge_msg(r));
- if (copy_file(git_path_merge_msg(r), rebase_path_message(), 0666))
+ if (copy_file(the_repository, git_path_merge_msg(r), rebase_path_message(), 0666))
return error(_("could not copy '%s' to '%s'"),
rebase_path_message(),
git_path_merge_msg(r));
diff --git a/setup.c b/setup.c
index 0de56a074f..91d61a5939 100644
--- a/setup.c
+++ b/setup.c
@@ -2331,7 +2331,7 @@ static void copy_templates_1(struct repository *repo,
strbuf_release(&lnk);
}
else if (S_ISREG(st_template.st_mode)) {
- if (copy_file(path->buf, template_path->buf, st_template.st_mode))
+ if (copy_file(repo, path->buf, template_path->buf, st_template.st_mode))
die_errno(_("cannot copy '%s' to '%s'"),
template_path->buf, path->buf);
}
---
base-commit: d35c5399e3e54ac277bb391fc2f6be3e816d312b
change-id: 20260716-pks-copy-wo-the-repository-aa01ccdbed76
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox