* Re: git config: unintuitive behaviour with --global and --no-includes
From: Jeff King @ 2026-07-20 12:51 UTC (permalink / raw)
To: Hendrik Jaeger; +Cc: git
In-Reply-To: <20260720113402.0dc16abe@frustcomp.hnjs.home.arpa>
On Mon, Jul 20, 2026 at 11:34:02AM +0200, Hendrik Jaeger wrote:
> The manpage says:
> > Respect include.* directives in config files when looking up
> > values. Defaults to off when a specific file is given (e.g., using
> > --file, --global, etc) and on when searching all config files.
>
> IMHO it makes sense the way it is phrased “when a specific file is
> given” but then seems to turn into non-sense when --global is given as
> an example. Giving --global is not “giving a specific file” but
> “restricting to a specific scope”, which may `include` other files.
> The results seem inconsistent and counterintuitive to me.
>
> Am I misunderstanding anything here?
> Is this behaviour intended?
> If it is intended, can someone please explain the rationale behind it? I don’t get it, it seems wrong to me.
The behavior you're seeing is intended. Regarding "a specific scope", I
don't think that's an unreasonable way to think about it. But it's not
how Git thinks about it, and in particular back when --include was added
and this behavior was set, "--global" was literally a synonym for
"--file=$HOME/.gitconfig".
As for the rationale, it is a mix of backwards compatibility and
least-surprise. The include functionality was tacked on to the existing
config parser, and we did not want to surprise anybody who asked for a
specific file by showing them results for another file. This is
especially important for reading untrusted input like .gitmodules, but
also for writing.
> Regarding the initial issue: I just added --includes to the call in
> lbmk and it works just fine, so there is no need to address this. I
> only mentioned it for context to how I got to looking into this
> behaviour.
IMHO lbmk is wrong to be using "--global" in the first place. Looking at
the source, it is trying to check whether the user has set up their
identity. But it is not lbmk's business whether you did it in the
--global config file, or elsewhere! So it should probably just use a
straight "git config user.name", which will do the same resolution that
Git will do internally.
The "--global" was added in their 4a280c62 (.gitcheck: re-write
entirely. force global config., 2023-08-27), but I don't see any
rationale given.
Depending on what they are trying to check, it might be even better
still for it to use "git var GIT_AUTHOR_IDENT". That will give the
actual ident Git will derive, including things like checking $EMAIL in
the environment and so on.
So if the intent is "will Git come up with some ident", then that is the
most accurate way to check it. But if the intent is "did the user
specifically configure Git (because we are worried that values derived
from GECOS and $EMAIL might not be accurate)", then checking user.*
specifically is closer to that.
Though note there is one other hitch, which is that the user can set
author.* and committer.* as specific variables, since 39ab4d0951
(config: allow giving separate author and committer idents, 2019-02-04).
I suspect not many people do that, but that would also be something that
a config-specific check would have to handle (but "git var" would do
automatically).
So I think you might consider sending a bug report to lbmk. Feel free to
point at this thread, and I'm happy to discuss further with them.
-Peff
^ permalink raw reply
* Re: git config: unintuitive behaviour with --global and --no-includes
From: Ben Knoble @ 2026-07-20 12:25 UTC (permalink / raw)
To: Hendrik Jaeger; +Cc: git
In-Reply-To: <20260720113402.0dc16abe@frustcomp.hnjs.home.arpa>
> Le 20 juil. 2026 à 06:02, Hendrik Jaeger <ml_git@henk.geekmail.org> a écrit :
>
> Hi
>
> I ran into a problem working with lbmk (https://codeberg.org/libreboot/lbmk).
> To see whether git is correctly configured, it runs `git config --global user.name` and that failed for my setup.
> The reason is that I have user.name and user.email not directly in the normal git config file but in an included file and when given a scope like --global `git config` does not by default check included files.
[snip]
> Regarding the initial issue: I just added --includes to the call in lbmk and it works just fine, so there is no need to address this.
I wonder why not use « git config user.name » without scope? That seems to sidestep the problem. Unless there’s a reason to use global-scope only? There’s also « git var GIT_AUTHOR_IDENT ».
^ permalink raw reply
* Re: [PATCH v3 0/9] sequencer: do not record dropped commits as rewritten
From: Oswald Buddenhagen @ 2026-07-20 12:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqecgyn5gk.fsf@gitster.g>
On Sun, Jul 19, 2026 at 12:29:31PM -0700, Junio C Hamano wrote:
>It looks like this is now ready to go? Any further comments?
>
you can add whatever footer is appropriate for "i read it, it seems to
make sense, but i didn't double-check" for me.
(same for phillip's new 2-patch series.)
(it feels silly to "spam" the list with such low-value verdicts. i
really miss gerrit code review here, where i'd leave a +1 in passing.)
^ permalink raw reply
* [PATCH v8 4/4] environment: move has_symlinks into repo_config_values
From: Tian Yuchen @ 2026-07-20 10:53 UTC (permalink / raw)
To: git; +Cc: ps, Tian Yuchen, Christian Couder, Ayush Chandekar,
Olamide Caleb Bello
In-Reply-To: <20260720105335.3202013-1-cat@malon.dev>
Move the global 'has_symlinks' configuration into the
repository-specific 'repo_config_values' struct.
Introduce 'repo_has_symlinks()' getter for readability.
Callers access this configuration by passing in 'repo'
when possible, and explicitly fall back to
'the_repository' the rest of the time.
Introduce 'platform_has_symlinks()' macro to allow
platform specific-customization, primarily to help MinGW.
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 | 3 ++-
environment.c | 12 ++++++++++--
environment.h | 4 +++-
git-compat-util.h | 4 ++++
read-cache.c | 7 ++++---
read-cache.h | 3 +--
10 files changed, 42 insertions(+), 15 deletions(-)
diff --git a/apply.c b/apply.c
index edb1502414..b748192ee2 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..5913a8b51f 100644
--- a/entry.c
+++ b/entry.c
@@ -321,7 +321,8 @@ 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 ?
+ state->istate->repo : the_repository) || to_tempfile)
goto write_file_entry;
ret = symlink(new_blob, path);
diff --git a/environment.c b/environment.c
index 32b110c405..e351043446 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,13 @@ int repo_trust_executable_bit(struct repository *repo)
: 1;
}
+int repo_has_symlinks(struct repository *repo)
+{
+ return repo->initialized
+ ? repo_config_values(repo)->has_symlinks
+ : platform_has_symlinks();
+}
+
int have_git_dir(void)
{
return startup_info->have_repository
@@ -336,7 +342,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 +734,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 1f8b5ed15f..c2c3c2e6cc 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -206,7 +206,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:
@@ -324,7 +324,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:
@@ -739,7 +739,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
diff --git a/read-cache.h b/read-cache.h
index 4b54cfc57c..ab9d40aa81 100644
--- a/read-cache.h
+++ b/read-cache.h
@@ -17,8 +17,7 @@ static inline unsigned int ce_mode_from_stat(struct repository *repo,
const struct cache_entry *ce,
unsigned int mode)
{
- extern int has_symlinks;
- if (S_ISREG(mode) && !has_symlinks &&
+ if (S_ISREG(mode) && !repo_has_symlinks(repo) &&
ce && S_ISLNK(ce->ce_mode))
return ce->ce_mode;
if (S_ISREG(mode) && !repo_trust_executable_bit(repo)) {
--
2.43.0
^ permalink raw reply related
* [PATCH v8 3/4] environment: move trust_executable_bit into repo_config_values
From: Tian Yuchen @ 2026-07-20 10:53 UTC (permalink / raw)
To: git; +Cc: ps, Tian Yuchen, Christian Couder, Ayush Chandekar,
Olamide Caleb Bello
In-Reply-To: <20260720105335.3202013-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 | 6 +++---
read-cache.h | 6 +++---
5 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/apply.c b/apply.c
index 26286eb57b..edb1502414 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(state->repo, *ce, st->st_mode);
else if (*ce)
st_mode = (*ce)->ce_mode;
diff --git a/environment.c b/environment.c
index fc3ed8bb1c..32b110c405 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->initialized
+ ? 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 b37bf688ec..1f8b5ed15f 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -208,7 +208,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:
@@ -318,7 +318,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;
@@ -739,7 +739,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
diff --git a/read-cache.h b/read-cache.h
index af8c657ecb..4b54cfc57c 100644
--- a/read-cache.h
+++ b/read-cache.h
@@ -13,15 +13,15 @@
* This function handles degradation for filesystems that lack
* symlink support or reliable executable bits.
*/
-static inline unsigned int ce_mode_from_stat(struct repository *repo UNUSED,
+static inline unsigned int ce_mode_from_stat(struct repository *repo,
const struct cache_entry *ce,
unsigned int mode)
{
- extern int trust_executable_bit, has_symlinks;
+ extern int has_symlinks;
if (S_ISREG(mode) && !has_symlinks &&
ce && S_ISLNK(ce->ce_mode))
return ce->ce_mode;
- if (S_ISREG(mode) && !trust_executable_bit) {
+ if (S_ISREG(mode) && !repo_trust_executable_bit(repo)) {
if (ce && S_ISREG(ce->ce_mode))
return ce->ce_mode;
return create_ce_mode(0666);
--
2.43.0
^ permalink raw reply related
* [PATCH v8 2/4] read-cache: pass 'repo' to 'ce_mode_from_stat()'
From: Tian Yuchen @ 2026-07-20 10:53 UTC (permalink / raw)
To: git; +Cc: ps, Tian Yuchen, Christian Couder, Ayush Chandekar,
Olamide Caleb Bello
In-Reply-To: <20260720105335.3202013-1-cat@malon.dev>
The ce_mode_from_stat() function is a performance-critical static
inline helper in 'read-cache.h'. As we migrate configuration
variables into the repository struct, this helper needs access
to the repository context.
Update the signature of ce_mode_from_stat() to take a 'struct
repository *' parameter, and update all callers to pass the
appropriate repository instance.
To prepare for the overhead of replacing cheap global variable
accesses with getter functions, the boolean expressions are
reordered to evaluate 'S_ISREG(mode)' first.
While at it, add a comment for ce_mode_from_stat().
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 +-
builtin/update-index.c | 2 +-
diff-lib.c | 10 +++++-----
read-cache.c | 2 +-
read-cache.h | 15 ++++++++++++---
5 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/apply.c b/apply.c
index 249248d4f2..26286eb57b 100644
--- a/apply.c
+++ b/apply.c
@@ -3894,7 +3894,7 @@ static int check_preimage(struct apply_state *state,
BUG("ce_mode == 0 for path '%s'", old_name);
if (trust_executable_bit || !S_ISREG(st->st_mode))
- st_mode = ce_mode_from_stat(*ce, st->st_mode);
+ st_mode = ce_mode_from_stat(state->repo, *ce, st->st_mode);
else if (*ce)
st_mode = (*ce)->ce_mode;
else
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 8a5907767b..7917bd286f 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -293,7 +293,7 @@ static int add_one_path(const struct cache_entry *old, const char *path, int len
ce->ce_flags = create_ce_flags(0);
ce->ce_namelen = len;
fill_stat_cache_info(the_repository->index, ce, st);
- ce->ce_mode = ce_mode_from_stat(old, st->st_mode);
+ ce->ce_mode = ce_mode_from_stat(the_repository, old, st->st_mode);
if (index_path(the_repository->index, &ce->oid, path, st,
info_only ? 0 : INDEX_WRITE_OBJECT)) {
diff --git a/diff-lib.c b/diff-lib.c
index ae91027a02..46cae637ec 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -160,7 +160,7 @@ void run_diff_files(struct rev_info *revs, unsigned int option)
changed = check_removed(ce, &st);
if (!changed)
- wt_mode = ce_mode_from_stat(ce, st.st_mode);
+ wt_mode = ce_mode_from_stat(revs->repo, ce, st.st_mode);
else {
if (changed < 0) {
perror(ce->name);
@@ -193,7 +193,7 @@ void run_diff_files(struct rev_info *revs, unsigned int option)
num_compare_stages++;
oidcpy(&dpath->parent[stage - 2].oid,
&nce->oid);
- dpath->parent[stage-2].mode = ce_mode_from_stat(nce, mode);
+ dpath->parent[stage-2].mode = ce_mode_from_stat(revs->repo, nce, mode);
dpath->parent[stage-2].status =
DIFF_STATUS_MODIFIED;
}
@@ -262,7 +262,7 @@ void run_diff_files(struct rev_info *revs, unsigned int option)
continue;
} else if (revs->diffopt.ita_invisible_in_index &&
ce_intent_to_add(ce)) {
- newmode = ce_mode_from_stat(ce, st.st_mode);
+ newmode = ce_mode_from_stat(revs->repo, ce, st.st_mode);
diff_addremove(&revs->diffopt, '+', newmode,
null_oid(the_hash_algo), 0, ce->name, 0);
continue;
@@ -270,7 +270,7 @@ void run_diff_files(struct rev_info *revs, unsigned int option)
changed = match_stat_with_submodule(&revs->diffopt, ce, &st,
ce_option, &dirty_submodule);
- newmode = ce_mode_from_stat(ce, st.st_mode);
+ newmode = ce_mode_from_stat(revs->repo, ce, st.st_mode);
}
if (!changed && !dirty_submodule) {
@@ -338,7 +338,7 @@ static int get_stat_data(const struct cache_entry *ce,
changed = match_stat_with_submodule(diffopt, ce, &st,
0, dirty_submodule);
if (changed) {
- mode = ce_mode_from_stat(ce, st.st_mode);
+ mode = ce_mode_from_stat(diffopt->repo, ce, st.st_mode);
oid = null_oid(the_hash_algo);
}
}
diff --git a/read-cache.c b/read-cache.c
index c44e4d128f..b37bf688ec 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -749,7 +749,7 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
int pos = index_name_pos_also_unmerged(istate, path, namelen);
ent = (0 <= pos) ? istate->cache[pos] : NULL;
- ce->ce_mode = ce_mode_from_stat(ent, st_mode);
+ ce->ce_mode = ce_mode_from_stat(istate->repo, ent, st_mode);
}
/* When core.ignorecase=true, determine if a directory of the same name but differing
diff --git a/read-cache.h b/read-cache.h
index 043da1f1aa..af8c657ecb 100644
--- a/read-cache.h
+++ b/read-cache.h
@@ -4,15 +4,24 @@
#include "read-cache-ll.h"
#include "object.h"
#include "pathspec.h"
+#include "environment.h"
-static inline unsigned int ce_mode_from_stat(const struct cache_entry *ce,
+/*
+ * 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.
+ */
+static inline unsigned int ce_mode_from_stat(struct repository *repo UNUSED,
+ const struct cache_entry *ce,
unsigned int mode)
{
extern int trust_executable_bit, has_symlinks;
- if (!has_symlinks && S_ISREG(mode) &&
+ if (S_ISREG(mode) && !has_symlinks &&
ce && S_ISLNK(ce->ce_mode))
return ce->ce_mode;
- if (!trust_executable_bit && S_ISREG(mode)) {
+ if (S_ISREG(mode) && !trust_executable_bit) {
if (ce && S_ISREG(ce->ce_mode))
return ce->ce_mode;
return create_ce_mode(0666);
--
2.43.0
^ permalink raw reply related
* [PATCH v8 1/4] read-cache: remove redundant extern declarations
From: Tian Yuchen @ 2026-07-20 10:53 UTC (permalink / raw)
To: git; +Cc: ps, Tian Yuchen, Christian Couder, Ayush Chandekar,
Olamide Caleb Bello
In-Reply-To: <20260720105335.3202013-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 v8 0/4] environment: migrate 'trust_executable_bit' and 'has_symlinks' into 'repo_config_values'
From: Tian Yuchen @ 2026-07-20 10:53 UTC (permalink / raw)
To: git; +Cc: ps, Tian Yuchen
In-Reply-To: <20260717063559.1633567-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?
Changes since V7:
- In commit 2/4, mark the 'struct repository' parameter of
ce_mode_from_stat() UNUSED. In commit 3/4, drop it.
- Don't check '!repo' in the getters, which lets the developers know
there is a bug to fix when NULL is passed in. Callers should be
responsible of passing non-null repos. Therefore, adjust the call in
write_entry().
Change back to check 'repo->initiaized' instead of 'repo->gitdir'.
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: pass 'repo' to 'ce_mode_from_stat()'
environment: move trust_executable_bit into repo_config_values
environment: move has_symlinks into repo_config_values
apply.c | 6 +++---
builtin/update-index.c | 2 +-
combine-diff.c | 2 +-
compat/mingw.c | 17 +++++++++++++----
compat/mingw.h | 3 +++
diff-lib.c | 10 +++++-----
entry.c | 3 ++-
environment.c | 23 +++++++++++++++++++----
environment.h | 8 ++++++--
git-compat-util.h | 4 ++++
read-cache.c | 15 +++++++--------
read-cache.h | 16 ++++++++++++----
12 files changed, 76 insertions(+), 33 deletions(-)
--
2.43.0
^ permalink raw reply
* Re: [RFC PATCH 1/7] builtin/repack.c: add --drop-filtered and --dry-run options
From: Siddharth Shrimali @ 2026-07-20 10:19 UTC (permalink / raw)
To: Christian Couder
Cc: git, gitster, siddharthasthana31, me, ps, johannes.schindelin,
l.s.r, karthik nayak
In-Reply-To: <CAP8UFD2ZNmWh4fjh+vFvKCihfebg2yif9=xLjqpKZFgF-O0RSg@mail.gmail.com>
Hey Christian,
On Sat, 18 Jul 2026 at 18:01, Christian Couder
<christian.couder@gmail.com> wrote:
> An alternative would be `--drop-filtered[=dry-run]`, which might be
> extended with other `--drop-filtered` specific options later.
>
> I think separating `--dry-run` from `--drop-filtered` like this patch
> does makes sense though if we think that `--dry-run` could be useful
> later without `--drop-filtered`. The fact that a number of other
> commands already have a `--dry-run` option might be a good sign.
Agreed. I chose a separate --dry-run mainly for consistency with the many other
Git commands that already use it that way, and because it leaves room
for --dry-run
to apply to other repack behavior later rather than being tied to
--drop-filtered.
That said, I do like the --drop-filtered[=dry-run] form for keeping future
--drop-filtered-specific options together, and it might end up being the tidier
choice.
Since this is an RFC, I'd genuinely welcome others' thoughts on which one
holds up better in the long run, before settling on any one command-line.
>
> Anyway it would be nice if the commit message explained a bit the
> choice to have a separate `--dry-run` option.
Either way, for v2 I'll add a note to the commit message explaining
the final choice.
Thanks!
^ permalink raw reply
* Re: [PATCH v7 2/4] read-cache: pass 'repo' to 'ce_mode_from_stat()'
From: Tian Yuchen @ 2026-07-20 10:12 UTC (permalink / raw)
To: SZEDER Gábor
Cc: git, ps, Christian Couder, Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <al3v0NVZJYS9SVZF@szeder.dev>
On 7/20/26 17:52, SZEDER Gábor wrote:
> On Mon, Jul 20, 2026 at 05:13:17PM +0800, Tian Yuchen wrote:
>> On 7/19/26 03:02, SZEDER Gábor wrote:
>>> On Fri, Jul 17, 2026 at 02:35:57PM +0800, Tian Yuchen wrote:
>>>> diff --git a/read-cache.h b/read-cache.h
>>>> index 043da1f1aa..94b8d3e547 100644
>>>> --- a/read-cache.h
>>>> +++ b/read-cache.h
>>>> @@ -4,15 +4,24 @@
>>>> #include "read-cache-ll.h"
>>>> #include "object.h"
>>>> #include "pathspec.h"
>>>> +#include "environment.h"
>>>> -static inline unsigned int ce_mode_from_stat(const struct cache_entry *ce,
>>>> +/*
>>>> + * 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.
>>>> + */
>>>> +static inline unsigned int ce_mode_from_stat(struct repository *repo,
>>>
>>> This new parameter is not yet used in this function, which causes
>>> compilation errors in all source files which include "read-cache.h"
>>> when trying to build this commit using DEVELOPER=1, e.g.:
>
>>> I think the new parameter should be marked as UNUSED in this patch,
>>> and then the UNUSED should be dropped in the next, where you start
>>> using the parameter.
>>>
>>>> + const struct cache_entry *ce,
>>>> unsigned int mode)
>>>> {
>>>> extern int trust_executable_bit, has_symlinks;
>>>> - if (!has_symlinks && S_ISREG(mode) &&
>>>> + if (S_ISREG(mode) && !has_symlinks &&
>>>> ce && S_ISLNK(ce->ce_mode))
>>>> return ce->ce_mode;
>>>> - if (!trust_executable_bit && S_ISREG(mode)) {
>>>> + if (S_ISREG(mode) && !trust_executable_bit) {
>>>> if (ce && S_ISREG(ce->ce_mode))
>>>> return ce->ce_mode;
>>>> return create_ce_mode(0666);
>>>> --
>>>> 2.43.0
>>>>
>>
>> But 'USUSED' cannot be used here since the corresponding header
>> (git-compat-util.h, or more specifically compat/posix.h) is not included.
>
> UNUSED _can_ be used here, because:
>
> - This is a header file, so it's not supposed to be compiled on its
> own.
> - All C source files including this header file must start with
> including "git-compat-util.h", so by the time they include
> "read-cache.h", the UNUSED macro is already defined.
>
I see.
Thanks, yuchen
^ permalink raw reply
* Re: [PATCH v7 3/4] environment: move trust_executable_bit into repo_config_values
From: Tian Yuchen @ 2026-07-20 10:09 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, ps, Christian Couder, Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <xmqq8q7961xe.fsf@gitster.g>
On 7/18/26 00:01, Junio C Hamano wrote:
> Tian Yuchen <cat@malon.dev> writes:
>
>> 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
>
> Two comments.
>
> * Missing SP before '?'. It may be easier to read if it is written
> like this:
>
> return repo->gitdir
> ? repo_config_values(repo)->trust_executable_bit
> : 1;
>
> which more clearly highlights the ternary structure. If you tilt
> your head 90 degrees to the left, you can almost see the parse
> tree of the expression.
>
Okay.
> * Does it make sense to protect against a NULL 'repo' case, as
> repo_protect_ntfs() and repo_protect_hfs() helpers do? Or is it
> better to crash loudly with a segfault to let the developer know
> they have a bug to fix? I lean toward the latter myself, and if
> we go that route, we should probably stop using 'repo &&
> repo->gitdir' elsewhere, rather than sweeping the problem under
> the rug with defensive checks.
Sounds sensible. I will drop the checks and adjust the calls themselves
instead.
Note that repo_protect_ntfs() looks like this:
int repo_protect_ntfs(struct repository *repo)
{
return (repo && repo->initialized) ?
repo_config_values(repo)->protect_ntfs :
PROTECT_NTFS_DEFAULT;
}
Do we need another fixup commit for it?
Regards, yuchen
^ permalink raw reply
* git config: unintuitive behaviour with --global and --no-includes
From: Hendrik Jaeger @ 2026-07-20 9:34 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 4788 bytes --]
Hi
I ran into a problem working with lbmk (https://codeberg.org/libreboot/lbmk).
To see whether git is correctly configured, it runs `git config --global user.name` and that failed for my setup.
The reason is that I have user.name and user.email not directly in the normal git config file but in an included file and when given a scope like --global `git config` does not by default check included files.
I tried to create a commented minimal example showing the problem with that:
```
# no config exists
~ % cat .gitconfig
cat: .gitconfig: No such file or directory
~ % cat .gitconfig_personal
cat: .gitconfig_personal: No such file or directory
~ % ls ~/.config/git
ls: cannot access '/home/resu/.config/git': No such file or directory
# config var is not set
~ % git config --show-scope --show-origin user.name
# set user.name
~ % git config --global user.name "Hendrik Jäger"
# check if it is set
~ % cat .gitconfig
[user]
name = Hendrik Jäger
# check where it is set
~ % git config --show-scope --show-origin user.name
global file:/home/resu/.gitconfig Hendrik Jäger
# check whether we can still retrieve it when explicitly giving the scope
~ % git config --show-scope --show-origin --global user.name
global file:/home/resu/.gitconfig Hendrik Jäger
# move setting to non-standard file
~ % cat .gitconfig > .gitconfig_personal
# include that non-standard file
~ % echo '[include]\npath = .gitconfig_personal' >| .gitconfig
# check config status
~ % cat .gitconfig
[include]
path = .gitconfig_personal
~ % cat .gitconfig_personal
[user]
name = Hendrik Jäger
# check whether git still finds that setting
~ % git config --show-scope --show-origin user.name
global file:/home/resu/.gitconfig_personal Hendrik Jäger
# check whether git still finds that setting in the scope it is in
~ % git config --show-scope --show-origin --global user.name
# set it again in the global scope
~ % git config --global user.name "Henk Hunter"
# check again
~ % git config --show-scope --show-origin user.name
global file:/home/resu/.gitconfig Henk Hunter
# check again with specific scope
~ % git config --show-scope --show-origin --global user.name
global file:/home/resu/.gitconfig Henk Hunter
# reset git config and check if the setting is really gone
~ % rm .gitconfig
~ % git config --show-scope --show-origin user.name
# set it again in global scope with different value
~ % git config --global user.name "Henk Hunter"
# check whether setting it was successful
~ % git config --show-scope --show-origin user.name
global file:/home/resu/.gitconfig Henk Hunter
# check again with specific scope
~ % git config --show-scope --show-origin --global user.name
global file:/home/resu/.gitconfig Henk Hunter
# add the include back
~ % echo '[include]\npath = .gitconfig_personal' >> .gitconfig
# check config status
~ % cat .gitconfig
[user]
name = Henk Hunter
[include]
path = .gitconfig_personal
# check the value and from which scope it comes
~ % git config --show-scope --show-origin user.name
global file:/home/resu/.gitconfig_personal Hendrik Jäger
# check again with specific scope
~ % git config --show-scope --show-origin --global user.name
global file:/home/resu/.gitconfig Henk Hunter
# check again while allowing includes
~ % git config --show-scope --show-origin --includes user.name
global file:/home/resu/.gitconfig_personal Hendrik Jäger
~ % git config --show-scope --show-origin --global --includes user.name
global file:/home/resu/.gitconfig_personal Hendrik Jäger
```
The manpage says:
> Respect include.* directives in config files when looking up values. Defaults to off when a specific file is given (e.g., using --file, --global, etc) and on when searching all config files.
IMHO it makes sense the way it is phrased “when a specific file is given” but then seems to turn into non-sense when --global is given as an example. Giving --global is not “giving a specific file” but “restricting to a specific scope”, which may `include` other files.
The results seem inconsistent and counterintuitive to me.
Am I misunderstanding anything here?
Is this behaviour intended?
If it is intended, can someone please explain the rationale behind it? I don’t get it, it seems wrong to me.
Regarding the initial issue: I just added --includes to the call in lbmk and it works just fine, so there is no need to address this. I only mentioned it for context to how I got to looking into this behaviour.
If any relevant information is missing in this bugreport, I’ll be happy to add it, please let me know!
Thank you very much
henk
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 0/4] send-pack: introduce a `no-ref-delta` capability
From: Jeff King @ 2026-07-20 9:57 UTC (permalink / raw)
To: Taylor Blau; +Cc: git, Junio C Hamano
In-Reply-To: <alvp8KsZPKqCyqma@com-79390>
On Sat, Jul 18, 2026 at 04:02:40PM -0500, Taylor Blau wrote:
> > The problem there is not REF_DELTA itself, but the fact that REF_DELTA
> > allows you to place a base after the delta which depends on it.
>
> Exactly.
>
> > If _that_ is your main concern, would it be worth a tighter capability
> > advertisement that insists that bases come before their deltas (if they
> > are in the pack at all)? We already generate packs that way by default,
> > and it would really just give the server a license to reject these
> > non-standard packs.
>
> That would address the ordering problem, but is weaker than the format
> restriction this receiver wants. Even a backward REF_DELTA requires an
> OID-to-entry lookup, whereas the retained pack's reconstruction metadata
> is addressed by offset alone. Supporting that is possible, of course,
> but adds another way to locate a base.
Yeah, but I don't think it's that much more complicated. You are
collecting the oids of the stuff you index anyway (since that is the
point of indexing), so it is just a matter of storing that in a
searchable data structure.
But what _is_ more complicated is the data dependency. Imagine you have
a pool of workers waiting to do delta resolution and hash computations.
You want to hand off each new object entry you parse to one of the
workers. With OFS_DELTA you know where the base is immediately, and if
its resolution is still pending, you know which worker you handed it off
to. But with REF_DELTA, you don't know which worker is processing your
base until it has finished (since that's when it reports back the oid).
That might or might not matter depending on your caching strategy for
intermediate states. For example, if you're trying to maintain locality
in what you hand to a worker (so if you have a delta chain A-B-C, when
you find C you want to give it to the worker who computed B, because
they may have that intermediate result at hand).
I do think in general that the intermediate-state caching is going to be
the trickiest part of a streaming resolution, though. Even with just
OFS_DELTA, there is no way to know in a single pass that the object at
position N is worth keeping around because N+K is going to reference it,
and K can be arbitrarily large. With Git's pack-objects implementation,
you will generally see delta families grouped together, but that's not
guaranteed.
So I'm growing more convinced that asking the client not to send
REF_DELTAs might open up some more opportunities for optimizations, but:
1. It would be neat if we could see those optimizations in git.git's
receiving code. ;)
2. Without seeing the whole scheme, I am still unclear on how some of
these other complications aren't much worse/harder.
I don't think you're asking for a lot of support on the git.git side, so
in that sense I'm not really opposed. I'm just not excited about
carrying protocol additions that wouldn't be used by git.git itself,
especially if we later find that the capability isn't a perfect match
for the optimizations we want to make.
-Peff
^ permalink raw reply
* Re: [PATCH] userdiff: add support for Swift
From: Shlok Kulshreshtha @ 2026-07-20 9:52 UTC (permalink / raw)
To: j6t
Cc: Shlok Kulshreshtha, D. Ben Knoble, Junio C Hamano,
René Scharfe, Eric Sunshine, Scott L. Burson, git
In-Reply-To: <2a3a73c5-5e90-44a3-bf6a-6e98ce5e5a59@kdbg.org>
Johannes Sixt <j6t@kdbg.org> writes:
>> - attributes, with or without arguments, whether on their own line
>> ("@objc" above a "func") or inline with the declaration
> AFAIC, the regular expression does not match attributes on their own
> line. What relevance does this statement have?
You are right. The pattern only matches attributes that are inline with
the declaration. An attribute on its own line is not matched, and does
not need to be, because the declaration line below it matches on its own.
I have reworded this.
> This test contains "RIGHT" twice. This is not good, because we do not
> know which one is picked.
[...]
> Again "RIGHT" twice in a harmful way.
Fixed in a coming v2: swift-init, swift-failable-init and
swift-generic-subscript now contain "RIGHT" only once, on the
declaration line.
> It may be worth considering to enumerate all keywords and permit any
> run of them:
> (public|final|etc.|func|init|...|actor)[ \t(?!<]+)+
Noted, and thanks for the follow-up on this one. I did check it anyway
out of curiosity: with that shape, a line that is only modifiers and
never reaches a real declaration keyword, such as
public var counter = 0
would still match, because it merges modifiers and declaration keywords
into one interchangeable run. The current pattern requires a real
keyword at the end, so that line correctly gets no header. I will keep
the current form for now, and can revisit if the backtracking turns out
to matter in practice.
> You could just throw all of them into a single pattern like this:
> 0[xXoObB][0-9a-fA-F_]+
> except when, for example, 0b1_abc
Right -- that is why I kept them as three separate patterns, so the
digit ranges stay correct (binary [01], octal [0-7]); merging would
mis-tokenize "0b1_abc".
> Is ".5" a correct floating-point number?
No -- Swift requires a leading digit, so ".5" is a syntax error (one must
write "0.5"). Tokenizing it as "." and "5" is therefore fine, and it
does not occur in valid Swift.
> You do not have to account for single-character operators; they are
> automatic. Drop the "?" from the first "=?".
Done in a coming v2, thanks; I had not realized PATTERNS appends
"|[^[:space:]]". It is a nice simplification, and it only touches the
word regex, not the funcname pattern.
Since neither of us speaks Swift, for your ease of judgement I have also
put together some coverage numbers, which the coming v2 cover note will
include:
- Grammar: I went through every declaration form listed in the "Summary
of the Grammar" in Swift's own language reference (func, init incl.
failable/generic, deinit, subscript incl. generic, class, struct,
enum, protocol, extension, actor, operator methods, stacked
modifiers, attributes with and without arguments, "where" clauses,
multi-line signatures -- 26 forms total) and wrote a case for each.
All 26 get the correct header.
- Real-world code: I ran the driver over the last 200 commits touching
*.swift in seven different Swift projects -- Alamofire,
apple/swift-argument-parser, vapor, Kingfisher, RxSwift, SnapKit, and
pointfreeco/swift-composable-architecture -- and checked every hunk
header by hand. Out of 20454 hunks, 15310 got a header, and 15296 of
those (99.9%) named a real declaration. None of the empty-header
hunks turned out to be a real miss (they were things like file
comment blocks, imports, or Package.swift, which have nothing to
attach a header to).
These numbers are unaffected by the changes in this reply: the funcname
pattern is identical in v1 and v2 (only the word regex and the test
files changed), and both measurements are of hunk headers, which come
from the funcname pattern alone. So the coverage above still holds for
v2.
Besides the fixes above, v2 will also carry the reworded attribute
description and the changelog explaining what changed since v1, so the
full picture is in one place when you look at it.
Thanks for the careful review.
Shlok
^ permalink raw reply
* Re: [PATCH v7 2/4] read-cache: pass 'repo' to 'ce_mode_from_stat()'
From: SZEDER Gábor @ 2026-07-20 9:52 UTC (permalink / raw)
To: Tian Yuchen
Cc: git, ps, Christian Couder, Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <8b9528b8-198b-489f-8f0e-fbd0c7d07b64@malon.dev>
On Mon, Jul 20, 2026 at 05:13:17PM +0800, Tian Yuchen wrote:
> On 7/19/26 03:02, SZEDER Gábor wrote:
> > On Fri, Jul 17, 2026 at 02:35:57PM +0800, Tian Yuchen wrote:
> > > diff --git a/read-cache.h b/read-cache.h
> > > index 043da1f1aa..94b8d3e547 100644
> > > --- a/read-cache.h
> > > +++ b/read-cache.h
> > > @@ -4,15 +4,24 @@
> > > #include "read-cache-ll.h"
> > > #include "object.h"
> > > #include "pathspec.h"
> > > +#include "environment.h"
> > > -static inline unsigned int ce_mode_from_stat(const struct cache_entry *ce,
> > > +/*
> > > + * 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.
> > > + */
> > > +static inline unsigned int ce_mode_from_stat(struct repository *repo,
> >
> > This new parameter is not yet used in this function, which causes
> > compilation errors in all source files which include "read-cache.h"
> > when trying to build this commit using DEVELOPER=1, e.g.:
> > I think the new parameter should be marked as UNUSED in this patch,
> > and then the UNUSED should be dropped in the next, where you start
> > using the parameter.
> >
> > > + const struct cache_entry *ce,
> > > unsigned int mode)
> > > {
> > > extern int trust_executable_bit, has_symlinks;
> > > - if (!has_symlinks && S_ISREG(mode) &&
> > > + if (S_ISREG(mode) && !has_symlinks &&
> > > ce && S_ISLNK(ce->ce_mode))
> > > return ce->ce_mode;
> > > - if (!trust_executable_bit && S_ISREG(mode)) {
> > > + if (S_ISREG(mode) && !trust_executable_bit) {
> > > if (ce && S_ISREG(ce->ce_mode))
> > > return ce->ce_mode;
> > > return create_ce_mode(0666);
> > > --
> > > 2.43.0
> > >
>
> But 'USUSED' cannot be used here since the corresponding header
> (git-compat-util.h, or more specifically compat/posix.h) is not included.
UNUSED _can_ be used here, because:
- This is a header file, so it's not supposed to be compiled on its
own.
- All C source files including this header file must start with
including "git-compat-util.h", so by the time they include
"read-cache.h", the UNUSED macro is already defined.
^ permalink raw reply
* Re: [PATCH 3/4] last-modified: check pathspec against Bloom filter first
From: Jeff King @ 2026-07-20 9:42 UTC (permalink / raw)
To: Taylor Blau; +Cc: Toon Claes, git, Gusted
In-Reply-To: <alvulw2fk67duo8n@com-79390>
On Sat, Jul 18, 2026 at 04:22:31PM -0500, Taylor Blau wrote:
> I think that we could feasibly get rid of "d" in the output in this
> particular case within last-modified. As you note, the command is marked
> EXPERIMENTAL for a reason, after all ;-).
>
> If we wanted to do that, it should be straightforward to do. I think the
> following (untested) patch would be sufficient:
>
> --- 8< ---
> diff --git a/builtin/last-modified.c b/builtin/last-modified.c
> index adc7cd8c74..0f0c1d1d17 100644
> --- a/builtin/last-modified.c
> +++ b/builtin/last-modified.c
> @@ -103,7 +103,7 @@ struct last_modified_callback_data {
> };
>
> static void add_path_from_diff(struct diff_queue_struct *q,
> - struct diff_options *opt UNUSED, void *data)
> + struct diff_options *opt, void *data)
> {
> struct last_modified *lm = data;
>
> @@ -112,6 +112,11 @@ static void add_path_from_diff(struct diff_queue_struct *q,
> struct last_modified_entry *ent;
> const char *path = p->two->path;
>
> + if (!match_pathspec(opt->repo->index, &opt->pathspec, path,
> + strlen(path), 0, NULL,
> + S_ISDIR(p->two->mode)))
> + continue;
> +
Yeah, that was exactly what I was thinking, but I wasn't sure if
match_pathspec() was the right tool. I mean, obviously it sounds like it
should be from the name, but I don't think it is actually what is used
in tree-diffs! There we have tree-walk.c:do_match() which does some
magic. And match_pathspec() is used more for dir.c callers.
I guess the two are supposed to be equivalent, or else we'd have weird
discrepancies between commands. So maybe just a weird existing oddity
that we don't need to worry about here.
There is one other interesting corner case here. If I do this in
git.git, for example:
git last-modified -t Documentation/technical/
it shows an entry for Documentation/, which we both find weird. And the
patch above would remove that. But it also shows an entry for
Documentation/technical/, which _is_ within the pathspec and would still
be shown after the patch above. That's OK for the optimization we're
talking about (it would be part of the filter key), but I do find it
still a little funny. The invocation above, at least as we used to use
it as blame-tree at GitHub, is really about asking for the entries
inside that directory, not the directory itself.
Perhaps not worth worrying too much about, though. The caller can easily
ignore the extra entry.
> If, on the other hand, we wanted to retain "d" in the output (which I am
> inclined to suggest is a bad idea), we could keep a list of paths which
> are not covered by the given pathspec.
Yeah, your analysis here makes sense, but I agree that it is not worth
retaining "d". Besides reducing our ability to optimize, it is IMHO just
plain confusing to have in the output.
-Peff
^ permalink raw reply
* Re: [PATCH v7 2/4] read-cache: pass 'repo' to 'ce_mode_from_stat()'
From: Tian Yuchen @ 2026-07-20 9:13 UTC (permalink / raw)
To: SZEDER Gábor
Cc: git, ps, Christian Couder, Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <alvNq8rXF/jofqUc@szeder.dev>
On 7/19/26 03:02, SZEDER Gábor wrote:
> On Fri, Jul 17, 2026 at 02:35:57PM +0800, Tian Yuchen wrote:
>> The ce_mode_from_stat() function is a performance-critical static
>> inline helper in 'read-cache.h'. As we migrate configuration
>> variables into the repository struct, this helper needs access
>> to the repository context.
>>
>> Update the signature of ce_mode_from_stat() to take a 'struct
>> repository *' parameter, and update all callers to pass the
>> appropriate repository instance.
>>
>> To prepare for the overhead of replacing cheap global variable
>> accesses with getter functions, the boolean expressions are
>> reordered to evaluate 'S_ISREG(mode)' first.
>>
>> While at it, add a comment for ce_mode_from_stat().
>>
>> 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>
>> ---
>
>> diff --git a/read-cache.h b/read-cache.h
>> index 043da1f1aa..94b8d3e547 100644
>> --- a/read-cache.h
>> +++ b/read-cache.h
>> @@ -4,15 +4,24 @@
>> #include "read-cache-ll.h"
>> #include "object.h"
>> #include "pathspec.h"
>> +#include "environment.h"
>>
>> -static inline unsigned int ce_mode_from_stat(const struct cache_entry *ce,
>> +/*
>> + * 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.
>> + */
>> +static inline unsigned int ce_mode_from_stat(struct repository *repo,
>
> This new parameter is not yet used in this function, which causes
> compilation errors in all source files which include "read-cache.h"
> when trying to build this commit using DEVELOPER=1, e.g.:
>
> CC pathspec.o
> In file included from pathspec.c:11:
> read-cache.h: In function ‘ce_mode_from_stat’:
> read-cache.h:16:65: error: unused parameter ‘repo’ [-Werror=unused-parameter]
> 16 | static inline unsigned int ce_mode_from_stat(struct repository *repo,
> | ~~~~~~~~~~~~~~~~~~~^~~~
> cc1: all warnings being treated as errors
> make: *** [Makefile:2921: pathspec.o] Error 1
> CC preload-index.o
> In file included from preload-index.c:16:
> read-cache.h: In function ‘ce_mode_from_stat’:
> read-cache.h:16:65: error: unused parameter ‘repo’ [-Werror=unused-parameter]
> 16 | static inline unsigned int ce_mode_from_stat(struct repository *repo,
> | ~~~~~~~~~~~~~~~~~~~^~~~
> cc1: all warnings being treated as errors
> make: *** [Makefile:2921: preload-index.o] Error 1
> CC read-cache.o
> In file included from read-cache.c:34:
> read-cache.h: In function ‘ce_mode_from_stat’:
> read-cache.h:16:65: error: unused parameter ‘repo’ [-Werror=unused-parameter]
> 16 | static inline unsigned int ce_mode_from_stat(struct repository *repo,
> | ~~~~~~~~~~~~~~~~~~~^~~~
> cc1: all warnings being treated as errors
> make: *** [Makefile:2921: read-cache.o] Error 1
>
Nice catch.
> I think the new parameter should be marked as UNUSED in this patch,
> and then the UNUSED should be dropped in the next, where you start
> using the parameter.
>
>> + const struct cache_entry *ce,
>> unsigned int mode)
>> {
>> extern int trust_executable_bit, has_symlinks;
>> - if (!has_symlinks && S_ISREG(mode) &&
>> + if (S_ISREG(mode) && !has_symlinks &&
>> ce && S_ISLNK(ce->ce_mode))
>> return ce->ce_mode;
>> - if (!trust_executable_bit && S_ISREG(mode)) {
>> + if (S_ISREG(mode) && !trust_executable_bit) {
>> if (ce && S_ISREG(ce->ce_mode))
>> return ce->ce_mode;
>> return create_ce_mode(0666);
>> --
>> 2.43.0
>>
But 'USUSED' cannot be used here since the corresponding header
(git-compat-util.h, or more specifically compat/posix.h) is not included.
Can we write..
(void)repo; /* TODO: use this parameter in the next patch */
..to keep it simple?
Regards, yuchen
^ permalink raw reply
* [PATCH v3 2/2] bisect: add --reset-when-found to leave when done
From: Harald Nordgren via GitGitGadget @ 2026-07-20 9:10 UTC (permalink / raw)
To: git; +Cc: Johannes Sixt, Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2335.v3.git.git.1784538619.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
When a bisection finishes, "git bisect" reports the first bad commit
but leaves the session active until "git bisect reset" is run by hand.
Add a "--reset-when-found[=<where>]" option, accepted by both "git
bisect start" and "git bisect run", that resets as soon as the first
bad commit is found. The "original" value returns to the commit checked
out before "git bisect start", while "found" leaves the first bad commit
checked out; omitting the value defaults to "original".
Persist the selected target in a BISECT_RESET_WHEN_FOUND state file
and perform the reset quietly.
For "git bisect run", defer the reset until after the captured output
is printed and BISECT_RUN is closed. This lets cleanup remove the file
on systems that cannot unlink an open file.
Reject this option together with "--no-checkout", since that mode must
not check out either target.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
Documentation/git-bisect.adoc | 14 +++-
bisect.c | 2 +
builtin/bisect.c | 134 ++++++++++++++++++++++++++++++----
t/t6030-bisect-porcelain.sh | 109 +++++++++++++++++++++++++++
4 files changed, 243 insertions(+), 16 deletions(-)
diff --git a/Documentation/git-bisect.adoc b/Documentation/git-bisect.adoc
index d2115b2990..aabddd42ca 100644
--- a/Documentation/git-bisect.adoc
+++ b/Documentation/git-bisect.adoc
@@ -10,7 +10,7 @@ SYNOPSIS
--------
[synopsis]
git bisect start [--term-(bad|new)=<term-new> --term-(good|old)=<term-old>]
- [--no-checkout] [--first-parent] [<bad> [<good>...]] [--] [<pathspec>...]
+ [--no-checkout] [--first-parent] [--reset-when-found[=<where>]] [<bad> [<good>...]] [--] [<pathspec>...]
git bisect (bad|new|<term-new>) [<rev>]
git bisect (good|old|<term-old>) [<rev>...]
git bisect terms [--term-(good|old) | --term-(bad|new)]
@@ -20,7 +20,7 @@ git bisect reset [<commit>]
git bisect (visualize|view)
git bisect replay <logfile>
git bisect log
-git bisect run <cmd> [<arg>...]
+git bisect run [--reset-when-found[=<where>]] <cmd> [<arg>...]
git bisect help
DESCRIPTION
@@ -385,6 +385,16 @@ ignored.
This option is particularly useful in avoiding false positives when a merged
branch contained broken or non-buildable commits, but the merge itself was OK.
+`--reset-when-found[=<where>]`::
+ Once the first bad commit is found, report it and clean up the
+ bisection state. `<where>` may be `original` to return to the commit
+ checked out before `git bisect start`, or `found` to leave the first
+ bad commit checked out. If `<where>` is omitted, it defaults to
+ `original`.
++
+This option may be given to `git bisect start` or to `git bisect run`. It
+cannot be used for a bisection started with `--no-checkout`.
+
EXAMPLES
--------
diff --git a/bisect.c b/bisect.c
index 94c7028d2a..d426fcd5a9 100644
--- a/bisect.c
+++ b/bisect.c
@@ -488,6 +488,7 @@ static GIT_PATH_FUNC(git_path_bisect_start, "BISECT_START")
static GIT_PATH_FUNC(git_path_bisect_log, "BISECT_LOG")
static GIT_PATH_FUNC(git_path_bisect_terms, "BISECT_TERMS")
static GIT_PATH_FUNC(git_path_bisect_first_parent, "BISECT_FIRST_PARENT")
+static GIT_PATH_FUNC(git_path_bisect_reset_when_found, "BISECT_RESET_WHEN_FOUND")
static void read_bisect_paths(struct strvec *array)
{
@@ -1211,6 +1212,7 @@ int bisect_clean_state(void)
unlink_or_warn(git_path_bisect_run());
unlink_or_warn(git_path_bisect_terms());
unlink_or_warn(git_path_bisect_first_parent());
+ unlink_or_warn(git_path_bisect_reset_when_found());
/*
* Cleanup BISECT_START last to support the --no-checkout option
* introduced in the commit 4796e823a.
diff --git a/builtin/bisect.c b/builtin/bisect.c
index 0e49ca23ae..de13f22f8a 100644
--- a/builtin/bisect.c
+++ b/builtin/bisect.c
@@ -24,11 +24,12 @@ static GIT_PATH_FUNC(git_path_bisect_start, "BISECT_START")
static GIT_PATH_FUNC(git_path_bisect_log, "BISECT_LOG")
static GIT_PATH_FUNC(git_path_bisect_names, "BISECT_NAMES")
static GIT_PATH_FUNC(git_path_bisect_first_parent, "BISECT_FIRST_PARENT")
+static GIT_PATH_FUNC(git_path_bisect_reset_when_found, "BISECT_RESET_WHEN_FOUND")
static GIT_PATH_FUNC(git_path_bisect_run, "BISECT_RUN")
#define BUILTIN_GIT_BISECT_START_USAGE \
N_("git bisect start [--term-(bad|new)=<term-new> --term-(good|old)=<term-old>]\n" \
- " [--no-checkout] [--first-parent] [<bad> [<good>...]] [--] [<pathspec>...]")
+ " [--no-checkout] [--first-parent] [--reset-when-found[=<where>]] [<bad> [<good>...]] [--] [<pathspec>...]")
#define BUILTIN_GIT_BISECT_BAD_USAGE \
N_("git bisect (bad|new|<term-new>) [<rev>]")
#define BUILTIN_GIT_BISECT_GOOD_USAGE \
@@ -48,7 +49,7 @@ static GIT_PATH_FUNC(git_path_bisect_run, "BISECT_RUN")
#define BUILTIN_GIT_BISECT_LOG_USAGE \
"git bisect log"
#define BUILTIN_GIT_BISECT_RUN_USAGE \
- N_("git bisect run <cmd> [<arg>...]")
+ N_("git bisect run [--reset-when-found[=<where>]] <cmd> [<arg>...]")
#define BUILTIN_GIT_BISECT_HELP_USAGE \
"git bisect help"
@@ -68,6 +69,12 @@ static const char * const git_bisect_usage[] = {
NULL
};
+enum reset_when_found_mode {
+ RESET_WHEN_FOUND_NONE,
+ RESET_WHEN_FOUND_TO_ORIGINAL,
+ RESET_WHEN_FOUND_TO_FOUND,
+};
+
struct add_bisect_ref_data {
struct rev_info *revs;
unsigned int object_flags;
@@ -272,6 +279,61 @@ static int bisect_reset(const char *commit, int quiet)
return bisect_clean_state();
}
+static int parse_reset_when_found(const char *value,
+ enum reset_when_found_mode *mode)
+{
+ if (!strcmp(value, "original"))
+ *mode = RESET_WHEN_FOUND_TO_ORIGINAL;
+ else if (!strcmp(value, "found"))
+ *mode = RESET_WHEN_FOUND_TO_FOUND;
+ else
+ return error(_("invalid value for '--reset-when-found': '%s'"),
+ value);
+
+ return 0;
+}
+
+static const char *reset_when_found_mode_name(enum reset_when_found_mode mode)
+{
+ switch (mode) {
+ case RESET_WHEN_FOUND_TO_ORIGINAL:
+ return "original";
+ case RESET_WHEN_FOUND_TO_FOUND:
+ return "found";
+ case RESET_WHEN_FOUND_NONE:
+ BUG("no name for unset reset-when-found mode");
+ }
+ BUG("unknown reset-when-found mode %d", mode);
+}
+
+static int bisect_reset_when_found(struct bisect_terms *terms)
+{
+ struct strbuf value = STRBUF_INIT;
+ enum reset_when_found_mode mode;
+ char *commit = NULL;
+ int res;
+
+ if (strbuf_read_file(&value, git_path_bisect_reset_when_found(), 0) < 0) {
+ res = error_errno(_("could not read '%s'"),
+ git_path_bisect_reset_when_found());
+ goto cleanup;
+ }
+ strbuf_trim(&value);
+ if (parse_reset_when_found(value.buf, &mode)) {
+ res = -1;
+ goto cleanup;
+ }
+
+ if (mode == RESET_WHEN_FOUND_TO_FOUND)
+ commit = xstrfmt("refs/bisect/%s", terms->term_bad);
+ res = bisect_reset(commit, 1);
+
+cleanup:
+ free(commit);
+ strbuf_release(&value);
+ return res;
+}
+
static void log_commit(FILE *fp,
const char *fmt, const char *state,
struct commit *commit)
@@ -677,7 +739,8 @@ static int bisect_successful(struct bisect_terms *terms)
return res;
}
-static enum bisect_error bisect_next(struct bisect_terms *terms, const char *prefix)
+static enum bisect_error bisect_next(struct bisect_terms *terms,
+ const char *prefix, bool defer_reset)
{
enum bisect_error res;
@@ -692,6 +755,9 @@ static enum bisect_error bisect_next(struct bisect_terms *terms, const char *pre
if (res == BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND) {
res = bisect_successful(terms);
+ if (!res && !defer_reset &&
+ !is_empty_or_missing_file(git_path_bisect_reset_when_found()))
+ res = bisect_reset_when_found(terms);
return res ? res : BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND;
} else if (res == BISECT_ONLY_SKIPPED_LEFT) {
res = bisect_skipped_commits(terms);
@@ -700,14 +766,15 @@ static enum bisect_error bisect_next(struct bisect_terms *terms, const char *pre
return res;
}
-static enum bisect_error bisect_auto_next(struct bisect_terms *terms, const char *prefix)
+static enum bisect_error bisect_auto_next(struct bisect_terms *terms,
+ const char *prefix, bool defer_reset)
{
if (bisect_next_check(terms, NULL)) {
bisect_print_status(terms);
return BISECT_OK;
}
- return bisect_next(terms, prefix);
+ return bisect_next(terms, prefix, defer_reset);
}
static enum bisect_error bisect_start(struct bisect_terms *terms, int argc,
@@ -715,6 +782,7 @@ static enum bisect_error bisect_start(struct bisect_terms *terms, int argc,
{
int no_checkout = 0;
int first_parent_only = 0;
+ enum reset_when_found_mode reset_when_found = RESET_WHEN_FOUND_NONE;
int i, has_double_dash = 0, must_write_terms = 0, bad_seen = 0;
int flags, pathspec_pos;
enum bisect_error res = BISECT_OK;
@@ -747,6 +815,13 @@ static enum bisect_error bisect_start(struct bisect_terms *terms, int argc,
no_checkout = 1;
} else if (!strcmp(arg, "--first-parent")) {
first_parent_only = 1;
+ } else if (!strcmp(arg, "--reset-when-found")) {
+ reset_when_found = RESET_WHEN_FOUND_TO_ORIGINAL;
+ } else if (skip_prefix(arg, "--reset-when-found=", &arg)) {
+ if (parse_reset_when_found(arg, &reset_when_found)) {
+ res = BISECT_FAILED;
+ goto finish;
+ }
} else if (!strcmp(arg, "--term-good") ||
!strcmp(arg, "--term-old")) {
i++;
@@ -784,6 +859,10 @@ static enum bisect_error bisect_start(struct bisect_terms *terms, int argc,
break;
}
}
+ if (reset_when_found != RESET_WHEN_FOUND_NONE && no_checkout) {
+ res = error(_("'--reset-when-found' cannot be used with '--no-checkout'"));
+ goto finish;
+ }
pathspec_pos = i;
/*
@@ -861,6 +940,10 @@ static enum bisect_error bisect_start(struct bisect_terms *terms, int argc,
if (first_parent_only)
write_file(git_path_bisect_first_parent(), "\n");
+ if (reset_when_found != RESET_WHEN_FOUND_NONE)
+ write_file(git_path_bisect_reset_when_found(), "%s\n",
+ reset_when_found_mode_name(reset_when_found));
+
if (no_checkout) {
if (repo_get_oid(the_repository, start_head.buf, &oid) < 0) {
res = error(_("invalid ref: '%s'"), start_head.buf);
@@ -902,7 +985,7 @@ finish:
if (res)
return res;
- res = bisect_auto_next(terms, NULL);
+ res = bisect_auto_next(terms, NULL, false);
if (!is_bisect_success(res))
bisect_clean_state();
return res;
@@ -941,7 +1024,7 @@ static int bisect_autostart(struct bisect_terms *terms)
}
static enum bisect_error bisect_state(struct bisect_terms *terms, int argc,
- const char **argv)
+ const char **argv, bool defer_reset)
{
const char *state;
int i, verify_expected = 1;
@@ -1018,7 +1101,7 @@ static enum bisect_error bisect_state(struct bisect_terms *terms, int argc,
}
oid_array_clear(&revs);
- return bisect_auto_next(terms, NULL);
+ return bisect_auto_next(terms, NULL, defer_reset);
}
static enum bisect_error bisect_log(void)
@@ -1107,7 +1190,7 @@ static enum bisect_error bisect_replay(struct bisect_terms *terms, const char *f
if (res)
return BISECT_FAILED;
- return bisect_auto_next(terms, NULL);
+ return bisect_auto_next(terms, NULL, false);
}
static enum bisect_error bisect_skip(struct bisect_terms *terms, int argc,
@@ -1141,7 +1224,7 @@ static enum bisect_error bisect_skip(struct bisect_terms *terms, int argc,
strvec_push(&argv_state, argv[i]);
}
}
- res = bisect_state(terms, argv_state.nr, argv_state.v);
+ res = bisect_state(terms, argv_state.nr, argv_state.v, false);
strvec_clear(&argv_state);
return res;
@@ -1239,6 +1322,8 @@ static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)
{
int res = BISECT_OK;
struct strbuf command = STRBUF_INIT;
+ enum reset_when_found_mode reset_when_found = RESET_WHEN_FOUND_NONE;
+ const char *reset_when_found_arg;
const char *new_state;
int temporary_stdout_fd, saved_stdout;
int is_first_run = 1;
@@ -1246,6 +1331,23 @@ static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)
if (bisect_next_check(terms, NULL))
return BISECT_FAILED;
+ if (argc && !strcmp(argv[0], "--reset-when-found"))
+ reset_when_found = RESET_WHEN_FOUND_TO_ORIGINAL;
+ else if (argc && skip_prefix(argv[0], "--reset-when-found=",
+ &reset_when_found_arg)) {
+ if (parse_reset_when_found(reset_when_found_arg, &reset_when_found))
+ return BISECT_FAILED;
+ }
+
+ if (reset_when_found != RESET_WHEN_FOUND_NONE) {
+ if (refs_ref_exists(get_main_ref_store(the_repository), "BISECT_HEAD"))
+ return error(_("'--reset-when-found' cannot be used with '--no-checkout'"));
+ write_file(git_path_bisect_reset_when_found(), "%s\n",
+ reset_when_found_mode_name(reset_when_found));
+ argc--;
+ argv++;
+ }
+
if (!argc) {
error(_("bisect run failed: no command provided."));
return BISECT_FAILED;
@@ -1304,7 +1406,7 @@ static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)
saved_stdout = dup(1);
dup2(temporary_stdout_fd, 1);
- res = bisect_state(terms, 1, &new_state);
+ res = bisect_state(terms, 1, &new_state, true);
fflush(stdout);
dup2(saved_stdout, 1);
@@ -1320,7 +1422,11 @@ static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)
res = BISECT_OK;
} else if (res == BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND) {
printf(_("bisect found first '%s' commit\n"), terms->term_bad);
- res = BISECT_OK;
+ if (!is_empty_or_missing_file(git_path_bisect_reset_when_found()) &&
+ bisect_reset_when_found(terms))
+ res = BISECT_FAILED;
+ else
+ res = BISECT_OK;
} else if (res) {
error(_("bisect run failed: 'git bisect %s'"
" exited with error code %d"), new_state, res);
@@ -1379,7 +1485,7 @@ static int cmd_bisect__next(int argc, const char **argv UNUSED, const char *pref
return error(_("'%s' requires 0 arguments"),
"git bisect next");
get_terms(&terms);
- res = bisect_next(&terms, prefix);
+ res = bisect_next(&terms, prefix, false);
free_terms(&terms);
return res;
}
@@ -1482,7 +1588,7 @@ int cmd_bisect(int argc,
!one_of(argv[0], terms.term_good, terms.term_bad, NULL))
usage_msg_optf(_("unknown command: '%s'"), git_bisect_usage,
options, argv[0]);
- res = bisect_state(&terms, argc, argv);
+ res = bisect_state(&terms, argc, argv, false);
free_terms(&terms);
} else {
argc--;
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 081116220a..7dfb871ab9 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -43,6 +43,38 @@ test_bisect_usage () {
test_cmp expect actual
}
+test_bisect_state_file () {
+ test_path_is_file "$(git rev-parse --git-path "$1")"
+}
+
+test_bisect_state_missing () {
+ test_path_is_missing "$(git rev-parse --git-path "$1")"
+}
+
+bisect_start_and_finish () {
+ git bisect start "$1" $HASH4 $HASH2 &&
+ git bisect bad
+}
+
+bisect_run_reset_when_found () {
+ write_script test_script.sh <<-\EOF &&
+ ! grep Another hello >/dev/null
+ EOF
+ git bisect start $HASH4 $HASH2 &&
+ git bisect run "$1" ./test_script.sh >my_bisect_log.txt &&
+ test_grep "$HASH3 is the first .bad. commit" my_bisect_log.txt &&
+ test_bisect_state_missing BISECT_RUN
+}
+
+test_reset_when_found_fails () {
+ local pattern="$1" &&
+ local state_file="$2" &&
+ shift 2 &&
+ test_must_fail "$@" 2>err &&
+ test_grep -- "$pattern" err &&
+ test_bisect_state_missing "$state_file"
+}
+
test_expect_success 'bisect usage' "
test_bisect_usage 1 git bisect reset extra1 extra2 <<-\EOF &&
error: 'git bisect reset' requires either no argument or a commit
@@ -453,6 +485,83 @@ test_expect_success '"git bisect run" simple case' '
git bisect reset
'
+test_expect_success '"git bisect start --reset-when-found" defaults to original' '
+ test_when_finished "git bisect reset; git checkout main" &&
+ git checkout main &&
+ bisect_start_and_finish --reset-when-found &&
+ test "$HASH4" = "$(git rev-parse HEAD)" &&
+ test main = "$(git branch --show-current)" &&
+ test_bisect_state_missing BISECT_START &&
+
+ bisect_start_and_finish --reset-when-found=original &&
+ test "$HASH4" = "$(git rev-parse HEAD)" &&
+ test main = "$(git branch --show-current)" &&
+ test_bisect_state_missing BISECT_START
+'
+
+test_expect_success '"git bisect start --reset-when-found=found" leaves first bad checked out' '
+ test_when_finished "git bisect reset; git checkout main" &&
+ bisect_start_and_finish --reset-when-found=found &&
+ test "$HASH3" = "$(git rev-parse HEAD)" &&
+ test_bisect_state_missing BISECT_START
+'
+
+test_expect_success '"git bisect run --reset-when-found" defaults to original' '
+ test_when_finished "git bisect reset; git checkout main" &&
+ bisect_run_reset_when_found --reset-when-found &&
+ test "$HASH4" = "$(git rev-parse HEAD)" &&
+ test main = "$(git branch --show-current)" &&
+ test_bisect_state_missing BISECT_START
+'
+
+test_expect_success '"git bisect run --reset-when-found=found" leaves first bad checked out' '
+ test_when_finished "git bisect reset; git checkout main" &&
+ bisect_run_reset_when_found --reset-when-found=found &&
+ test "$HASH3" = "$(git rev-parse HEAD)" &&
+ test_bisect_state_missing BISECT_START
+'
+
+test_expect_success '--reset-when-found rejects an unknown reset target' '
+ test_when_finished "git bisect reset; git checkout main" &&
+ test_reset_when_found_fails \
+ "invalid value for.*--reset-when-found.*unknown" BISECT_START \
+ git bisect start --reset-when-found=unknown $HASH4 $HASH2 &&
+
+ git bisect start $HASH4 $HASH2 &&
+ test_reset_when_found_fails \
+ "invalid value for.*--reset-when-found.*unknown" \
+ BISECT_RESET_WHEN_FOUND \
+ git bisect run --reset-when-found=unknown true
+'
+
+test_expect_success '--reset-when-found cannot be used with --no-checkout' '
+ test_when_finished "git bisect reset" &&
+ test_reset_when_found_fails \
+ "cannot be used with.*--no-checkout" BISECT_START \
+ git bisect start --reset-when-found=original --no-checkout $HASH4 $HASH2 &&
+
+ git bisect start --no-checkout $HASH4 $HASH2 &&
+ test_reset_when_found_fails \
+ "cannot be used with.*--no-checkout" BISECT_RESET_WHEN_FOUND \
+ git bisect run --reset-when-found=found true
+'
+
+test_expect_success 'without --reset-when-found the bisection state is kept' '
+ test_when_finished "git bisect reset" &&
+ git bisect start $HASH4 $HASH2 &&
+ git bisect bad &&
+ test_bisect_state_file BISECT_START
+'
+
+test_expect_success '--reset-when-found does not leak into a later bisection' '
+ test_when_finished "git bisect reset; git checkout main" &&
+ bisect_start_and_finish --reset-when-found &&
+
+ git bisect start $HASH4 $HASH2 &&
+ git bisect bad &&
+ test_bisect_state_file BISECT_START
+'
+
# We want to automatically find the commit that
# added "Ciao" into hello.
test_expect_success '"git bisect run" with more complex "git bisect start"' '
--
gitgitgadget
^ permalink raw reply related
* [PATCH 2/2] remote: resolve URL-valued push tracking remotes
From: Harald Nordgren via GitGitGadget @ 2026-07-20 9:10 UTC (permalink / raw)
To: git; +Cc: Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2358.git.git.1784538618.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
A branch may name its push destination with a URL instead of a
configured remote. This is useful in fork workflows, where the original
remote is renamed to "upstream", the fork is added as "origin", and an
existing branch.<name>.pushRemote continues to contain the fork URL.
Git can still push through the anonymous remote created for that URL.
However, the anonymous remote has no fetch refspec. Git therefore cannot
resolve @{push} to origin/<branch> or update that remote-tracking branch
after a push. The push can succeed, or report that everything is up to
date, while status continues to compare against a stale tracking ref or
cannot show the push branch at all.
A uniquely matching configured remote already provides the missing
mapping. Use its fetch refspec when resolving the push tracking branch
and when updating tracking refs after a push. This changes neither the
push destination nor configuration. Keep the existing behavior when no
remote matches or multiple remotes share the URL, since either case is
ambiguous.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
Documentation/revisions.adoc | 3 +
remote.c | 27 ++++++++-
remote.h | 2 +
t/t5505-remote.sh | 104 +++++++++++++++++++++++++++++++++++
transport.c | 5 +-
5 files changed, 139 insertions(+), 2 deletions(-)
diff --git a/Documentation/revisions.adoc b/Documentation/revisions.adoc
index 6ea6c7cead..b691691c8c 100644
--- a/Documentation/revisions.adoc
+++ b/Documentation/revisions.adoc
@@ -127,6 +127,9 @@ some output processing may assume ref names in UTF-8.
`git push` were run while `branchname` was checked out (or the current
`HEAD` if no branchname is specified). Like for '@\{upstream\}', we report
the remote-tracking branch that corresponds to that branch at the remote.
+ If the push remote is specified as a URL, the fetch refspec of a uniquely
+ matching configured remote is used to find and update the remote-tracking
+ branch.
+
Here's an example to make it more clear:
+
diff --git a/remote.c b/remote.c
index 89d0f9e2d8..03908dfe8d 100644
--- a/remote.c
+++ b/remote.c
@@ -1887,13 +1887,38 @@ const char *branch_get_upstream(struct branch *branch, struct strbuf *err)
return branch->merge[0]->dst;
}
-static char *tracking_for_push_dest(struct repository *repo UNUSED,
+struct remote *repo_remote_for_push_tracking(struct repository *repo,
+ struct remote *remote)
+{
+ struct remote *first_match = NULL;
+ struct remote_state *remote_state = repo->remote_state;
+
+ if (remote->origin != REMOTE_UNCONFIGURED || remote->url.nr != 1)
+ return remote;
+
+ for (int i = 0; i < remote_state->remotes_nr; i++) {
+ struct remote *candidate = remote_state->remotes[i];
+
+ if (!candidate || candidate == remote ||
+ !remote_is_configured(candidate, 0) ||
+ !remote_has_url(candidate, remote->url.v[0]))
+ continue;
+ if (first_match)
+ return remote;
+ first_match = candidate;
+ }
+
+ return first_match ? first_match : remote;
+}
+
+static char *tracking_for_push_dest(struct repository *repo,
struct remote *remote,
const char *refname,
struct strbuf *err)
{
char *ret;
+ remote = repo_remote_for_push_tracking(repo, remote);
ret = apply_refspecs(&remote->fetch, refname);
if (!ret)
return error_buf(err,
diff --git a/remote.h b/remote.h
index 72a54d84ad..cca02033b9 100644
--- a/remote.h
+++ b/remote.h
@@ -345,6 +345,8 @@ char *remote_ref_for_branch(struct branch *branch, int for_push);
const char *repo_default_remote(struct repository *repo);
const char *repo_remote_from_url(struct repository *repo, const char *url);
+struct remote *repo_remote_for_push_tracking(struct repository *repo,
+ struct remote *remote);
/* returns true if the given branch has merge configuration given. */
int branch_has_merge_config(struct branch *branch);
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index e592c0bcde..e16b3f320a 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -24,6 +24,28 @@ setup_repository () {
)
}
+setup_url_pushremote () {
+ rm -rf fork.git client &&
+ git clone --bare one fork.git &&
+ git clone one client &&
+ fork_url="$TRASH_DIRECTORY/fork.git" &&
+ (
+ cd client &&
+ git checkout -b topic --track origin/main &&
+ git commit --allow-empty -m topic-change &&
+ git config push.default current &&
+ git config status.compareBranches "@{upstream} @{push}" &&
+ git config branch.topic.pushRemote "$fork_url" &&
+ git push
+ )
+}
+
+check_status () {
+ git -C client status >actual &&
+ cat >expected &&
+ test_cmp expected actual
+}
+
tokens_match () {
echo "$1" | tr ' ' '\012' | sort | sed -e '/^$/d' >expect &&
echo "$2" | tr ' ' '\012' | sort | sed -e '/^$/d' >actual &&
@@ -1018,6 +1040,88 @@ test_expect_success 'rename a remote renames repo remote.pushDefault but keeps g
)
'
+test_expect_success 'URL-valued pushRemote without matching remote is not trackable' '
+ setup_url_pushremote &&
+
+ check_status <<-EOF
+ On branch topic
+ Your branch is ahead of ${SQ}origin/main${SQ} by 1 commit.
+ (use "git push" to publish your local commits)
+
+ nothing to commit, working tree clean
+ EOF
+'
+
+test_expect_success 'adding fork remote makes URL-valued pushRemote trackable' '
+ setup_url_pushremote &&
+
+ (
+ cd client &&
+ git remote rename origin upstream &&
+ git remote add -f origin "$fork_url"
+ ) &&
+
+ check_status <<-EOF
+ On branch topic
+ Your branch is ahead of ${SQ}upstream/main${SQ} by 1 commit.
+
+ Your branch is up to date with ${SQ}origin/topic${SQ}.
+
+ nothing to commit, working tree clean
+ EOF
+'
+
+test_expect_success 'up-to-date URL push refreshes stale tracking branch' '
+ setup_url_pushremote &&
+ (
+ cd client &&
+ git remote rename origin upstream &&
+ git remote add -f origin "$fork_url" &&
+ git commit --allow-empty -m another-topic-change &&
+ git -C ../fork.git fetch ../client topic:topic
+ ) &&
+
+ check_status <<-EOF &&
+ On branch topic
+ Your branch is ahead of ${SQ}upstream/main${SQ} by 2 commits.
+
+ Your branch is ahead of ${SQ}origin/topic${SQ} by 1 commit.
+ (use "git push" to publish your local commits)
+
+ nothing to commit, working tree clean
+ EOF
+
+ git -C client push >actual 2>&1 &&
+ test_grep "Everything up-to-date" actual &&
+
+ check_status <<-EOF
+ On branch topic
+ Your branch is ahead of ${SQ}upstream/main${SQ} by 2 commits.
+
+ Your branch is up to date with ${SQ}origin/topic${SQ}.
+
+ nothing to commit, working tree clean
+ EOF
+'
+
+test_expect_success 'duplicate remote URL leaves URL-valued pushRemote ambiguous' '
+ setup_url_pushremote &&
+ (
+ cd client &&
+ git remote rename origin upstream &&
+ git remote add -f origin "$fork_url" &&
+ git remote add duplicate "$fork_url"
+ ) &&
+
+ check_status <<-EOF
+ On branch topic
+ Your branch is ahead of ${SQ}upstream/main${SQ} by 1 commit.
+ (use "git push" to publish your local commits)
+
+ nothing to commit, working tree clean
+ EOF
+'
+
test_expect_success 'rename handles remote without fetch refspec' '
git clone --bare one no-refspec.git &&
# confirm assumption that bare clone does not create refspec
diff --git a/transport.c b/transport.c
index fc144f0aed..30a4ab2cd5 100644
--- a/transport.c
+++ b/transport.c
@@ -1553,8 +1553,11 @@ int transport_push(struct repository *r,
if (!(flags & (TRANSPORT_PUSH_DRY_RUN |
TRANSPORT_RECURSE_SUBMODULES_ONLY))) {
struct ref *ref;
+ struct remote *tracking_remote = repo_remote_for_push_tracking(
+ r, transport->remote);
+
for (ref = remote_refs; ref; ref = ref->next)
- transport_update_tracking_ref(transport->remote, ref, verbose);
+ transport_update_tracking_ref(tracking_remote, ref, verbose);
}
if (porcelain && !push_ret)
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 1/2] bisect: let bisect_reset() optionally check out quietly
From: Harald Nordgren via GitGitGadget @ 2026-07-20 9:10 UTC (permalink / raw)
To: git; +Cc: Johannes Sixt, Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2335.v3.git.git.1784538619.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
Add a "quiet" parameter to bisect_reset() that passes "--quiet" to the
checkout restoring the original HEAD, suppressing its progress and
branch-status output.
No caller sets the flag yet, so behavior is unchanged.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
builtin/bisect.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/builtin/bisect.c b/builtin/bisect.c
index 798e28f501..0e49ca23ae 100644
--- a/builtin/bisect.c
+++ b/builtin/bisect.c
@@ -234,7 +234,7 @@ static int write_terms(const char *bad, const char *good)
return res;
}
-static int bisect_reset(const char *commit)
+static int bisect_reset(const char *commit, int quiet)
{
struct strbuf branch = STRBUF_INIT;
@@ -255,8 +255,10 @@ static int bisect_reset(const char *commit)
struct child_process cmd = CHILD_PROCESS_INIT;
cmd.git_cmd = 1;
- strvec_pushl(&cmd.args, "checkout", "--ignore-other-worktrees",
- branch.buf, "--", NULL);
+ strvec_pushl(&cmd.args, "checkout", "--ignore-other-worktrees", NULL);
+ if (quiet)
+ strvec_push(&cmd.args, "--quiet");
+ strvec_pushl(&cmd.args, branch.buf, "--", NULL);
if (run_command(&cmd)) {
error(_("could not check out original"
" HEAD '%s'. Try 'git bisect"
@@ -1089,7 +1091,7 @@ static enum bisect_error bisect_replay(struct bisect_terms *terms, const char *f
if (is_empty_or_missing_file(filename))
return error(_("cannot read file '%s' for replaying"), filename);
- if (bisect_reset(NULL))
+ if (bisect_reset(NULL, 0))
return BISECT_FAILED;
fp = fopen(filename, "r");
@@ -1338,7 +1340,7 @@ static int cmd_bisect__reset(int argc, const char **argv, const char *prefix UNU
if (argc > 1)
return error(_("'%s' requires either no argument or a commit"),
"git bisect reset");
- return bisect_reset(argc ? argv[0] : NULL);
+ return bisect_reset(argc ? argv[0] : NULL, 0);
}
static int cmd_bisect__terms(int argc, const char **argv, const char *prefix UNUSED,
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 0/2] bisect: add --auto-reset to leave when done
From: Harald Nordgren via GitGitGadget @ 2026-07-20 9:10 UTC (permalink / raw)
To: git; +Cc: Johannes Sixt, Harald Nordgren
In-Reply-To: <pull.2335.v2.git.git.1784312854.gitgitgadget@gmail.com>
Add a --reset-when-found option to git bisect that resets the bisect session
when culprit is found.
Changes in v3:
* Rename --auto-reset to --reset-when-found, including internal names.
* Defer git bisect run cleanup until captured output is printed and
BISECT_RUN is closed. Drop the open-descriptor preparatory change,
retaining the existing filename-based output handling.
Changes in v2:
* Add option --auto-reset[=<where>] with option to go to final commit as
well as original.
* Refactored tests.
Harald Nordgren (2):
bisect: let bisect_reset() optionally check out quietly
bisect: add --reset-when-found to leave when done
Documentation/git-bisect.adoc | 14 +++-
bisect.c | 2 +
builtin/bisect.c | 146 +++++++++++++++++++++++++++++-----
t/t6030-bisect-porcelain.sh | 109 +++++++++++++++++++++++++
4 files changed, 250 insertions(+), 21 deletions(-)
base-commit: 41365c2a9ba347870b80881c0d67454edd22fd49
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2335%2FHaraldNordgren%2Fbisect-auto-reset-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2335/HaraldNordgren/bisect-auto-reset-v3
Pull-Request: https://github.com/git/git/pull/2335
Range-diff vs v2:
1: 0de8b12f65 < -: ---------- bisect: read run output from the open descriptor
2: 8a2dcdf305 = 1: 59920c51ae bisect: let bisect_reset() optionally check out quietly
3: 5b3704fbd4 ! 2: 542f4b2c80 bisect: add --auto-reset to leave when done
@@ Metadata
Author: Harald Nordgren <haraldnordgren@gmail.com>
## Commit message ##
- bisect: add --auto-reset to leave when done
+ bisect: add --reset-when-found to leave when done
When a bisection finishes, "git bisect" reports the first bad commit
but leaves the session active until "git bisect reset" is run by hand.
- Add an "--auto-reset[=<where>]" option, accepted by both "git bisect
- start" and "git bisect run", that resets as soon as the first bad commit
- is found. The "original" value returns to the commit checked out before
- "git bisect start", while "found" leaves the first bad commit checked
- out; omitting the value defaults to "original".
+ Add a "--reset-when-found[=<where>]" option, accepted by both "git
+ bisect start" and "git bisect run", that resets as soon as the first
+ bad commit is found. The "original" value returns to the commit checked
+ out before "git bisect start", while "found" leaves the first bad commit
+ checked out; omitting the value defaults to "original".
- Persist the selected target in a BISECT_AUTO_RESET state file and perform
- the reset quietly. Reject this option together with "--no-checkout",
- since that mode must not check out either target.
+ Persist the selected target in a BISECT_RESET_WHEN_FOUND state file
+ and perform the reset quietly.
+
+ For "git bisect run", defer the reset until after the captured output
+ is printed and BISECT_RUN is closed. This lets cleanup remove the file
+ on systems that cannot unlink an open file.
+
+ Reject this option together with "--no-checkout", since that mode must
+ not check out either target.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
@@ Documentation/git-bisect.adoc: SYNOPSIS
[synopsis]
git bisect start [--term-(bad|new)=<term-new> --term-(good|old)=<term-old>]
- [--no-checkout] [--first-parent] [<bad> [<good>...]] [--] [<pathspec>...]
-+ [--no-checkout] [--first-parent] [--auto-reset[=<where>]] [<bad> [<good>...]] [--] [<pathspec>...]
++ [--no-checkout] [--first-parent] [--reset-when-found[=<where>]] [<bad> [<good>...]] [--] [<pathspec>...]
git bisect (bad|new|<term-new>) [<rev>]
git bisect (good|old|<term-old>) [<rev>...]
git bisect terms [--term-(good|old) | --term-(bad|new)]
@@ Documentation/git-bisect.adoc: git bisect reset [<commit>]
git bisect replay <logfile>
git bisect log
-git bisect run <cmd> [<arg>...]
-+git bisect run [--auto-reset[=<where>]] <cmd> [<arg>...]
++git bisect run [--reset-when-found[=<where>]] <cmd> [<arg>...]
git bisect help
DESCRIPTION
@@ Documentation/git-bisect.adoc: ignored.
This option is particularly useful in avoiding false positives when a merged
branch contained broken or non-buildable commits, but the merge itself was OK.
-+`--auto-reset[=<where>]`::
++`--reset-when-found[=<where>]`::
+ Once the first bad commit is found, report it and clean up the
+ bisection state. `<where>` may be `original` to return to the commit
+ checked out before `git bisect start`, or `found` to leave the first
@@ bisect.c: static GIT_PATH_FUNC(git_path_bisect_start, "BISECT_START")
static GIT_PATH_FUNC(git_path_bisect_log, "BISECT_LOG")
static GIT_PATH_FUNC(git_path_bisect_terms, "BISECT_TERMS")
static GIT_PATH_FUNC(git_path_bisect_first_parent, "BISECT_FIRST_PARENT")
-+static GIT_PATH_FUNC(git_path_bisect_auto_reset, "BISECT_AUTO_RESET")
++static GIT_PATH_FUNC(git_path_bisect_reset_when_found, "BISECT_RESET_WHEN_FOUND")
static void read_bisect_paths(struct strvec *array)
{
@@ bisect.c: int bisect_clean_state(void)
unlink_or_warn(git_path_bisect_run());
unlink_or_warn(git_path_bisect_terms());
unlink_or_warn(git_path_bisect_first_parent());
-+ unlink_or_warn(git_path_bisect_auto_reset());
++ unlink_or_warn(git_path_bisect_reset_when_found());
/*
* Cleanup BISECT_START last to support the --no-checkout option
* introduced in the commit 4796e823a.
@@ builtin/bisect.c: static GIT_PATH_FUNC(git_path_bisect_start, "BISECT_START")
static GIT_PATH_FUNC(git_path_bisect_log, "BISECT_LOG")
static GIT_PATH_FUNC(git_path_bisect_names, "BISECT_NAMES")
static GIT_PATH_FUNC(git_path_bisect_first_parent, "BISECT_FIRST_PARENT")
-+static GIT_PATH_FUNC(git_path_bisect_auto_reset, "BISECT_AUTO_RESET")
++static GIT_PATH_FUNC(git_path_bisect_reset_when_found, "BISECT_RESET_WHEN_FOUND")
static GIT_PATH_FUNC(git_path_bisect_run, "BISECT_RUN")
#define BUILTIN_GIT_BISECT_START_USAGE \
N_("git bisect start [--term-(bad|new)=<term-new> --term-(good|old)=<term-old>]\n" \
- " [--no-checkout] [--first-parent] [<bad> [<good>...]] [--] [<pathspec>...]")
-+ " [--no-checkout] [--first-parent] [--auto-reset[=<where>]] [<bad> [<good>...]] [--] [<pathspec>...]")
++ " [--no-checkout] [--first-parent] [--reset-when-found[=<where>]] [<bad> [<good>...]] [--] [<pathspec>...]")
#define BUILTIN_GIT_BISECT_BAD_USAGE \
N_("git bisect (bad|new|<term-new>) [<rev>]")
#define BUILTIN_GIT_BISECT_GOOD_USAGE \
@@ builtin/bisect.c: static GIT_PATH_FUNC(git_path_bisect_run, "BISECT_RUN")
"git bisect log"
#define BUILTIN_GIT_BISECT_RUN_USAGE \
- N_("git bisect run <cmd> [<arg>...]")
-+ N_("git bisect run [--auto-reset[=<where>]] <cmd> [<arg>...]")
++ N_("git bisect run [--reset-when-found[=<where>]] <cmd> [<arg>...]")
#define BUILTIN_GIT_BISECT_HELP_USAGE \
"git bisect help"
@@ builtin/bisect.c: static const char * const git_bisect_usage[] = {
NULL
};
-+enum auto_reset_mode {
-+ AUTO_RESET_NONE,
-+ AUTO_RESET_ORIGINAL,
-+ AUTO_RESET_FOUND,
++enum reset_when_found_mode {
++ RESET_WHEN_FOUND_NONE,
++ RESET_WHEN_FOUND_TO_ORIGINAL,
++ RESET_WHEN_FOUND_TO_FOUND,
+};
+
struct add_bisect_ref_data {
@@ builtin/bisect.c: static int bisect_reset(const char *commit, int quiet)
return bisect_clean_state();
}
-+static int parse_auto_reset(const char *value, enum auto_reset_mode *mode)
++static int parse_reset_when_found(const char *value,
++ enum reset_when_found_mode *mode)
+{
+ if (!strcmp(value, "original"))
-+ *mode = AUTO_RESET_ORIGINAL;
++ *mode = RESET_WHEN_FOUND_TO_ORIGINAL;
+ else if (!strcmp(value, "found"))
-+ *mode = AUTO_RESET_FOUND;
++ *mode = RESET_WHEN_FOUND_TO_FOUND;
+ else
-+ return error(_("invalid value for '--auto-reset': '%s'"), value);
++ return error(_("invalid value for '--reset-when-found': '%s'"),
++ value);
+
+ return 0;
+}
+
-+static const char *auto_reset_mode_name(enum auto_reset_mode mode)
++static const char *reset_when_found_mode_name(enum reset_when_found_mode mode)
+{
+ switch (mode) {
-+ case AUTO_RESET_ORIGINAL:
++ case RESET_WHEN_FOUND_TO_ORIGINAL:
+ return "original";
-+ case AUTO_RESET_FOUND:
++ case RESET_WHEN_FOUND_TO_FOUND:
+ return "found";
-+ case AUTO_RESET_NONE:
-+ BUG("no name for unset auto-reset mode");
++ case RESET_WHEN_FOUND_NONE:
++ BUG("no name for unset reset-when-found mode");
+ }
-+ BUG("unknown auto-reset mode %d", mode);
++ BUG("unknown reset-when-found mode %d", mode);
+}
+
-+static int bisect_auto_reset(struct bisect_terms *terms)
++static int bisect_reset_when_found(struct bisect_terms *terms)
+{
+ struct strbuf value = STRBUF_INIT;
-+ enum auto_reset_mode mode;
++ enum reset_when_found_mode mode;
+ char *commit = NULL;
+ int res;
+
-+ if (strbuf_read_file(&value, git_path_bisect_auto_reset(), 0) < 0) {
++ if (strbuf_read_file(&value, git_path_bisect_reset_when_found(), 0) < 0) {
+ res = error_errno(_("could not read '%s'"),
-+ git_path_bisect_auto_reset());
++ git_path_bisect_reset_when_found());
+ goto cleanup;
+ }
+ strbuf_trim(&value);
-+ if (parse_auto_reset(value.buf, &mode)) {
++ if (parse_reset_when_found(value.buf, &mode)) {
+ res = -1;
+ goto cleanup;
+ }
+
-+ if (mode == AUTO_RESET_FOUND)
++ if (mode == RESET_WHEN_FOUND_TO_FOUND)
+ commit = xstrfmt("refs/bisect/%s", terms->term_bad);
+ res = bisect_reset(commit, 1);
+
@@ builtin/bisect.c: static int bisect_reset(const char *commit, int quiet)
static void log_commit(FILE *fp,
const char *fmt, const char *state,
struct commit *commit)
+@@ builtin/bisect.c: static int bisect_successful(struct bisect_terms *terms)
+ return res;
+ }
+
+-static enum bisect_error bisect_next(struct bisect_terms *terms, const char *prefix)
++static enum bisect_error bisect_next(struct bisect_terms *terms,
++ const char *prefix, bool defer_reset)
+ {
+ enum bisect_error res;
+
@@ builtin/bisect.c: static enum bisect_error bisect_next(struct bisect_terms *terms, const char *pre
if (res == BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND) {
res = bisect_successful(terms);
-+ if (!res && !is_empty_or_missing_file(git_path_bisect_auto_reset()))
-+ res = bisect_auto_reset(terms);
++ if (!res && !defer_reset &&
++ !is_empty_or_missing_file(git_path_bisect_reset_when_found()))
++ res = bisect_reset_when_found(terms);
return res ? res : BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND;
} else if (res == BISECT_ONLY_SKIPPED_LEFT) {
res = bisect_skipped_commits(terms);
+@@ builtin/bisect.c: static enum bisect_error bisect_next(struct bisect_terms *terms, const char *pre
+ return res;
+ }
+
+-static enum bisect_error bisect_auto_next(struct bisect_terms *terms, const char *prefix)
++static enum bisect_error bisect_auto_next(struct bisect_terms *terms,
++ const char *prefix, bool defer_reset)
+ {
+ if (bisect_next_check(terms, NULL)) {
+ bisect_print_status(terms);
+ return BISECT_OK;
+ }
+
+- return bisect_next(terms, prefix);
++ return bisect_next(terms, prefix, defer_reset);
+ }
+
+ static enum bisect_error bisect_start(struct bisect_terms *terms, int argc,
@@ builtin/bisect.c: static enum bisect_error bisect_start(struct bisect_terms *terms, int argc,
{
int no_checkout = 0;
int first_parent_only = 0;
-+ enum auto_reset_mode auto_reset = AUTO_RESET_NONE;
++ enum reset_when_found_mode reset_when_found = RESET_WHEN_FOUND_NONE;
int i, has_double_dash = 0, must_write_terms = 0, bad_seen = 0;
int flags, pathspec_pos;
enum bisect_error res = BISECT_OK;
@@ builtin/bisect.c: static enum bisect_error bisect_start(struct bisect_terms *ter
no_checkout = 1;
} else if (!strcmp(arg, "--first-parent")) {
first_parent_only = 1;
-+ } else if (!strcmp(arg, "--auto-reset")) {
-+ auto_reset = AUTO_RESET_ORIGINAL;
-+ } else if (skip_prefix(arg, "--auto-reset=", &arg)) {
-+ if (parse_auto_reset(arg, &auto_reset)) {
++ } else if (!strcmp(arg, "--reset-when-found")) {
++ reset_when_found = RESET_WHEN_FOUND_TO_ORIGINAL;
++ } else if (skip_prefix(arg, "--reset-when-found=", &arg)) {
++ if (parse_reset_when_found(arg, &reset_when_found)) {
+ res = BISECT_FAILED;
+ goto finish;
+ }
@@ builtin/bisect.c: static enum bisect_error bisect_start(struct bisect_terms *ter
break;
}
}
-+ if (auto_reset != AUTO_RESET_NONE && no_checkout) {
-+ res = error(_("'--auto-reset' cannot be used with '--no-checkout'"));
++ if (reset_when_found != RESET_WHEN_FOUND_NONE && no_checkout) {
++ res = error(_("'--reset-when-found' cannot be used with '--no-checkout'"));
+ goto finish;
+ }
pathspec_pos = i;
@@ builtin/bisect.c: static enum bisect_error bisect_start(struct bisect_terms *ter
if (first_parent_only)
write_file(git_path_bisect_first_parent(), "\n");
-+ if (auto_reset != AUTO_RESET_NONE)
-+ write_file(git_path_bisect_auto_reset(), "%s\n",
-+ auto_reset_mode_name(auto_reset));
++ if (reset_when_found != RESET_WHEN_FOUND_NONE)
++ write_file(git_path_bisect_reset_when_found(), "%s\n",
++ reset_when_found_mode_name(reset_when_found));
+
if (no_checkout) {
if (repo_get_oid(the_repository, start_head.buf, &oid) < 0) {
res = error(_("invalid ref: '%s'"), start_head.buf);
+@@ builtin/bisect.c: finish:
+ if (res)
+ return res;
+
+- res = bisect_auto_next(terms, NULL);
++ res = bisect_auto_next(terms, NULL, false);
+ if (!is_bisect_success(res))
+ bisect_clean_state();
+ return res;
+@@ builtin/bisect.c: static int bisect_autostart(struct bisect_terms *terms)
+ }
+
+ static enum bisect_error bisect_state(struct bisect_terms *terms, int argc,
+- const char **argv)
++ const char **argv, bool defer_reset)
+ {
+ const char *state;
+ int i, verify_expected = 1;
+@@ builtin/bisect.c: static enum bisect_error bisect_state(struct bisect_terms *terms, int argc,
+ }
+
+ oid_array_clear(&revs);
+- return bisect_auto_next(terms, NULL);
++ return bisect_auto_next(terms, NULL, defer_reset);
+ }
+
+ static enum bisect_error bisect_log(void)
+@@ builtin/bisect.c: static enum bisect_error bisect_replay(struct bisect_terms *terms, const char *f
+ if (res)
+ return BISECT_FAILED;
+
+- return bisect_auto_next(terms, NULL);
++ return bisect_auto_next(terms, NULL, false);
+ }
+
+ static enum bisect_error bisect_skip(struct bisect_terms *terms, int argc,
+@@ builtin/bisect.c: static enum bisect_error bisect_skip(struct bisect_terms *terms, int argc,
+ strvec_push(&argv_state, argv[i]);
+ }
+ }
+- res = bisect_state(terms, argv_state.nr, argv_state.v);
++ res = bisect_state(terms, argv_state.nr, argv_state.v, false);
+
+ strvec_clear(&argv_state);
+ return res;
@@ builtin/bisect.c: static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)
{
int res = BISECT_OK;
struct strbuf command = STRBUF_INIT;
-+ enum auto_reset_mode auto_reset = AUTO_RESET_NONE;
-+ const char *auto_reset_arg;
++ enum reset_when_found_mode reset_when_found = RESET_WHEN_FOUND_NONE;
++ const char *reset_when_found_arg;
const char *new_state;
int temporary_stdout_fd, saved_stdout;
int is_first_run = 1;
@@ builtin/bisect.c: static int bisect_run(struct bisect_terms *terms, int argc, co
if (bisect_next_check(terms, NULL))
return BISECT_FAILED;
-+ if (argc && !strcmp(argv[0], "--auto-reset"))
-+ auto_reset = AUTO_RESET_ORIGINAL;
-+ else if (argc && skip_prefix(argv[0], "--auto-reset=", &auto_reset_arg)) {
-+ if (parse_auto_reset(auto_reset_arg, &auto_reset))
++ if (argc && !strcmp(argv[0], "--reset-when-found"))
++ reset_when_found = RESET_WHEN_FOUND_TO_ORIGINAL;
++ else if (argc && skip_prefix(argv[0], "--reset-when-found=",
++ &reset_when_found_arg)) {
++ if (parse_reset_when_found(reset_when_found_arg, &reset_when_found))
+ return BISECT_FAILED;
+ }
+
-+ if (auto_reset != AUTO_RESET_NONE) {
++ if (reset_when_found != RESET_WHEN_FOUND_NONE) {
+ if (refs_ref_exists(get_main_ref_store(the_repository), "BISECT_HEAD"))
-+ return error(_("'--auto-reset' cannot be used with '--no-checkout'"));
-+ write_file(git_path_bisect_auto_reset(), "%s\n",
-+ auto_reset_mode_name(auto_reset));
++ return error(_("'--reset-when-found' cannot be used with '--no-checkout'"));
++ write_file(git_path_bisect_reset_when_found(), "%s\n",
++ reset_when_found_mode_name(reset_when_found));
+ argc--;
+ argv++;
+ }
@@ builtin/bisect.c: static int bisect_run(struct bisect_terms *terms, int argc, co
if (!argc) {
error(_("bisect run failed: no command provided."));
return BISECT_FAILED;
+@@ builtin/bisect.c: static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)
+ saved_stdout = dup(1);
+ dup2(temporary_stdout_fd, 1);
+
+- res = bisect_state(terms, 1, &new_state);
++ res = bisect_state(terms, 1, &new_state, true);
+
+ fflush(stdout);
+ dup2(saved_stdout, 1);
+@@ builtin/bisect.c: static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)
+ res = BISECT_OK;
+ } else if (res == BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND) {
+ printf(_("bisect found first '%s' commit\n"), terms->term_bad);
+- res = BISECT_OK;
++ if (!is_empty_or_missing_file(git_path_bisect_reset_when_found()) &&
++ bisect_reset_when_found(terms))
++ res = BISECT_FAILED;
++ else
++ res = BISECT_OK;
+ } else if (res) {
+ error(_("bisect run failed: 'git bisect %s'"
+ " exited with error code %d"), new_state, res);
+@@ builtin/bisect.c: static int cmd_bisect__next(int argc, const char **argv UNUSED, const char *pref
+ return error(_("'%s' requires 0 arguments"),
+ "git bisect next");
+ get_terms(&terms);
+- res = bisect_next(&terms, prefix);
++ res = bisect_next(&terms, prefix, false);
+ free_terms(&terms);
+ return res;
+ }
+@@ builtin/bisect.c: int cmd_bisect(int argc,
+ !one_of(argv[0], terms.term_good, terms.term_bad, NULL))
+ usage_msg_optf(_("unknown command: '%s'"), git_bisect_usage,
+ options, argv[0]);
+- res = bisect_state(&terms, argc, argv);
++ res = bisect_state(&terms, argc, argv, false);
+ free_terms(&terms);
+ } else {
+ argc--;
## t/t6030-bisect-porcelain.sh ##
@@ t/t6030-bisect-porcelain.sh: test_bisect_usage () {
@@ t/t6030-bisect-porcelain.sh: test_bisect_usage () {
+ git bisect bad
+}
+
-+bisect_run_auto_reset () {
++bisect_run_reset_when_found () {
+ write_script test_script.sh <<-\EOF &&
+ ! grep Another hello >/dev/null
+ EOF
+ git bisect start $HASH4 $HASH2 &&
+ git bisect run "$1" ./test_script.sh >my_bisect_log.txt &&
-+ test_grep "$HASH3 is the first .bad. commit" my_bisect_log.txt
++ test_grep "$HASH3 is the first .bad. commit" my_bisect_log.txt &&
++ test_bisect_state_missing BISECT_RUN
+}
+
-+test_auto_reset_fails () {
++test_reset_when_found_fails () {
+ local pattern="$1" &&
+ local state_file="$2" &&
+ shift 2 &&
@@ t/t6030-bisect-porcelain.sh: test_expect_success '"git bisect run" simple case'
git bisect reset
'
-+test_expect_success '"git bisect start --auto-reset" defaults to original' '
++test_expect_success '"git bisect start --reset-when-found" defaults to original' '
+ test_when_finished "git bisect reset; git checkout main" &&
+ git checkout main &&
-+ bisect_start_and_finish --auto-reset &&
++ bisect_start_and_finish --reset-when-found &&
+ test "$HASH4" = "$(git rev-parse HEAD)" &&
+ test main = "$(git branch --show-current)" &&
+ test_bisect_state_missing BISECT_START &&
+
-+ bisect_start_and_finish --auto-reset=original &&
++ bisect_start_and_finish --reset-when-found=original &&
+ test "$HASH4" = "$(git rev-parse HEAD)" &&
+ test main = "$(git branch --show-current)" &&
+ test_bisect_state_missing BISECT_START
+'
+
-+test_expect_success '"git bisect start --auto-reset=found" leaves first bad checked out' '
++test_expect_success '"git bisect start --reset-when-found=found" leaves first bad checked out' '
+ test_when_finished "git bisect reset; git checkout main" &&
-+ bisect_start_and_finish --auto-reset=found &&
++ bisect_start_and_finish --reset-when-found=found &&
+ test "$HASH3" = "$(git rev-parse HEAD)" &&
+ test_bisect_state_missing BISECT_START
+'
+
-+test_expect_success '"git bisect run --auto-reset" defaults to original' '
++test_expect_success '"git bisect run --reset-when-found" defaults to original' '
+ test_when_finished "git bisect reset; git checkout main" &&
-+ bisect_run_auto_reset --auto-reset &&
++ bisect_run_reset_when_found --reset-when-found &&
+ test "$HASH4" = "$(git rev-parse HEAD)" &&
+ test main = "$(git branch --show-current)" &&
+ test_bisect_state_missing BISECT_START
+'
+
-+test_expect_success '"git bisect run --auto-reset=found" leaves first bad checked out' '
++test_expect_success '"git bisect run --reset-when-found=found" leaves first bad checked out' '
+ test_when_finished "git bisect reset; git checkout main" &&
-+ bisect_run_auto_reset --auto-reset=found &&
++ bisect_run_reset_when_found --reset-when-found=found &&
+ test "$HASH3" = "$(git rev-parse HEAD)" &&
+ test_bisect_state_missing BISECT_START
+'
+
-+test_expect_success '--auto-reset rejects an unknown reset target' '
++test_expect_success '--reset-when-found rejects an unknown reset target' '
+ test_when_finished "git bisect reset; git checkout main" &&
-+ test_auto_reset_fails \
-+ "invalid value for.*--auto-reset.*unknown" BISECT_START \
-+ git bisect start --auto-reset=unknown $HASH4 $HASH2 &&
++ test_reset_when_found_fails \
++ "invalid value for.*--reset-when-found.*unknown" BISECT_START \
++ git bisect start --reset-when-found=unknown $HASH4 $HASH2 &&
+
+ git bisect start $HASH4 $HASH2 &&
-+ test_auto_reset_fails \
-+ "invalid value for.*--auto-reset.*unknown" BISECT_AUTO_RESET \
-+ git bisect run --auto-reset=unknown true
++ test_reset_when_found_fails \
++ "invalid value for.*--reset-when-found.*unknown" \
++ BISECT_RESET_WHEN_FOUND \
++ git bisect run --reset-when-found=unknown true
+'
+
-+test_expect_success '--auto-reset cannot be used with --no-checkout' '
++test_expect_success '--reset-when-found cannot be used with --no-checkout' '
+ test_when_finished "git bisect reset" &&
-+ test_auto_reset_fails \
++ test_reset_when_found_fails \
+ "cannot be used with.*--no-checkout" BISECT_START \
-+ git bisect start --auto-reset=original --no-checkout $HASH4 $HASH2 &&
++ git bisect start --reset-when-found=original --no-checkout $HASH4 $HASH2 &&
+
+ git bisect start --no-checkout $HASH4 $HASH2 &&
-+ test_auto_reset_fails \
-+ "cannot be used with.*--no-checkout" BISECT_AUTO_RESET \
-+ git bisect run --auto-reset=found true
++ test_reset_when_found_fails \
++ "cannot be used with.*--no-checkout" BISECT_RESET_WHEN_FOUND \
++ git bisect run --reset-when-found=found true
+'
+
-+test_expect_success 'without --auto-reset the bisection state is kept' '
++test_expect_success 'without --reset-when-found the bisection state is kept' '
+ test_when_finished "git bisect reset" &&
+ git bisect start $HASH4 $HASH2 &&
+ git bisect bad &&
+ test_bisect_state_file BISECT_START
+'
+
-+test_expect_success '--auto-reset does not leak into a later bisection' '
++test_expect_success '--reset-when-found does not leak into a later bisection' '
+ test_when_finished "git bisect reset; git checkout main" &&
-+ bisect_start_and_finish --auto-reset &&
++ bisect_start_and_finish --reset-when-found &&
+
+ git bisect start $HASH4 $HASH2 &&
+ git bisect bad &&
--
gitgitgadget
^ permalink raw reply
* [PATCH 1/2] remote: pass repository to push tracking helper
From: Harald Nordgren via GitGitGadget @ 2026-07-20 9:10 UTC (permalink / raw)
To: git; +Cc: Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2358.git.git.1784538618.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
The push tracking helper currently only needs the push remote. However,
resolving a URL-valued remote requires access to the repository's list
of configured remotes.
Pass the repository through the existing callers and mark the parameter
as unused for now. This prepares the helper for that lookup without
changing its behavior.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
remote.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/remote.c b/remote.c
index e6c52c850c..89d0f9e2d8 100644
--- a/remote.c
+++ b/remote.c
@@ -1887,7 +1887,8 @@ const char *branch_get_upstream(struct branch *branch, struct strbuf *err)
return branch->merge[0]->dst;
}
-static char *tracking_for_push_dest(struct remote *remote,
+static char *tracking_for_push_dest(struct repository *repo UNUSED,
+ struct remote *remote,
const char *refname,
struct strbuf *err)
{
@@ -1925,13 +1926,13 @@ static char *branch_get_push_1(struct repository *repo,
_("push refspecs for '%s' do not include '%s'"),
remote->name, branch->name);
- ret = tracking_for_push_dest(remote, dst, err);
+ ret = tracking_for_push_dest(repo, remote, dst, err);
free(dst);
return ret;
}
if (remote->mirror)
- return tracking_for_push_dest(remote, branch->refname, err);
+ return tracking_for_push_dest(repo, remote, branch->refname, err);
switch (push_default) {
case PUSH_DEFAULT_NOTHING:
@@ -1939,7 +1940,7 @@ static char *branch_get_push_1(struct repository *repo,
case PUSH_DEFAULT_MATCHING:
case PUSH_DEFAULT_CURRENT:
- return tracking_for_push_dest(remote, branch->refname, err);
+ return tracking_for_push_dest(repo, remote, branch->refname, err);
case PUSH_DEFAULT_UPSTREAM:
return xstrdup_or_null(branch_get_upstream(branch, err));
@@ -1953,7 +1954,7 @@ static char *branch_get_push_1(struct repository *repo,
up = branch_get_upstream(branch, err);
if (!up)
return NULL;
- cur = tracking_for_push_dest(remote, branch->refname, err);
+ cur = tracking_for_push_dest(repo, remote, branch->refname, err);
if (!cur)
return NULL;
if (strcmp(cur, up)) {
--
gitgitgadget
^ permalink raw reply related
* [PATCH 0/2] remote: resolve url push tracking
From: Harald Nordgren via GitGitGadget @ 2026-07-20 9:10 UTC (permalink / raw)
To: git; +Cc: Harald Nordgren
After renaming remotes, git status may stop showing the push branch even
though Git still pushes to the right URL, use the remote with the same URL
to find it.
Harald Nordgren (2):
remote: pass repository to push tracking helper
remote: resolve URL-valued push tracking remotes
Documentation/revisions.adoc | 3 +
remote.c | 36 ++++++++++--
remote.h | 2 +
t/t5505-remote.sh | 104 +++++++++++++++++++++++++++++++++++
transport.c | 5 +-
5 files changed, 144 insertions(+), 6 deletions(-)
base-commit: 41365c2a9ba347870b80881c0d67454edd22fd49
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2358%2FHaraldNordgren%2Fremote-resolve-url-push-tracking-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2358/HaraldNordgren/remote-resolve-url-push-tracking-v1
Pull-Request: https://github.com/git/git/pull/2358
--
gitgitgadget
^ permalink raw reply
* Re: [PATCH] userdiff: add support for Swift
From: Johannes Sixt @ 2026-07-20 8:49 UTC (permalink / raw)
To: Shlok Kulshreshtha
Cc: D. Ben Knoble, Junio C Hamano, René Scharfe, Eric Sunshine,
Scott L. Burson, git
In-Reply-To: <2a3a73c5-5e90-44a3-bf6a-6e98ce5e5a59@kdbg.org>
Am 18.07.26 um 20:11 schrieb Johannes Sixt:
> Am 17.07.26 um 16:02 schrieb Shlok Kulshreshtha:
>> +PATTERNS("swift",
>> + "^[ \t]*((@[A-Za-z_][A-Za-z0-9_]*(\\([^()]*\\))?[ \t]+)*([a-z]+[ \t]+)*(func|init|deinit|subscript|class|struct|enum|protocol|extension|actor)[ \t(?!<].*)$",
>
> This looks good.
>
> Notice, however, how the regular expression matcher has to backtrack on
> even simple lines such as
[...]
> It may be worth considering to enumerate all keywords and permit any run
> of them:
Let me back-paddle on this one. As I said, the original RE is good. I am
making up a problem here without providing evidence. Modern RE matchers
may be clever enough that there is no problem. If it turns out there is
a problem, we can improve later something that already works.
-- Hannes
^ permalink raw reply
* [PATCH v10 5/5] history: re-edit a squash with every message
From: Harald Nordgren via GitGitGadget @ 2026-07-20 8:27 UTC (permalink / raw)
To: git
Cc: Phillip Wood, D. Ben Knoble, Patrick Steinhardt, Matt Hunter,
Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2337.v10.git.git.1784536024.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
By default "git history squash" reuses the oldest commit's message, or
the replacement body from an amend! commit targeting it. When
--reedit-message is given it only reopened that selected message, so the
messages of the other commits in the range were lost.
Gather the message of every commit in the range and build the same editor
template that "git rebase -i --autosquash" shows for a squash, reusing
add_squash_combination_header(), add_squash_message_header() and
squash_subject_comment_len(). Feed the range through
todo_list_rearrange_squash() so that each fixup!, squash! or amend! is
grouped under the commit it targets rather than shown in commit order,
exactly as autosquash would arrange them.
Only the message text differs, the changes are always folded in. A fixup!
message is commented out in full under a "will be skipped" header, a
squash! keeps its body with only the marker subject commented, and an
amend! replaces its target's message unless a squash! already folded into
that target, in which case it behaves like a squash!.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
Documentation/git-history.adoc | 22 +++-
builtin/history.c | 104 +++++++++++++++++
t/t3455-history-squash.sh | 201 +++++++++++++++++++++++++++++++++
3 files changed, 321 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-history.adoc b/Documentation/git-history.adoc
index e1e930f355..6f3b031d2a 100644
--- a/Documentation/git-history.adoc
+++ b/Documentation/git-history.adoc
@@ -134,11 +134,12 @@ already on `topic`. Rev-list options may also be given, but any that would
change how the range is walked are overridden with a warning.
+
The oldest commit's message is preserved by default, except that an `amend!`
-commit targeting it replaces its message. Specify `--reedit-message` to edit
-the resulting message. A merge commit inside the range is folded like any
-other, but the range must have a single base, so a range that reaches more
-than one entry point (for example a side branch that forked before the range
-and was later merged into it) is rejected.
+commit targeting it replaces its message. With `--reedit-message`, an editor
+opens pre-filled with the messages of all the folded commits so you can
+combine them. A merge commit inside the range is folded like any other, but
+the range must have a single base, so a range that reaches more than one entry
+point (for example a side branch that forked before the range and was later
+merged into it) is rejected.
+
A `fixup!`, `squash!`, or `amend!` commit is refused unless the commit it
targets is also in the range, so the fold does not silently absorb a
@@ -146,6 +147,14 @@ marker meant for a commit outside it. As an exception, a range made up entirely
of markers for one target is combined into a single commit, keeping the last
`amend!` message if there is one.
+
+With `--reedit-message` the template mirrors `git rebase -i --autosquash`:
+each `fixup!`, `squash!`, or `amend!` is grouped under the commit it
+targets rather than shown in commit order. A `fixup!` message is dropped
+(commented out in full), a `squash!` keeps its body with only the marker
+subject commented, and an `amend!` replaces its target's message, unless
+a `squash!` folded into that target first, in which case it keeps its
+body like a `squash!`.
++
A branch or tag that points at a commit inside the range would be left
dangling once those commits are folded away, so with the default
`--update-refs=branches` the command refuses. Rerun with
@@ -162,7 +171,8 @@ OPTIONS
ref updates is generally safe.
`--reedit-message`::
- Open an editor to modify the rewritten commit's message.
+ Open an editor to modify the rewritten commit's message. For `squash`
+ the editor is pre-filled with the messages of all the folded commits.
`--empty=(drop|keep|abort)`::
Control what happens when a commit becomes empty as a result of the
diff --git a/builtin/history.c b/builtin/history.c
index 423c8beaaf..2542ea33a0 100644
--- a/builtin/history.c
+++ b/builtin/history.c
@@ -1255,6 +1255,102 @@ static int find_interior_ref(const struct reference *ref, void *cb_data)
return 0;
}
+static bool amend_replaces_target(struct todo_list *todo, int target)
+{
+ int i;
+
+ for (i = target + 1; i < todo->nr &&
+ todo->items[i].command != TODO_PICK; i++) {
+ if (todo->items[i].command == TODO_SQUASH)
+ return false;
+ if (todo->items[i].flags & TODO_REPLACE_FIXUP_MSG)
+ return true;
+ }
+ return false;
+}
+
+static int build_squash_message(struct repository *repo,
+ struct commit *base,
+ struct commit *tip,
+ struct strbuf *out)
+{
+ struct rev_info revs;
+ struct commit *commit;
+ struct strvec args = STRVEC_INIT;
+ struct todo_list todo = TODO_LIST_INIT;
+ struct replay_opts opts = REPLAY_OPTS_INIT;
+ int i, nr_commits, ret;
+
+ repo_init_revisions(repo, &revs, NULL);
+ strvec_push(&args, "ignored");
+ strvec_push(&args, "--reverse");
+ strvec_push(&args, "--topo-order");
+ strvec_pushf(&args, "%s..%s", oid_to_hex(&base->object.oid),
+ oid_to_hex(&tip->object.oid));
+ setup_revisions_from_strvec(&args, &revs, NULL);
+
+ if (prepare_revision_walk(&revs) < 0) {
+ ret = error(_("error preparing revisions"));
+ goto out;
+ }
+
+ while ((commit = get_revision(&revs)))
+ strbuf_addf(&todo.buf, "pick %s\n",
+ oid_to_hex(&commit->object.oid));
+
+ if (todo_list_parse_insn_buffer(repo, &opts, todo.buf.buf, &todo) < 0 ||
+ todo_list_rearrange_squash(&todo) < 0) {
+ ret = error(_("could not prepare the squash message"));
+ goto out;
+ }
+
+ nr_commits = todo.nr;
+ for (i = 0; i < nr_commits; i++) {
+ struct todo_item *item = &todo.items[i];
+ const char *message, *body;
+ size_t commented_len;
+ bool skip, squashing;
+
+ squashing = item->command == TODO_SQUASH ||
+ (item->flags & TODO_REPLACE_FIXUP_MSG);
+ if (item->command == TODO_PICK)
+ skip = amend_replaces_target(&todo, i);
+ else
+ skip = !squashing;
+
+ message = repo_logmsg_reencode(repo, item->commit, NULL, NULL);
+ find_commit_subject(message, &body);
+
+ if (skip)
+ commented_len = strlen(body);
+ else if (squashing)
+ commented_len = squash_subject_comment_len(body, 1);
+ else
+ commented_len = 0;
+
+ if (!i)
+ add_squash_combination_header(out, nr_commits);
+ strbuf_addch(out, '\n');
+ add_squash_message_header(out, i + 1, skip);
+ strbuf_addstr(out, "\n\n");
+ strbuf_add_commented_lines(out, body, commented_len, comment_line_str);
+ strbuf_addstr(out, body + commented_len);
+ strbuf_complete_line(out);
+
+ repo_unuse_commit_buffer(repo, item->commit, message);
+ }
+
+ ret = 0;
+
+out:
+ todo_list_release(&todo);
+ replay_opts_release(&opts);
+ reset_revision_walk();
+ release_revisions(&revs);
+ strvec_clear(&args);
+ return ret;
+}
+
static int cmd_history_squash(int argc,
const char **argv,
const char *prefix,
@@ -1338,6 +1434,14 @@ static int cmd_history_squash(int argc,
}
}
+ if (flags & COMMIT_TREE_EDIT_MESSAGE) {
+ strbuf_reset(&message);
+ ret = build_squash_message(repo, base, tip, &message);
+ if (ret < 0)
+ goto out;
+ message_template = message.buf;
+ }
+
ret = setup_revwalk(repo, action, tip, &revs);
if (ret < 0)
goto out;
diff --git a/t/t3455-history-squash.sh b/t/t3455-history-squash.sh
index 9c362f3094..6fce924327 100755
--- a/t/t3455-history-squash.sh
+++ b/t/t3455-history-squash.sh
@@ -271,6 +271,207 @@ test_expect_success 'preserves authorship of the oldest commit' '
test_cmp expect actual
'
+test_expect_success '--reedit-message offers every folded-in message' '
+ git reset --hard start &&
+ stage_file b &&
+ git commit -m "re-one subject" -m "re-one body line" &&
+ test_commit --no-tag re-two file c &&
+ test_commit re-three file d &&
+
+ write_script editor <<-\EOF &&
+ cat "$1" >edited &&
+ echo combined >"$1"
+ EOF
+ test_set_editor "$(pwd)/editor" &&
+ git history squash --reedit-message start.. &&
+
+ cat >expect <<-EOF &&
+ # This is a combination of 3 commits.
+ # This is the 1st commit message:
+
+ re-one subject
+
+ re-one body line
+
+ # This is the commit message #2:
+
+ re-two
+
+ # This is the commit message #3:
+
+ re-three
+
+ # Please enter the commit message for the squash changes. Lines starting
+ # with ${SQ}#${SQ} will be ignored, and an empty message aborts the commit.
+ # Changes to be committed:
+ # modified: file
+ #
+ EOF
+ test_cmp expect edited &&
+ check_log_subjects -1 <<-\EOF
+ combined
+ EOF
+'
+
+test_expect_success '--reedit-message handles fixup!, squash! and amend! like rebase' '
+ git reset --hard start &&
+ test_commit --no-tag mark-base file b &&
+ stage_file c &&
+ commit_with_message "fixup! mark-base\n\nfixup body\n" &&
+ stage_file d &&
+ commit_with_message "squash! mark-base\n\nsquash remark\n" &&
+ stage_file e &&
+ commit_with_message "amend! mark-base\n\namended message\n" &&
+
+ write_script editor <<-\EOF &&
+ cat "$1" >edited
+ EOF
+ test_set_editor "$(pwd)/editor" &&
+ git history squash --reedit-message start.. &&
+
+ cat >expect <<-EOF &&
+ # This is a combination of 4 commits.
+ # This is the 1st commit message:
+
+ mark-base
+
+ # The commit message #2 will be skipped:
+
+ # fixup! mark-base
+ #
+ # fixup body
+
+ # This is the commit message #3:
+
+ # squash! mark-base
+
+ squash remark
+
+ # This is the commit message #4:
+
+ # amend! mark-base
+
+ amended message
+
+ # Please enter the commit message for the squash changes. Lines starting
+ # with ${SQ}#${SQ} will be ignored, and an empty message aborts the commit.
+ # Changes to be committed:
+ # modified: file
+ #
+ EOF
+ test_cmp expect edited &&
+ check_log_messages -1 <<-\EOF
+ mark-base
+
+ squash remark
+
+ amended message
+
+ EOF
+'
+
+test_expect_success '--reedit-message groups fixups under their targets' '
+ git reset --hard start &&
+ test_commit --no-tag alpha file a1 &&
+ test_commit --no-tag beta file b1 &&
+ stage_file a2 &&
+ commit_with_message "fixup! alpha\n" &&
+ stage_file b2 &&
+ commit_with_message "fixup! beta\n" &&
+
+ write_script editor <<-\EOF &&
+ cat "$1" >edited
+ EOF
+ test_set_editor "$(pwd)/editor" &&
+ git history squash --reedit-message start.. &&
+
+ cat >expect <<-EOF &&
+ # This is a combination of 4 commits.
+ # This is the 1st commit message:
+
+ alpha
+
+ # The commit message #2 will be skipped:
+
+ # fixup! alpha
+
+ # This is the commit message #3:
+
+ beta
+
+ # The commit message #4 will be skipped:
+
+ # fixup! beta
+
+ # Please enter the commit message for the squash changes. Lines starting
+ # with ${SQ}#${SQ} will be ignored, and an empty message aborts the commit.
+ # Changes to be committed:
+ # modified: file
+ #
+ EOF
+ test_cmp expect edited
+'
+
+test_expect_success '--reedit-message lets amend! replace its target message' '
+ git reset --hard start &&
+ test_commit --no-tag mark-base file b &&
+ stage_file c &&
+ commit_with_message "amend! mark-base\n\namended message\n" &&
+ stage_file d &&
+ commit_with_message "squash! mark-base\n\nsquash remark\n" &&
+
+ write_script editor <<-\EOF &&
+ cat "$1" >edited
+ EOF
+ test_set_editor "$(pwd)/editor" &&
+ git history squash --reedit-message start.. &&
+
+ cat >expect <<-EOF &&
+ # This is a combination of 3 commits.
+ # The 1st commit message will be skipped:
+
+ # mark-base
+
+ # This is the commit message #2:
+
+ # amend! mark-base
+
+ amended message
+
+ # This is the commit message #3:
+
+ # squash! mark-base
+
+ squash remark
+
+ # Please enter the commit message for the squash changes. Lines starting
+ # with ${SQ}#${SQ} will be ignored, and an empty message aborts the commit.
+ # Changes to be committed:
+ # modified: file
+ #
+ EOF
+ test_cmp expect edited &&
+ check_log_messages -1 <<-\EOF
+ amended message
+
+ squash remark
+
+ EOF
+'
+
+test_expect_success '--reedit-message aborts on an empty message' '
+ git reset --hard three &&
+ head_before=$(git rev-parse HEAD) &&
+
+ write_script editor <<-\EOF &&
+ >"$1"
+ EOF
+ test_set_editor "$(pwd)/editor" &&
+ test_must_fail git history squash --reedit-message start.. &&
+
+ test_cmp_rev "$head_before" HEAD
+'
+
test_expect_success '--update-refs=head only moves HEAD' '
git reset --hard three &&
git branch -f other HEAD &&
--
gitgitgadget
^ 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;
as well as URLs for NNTP newsgroup(s).