* [PATCH 1/2] commit: extract commit_index_files_or_die() helper
From: Harald Nordgren via GitGitGadget @ 2026-06-13 9:16 UTC (permalink / raw)
To: git; +Cc: Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2334.git.git.1781342189.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
A later change adds a second caller that commits the index lock and dies
on failure, so wrap that into a helper to avoid duplicating its message.
No functional change intended.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
builtin/commit.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 28f6174503..1a51450660 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -248,6 +248,14 @@ static int commit_index_files(void)
return err;
}
+static void commit_index_files_or_die(void)
+{
+ if (commit_index_files())
+ die(_("repository has been updated, but unable to write\n"
+ "new index file. Check that disk is not full and quota is\n"
+ "not exceeded, and then \"git restore --staged :/\" to recover."));
+}
+
/*
* Take a union of paths in the index and the named tree (typically, "HEAD"),
* and return the paths that match the given pattern in list.
@@ -1954,10 +1962,7 @@ int cmd_commit(int argc,
unlink(git_path_merge_mode(the_repository));
unlink(git_path_squash_msg(the_repository));
- if (commit_index_files())
- die(_("repository has been updated, but unable to write\n"
- "new index file. Check that disk is not full and quota is\n"
- "not exceeded, and then \"git restore --staged :/\" to recover."));
+ commit_index_files_or_die();
git_test_write_commit_graph_or_die(the_repository->objects->sources);
--
gitgitgadget
^ permalink raw reply related
* [PATCH 2/2] commit: keep the commit on a no-op amend
From: Harald Nordgren via GitGitGadget @ 2026-06-13 9:16 UTC (permalink / raw)
To: git; +Cc: Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2334.git.git.1781342189.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
"git commit --amend --no-edit" reset the committer date to "now" and
rewrote the commit even when nothing else changed, moving the branch tip
to a new hash for an effective no-op.
Build the amended commit reusing the existing committer date: if that
reproduces the current commit, leave the branch alone, report "nothing
to amend", and skip the reflog entry and the post-commit and post-rewrite
hooks.
Signing always rewrites the commit, since its signature cannot reproduce
the original, so it skips this detection.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
Documentation/git-commit.adoc | 6 ++
builtin/commit.c | 56 ++++++++++++
t/t7501-commit-basic-functionality.sh | 119 ++++++++++++++++++++++++++
3 files changed, 181 insertions(+)
diff --git a/Documentation/git-commit.adoc b/Documentation/git-commit.adoc
index 8329c1034b..c433c60929 100644
--- a/Documentation/git-commit.adoc
+++ b/Documentation/git-commit.adoc
@@ -282,6 +282,12 @@ variable (see linkgit:git-config[1]).
parents and author as the current one (the `--reset-author`
option can countermand this).
+
+If the amended commit would be identical to the original (its tree,
+message, author, parents, and committer are all unchanged), the original
+committer date is kept so that the commit, and thus the branch tip, is
+left untouched. A commit that is being signed (`-S`, or `commit.gpgsign`)
+is always rewritten, since its signature cannot reproduce the original.
++
--
It is a rough equivalent for:
diff --git a/builtin/commit.c b/builtin/commit.c
index 1a51450660..e330a53d5c 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -17,6 +17,7 @@
#include "dir.h"
#include "editor.h"
#include "environment.h"
+#include "ident.h"
#include "diff.h"
#include "commit.h"
#include "add-interactive.h"
@@ -760,6 +761,49 @@ static void prepare_amend_commit(struct commit *commit, struct strbuf *sb,
repo_unuse_commit_buffer(the_repository, commit, buffer);
}
+/*
+ * Rebuild the amended commit reusing the existing committer date and report
+ * whether it reproduces the current commit. Because the committer date is the
+ * only field that an amend would otherwise replace with "now", an exact match
+ * means everything else (tree, message, author, parents, committer identity)
+ * is unchanged too.
+ */
+static int amend_is_noop(struct commit *current_head,
+ const struct strbuf *message,
+ const struct commit_list *parents,
+ const char *author,
+ const struct commit_extra_header *extra,
+ struct object_id *oid)
+{
+ const char *buffer, *committer_line;
+ size_t len;
+ struct ident_split ident;
+ struct strbuf date = STRBUF_INIT;
+ int unchanged = 0;
+
+ buffer = repo_get_commit_buffer(the_repository, current_head, NULL);
+ committer_line = find_commit_header(buffer, "committer", &len);
+ if (committer_line && !split_ident_line(&ident, committer_line, len) &&
+ ident.date_begin) {
+ const char *committer;
+
+ strbuf_add(&date, ident.date_begin,
+ ident.tz_end - ident.date_begin);
+ committer = fmt_ident(getenv("GIT_COMMITTER_NAME"),
+ getenv("GIT_COMMITTER_EMAIL"),
+ WANT_COMMITTER_IDENT, date.buf,
+ IDENT_STRICT);
+ if (!commit_tree_extended(message->buf, message->len,
+ &the_repository->index->cache_tree->oid,
+ parents, oid, author, committer, NULL,
+ extra))
+ unchanged = oideq(oid, ¤t_head->object.oid);
+ }
+ repo_unuse_commit_buffer(the_repository, current_head, buffer);
+ strbuf_release(&date);
+ return unchanged;
+}
+
static void change_data_free(void *util, const char *str UNUSED)
{
struct wt_status_change_data *d = util;
@@ -1943,6 +1987,18 @@ int cmd_commit(int argc,
append_merge_tag_headers(parents, &tail);
}
+ if (amend && current_head && !sign_commit &&
+ amend_is_noop(current_head, &sb, parents, author_ident.buf,
+ extra, &oid)) {
+ commit_index_files_or_die();
+ if (!quiet)
+ fprintf(stderr,
+ _("nothing to amend; %s left unchanged\n"),
+ repo_find_unique_abbrev(the_repository, &oid,
+ DEFAULT_ABBREV));
+ goto cleanup;
+ }
+
if (commit_tree_extended(sb.buf, sb.len, &the_repository->index->cache_tree->oid,
parents, &oid, author_ident.buf, NULL,
sign_commit, extra)) {
diff --git a/t/t7501-commit-basic-functionality.sh b/t/t7501-commit-basic-functionality.sh
index a37509f004..160edb9c0a 100755
--- a/t/t7501-commit-basic-functionality.sh
+++ b/t/t7501-commit-basic-functionality.sh
@@ -11,6 +11,7 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
. ./test-lib.sh
. "$TEST_DIRECTORY/lib-diff.sh"
+. "$TEST_DIRECTORY/lib-gpg.sh"
author='The Real Author <someguy@his.email.org>'
@@ -654,6 +655,124 @@ test_expect_success 'amend commit to fix author' '
'
+test_expect_success 'amend --no-edit that changes nothing keeps the commit' '
+ git reset --hard &&
+ old=$(git rev-parse HEAD) &&
+ test_tick &&
+ git commit --amend --no-edit 2>err &&
+ test_cmp_rev $old HEAD &&
+ test_grep "nothing to amend" err
+'
+
+test_expect_success 'amend --no-edit keeps the commit out of the reflog' '
+ git reset --hard &&
+ git rev-parse HEAD@{0} >before &&
+ test_tick &&
+ git commit --amend --no-edit &&
+ git rev-parse HEAD@{0} >after &&
+ test_cmp before after
+'
+
+test_expect_success 'amend --signoff is idempotent once signed off' '
+ git reset --hard &&
+ test_tick &&
+ git commit --amend --no-edit --signoff &&
+ signed=$(git rev-parse HEAD) &&
+ git log -1 --format=%B | grep "^Signed-off-by:" &&
+ test_tick &&
+ git commit --amend --no-edit --signoff &&
+ test_cmp_rev $signed HEAD
+'
+
+test_expect_success 'amend that changes the tree still rewrites the commit' '
+ git reset --hard &&
+ old=$(git rev-parse HEAD) &&
+ echo changed >>file &&
+ git add file &&
+ test_tick &&
+ git commit --amend --no-edit &&
+ test_cmp_rev ! $old HEAD
+'
+
+test_expect_success 'amend that changes the committer still rewrites the commit' '
+ git reset --hard &&
+ old=$(git rev-parse HEAD) &&
+ test_tick &&
+ GIT_COMMITTER_EMAIL=other@example.com \
+ git commit --amend --no-edit &&
+ test_cmp_rev ! $old HEAD
+'
+
+test_expect_success 'amend that changes only the message still rewrites the commit' '
+ git reset --hard &&
+ old=$(git rev-parse HEAD) &&
+ test_tick &&
+ git commit --amend -m "new message" &&
+ test_cmp_rev ! $old HEAD &&
+ echo "new message" >expect &&
+ git log -1 --format=%s >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'amend --allow-empty of an empty commit that changes nothing keeps it' '
+ test_when_finished "git reset --hard parent && git tag -d parent" &&
+ git tag parent &&
+ git commit --allow-empty -m "empty" &&
+ old=$(git rev-parse HEAD) &&
+ test_tick &&
+ git commit --amend --no-edit --allow-empty 2>err &&
+ test_cmp_rev $old HEAD &&
+ test_grep "nothing to amend" err
+'
+
+test_expect_success GPG 'amend --no-edit of a signed commit is not a no-op' '
+ git reset --hard &&
+ test_tick &&
+ git commit --amend --no-edit -S &&
+ signed=$(git rev-parse HEAD) &&
+ git verify-commit HEAD &&
+ test_tick &&
+ git commit --amend --no-edit -S &&
+ test_cmp_rev ! $signed HEAD &&
+ git verify-commit HEAD
+'
+
+test_expect_success GPG 'amend --no-edit with commit.gpgsign is not a no-op' '
+ git reset --hard &&
+ test_tick &&
+ old=$(git rev-parse HEAD) &&
+ git -c commit.gpgsign=true commit --amend --no-edit &&
+ test_cmp_rev ! $old HEAD &&
+ git verify-commit HEAD
+'
+
+test_expect_success 'amend --reset-author rewrites the commit' '
+ git reset --hard &&
+ old=$(git rev-parse HEAD) &&
+ test_tick &&
+ git commit --amend --no-edit --reset-author &&
+ test_cmp_rev ! $old HEAD
+'
+
+test_expect_success 'amend --date rewrites the commit' '
+ git reset --hard &&
+ old=$(git rev-parse HEAD) &&
+ test_tick &&
+ git commit --amend --no-edit --date="@1234567890 +0000" &&
+ test_cmp_rev ! $old HEAD
+'
+
+test_expect_success 'amend that changes nothing skips the post-commit hook' '
+ test_when_finished "rm -f post-commit.ran" &&
+ test_hook post-commit <<-\EOF &&
+ >post-commit.ran
+ EOF
+ git reset --hard &&
+ test_tick &&
+ git commit --amend --no-edit &&
+ test_path_is_missing post-commit.ran
+'
+
test_expect_success 'git commit <file> with dirty index' '
echo tacocat >elif &&
echo tehlulz >chz &&
--
gitgitgadget
^ permalink raw reply related
* Re: [RFC] commit-reach: terminate merge-base walk when one paint side is exhausted
From: Kristofer Karlsson @ 2026-06-13 9:42 UTC (permalink / raw)
To: Derrick Stolee; +Cc: git
In-Reply-To: <8c06cc48-d036-4d01-98d3-e94b5edb389c@gmail.com>
On Fri, 12 Jun 2026 at 17:48, Derrick Stolee <stolee@gmail.com> wrote:
> Suppose developers are merging into 'main' frequently. On occasion,
> the tip of 'main' is merged into a new 'release' branch. Thus, the
> first-parent history of 'release' is long and completely separate
> from the commit history of 'main'. To reach the queue_has_nonstale()
> exit condition, we'd need to walk the entire history.
>
> However, if we focus on the single-side condition you are proposing,
> we can stop walking once everything in the queue that is reachable
> form 'main' is also reachable from that top merge-base.
Exactly - I have a similar example and a minimal reproduction idea
for the current problem:
Consider two graph shapes for `git merge-base H B`:
Shape 1 (fast): Shape 2 (slow):
H B H B
| | | |
A | A |
\ / / \ /
C X C
| | |
... \ /
D
|
...
In shape 2, A is a merge commit with parents C and X. X
branches off from an older commit D on the main line and gets
merged back. This is extremely common in repositories that
use merge commits (monorepos, release-branch workflows).
In both shapes, C is the only merge-base. But in shape 2, the
walk through X's ancestry is P1-only: STALE propagates through
C's ancestors but never reaches D's lineage. The max_nonstale
pointer stays alive until D's entire history is drained.
On a 2.5M-commit monorepo, we measured this directly by creating
test commits with `git commit-tree`:
Shape 1: 10ms
Shape 2: 4.85s
Both running with stock git 2.53 and both found the merge-base C.
A single merge bypass to old history is enough to force the walk
through the entire graph. In practice, master's history contains
many such merge commits, which is why we consistently see 5-7s
wall-clock time for merge-base queries.
With per-side tracking, the P2 side exhausts immediately after C
is found (B's only parent C has been processed), and the walk
terminates in 6ms regardless of how deep the P1-only bypass goes.
As you noted, the "release branch" shape is another case where
this helps -- the main side exhausts at the merge-base while
release's first-parent history is entirely one-sided.
> But I think your single-sided approach is a better way to get the gains
> that you want. I think that case is much more likely to occur.
Thanks for working through this with me. I started thinking the idea
itself is not strong enough on its own, so I have attempted to
write a more formal correctness proof covering the
drain phase, result exactness, and the INFINITY/finite region
boundary. It is too long to inline (~2000 words) and the high
level argument is already in this thread, so linking it here
instead - I consider it optional reading, since it's not really
light reading and we can likely make progress without it:
https://gist.github.com/spkrka/621695aa464df2a8c1837e9abca822e3
The proof assumes finite generation numbers (commit-graph
present). The side-exhaustion check is guarded by
generation < INFINITY in the implementation.
I am far from an expert in logical proofs though, so this may
not be strong enough to be useful, but it may be possible to
fix it - or it will uncover some flaw that invalidates the idea.
> Thanks for your persistence in working on this through my
> misunderstanding.
>
To be fair, I think this was more a case of me not being fully
able to explain the idea with enough precision initially, so I
appreciate getting the opportunity to refine it with your very useful
example case.
Thanks,
Kristofer
^ permalink raw reply
* Re: [PATCH 0/2] commit: preserve commit hash on a no-op amend
From: Johannes Sixt @ 2026-06-13 9:59 UTC (permalink / raw)
To: Harald Nordgren; +Cc: git, Harald Nordgren via GitGitGadget
In-Reply-To: <pull.2334.git.git.1781342189.gitgitgadget@gmail.com>
Am 13.06.26 um 11:16 schrieb Harald Nordgren via GitGitGadget:
> git commit --amend --no-edit rewrote the commit and moved the branch tip
> even when nothing changed, because the committer date was reset to "now".
> Reuse the existing committer date so a no-op amend keeps the commit hash and
> leaves the branch untouched.
`git commit --amend --no-edit` is a way to set the committer timestamp
to the current time without changing other aspects of the commit. This
takes away this ability, doesn't it?
Is this keyed to --no-edit? Why is this mode special? Wouldn't it be an
identical case when the commit message is passed to the editor, but
comes back unchanged?
An invocation of `git commit` asks to "please make a new commit". But in
the suggested mode, no new commit is created. Shouldn't this then be
regarded as failure?
What happens with the current branch? Is it left unchanged (no ref
update occurs) or is it changed (a ref update occurs, but it happens to
be a no-op)? And does this then generate a reflog entry?
The updated documentation says about signed commits (note: I am totally
clueless about commit signing procedures):
> A commit that is being signed (`-S`, or `commit.gpgsign`)
> is always rewritten, since its signature cannot reproduce the original.
But if the commit doesn't change in any way, why should the signature be
invalidated, rewritten, or updated?
-- Hannes
^ permalink raw reply
* [PATCH v4 0/3] compat/posix.h: enable UNUSED warning messages for Clang
From: Dominik Loidolt @ 2026-06-13 12:27 UTC (permalink / raw)
To: git; +Cc: ps, gitster, asedeno, asedeno, avarab, Dominik Loidolt
In-Reply-To: <20260608124419.38905-1-dominik.loidolt@univie.ac.at>
This series enables the intended UNUSED warning message with Clang by
adding a dedicated Clang version check. It also cleans up the nearby
GIT_GNUC_PREREQ() and UNUSED macros.
Changes since v3:
- split style-only cleanups into their own patch
- fix the UNUSED preprocessor indentation style
- simplify the GIT_GNUC_PREREQ() comparison commit message
- keep the Clang-specific note in the patch that adds GIT_CLANG_PREREQ()
Thanks,
Dominik
Dominik Loidolt (3):
compat/posix.h: enable UNUSED warning messages for Clang
compat/posix.h: clean up GIT_GNUC_PREREQ() and UNUSED
compat/posix.h: simplify GIT_GNUC_PREREQ() comparison
compat/posix.h | 35 +++++++++++++++++++++++------------
1 file changed, 23 insertions(+), 12 deletions(-)
Range-diff against v3:
1: 62f5bce297 ! 1: 9ec87cd815 compat/posix.h: enable UNUSED warning messages for Clang
@@ Commit message
Signed-off-by: Dominik Loidolt <dominik.loidolt@univie.ac.at>
## compat/posix.h ##
+@@
+ * ... code requiring gcc 2.8 or later ...
+ * #endif
+ *
++ * Note that Clang and other compilers define __GNUC__ for compatibility; use
++ * GIT_CLANG_PREREQ() to check for specific Clang versions.
++ *
+ * This macro of course is not part of POSIX, but we need it for the UNUSED
+ * macro which is used by some of our POSIX compatibility wrappers.
+ */
@@
#define GIT_GNUC_PREREQ(maj, min) 0
#endif
2: a8fe5047a4 ! 2: 1a695af9ca compat/posix.h: simplify GIT_GNUC_PREREQ() comparison
@@ Metadata
Author: Dominik Loidolt <dominik.loidolt@univie.ac.at>
## Commit message ##
- compat/posix.h: simplify GIT_GNUC_PREREQ() comparison
+ compat/posix.h: clean up GIT_GNUC_PREREQ() and UNUSED
- Replace the glibc-style bit-shift version comparison with an explicit
- major/minor comparison. This is easier to read and is consistent with
- the format already used by GIT_CLANG_PREREQ() and many BSD
- <sys/cdefs.h> headers.
+ Fix the preprocessor indentation of the GIT_GNUC_PREREQ() and UNUSED
+ macros according to the CodingGuidelines, without changing their
+ behavior.
- This has no runtime impact, as the macro is evaluated at compile time.
- It is also more future-proof, as it no longer assumes that GCC version
- components stay below 65536.
+ Adjust the spelling in the GIT_GNUC_PREREQ() comment block.
Signed-off-by: Dominik Loidolt <dominik.loidolt@univie.ac.at>
## compat/posix.h ##
@@
- #define _FILE_OFFSET_BITS 64
/*
-- * Derived from Linux "Features Test Macro" header
+ * Derived from Linux "Features Test Macro" header
- * Convenience macros to test the versions of gcc (or
- * a compatible compiler).
+ * Convenience macros to test the versions of GCC (or a compatible compiler).
@@ compat/posix.h
+ * ... code requiring GCC 2.8 or later ...
* #endif
*
-+ * Note that Clang and other compilers define __GNUC__ for compatibility; use
-+ * GIT_CLANG_PREREQ() to check for specific Clang versions.
-+ *
+ * Note that Clang and other compilers define __GNUC__ for compatibility; use
+@@
+ *
* This macro of course is not part of POSIX, but we need it for the UNUSED
* macro which is used by some of our POSIX compatibility wrappers.
-*/
+ */
#if defined(__GNUC__) && defined(__GNUC_MINOR__)
# define GIT_GNUC_PREREQ(maj, min) \
-- ((__GNUC__ << 16) + __GNUC_MINOR__ >= ((maj) << 16) + (min))
-+ ((__GNUC__ > (maj)) || \
-+ (__GNUC__ == (maj) && __GNUC_MINOR__ >= (min)))
+ ((__GNUC__ << 16) + __GNUC_MINOR__ >= ((maj) << 16) + (min))
#else
- #define GIT_GNUC_PREREQ(maj, min) 0
+# define GIT_GNUC_PREREQ(maj, min) 0
#endif
/* Similar for Clang. */
+@@
+ * compilation, consider using MAYBE_UNUSED instead.
+ */
+ #if GIT_GNUC_PREREQ(4, 5) || GIT_CLANG_PREREQ(2, 9)
+-#define UNUSED __attribute__((unused)) \
+- __attribute__((deprecated ("parameter declared as UNUSED")))
++# define UNUSED __attribute__((unused)) \
++ __attribute__((deprecated("parameter declared as UNUSED")))
+ #elif defined(__GNUC__)
+-#define UNUSED __attribute__((unused)) \
++# define UNUSED __attribute__((unused)) \
+ __attribute__((deprecated))
+ #else
+-#define UNUSED
++# define UNUSED
+ #endif
+
+ #ifdef __MINGW64__
-: ---------- > 3: 289b7d9f8e compat/posix.h: simplify GIT_GNUC_PREREQ() comparison
base-commit: a89346e34a937f001e5d397ee62224e3e9852040
--
2.54.0
^ permalink raw reply
* [PATCH v4 2/3] compat/posix.h: clean up GIT_GNUC_PREREQ() and UNUSED
From: Dominik Loidolt @ 2026-06-13 12:27 UTC (permalink / raw)
To: git; +Cc: ps, gitster, asedeno, asedeno, avarab, Dominik Loidolt
In-Reply-To: <20260613122711.38662-1-dominik.loidolt@univie.ac.at>
Fix the preprocessor indentation of the GIT_GNUC_PREREQ() and UNUSED
macros according to the CodingGuidelines, without changing their
behavior.
Adjust the spelling in the GIT_GNUC_PREREQ() comment block.
Signed-off-by: Dominik Loidolt <dominik.loidolt@univie.ac.at>
---
compat/posix.h | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/compat/posix.h b/compat/posix.h
index 273cb87101..d2de5cedf5 100644
--- a/compat/posix.h
+++ b/compat/posix.h
@@ -5,11 +5,10 @@
/*
* Derived from Linux "Features Test Macro" header
- * Convenience macros to test the versions of gcc (or
- * a compatible compiler).
+ * Convenience macros to test the versions of GCC (or a compatible compiler).
* Use them like this:
* #if GIT_GNUC_PREREQ (2,8)
- * ... code requiring gcc 2.8 or later ...
+ * ... code requiring GCC 2.8 or later ...
* #endif
*
* Note that Clang and other compilers define __GNUC__ for compatibility; use
@@ -17,12 +16,12 @@
*
* This macro of course is not part of POSIX, but we need it for the UNUSED
* macro which is used by some of our POSIX compatibility wrappers.
-*/
+ */
#if defined(__GNUC__) && defined(__GNUC_MINOR__)
# define GIT_GNUC_PREREQ(maj, min) \
((__GNUC__ << 16) + __GNUC_MINOR__ >= ((maj) << 16) + (min))
#else
- #define GIT_GNUC_PREREQ(maj, min) 0
+# define GIT_GNUC_PREREQ(maj, min) 0
#endif
/* Similar for Clang. */
@@ -48,13 +47,13 @@
* compilation, consider using MAYBE_UNUSED instead.
*/
#if GIT_GNUC_PREREQ(4, 5) || GIT_CLANG_PREREQ(2, 9)
-#define UNUSED __attribute__((unused)) \
- __attribute__((deprecated ("parameter declared as UNUSED")))
+# define UNUSED __attribute__((unused)) \
+ __attribute__((deprecated("parameter declared as UNUSED")))
#elif defined(__GNUC__)
-#define UNUSED __attribute__((unused)) \
+# define UNUSED __attribute__((unused)) \
__attribute__((deprecated))
#else
-#define UNUSED
+# define UNUSED
#endif
#ifdef __MINGW64__
--
2.54.0
^ permalink raw reply related
* [PATCH v4 1/3] compat/posix.h: enable UNUSED warning messages for Clang
From: Dominik Loidolt @ 2026-06-13 12:27 UTC (permalink / raw)
To: git; +Cc: ps, gitster, asedeno, asedeno, avarab, Dominik Loidolt
In-Reply-To: <20260613122711.38662-1-dominik.loidolt@univie.ac.at>
Use a dedicated Clang version check for the UNUSED macro.
Commit 7c07f36ad2 (git-compat-util.h: GCC deprecated message arg only in
GCC 4.5+, 2022-10-05) restricted use of the deprecated attribute's
message argument in the UNUSED macro to GCC 4.5 or newer.
Clang identifies itself as GNUC 4.2.1 for compatibility, so
GIT_GNUC_PREREQ(4, 5) does not detect whether Clang supports the
deprecated("...") form. Add GIT_CLANG_PREREQ() macro and use it to
enable the UNUSED warning message for Clang 2.9 and newer.
Signed-off-by: Dominik Loidolt <dominik.loidolt@univie.ac.at>
---
compat/posix.h | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/compat/posix.h b/compat/posix.h
index faaae1b655..273cb87101 100644
--- a/compat/posix.h
+++ b/compat/posix.h
@@ -12,6 +12,9 @@
* ... code requiring gcc 2.8 or later ...
* #endif
*
+ * Note that Clang and other compilers define __GNUC__ for compatibility; use
+ * GIT_CLANG_PREREQ() to check for specific Clang versions.
+ *
* This macro of course is not part of POSIX, but we need it for the UNUSED
* macro which is used by some of our POSIX compatibility wrappers.
*/
@@ -22,6 +25,15 @@
#define GIT_GNUC_PREREQ(maj, min) 0
#endif
+/* Similar for Clang. */
+#if defined(__clang__) && defined(__clang_minor__) && defined(__clang_major__)
+# define GIT_CLANG_PREREQ(maj, min) \
+ ((__clang_major__ > (maj)) || \
+ (__clang_major__ == (maj) && __clang_minor__ >= (min)))
+#else
+# define GIT_CLANG_PREREQ(maj, min) 0
+#endif
+
/*
* UNUSED marks a function parameter that is always unused. It also
* can be used to annotate a function, a variable, or a type that is
@@ -35,7 +47,7 @@
* When a parameter may be used or unused, depending on conditional
* compilation, consider using MAYBE_UNUSED instead.
*/
-#if GIT_GNUC_PREREQ(4, 5)
+#if GIT_GNUC_PREREQ(4, 5) || GIT_CLANG_PREREQ(2, 9)
#define UNUSED __attribute__((unused)) \
__attribute__((deprecated ("parameter declared as UNUSED")))
#elif defined(__GNUC__)
--
2.54.0
^ permalink raw reply related
* [PATCH v4 3/3] compat/posix.h: simplify GIT_GNUC_PREREQ() comparison
From: Dominik Loidolt @ 2026-06-13 12:27 UTC (permalink / raw)
To: git; +Cc: ps, gitster, asedeno, asedeno, avarab, Dominik Loidolt
In-Reply-To: <20260613122711.38662-1-dominik.loidolt@univie.ac.at>
GIT_GNUC_PREREQ() uses a glibc-style bit-shift version comparison,
which is harder to read than an explicit major/minor comparison.
Use an explicit comparison, as in many BSD <sys/cdefs.h> headers, and
drop the Linux header attribution comment because it no longer applies.
Signed-off-by: Dominik Loidolt <dominik.loidolt@univie.ac.at>
---
compat/posix.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/compat/posix.h b/compat/posix.h
index d2de5cedf5..2f01564b0d 100644
--- a/compat/posix.h
+++ b/compat/posix.h
@@ -4,7 +4,6 @@
#define _FILE_OFFSET_BITS 64
/*
- * Derived from Linux "Features Test Macro" header
* Convenience macros to test the versions of GCC (or a compatible compiler).
* Use them like this:
* #if GIT_GNUC_PREREQ (2,8)
@@ -19,7 +18,8 @@
*/
#if defined(__GNUC__) && defined(__GNUC_MINOR__)
# define GIT_GNUC_PREREQ(maj, min) \
- ((__GNUC__ << 16) + __GNUC_MINOR__ >= ((maj) << 16) + (min))
+ ((__GNUC__ > (maj)) || \
+ (__GNUC__ == (maj) && __GNUC_MINOR__ >= (min)))
#else
# define GIT_GNUC_PREREQ(maj, min) 0
#endif
--
2.54.0
^ permalink raw reply related
* Re: [PATCH 0/9] refs: stop using `chdir_notify_reparent()`
From: Jeff King @ 2026-06-13 14:00 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Karthik Nayak
In-Reply-To: <aiukqI0Nj_RRn-wZ@pks.im>
On Fri, Jun 12, 2026 at 08:18:16AM +0200, Patrick Steinhardt wrote:
> > If we move to a world of all absolute paths where chdir-notify is not
> > necessary, will we lose that optimization?
>
> Probably. Unfortunately, the commit doesn't have any repeatable
> benchmarks in there, so it's hard to say whether we could still
> reproduce those issues or not.
Here's an easy-ish reproduction specific to the ref code:
rm -rf a/
dir=$(perl -e 'print "a/" x 1024')
mkdir -p $dir &&
cd $dir &&
git init &&
git commit --allow-empty -m foo &&
seq -f 'create refs/heads/foo%05g HEAD' 10000 |
git update-ref --stdin &&
time git show-ref
Before your series, I get timings like this:
real 0m0.078s
user 0m0.020s
sys 0m0.057s
After, I get:
real 0m0.876s
user 0m0.004s
sys 0m0.872s
So it really is measurable (and I did not expect the effect to be nearly
so large). Unsurprisingly the extra CPU goes to system time.
But obviously that case is quite silly. It's an absurdly deep hierarchy,
and 10,000 loose refs is a lot. Just running "git pack-refs --all"
brings the before/after to roughly the same timings (around 40ms --
faster even than the before timing).
So it _can_ matter, but I think ultimately the better direction is
probably "make fewer syscalls". Which we do via packfiles, and via
packed-refs, and eventually via reftables, all of which put more data
into a single file.
I offer the script above more as food for thought, and not necessarily
an argument against your series.
> Ideally, we'd have the best of both worlds: absolute paths everywhere
> without the performance hit. A while back I had a discussion with
> Torvalds on the securiy mailing list around this issue, and ultimately
> the conclusion was that the best way forward would be to use openat(3p).
>
> This wouldn't only allow us to optimize cases like this, but it also has
> the added benefit that we're much less prone to TOCTOU-style issues and
> we might even be able to use flags like O_BENEATH. So it would basically
> be win-win. The only problem is of course that Windows doesn't have
> openat(3p), so we'd have to emulate it, and that's where I always lost
> the desire to do this.
>
> When waking up this morning though I had the thought that we shouldn't
> try to emulate openat(3p) directly, but instead create a higher-level
> interface.
> [...]
Yeah, I think given a decent interface it might not be so bad. It would
mean code thinking about filesystem syscalls in a different way, but if
done subsystem-by-subsystem it might be OK to do incrementally. Much of
the code that would want to switch to this is using repo_git_path() or
similar already (and getting rid of those remaining static-buffer
functions would be a nice bonus).
I do wonder if your series here to move to absolute paths makes the
TOCTOU situation a little worse. With a relative path, once we are
"inside" the repo then we are only susceptible to changes within it.
Whereas with an absolute path, if one of the intermediate paths changes
from under us, there may be confusion.
Without thinking on it too hard, though, I'd guess if any such case is a
security problem, it already was during the "open" part (because it
implies that the attacker controls paths below you in the hierarchy, and
you had to get to your cwd _somehow_, at which point they could have
attacked you then).
-Peff
^ permalink raw reply
* Re: [PATCH 0/2] commit: preserve commit hash on a no-op amend
From: Ben Knoble @ 2026-06-13 14:07 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Harald Nordgren, git, Harald Nordgren via GitGitGadget
In-Reply-To: <355d4f1a-147c-47e3-ab09-06810d7910c0@kdbg.org>
> Le 13 juin 2026 à 05:59, Johannes Sixt <j6t@kdbg.org> a écrit :
>
> Am 13.06.26 um 11:16 schrieb Harald Nordgren via GitGitGadget:
>> git commit --amend --no-edit rewrote the commit and moved the branch tip
>> even when nothing changed, because the committer date was reset to "now".
>> Reuse the existing committer date so a no-op amend keeps the commit hash and
>> leaves the branch untouched.
>
> `git commit --amend --no-edit` is a way to set the committer timestamp
> to the current time without changing other aspects of the commit. This
> takes away this ability, doesn't it?
Indeed. This is a convenient formula to force CI re-runs in certain environments, and so on.
^ permalink raw reply
* [RFC PATCH 0/2] doc: clarify review replies and reroll timing
From: Weijie Yuan @ 2026-06-13 14:08 UTC (permalink / raw)
To: git; +Cc: gitster, ps
Hi,
This small series updates the 2 documentations: MyFirstContribution and
SubmittingPatches.
The first patch clarifies that review feedback should not be answered
only by sending a new version of the patches, which is talked in [1].
Contributors are encouraged to and should discuss their planned response in
the existing review thread, so that the next version does not become the
only place where reviewers can infer the author's reasoning.
The second patch is originally from an email from Patrick [2], which
documents a rough expectation around reroll frequency.
Patrick suggests: There is no hard rule for when to send a new version,
but batching feedback and avoiding multiple rerolls of the same series
in a single day is a useful default. The text also mentions factors that
may affect this, such as the size of the series, the depth of review,
and whether the topic is close to being picked up.
Since I am the newbie here, please tell me how to attribute the credit
to Patrick. Thank you Patrick!
I am sending this as RFC because I think my wording is quite rough, and
there must be a better way to express all of these, including how to
manage the structures of both these two documents. Any comments are
appreciated, thank you!
[1]: <xmqq7bo5nf31.fsf@gitster.g>
[2]: <aietF4BX1Ewt3cpG@pks.im>
Weijie Yuan (2):
doc: encourage review replies before rerolling
doc: advise batching patch rerolls
Documentation/MyFirstContribution.adoc | 27 +++++++++++++++++++++-----
Documentation/SubmittingPatches | 17 +++++++++++++---
2 files changed, 36 insertions(+), 8 deletions(-)
--
2.54.0
^ permalink raw reply
* Re: [RFC] commit-reach: terminate merge-base walk when one paint side is exhausted
From: Kristofer Karlsson @ 2026-06-13 14:08 UTC (permalink / raw)
To: Derrick Stolee; +Cc: git
In-Reply-To: <CAL71e4NRvmDagFAJE-0HYwiLPSfhVVQO2qZe-EJPVXxeC4PWqg@mail.gmail.com>
On Fri, 13 Jun 2026, Kristofer Karlsson wrote:
> In both shapes, C is the only merge-base. But in shape 2, the
> walk through X's ancestry is P1-only: STALE propagates through
> C's ancestors but never reaches D's lineage.
I have to add a self-correction here:
STALE does reach D through the main line. The real
issue is that the bypass branch has a low generation number, so
max_nonstale keeps the loop alive until the stale frontier
drains all the way down.
In our monorepo, the concrete trigger is imported repositories.
An import merge brings in a separate history with its own root
at generation 0. As soon as the walk crosses one such import
above the merge-base, max_nonstale forces it to drain the
entire main graph. Instrumenting paint_down_to_common confirms
this: `merge-base --all HEAD HEAD~1000` takes 2.3M steps, of
which almost all are stale.
Thanks,
Kristofer
^ permalink raw reply
* [RFC PATCH 1/2] doc: encourage review replies before rerolling
From: Weijie Yuan @ 2026-06-13 14:08 UTC (permalink / raw)
To: git; +Cc: gitster, ps
In-Reply-To: <cover.1781358364.git.wy@wyuan.org>
Review feedback should not be answered only by sending a new patch
version. Encourage contributors to discuss their planned response in the
mailing-list thread before rerolling.
This makes the author's reasoning explicit before the next version is
prepared, instead of forcing reviewers to infer it from the rerolled
patches.
Signed-off-by: Weijie Yuan <wy@wyuan.org>
---
Documentation/MyFirstContribution.adoc | 12 +++++++-----
Documentation/SubmittingPatches | 12 +++++++++---
2 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/Documentation/MyFirstContribution.adoc b/Documentation/MyFirstContribution.adoc
index 0e2a9313ce..59891e3c14 100644
--- a/Documentation/MyFirstContribution.adoc
+++ b/Documentation/MyFirstContribution.adoc
@@ -1423,11 +1423,13 @@ fewer mistakes were the only one they would need to review.
After a few days, you will hopefully receive a reply to your patchset with some
comments. Woohoo! Now you can get back to work.
-It's good manners to reply to each comment, notifying the reviewer that you have
-made the change suggested, feel the original is better, or that the comment
-inspired you to do something a new way which is superior to both the original
-and the suggested change. This way reviewers don't need to inspect your v2 to
-figure out whether you implemented their comment or not.
+It's good manners to reply to each comment in the mailing list discussion
+instead of letting the next version of your patch be your only response. Tell
+the reviewer whether you plan to make the suggested change, keep the original,
+or pursue a different approach. This way reviewers can respond to your reasoning
+before you spend time preparing a version they may not agree with, and later do
+not need to inspect your v2 to figure out whether you implemented their comment
+or not.
Reviewers may ask you about what you wrote in the patchset, either in
the proposed commit log message or in the changes themselves. You
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 6b83b6c89e..d8ad7fb73e 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -48,8 +48,12 @@ area.
. You get comments and suggestions for improvements. You may even get
them in an "on top of your change" patch form. You are expected to
- respond to them with "Reply-All" on the mailing list, while taking
- them into account while preparing an updated set of patches.
+ respond to them with "Reply-All" on the mailing list, instead of
+ letting an updated patch series be your only response. Tell
+ reviewers which suggestions you plan to use, which ones you disagree
+ with, and when a comment leads you to consider a different approach.
+ Use these replies and any follow-up discussion as input when
+ preparing an updated set of patches.
+
It is often beneficial to allow some time for reviewers to provide
feedback before sending a new version, rather than sending an updated
@@ -639,7 +643,9 @@ grouped into their own e-mail thread to help readers find all parts of the
series. To that end, send them as replies to either an additional "cover
letter" message (see below), the first patch, or the respective preceding patch.
Here is a link:MyFirstContribution.html#v2-git-send-email[step-by-step guide] on
-how to submit updated versions of a patch series.
+how to submit updated versions of a patch series. Before sending another
+version, make sure you have answered meaningful review comments in the existing
+discussion.
If your log message (including your name on the
`Signed-off-by:` trailer) is not writable in ASCII, make sure that
--
2.54.0
^ permalink raw reply related
* [RFC PATCH 2/2] doc: advise batching patch rerolls
From: Weijie Yuan @ 2026-06-13 14:09 UTC (permalink / raw)
To: git; +Cc: gitster, ps
In-Reply-To: <cover.1781358364.git.wy@wyuan.org>
Contributors often need guidance on how quickly to send later iterations
of a patch series. Add a rough default of no more than one new version
of the same series per day so feedback can be batched and reviewers have
time to comment.
Mention factors that can affect the timing, such as series size, review
depth, substantial rework, and how close the topic is to being accepted.
Signed-off-by: Weijie Yuan <wy@wyuan.org>
---
Documentation/MyFirstContribution.adoc | 15 +++++++++++++++
Documentation/SubmittingPatches | 7 ++++++-
2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/Documentation/MyFirstContribution.adoc b/Documentation/MyFirstContribution.adoc
index 59891e3c14..9d76c72d05 100644
--- a/Documentation/MyFirstContribution.adoc
+++ b/Documentation/MyFirstContribution.adoc
@@ -1416,6 +1416,21 @@ previous one" patches over 2 days), reviewers would strongly prefer if a
single polished version came 2 days later instead, and that version with
fewer mistakes were the only one they would need to review.
+This consideration applies not only when going from the initial patch to v2, but
+also to later iterations of the same series. There is no fixed rule for how long
+to wait before sending a new version. A useful default is to send at most one
+new version of the same patch series per day. This gives multiple reviewers time
+to comment, lets you batch feedback together, and gives you time to think
+through the comments you received.
+
+The right timing depends on the topic and the feedback. Larger series usually
+need more review time. If the only comments so far are minor, such as typo
+fixes, it often makes sense to wait a little longer in case deeper reviews are
+still coming. If the comments require substantial rework, sending a new version
+sooner may save reviewers from spending time on a version you already know will
+change significantly. If the topic is close to being accepted and the remaining
+comments are small, a quicker new version may also be fine.
+
[[reviewing]]
=== Responding to Reviews
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index d8ad7fb73e..1bc2684c54 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -59,6 +59,10 @@ It is often beneficial to allow some time for reviewers to provide
feedback before sending a new version, rather than sending an updated
series immediately after receiving a review. This helps collect broader
input and avoids unnecessary churn from many rapid iterations.
++
+As a rough default, avoid sending more than one new version of the same
+series per day, while considering the size of the series, the depth of
+review, and how close the topic is to being accepted.
. These early update iterations are expected to be full replacements,
not incremental updates on top of what you posted already. If you
@@ -645,7 +649,8 @@ letter" message (see below), the first patch, or the respective preceding patch.
Here is a link:MyFirstContribution.html#v2-git-send-email[step-by-step guide] on
how to submit updated versions of a patch series. Before sending another
version, make sure you have answered meaningful review comments in the existing
-discussion.
+discussion. Also give reviewers enough time to comment before sending another
+version.
If your log message (including your name on the
`Signed-off-by:` trailer) is not writable in ASCII, make sure that
--
2.54.0
^ permalink raw reply related
* Re: [PATCH] clone: accept DEPTH env var as fallback for --depth
From: Junio C Hamano @ 2026-06-13 15:20 UTC (permalink / raw)
To: Matt Hunter; +Cc: h8d13 via GitGitGadget, git, h8d13
In-Reply-To: <DJ7MJMIFZR5N.2SG1RWB46WPQB@lfurio.us>
"Matt Hunter" <m@lfurio.us> writes:
> On Fri Jun 12, 2026 at 9:39 PM EDT, h8d13 via GitGitGadget wrote:
>> @@ -1022,6 +1022,12 @@ int cmd_clone(int argc,
>> usage_msg_opt(_("You must specify a repository to clone."),
>> builtin_clone_usage, builtin_clone_options);
>>
>> + if (!option_depth) {
>> + const char *env_depth = getenv("DEPTH");
>
> Nearly all of the non-standard environment variables used by git start
> with "GIT_". "GIT_CLONE_DEPTH" may be a better choice.
Isn't it sufficient to add a new configuration variable in the
clone.* namespace? Unless there is a reason why it does not work, I
won't accept a patch that adds a random environment support like
this. We do not want to end up having to add other random
environment variables like GIT_CLONE_DEFAULTREMOTENAME,
CLONE_REJECTSHALLOW, CLONE_FILTERSUBMODULES for consistency.
^ permalink raw reply
* [PATCH v3 0/1] environment: move protect_hfs and protect_ntfs into repo_config_values
From: Tian Yuchen @ 2026-06-13 15:33 UTC (permalink / raw)
To: git
Cc: phillip.wood123, Tian Yuchen, Christian Couder, Ayush Chandekar,
Olamide Caleb Bello
In-Reply-To: <20260610124353.149874-1-cat@malon.dev>
Hi everyone,
This series continues the ongoing libification effort by moving the
global filesystem variables, 'protect_hfs' and 'protect_ntfs', into
'struct repo_config_values'.
Place them within the per-repository configuration structure
aligns with our goal of removing global states.
For reviewers familiar with previous libification efforts, Derrick Stolee
attempted to wrap this kind of filesystem-level variable using a
lazy-loaded global accessor get_int_config_global() [1].
However, as Glen Choo pointed out in his review of that series [2],
it is strongly preferred to use plain fields in a repository-scoped
struct over global lazy-loaders, provided those fields are properly
initialized during the setup process.
By moving these variables into repo_config_values and parsing
them eagerly, we successfully tie the filesystem security flags
to the specific repository instance without altering the timing
of configuration warnings or introducing new global states.
Thanks!
Recent related patch (environment.c: migrate 'trust_executable_bit' into 'repo_config_values'): [3]
Changes since V2:
1. s/environment.c/environment
2. Updated the link for "Recent related patch"
[1] https://lore.kernel.org/git/a42dd9397d07b2dc4a0d7e75bfe1af2e46cad262.1685716420.git.gitgitgadget@gmail.com/
[2] https://lore.kernel.org/git/kl6lbkhpzujf.fsf@chooglen-macbookpro.roam.corp.google.com/
[3] https://lore.kernel.org/git/20260612160527.167203-1-cat@malon.dev/
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>
Tian Yuchen (1):
environment: move 'protect_hfs' and 'protect_ntfs' into
'repo_config_values'
compat/mingw.c | 2 +-
environment.c | 22 ++++++++++++++++++----
environment.h | 12 ++++++++++--
read-cache.c | 7 ++++---
t/helper/test-path-utils.c | 24 +++++++++++++++---------
5 files changed, 48 insertions(+), 19 deletions(-)
--
2.43.0
^ permalink raw reply
* [PATCH v3 1/1] environment: move 'protect_hfs' and 'protect_ntfs' into 'repo_config_values'
From: Tian Yuchen @ 2026-06-13 15:33 UTC (permalink / raw)
To: git
Cc: phillip.wood123, Tian Yuchen, Christian Couder, Ayush Chandekar,
Olamide Caleb Bello
In-Reply-To: <20260613153302.168801-1-cat@malon.dev>
Move the global 'protect_hfs' and 'protect_ntfs' configurations
into the repository-specific 'repo_config_values' struct.
This will help with the elimination of 'the_repository'
To ensure code readability, the getter functions
'repo_protect_hfs()' and 'repo_protect_ntfs()'
have been introduced.
For now, associated functions access this configuration by
explicitly falling back to 'the_repository', which needs to
be addressed in the future.
Note: In 't/helper/test-path-utils.c', there is a function
'protect_ntfs_hfs_benchmark()' where these two global
variables are used as loop iterators. New local variables
have been created to replace them.
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>
---
compat/mingw.c | 2 +-
environment.c | 22 ++++++++++++++++++----
environment.h | 12 ++++++++++--
read-cache.c | 7 ++++---
t/helper/test-path-utils.c | 24 +++++++++++++++---------
5 files changed, 48 insertions(+), 19 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index aa7525f419..af87df77fd 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -3392,7 +3392,7 @@ int is_valid_win32_path(const char *path, int allow_literal_nul)
const char *p = path;
int preceding_space_or_period = 0, i = 0, periods = 0;
- if (!protect_ntfs)
+ if (!repo_protect_ntfs(the_repository))
return 1;
skip_dos_drive_prefix((char **)&path);
diff --git a/environment.c b/environment.c
index fc3ed8bb1c..683fe1b4d3 100644
--- a/environment.c
+++ b/environment.c
@@ -82,12 +82,10 @@ unsigned long pack_size_limit_cfg;
#ifndef PROTECT_HFS_DEFAULT
#define PROTECT_HFS_DEFAULT 0
#endif
-int protect_hfs = PROTECT_HFS_DEFAULT;
#ifndef PROTECT_NTFS_DEFAULT
#define PROTECT_NTFS_DEFAULT 1
#endif
-int protect_ntfs = PROTECT_NTFS_DEFAULT;
/*
* The character that begins a commented line in user-editable file
@@ -142,6 +140,20 @@ int is_bare_repository(void)
return is_bare_repository_cfg && !repo_get_work_tree(the_repository);
}
+int repo_protect_ntfs(struct repository *repo)
+{
+ return repo->gitdir ?
+ repo_config_values(repo)->protect_ntfs :
+ PROTECT_NTFS_DEFAULT;
+}
+
+int repo_protect_hfs(struct repository *repo)
+{
+ return repo->gitdir ?
+ repo_config_values(repo)->protect_hfs :
+ PROTECT_HFS_DEFAULT;
+}
+
int have_git_dir(void)
{
return startup_info->have_repository
@@ -541,12 +553,12 @@ int git_default_core_config(const char *var, const char *value,
}
if (!strcmp(var, "core.protecthfs")) {
- protect_hfs = git_config_bool(var, value);
+ cfg->protect_hfs = git_config_bool(var, value);
return 0;
}
if (!strcmp(var, "core.protectntfs")) {
- protect_ntfs = git_config_bool(var, value);
+ cfg->protect_ntfs = git_config_bool(var, value);
return 0;
}
@@ -720,5 +732,7 @@ void repo_config_values_init(struct repo_config_values *cfg)
{
cfg->attributes_file = NULL;
cfg->apply_sparse_checkout = 0;
+ cfg->protect_hfs = PROTECT_HFS_DEFAULT;
+ cfg->protect_ntfs = PROTECT_NTFS_DEFAULT;
cfg->branch_track = BRANCH_TRACK_REMOTE;
}
diff --git a/environment.h b/environment.h
index 9eb97b3869..fdd9775900 100644
--- a/environment.h
+++ b/environment.h
@@ -91,6 +91,8 @@ struct repo_config_values {
/* section "core" config values */
char *attributes_file;
int apply_sparse_checkout;
+ int protect_hfs;
+ int protect_ntfs;
/* section "branch" config values */
enum branch_track branch_track;
@@ -123,6 +125,14 @@ 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);
+/*
+ * Getters for the `protect_hfs` and `protect_ntfs` fields of `struct repo_config_values`.
+ * They check `repo->gitdir` to prevent calling repo_config_values()
+ * before the configuration is loaded or in bare environments.
+ */
+int repo_protect_hfs(struct repository *repo);
+int repo_protect_ntfs(struct repository *repo);
+
void repo_config_values_init(struct repo_config_values *cfg);
/*
@@ -173,8 +183,6 @@ extern int pack_compression_level;
extern unsigned long pack_size_limit_cfg;
extern int precomposed_unicode;
-extern int protect_hfs;
-extern int protect_ntfs;
extern int core_sparse_checkout_cone;
extern int sparse_expect_files_outside_of_patterns;
diff --git a/read-cache.c b/read-cache.c
index 21829102ae..2c6a60c756 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1002,7 +1002,7 @@ static enum verify_path_result verify_path_internal(const char *path,
return PATH_OK;
if (is_dir_sep(c)) {
inside:
- if (protect_hfs) {
+ if (repo_protect_hfs(the_repository)) {
if (is_hfs_dotgit(path))
return PATH_INVALID;
@@ -1011,7 +1011,7 @@ static enum verify_path_result verify_path_internal(const char *path,
return PATH_INVALID;
}
}
- if (protect_ntfs) {
+ if (repo_protect_ntfs(the_repository)) {
#if defined GIT_WINDOWS_NATIVE || defined __CYGWIN__
if (c == '\\')
return PATH_INVALID;
@@ -1035,7 +1035,8 @@ static enum verify_path_result verify_path_internal(const char *path,
if (c == '\0')
return S_ISDIR(mode) ? PATH_DIR_WITH_SEP :
PATH_INVALID;
- } else if (c == '\\' && protect_ntfs) {
+ } else if (c == '\\' &&
+ repo_protect_ntfs(the_repository)) {
if (is_ntfs_dotgit(path))
return PATH_INVALID;
if (S_ISLNK(mode)) {
diff --git a/t/helper/test-path-utils.c b/t/helper/test-path-utils.c
index 15eb44485c..f77b3f9d70 100644
--- a/t/helper/test-path-utils.c
+++ b/t/helper/test-path-utils.c
@@ -250,6 +250,7 @@ static int protect_ntfs_hfs_benchmark(int argc, const char **argv)
double m[3][2], v[3][2];
uint64_t cumul;
double cumul2;
+ int ntfs, hfs;
if (argc > 1 && !strcmp(argv[1], "--with-symlink-mode")) {
file_mode = 0120000;
@@ -276,8 +277,13 @@ static int protect_ntfs_hfs_benchmark(int argc, const char **argv)
names[i][--len] = (char)(' ' + (my_random() % ('\x7f' - ' ')));
}
- for (protect_ntfs = 0; protect_ntfs < 2; protect_ntfs++)
- for (protect_hfs = 0; protect_hfs < 2; protect_hfs++) {
+ if (!the_repository->gitdir)
+ the_repository->gitdir = xstrdup(".git");
+
+ for (ntfs = 0; ntfs < 2; ntfs++)
+ for (hfs = 0; hfs < 2; hfs++) {
+ repo_config_values(the_repository)->protect_ntfs = ntfs;
+ repo_config_values(the_repository)->protect_hfs = hfs;
cumul = 0;
cumul2 = 0;
for (i = 0; i < repetitions; i++) {
@@ -285,18 +291,18 @@ static int protect_ntfs_hfs_benchmark(int argc, const char **argv)
for (j = 0; j < nr; j++)
verify_path(names[j], file_mode);
end = getnanotime();
- printf("protect_ntfs = %d, protect_hfs = %d: %lfms\n", protect_ntfs, protect_hfs, (end-begin) / (double)1e6);
+ printf("protect_ntfs = %d, protect_hfs = %d: %lfms\n", ntfs, hfs, (end-begin) / (double)1e6);
cumul += end - begin;
cumul2 += (end - begin) * (end - begin);
}
- m[protect_ntfs][protect_hfs] = cumul / (double)repetitions;
- v[protect_ntfs][protect_hfs] = my_sqrt(cumul2 / (double)repetitions - m[protect_ntfs][protect_hfs] * m[protect_ntfs][protect_hfs]);
- printf("mean: %lfms, stddev: %lfms\n", m[protect_ntfs][protect_hfs] / (double)1e6, v[protect_ntfs][protect_hfs] / (double)1e6);
+ m[ntfs][hfs] = cumul / (double)repetitions;
+ v[ntfs][hfs] = my_sqrt(cumul2 / (double)repetitions - m[ntfs][hfs] * m[ntfs][hfs]);
+ printf("mean: %lfms, stddev: %lfms\n", m[ntfs][hfs] / (double)1e6, v[ntfs][hfs] / (double)1e6);
}
- for (protect_ntfs = 0; protect_ntfs < 2; protect_ntfs++)
- for (protect_hfs = 0; protect_hfs < 2; protect_hfs++)
- printf("ntfs=%d/hfs=%d: %lf%% slower\n", protect_ntfs, protect_hfs, (m[protect_ntfs][protect_hfs] - m[0][0]) * 100 / m[0][0]);
+ for (ntfs = 0; ntfs < 2; ntfs++)
+ for (hfs = 0; hfs < 2; hfs++)
+ printf("ntfs=%d/hfs=%d: %lf%% slower\n", ntfs, hfs, (m[ntfs][hfs] - m[0][0]) * 100 / m[0][0]);
return 0;
}
--
2.43.0
^ permalink raw reply related
* Re: [PATCH 0/2] commit: preserve commit hash on a no-op amend
From: Junio C Hamano @ 2026-06-13 15:44 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget; +Cc: git, Harald Nordgren
In-Reply-To: <pull.2334.git.git.1781342189.gitgitgadget@gmail.com>
"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:
> git commit --amend --no-edit rewrote the commit and moved the branch tip
> even when nothing changed, because the committer date was reset to "now".
> Reuse the existing committer date so a no-op amend keeps the commit hash and
> leaves the branch untouched.
>
> A real change (tree, message, author, committer, or signing) still rewrites
> as before.
I think this change brings nothing but regression.
Isn't it obvious that "commit --amend --no-edit" without updating
any tree contents would record exactly the same contents as before,
without a "real change" (as you said above), to any and all users,
expert and casual alike?
The end-user who runs such a command must have a reason to do so.
The *ONLY* valid reason anybody might want to such an amend is to
make sure the result is a new object, even if it records otherwise
the same content.
Why would they want to do so? Perhaps it is so that future merges
of the topic branch that contains the commit will work more smoothly
into an integration branch that had earlier merged the topic branch,
and then that earlier merge was reverted. This change will rob an
effective way to ensure a successful final merge in a workflow to
(1) merge a topic, (2) revert the topic, (3) update near the tip of
the topic while keeping earlier topic intact, and then (4) merge the
result again.
So, no. I do not think this is a good change.
Let's digress and imagine an alternate universe where rebase/commit
--amend/history were "smart" from day one. These command in such a
hypothetical world may not be capable of refreshing an existing
commit without making any "real change".
Making a change to these commands to _optionally_ allow them to
recreate an otherwise unchanged commit, so that it will get a new
object name, would be a welcome change that would allow users who
would use "commit --amend --no-edit" with today's system for such a
use case.
And that would have been a logical evolution of the system in such a
hypothetical world.
But the thing is, we do not live in such a world.
If we still think that alternate hypothetical world is a better
place, we'd need to actively move things around, carefully designing
the transition to avoid harming existing users along the way, to get
there. Changing the behaviour all of a sudden and breaking existing
workflows is not something we do around here.
One way to get to such a world might be:
* Introduce an "committer timestamp is a trashable information"
option, and teach commands like "commit --amend", "rebase", and
"history" to cheat and yield the existing commit without
refreshing when they are asked to recreate an existing commit
while the option is in effect. Give people the opposite
"committer timestamp is not trashable information" option, so an
earlier "is trashable" option on the command line can be
countermanded by giving it later on the command line.
* Have users discuss if "is trashable" is a better default, and
gain consensus to make it the default in a future version of Git.
Advertise the fact that we achieved consensus LOUDLY, while
telling dissidents that "is not trashable" option will forever be
available for them.
* At a big version boundary, switch the default.
And I do not think I in principle would object to the first step of
such a three step process.
Thanks.
^ permalink raw reply
* Re: [RFC PATCH 2/2] doc: advise batching patch rerolls
From: Junio C Hamano @ 2026-06-13 16:02 UTC (permalink / raw)
To: Weijie Yuan; +Cc: git, ps
In-Reply-To: <8166623d1599fca2cd4614889e4a69b2006c12c1.1781358364.git.wy@wyuan.org>
Weijie Yuan <wy@wyuan.org> writes:
> Contributors often need guidance on how quickly to send later iterations
> of a patch series. Add a rough default of no more than one new version
> of the same series per day so feedback can be batched and reviewers have
> time to comment.
>
> Mention factors that can affect the timing, such as series size, review
> depth, substantial rework, and how close the topic is to being accepted.
Another good thing to discourage yourself from rerolling too quickly
is that such a practice forces you to think twice and be very
careful before sending patches out. As you have only one chance to
get it right before, say, 24 hours, you'd want to make sure that you
would not distract your reviewers with stupid typoes, off-by-one
errors, and such, and concentrate their reviews more on what matters
more, i.e., the higher level design, choice of algorithms, etc.
> +This consideration applies not only when going from the initial patch to v2, but
> +also to later iterations of the same series. There is no fixed rule for how long
> +to wait before sending a new version. A useful default is to send at most one
> +new version of the same patch series per day. This gives multiple reviewers time
> +to comment, lets you batch feedback together, and gives you time to think
> +through the comments you received.
And the 24-hour gives equal chance to comment on your patches to
anybody no matter where they live ;-)
I see you CC'ed Patrick, and I am sure he'll give us more useful
suggestions than I do here ;-)
Thanks.
^ permalink raw reply
* Re: [PATCH 0/2] commit: preserve commit hash on a no-op amend
From: Harald Nordgren @ 2026-06-13 16:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Harald Nordgren via GitGitGadget, git
In-Reply-To: <xmqq33yqfnsa.fsf@gitster.g>
Interesting discussions! This sounds like showstopper, seems more
reasonable to leave this topic for now.
I just want to share that I've been running this for years to
re-trigger CI (because up until a few days ago I didn't realize that
the hash did indeed change even when nothing had changed), I had the
wrong mental model for commit hashes:
git commit --amend --no-edit --date="now"
Harald
^ permalink raw reply
* Re: [PATCH 0/6] t: add lint-style.pl and convert grep to test_grep
From: Michael Montalbo @ 2026-06-13 16:28 UTC (permalink / raw)
To: Junio C Hamano
Cc: Michael Montalbo via GitGitGadget, git, D. Ben Knoble,
Eric Sunshine
In-Reply-To: <xmqqldcovhnf.fsf@gitster.g>
On Mon, Jun 8, 2026 at 2:36 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> I do not think we want an automated tool that rewrites the source
> files. I was hoping that we would get a patch or two that _adds_ to
> existing test-lint framework (i.e., 'test-grep' that 'test-lint'
> target depends on in t/Makefile) that gives diagnosis in a similar
> fashion as test-lint-shell-syntax and test-chainlint do.
>
> Also some existing uses of "grep" are not end-user facing and should
> not be rewritten to "test_grep".
Apologies for not responding explicitly before sending a re-roll, I
will do that in
the future. v2 attempts to address these points as noted in the "changes"
section.
^ permalink raw reply
* Re: [PATCH v4 0/3] compat/posix.h: enable UNUSED warning messages for Clang
From: Junio C Hamano @ 2026-06-13 16:39 UTC (permalink / raw)
To: Dominik Loidolt; +Cc: git, ps, asedeno, asedeno, avarab
In-Reply-To: <20260613122711.38662-1-dominik.loidolt@univie.ac.at>
Dominik Loidolt <dominik.loidolt@univie.ac.at> writes:
> This series enables the intended UNUSED warning message with Clang by
> adding a dedicated Clang version check. It also cleans up the nearby
> GIT_GNUC_PREREQ() and UNUSED macros.
>
> Changes since v3:
> - split style-only cleanups into their own patch
> - fix the UNUSED preprocessor indentation style
> - simplify the GIT_GNUC_PREREQ() comparison commit message
> - keep the Clang-specific note in the patch that adds GIT_CLANG_PREREQ()
>
> Thanks,
> Dominik
>
> Dominik Loidolt (3):
> compat/posix.h: enable UNUSED warning messages for Clang
> compat/posix.h: clean up GIT_GNUC_PREREQ() and UNUSED
> compat/posix.h: simplify GIT_GNUC_PREREQ() comparison
Looking good and all the points Patrick raised during the review of
the previous round seem to have been addressed nicely.
Will replace. Shall we mark it for 'next' now?
Thanks.
^ permalink raw reply
* Re: [RFC PATCH 2/2] doc: advise batching patch rerolls
From: Weijie Yuan @ 2026-06-13 17:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, ps
In-Reply-To: <xmqqwlw2e8dc.fsf@gitster.g>
On Sat, Jun 13, 2026 at 09:02:39AM -0700, Junio C Hamano wrote:
> Weijie Yuan <wy@wyuan.org> writes:
>
> > Contributors often need guidance on how quickly to send later iterations
> > of a patch series. Add a rough default of no more than one new version
> > of the same series per day so feedback can be batched and reviewers have
> > time to comment.
> >
> > Mention factors that can affect the timing, such as series size, review
> > depth, substantial rework, and how close the topic is to being accepted.
>
> Another good thing to discourage yourself from rerolling too quickly
> is that such a practice forces you to think twice and be very
> careful before sending patches out. As you have only one chance to
> get it right before, say, 24 hours, you'd want to make sure that you
> would not distract your reviewers with stupid typoes, off-by-one
> errors, and such, and concentrate their reviews more on what matters
> more, i.e., the higher level design, choice of algorithms, etc.
>
> > +This consideration applies not only when going from the initial patch to v2, but
> > +also to later iterations of the same series. There is no fixed rule for how long
> > +to wait before sending a new version. A useful default is to send at most one
> > +new version of the same patch series per day. This gives multiple reviewers time
> > +to comment, lets you batch feedback together, and gives you time to think
> > +through the comments you received.
>
> And the 24-hour gives equal chance to comment on your patches to
> anybody no matter where they live ;-)
Thanks for your comments above! Let me think about how to integrate
these contents with the patch.
> I see you CC'ed Patrick, and I am sure he'll give us more useful
> suggestions than I do here ;-)
This is his practical advice, and I just stole Patrick´s wording, to be
fair ;-) so of course I should CC him and let him know I am a wording
thief :-P, hope it wouldn't disturb him ;-)
Thank you very much.
^ permalink raw reply
* Re: [PATCH v11] checkout: extend --track with a "fetch" mode to refresh start-point
From: Harald Nordgren @ 2026-06-13 17:38 UTC (permalink / raw)
To: phillip.wood
Cc: Junio C Hamano, Harald Nordgren via GitGitGadget, git,
Ramsay Jones, D. Ben Knoble, Kristoffer Haugsbakk, Marc Branchaud
In-Reply-To: <01526f43-86aa-466f-a1e8-054284e1a2e1@gmail.com>
Hi Phillip and JCH!
Would it be possible to get another look here to know if it's worth
continuing with this topic. I think it's a useful feature, but the
feedback from this list has been a bit lukewarm.
Harald
On Thu, May 21, 2026 at 4:06 PM Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> On 21/05/2026 13:58, Junio C Hamano wrote:
> > Phillip Wood <phillip.wood123@gmail.com> writes:
> >
> >>> One. Have you considered the case where the remote-tracking refs
> >>> are overlapping, e.g., where "origin" and "upstream" point at
> >>> different URLs but they both store in "refs/remotes/upstream/*"?
> >>> Perhaps their URLs may textually be different but are pointing
> >>> logically at the same place (e.g., one ssh:// the other https:// for
> >>> example).
> >>>
> >>> What should happen? What does happen after you apply this patch?
> >>
> >> It would be worth looking at what "git checkout --track" does in that
> >> case and seeing if we can share the code.
> >
> > It always is a good idea to think how we can share code for
> > different purposes to solve a new problem, but in this particular
> > one, I am not sure if "git checkout -t -b topic upstream/main"
> > codepath has much to offer to solve what the new "before the
> > checkout, update from the remote" feature wants to do. To the
> > former, it does not matter how refs/remotes/upstream/* are updated
> > and by fetching which remote at all.
>
> Don't we want to avoid creating a branch with an ambiguous upstream so
> that a subsequent "git pull" works though? Looking at
> branch.c:setup_tracking() it seems to reject upstream branches that
> match more than one remote.
>
> Thanks
>
> Phillip
>
> > The only thing it cares about
> > is to leave the record that this new "topic" branch works with
> > refs/remotes/upstrea/main. But the latter needs to be able to
> > compute which remote it should fetch from. It is a problem that
> > existing code had no need to solve.
> >
>
^ permalink raw reply
* Re: [PATCH] clone: accept DEPTH env var as fallback for --depth
From: Hadrien Loge @ 2026-06-13 17:43 UTC (permalink / raw)
To: gitster; +Cc: git, gitgitgadget, hadean-eon-dev, m
Well mainly I'm asking this for packaging (Arch/Alpine/Etc)
These all follow similar conventions (PKGBUILD/APKBUILD).
But in nested flows the ENV var seems like the proper solution.
Mainly I gave this example on github:
git clone --depth 1 url dest
cd dest
bash run.sh
here run.sh has its own clone deps (perhaps even multiple)
--depth 1 is now lost
And only ENV vars that I can think of properly propagate for CI
flows/clean chroot envirs.
Thank you for considering the solution. It would be very useful
for speeding up packaging.
Even on 5k commits history it's 900kb vs 17mb.
I have also reworked the commit to include tests/docs.
and rename to GIT_CLONE_DEPTH
Kind regards,
Hade
^ permalink raw reply
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