* [GSoC Patch v7 2/3] repo: add path.commondir with absolute and relative suffix formatting
From: K Jayatheerth @ 2026-06-21 5:55 UTC (permalink / raw)
To: jayatheerthkulkarni2005
Cc: a3205153416, git, gitster, jltobler, kumarayushjha123,
lucasseikioshiro, phillip.wood, sandals
In-Reply-To: <20260621055534.46798-1-jayatheerthkulkarni2005@gmail.com>
Scripts working with worktree setups need a reliable way to discover
the common directory, which diverges from the git directory when
multiple worktrees are in use. There is no way to retrieve this path
from git repo info today.
Introduce path.commondir.absolute and path.commondir.relative keys.
Exposing explicit format variants rather than a single key with a
default avoids ambiguity for scripts that require predictable output.
Mentored-by: Justin Tobler <jltobler@gmail.com>
Mentored-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
---
Documentation/git-repo.adoc | 9 +++++++
builtin/repo.c | 26 +++++++++++++++++++
t/t1900-repo-info.sh | 52 +++++++++++++++++++++++++++++++++++++
3 files changed, 87 insertions(+)
diff --git a/Documentation/git-repo.adoc b/Documentation/git-repo.adoc
index 42262c1983..890c34051d 100644
--- a/Documentation/git-repo.adoc
+++ b/Documentation/git-repo.adoc
@@ -104,6 +104,15 @@ values that they return:
`object.format`::
The object format (hash algorithm) used in the repository.
+`path.commondir.absolute`::
+ The canonical absolute path to the Git repository's common
+ directory (the shared `.git` directory containing objects,
+ refs, and global configuration).
+
+`path.commondir.relative`::
+ The path to the Git repository's common directory relative to
+ the current working directory.
+
`references.format`::
The reference storage format. The valid values are:
+
diff --git a/builtin/repo.c b/builtin/repo.c
index 71a5c1c29c..c4cc3bf3fc 100644
--- a/builtin/repo.c
+++ b/builtin/repo.c
@@ -7,12 +7,14 @@
#include "hex.h"
#include "odb.h"
#include "parse-options.h"
+#include "path.h"
#include "path-walk.h"
#include "progress.h"
#include "quote.h"
#include "ref-filter.h"
#include "refs.h"
#include "revision.h"
+#include "setup.h"
#include "strbuf.h"
#include "string-list.h"
#include "shallow.h"
@@ -75,6 +77,28 @@ static int get_object_format(struct repository *repo, struct strbuf *buf)
return 0;
}
+static int get_path_commondir_absolute(struct repository *repo, struct strbuf *buf)
+{
+ const char *common_dir = repo_get_common_dir(repo);
+
+ if (!common_dir)
+ return error(_("unable to get common directory"));
+
+ append_formatted_path(buf, common_dir, startup_info->prefix, PATH_FORMAT_CANONICAL);
+ return 0;
+}
+
+static int get_path_commondir_relative(struct repository *repo, struct strbuf *buf)
+{
+ const char *common_dir = repo_get_common_dir(repo);
+
+ if (!common_dir)
+ return error(_("unable to get common directory"));
+
+ append_formatted_path(buf, common_dir, startup_info->prefix, PATH_FORMAT_RELATIVE);
+ return 0;
+}
+
static int get_references_format(struct repository *repo, struct strbuf *buf)
{
strbuf_addstr(buf,
@@ -87,6 +111,8 @@ static const struct repo_info_field repo_info_field[] = {
{ "layout.bare", get_layout_bare },
{ "layout.shallow", get_layout_shallow },
{ "object.format", get_object_format },
+ { "path.commondir.absolute", get_path_commondir_absolute },
+ { "path.commondir.relative", get_path_commondir_relative },
{ "references.format", get_references_format },
};
diff --git a/t/t1900-repo-info.sh b/t/t1900-repo-info.sh
index 39bb77dda0..09158d29f9 100755
--- a/t/t1900-repo-info.sh
+++ b/t/t1900-repo-info.sh
@@ -155,4 +155,56 @@ test_expect_success 'git repo info -h shows only repo info usage' '
test_grep ! "git repo structure" actual
'
+# Helper function to test path keys in both absolute and relative formats.
+# $1: label for the test
+# $2: field_name (e.g., commondir)
+# $3: expected_dir (the directory name, e.g., .git or custom-common)
+# $4: init_command (extra setup like exporting env vars)
+test_repo_info_path () {
+ label=$1
+ field_name=$2
+ expected_dir=$3
+ init_command=$4
+
+ test_expect_success "absolute: $label" '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ mkdir -p repo/sub &&
+ cd repo/sub &&
+ ROOT="$(test-tool path-utils real_path ..)" && export ROOT &&
+ eval "$init_command" &&
+ echo "path.$field_name.absolute=$ROOT/$expected_dir" >expect &&
+ git repo info "path.$field_name.absolute" >actual &&
+ test_cmp expect actual
+ )
+ '
+
+ test_expect_success "relative: $label" '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ mkdir -p repo/sub &&
+ cd repo/sub &&
+ ROOT="$(test-tool path-utils real_path ..)" && export ROOT &&
+ eval "$init_command" &&
+ echo "path.$field_name.relative=../$expected_dir" >expect &&
+ git repo info "path.$field_name.relative" >actual &&
+ test_cmp expect actual
+ )
+ '
+}
+
+test_repo_info_path 'commondir standard' 'commondir' '.git'
+
+test_repo_info_path 'commondir with GIT_COMMON_DIR and GIT_DIR' 'commondir' \
+ 'custom-common' \
+ 'GIT_COMMON_DIR="$ROOT/custom-common" && export GIT_COMMON_DIR &&
+ GIT_DIR="../.git" && export GIT_DIR &&
+ git init --bare "$ROOT/custom-common"'
+
+test_repo_info_path 'commondir with only GIT_DIR' 'commondir' \
+ '.git' \
+ 'GIT_DIR="../.git" && export GIT_DIR'
+
test_done
--
2.55.0-rc1
^ permalink raw reply related
* [GSoC Patch v7 3/3] repo: add path.gitdir with absolute and relative suffix formatting
From: K Jayatheerth @ 2026-06-21 5:55 UTC (permalink / raw)
To: jayatheerthkulkarni2005
Cc: a3205153416, git, gitster, jltobler, kumarayushjha123,
lucasseikioshiro, phillip.wood, sandals
In-Reply-To: <20260621055534.46798-1-jayatheerthkulkarni2005@gmail.com>
Scripts need a stable way to locate the git directory without
parsing rev-parse output or relying on its flag-driven path format
selection. There is no way to retrieve this path from git repo info
today.
Introduce path.gitdir.absolute and path.gitdir.relative keys,
consistent with the path.commondir keys added in the previous patch.
Reuse the test_repo_info_path helper introduced there to validate
both variants.
Mentored-by: Justin Tobler <jltobler@gmail.com>
Mentored-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
---
Documentation/git-repo.adoc | 6 ++++++
builtin/repo.c | 24 ++++++++++++++++++++++++
t/t1900-repo-info.sh | 6 ++++++
3 files changed, 36 insertions(+)
diff --git a/Documentation/git-repo.adoc b/Documentation/git-repo.adoc
index 890c34051d..ed7d80c690 100644
--- a/Documentation/git-repo.adoc
+++ b/Documentation/git-repo.adoc
@@ -113,6 +113,12 @@ values that they return:
The path to the Git repository's common directory relative to
the current working directory.
+`path.gitdir.absolute`::
+ The canonical absolute path to the Git repository directory (the `.git` directory).
+
+`path.gitdir.relative`::
+ The path to the Git repository directory relative to the current working directory.
+
`references.format`::
The reference storage format. The valid values are:
+
diff --git a/builtin/repo.c b/builtin/repo.c
index c4cc3bf3fc..9a312d127a 100644
--- a/builtin/repo.c
+++ b/builtin/repo.c
@@ -99,6 +99,28 @@ static int get_path_commondir_relative(struct repository *repo, struct strbuf *b
return 0;
}
+static int get_path_gitdir_absolute(struct repository *repo, struct strbuf *buf)
+{
+ const char *git_dir = repo_get_git_dir(repo);
+
+ if (!git_dir)
+ return error(_("unable to get git directory"));
+
+ append_formatted_path(buf, git_dir, startup_info->prefix, PATH_FORMAT_CANONICAL);
+ return 0;
+}
+
+static int get_path_gitdir_relative(struct repository *repo, struct strbuf *buf)
+{
+ const char *git_dir = repo_get_git_dir(repo);
+
+ if (!git_dir)
+ return error(_("unable to get git directory"));
+
+ append_formatted_path(buf, git_dir, startup_info->prefix, PATH_FORMAT_RELATIVE);
+ return 0;
+}
+
static int get_references_format(struct repository *repo, struct strbuf *buf)
{
strbuf_addstr(buf,
@@ -113,6 +135,8 @@ static const struct repo_info_field repo_info_field[] = {
{ "object.format", get_object_format },
{ "path.commondir.absolute", get_path_commondir_absolute },
{ "path.commondir.relative", get_path_commondir_relative },
+ { "path.gitdir.absolute", get_path_gitdir_absolute },
+ { "path.gitdir.relative", get_path_gitdir_relative },
{ "references.format", get_references_format },
};
diff --git a/t/t1900-repo-info.sh b/t/t1900-repo-info.sh
index 09158d29f9..ae8c22c817 100755
--- a/t/t1900-repo-info.sh
+++ b/t/t1900-repo-info.sh
@@ -207,4 +207,10 @@ test_repo_info_path 'commondir with only GIT_DIR' 'commondir' \
'.git' \
'GIT_DIR="../.git" && export GIT_DIR'
+test_repo_info_path 'gitdir standard' 'gitdir' '.git'
+
+test_repo_info_path 'gitdir with explicit GIT_DIR' 'gitdir' \
+ '.git' \
+ 'GIT_DIR="../.git" && export GIT_DIR'
+
test_done
--
2.55.0-rc1
^ permalink raw reply related
* Re: [PATCH GSoC RFC v13 10/12] cat-file: add remote-object-info to batch-command
From: Chandra Pratap @ 2026-06-21 6:01 UTC (permalink / raw)
To: Pablo Sabater
Cc: gitster, peff, eric.peijian, chriscool, git, jltobler,
karthik.188, toon, Jonathan Tan, Calvin Wan
In-Reply-To: <20260619-ps-eric-work-rebase-v13-10-3d4c7315d2f8@gmail.com>
[snip]
> +static void parse_cmd_remote_object_info(struct batch_options *opt,
> + const char *line, struct strbuf *output,
> + struct expand_data *data)
> +{
> + int count;
> + const char **argv;
> + char *line_to_split;
> + static struct object_info *remote_object_info;
> + static struct oid_array object_info_oids = OID_ARRAY_INIT;
I don't get the point of remote_object_info and object_info_oids
being static here? These variables are allocated, utilized, and
completely freed/disconnected within a single command cycle.
Making them static gives me the false impression that state
needs to persist between calls.
> + if (strlen(line) >= MAX_REMOTE_OBJ_INFO_LINE)
> + die(_("remote-object-info command too long"));
> +
> + line_to_split = xstrdup(line);
> + count = split_cmdline(line_to_split, &argv);
> + if (count < 0)
> + die(_("split remote-object-info command"));
> + if (count - 1 > MAX_ALLOWED_OBJ_LIMIT)
> + die(_("remote-object-info supports at most %d objects"),
> + MAX_ALLOWED_OBJ_LIMIT);
> +
> + if (get_remote_info(opt, count, argv, &remote_object_info,
> + &object_info_oids))
> + goto cleanup;
> +
> + data->skip_object_info = 1;
> + for (size_t i = 0; i < object_info_oids.nr; i++) {
> + data->oid = object_info_oids.oid[i];
> + if (remote_object_info[i].sizep) {
> + /*
> + * When reaching here, it means remote-object-info can retrieve
> + * information from server without downloading them.
> + */
> + data->size = *remote_object_info[i].sizep;
> + opt->batch_mode = BATCH_MODE_INFO;
> + batch_object_write(argv[i + 1], output, opt, data, NULL, 0);
> + } else {
> + report_object_status(opt, oid_to_hex(&data->oid), &data->oid, "missing");
> + }
> + }
> + data->skip_object_info = 0;
> +
> +cleanup:
> + for (size_t i = 0; i < object_info_oids.nr; i++)
> + free_object_info_contents(&remote_object_info[i]);
> + free(line_to_split);
> + free(argv);
> + free(remote_object_info);
> +}
> +
[snip]
^ permalink raw reply
* Re: [PATCH GSoC RFC v13 12/12] cat-file: make remote-object-info allow-list dynamic
From: Chandra Pratap @ 2026-06-21 6:19 UTC (permalink / raw)
To: Pablo Sabater
Cc: gitster, peff, eric.peijian, chriscool, git, jltobler,
karthik.188, toon
In-Reply-To: <20260619-ps-eric-work-rebase-v13-12-3d4c7315d2f8@gmail.com>
On Fri, 19 Jun 2026 at 20:27, Pablo Sabater <pabloosabaterr@gmail.com> wrote:
>
> The static allow-list in expand_atom() is hardcoded to only allow
> "objectname" and "objectsize" for remote queries. This works because
> up to this point all servers will either support object-info with name
> and size or they do not support them at all, but we cannot expect that
> in a future different servers with different git versions to have the
> same object-info capabilities. Therefore, the allow_list needs to be
> dynamic depending on what does the server advertise.
Nit: "...depending on what does the server advertise." ->
"...depending on what the server advertises."
^ permalink raw reply
* Re: [PATCH GSoC RFC v13 12/12] cat-file: make remote-object-info allow-list dynamic
From: Chandra Pratap @ 2026-06-21 6:27 UTC (permalink / raw)
To: Pablo Sabater
Cc: gitster, peff, eric.peijian, chriscool, git, jltobler,
karthik.188, toon
In-Reply-To: <CA+J6zkRoS5uZFkW1jJv1JO7jPMPO-ZANOYerbUxn4WPaApPV6g@mail.gmail.com>
Forgot to mention this in the earlier email, but the comment explaining
why the backward iteration of the list is needed should also explain why
it is fine to cast `args->object_info_options->nr` to `int` from `size_t`
(it will always be a small number, so no risk of overflow).
^ permalink raw reply
* Re: [PATCH v6 2/3] revision: add peek functions for lookahead
From: Chandra Pratap @ 2026-06-21 6:42 UTC (permalink / raw)
To: Pablo Sabater
Cc: git, krka, ayu.chandekar, christian.couder, gitster, jltobler,
karthik.188, peff, phillip.wood, siddharthasthana31,
Kristofer Karlsson
In-Reply-To: <20260620-ps-pre-commit-indent-v6-2-cdc6d8fd5fbc@gmail.com>
> +int revision_has_commits_after (struct rev_info *revs, int n)
> +{
> + struct topo_walk_info *info = revs->topo_walk_info;
> +
> + if (info) {
> + int visible = 0;
> + for (size_t i = 0; i < info->topo_queue.nr && visible < n; i++) {
> + struct commit *c = info->topo_queue.array[i].data;
> + if (get_commit_action(revs, c) == commit_show)
> + visible++;
> + }
> + return visible > n-1;
Nit: I think 'return visible >= n' will be more readable here. As in,
more in-line with this function's description (below).
> + if (revs->commits) {
> + struct commit_list *cl;
> + int visible = 0;
> + for (cl = revs->commits; cl && visible < n; cl = cl->next) {
> + if (get_commit_action(revs, cl->item) == commit_show)
> + visible++;
> + }
> + return visible > n-1;
Same here.
> + }
> +
> + return 0;
> +}
> +
> static void trace2_topo_walk_statistics_atexit(void)
> {
> struct json_writer jw = JSON_WRITER_INIT;
> diff --git a/revision.h b/revision.h
> index 00c392be37..a10c6b0940 100644
> --- a/revision.h
> +++ b/revision.h
> @@ -572,4 +572,14 @@ int rewrite_parents(struct rev_info *revs,
> */
> struct commit_list *get_saved_parents(struct rev_info *revs, const struct commit *commit);
>
> +/*
> + * Peek into revision's next commit without consuming it.
> + */
> +struct commit *revision_peek_next_commit(struct rev_info *revs);
> +
> +/*
> + * Check if there are n more commits to be shown yet.
Shouldn't this be "n or more"?
> +int revision_has_commits_after(struct rev_info *revs, int n);
> +
> #endif
>
> --
> 2.54.0
^ permalink raw reply
* [PATCH v3 0/2] doc: clarify review replies and reroll timing
From: Weijie Yuan @ 2026-06-21 8:04 UTC (permalink / raw)
To: git; +Cc: gitster, ps
In-Reply-To: <cover.1781714757.git.wy@wyuan.org>
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.
(Edit: the last point is discussed by Junio[3], which is improved in v3)
Since I am the newbie here, please tell me how to attribute the credit
to Patrick. Thank you Patrick!
(Edit: finished with the Helped-by trailer in v2)
[1]: <xmqq7bo5nf31.fsf@gitster.g>
[2]: <aietF4BX1Ewt3cpG@pks.im>
[3]: <xmqq4ij1vywy.fsf@gitster.g>
---
Changes in v3:
- Reworked the substantial-rework case. Instead of suggesting that
authors send a new version sooner, the text now advises authors not
to rush out an updated version before reviewing the larger changes
carefully. It recommends replying to the review that prompted the
rewrite, saying that a substantial rework is planned, and pointing
out which parts of the current series will become obsolete.
- Dropped the advice that a topic close to being accepted may justify
a quicker reroll.
- Removed "how close the topic is to being accepted" from the short
reroll-timing guidance in Documentation/SubmittingPatches.
- Updated the commit message of patch 2 accordingly.
Changes in v2:
For [PATCH 1/2] doc: encourage review replies before rerolling:
- Add "social interactions" in commit message.
I didn't do any changes for Patrick's comments in [a]:
> I feel like the new version doesn't really add anything significant
> to this paragraph that it didn't already say before your patch, but
> it does so with more words.
> I'm of course biased though, so maybe more words help newcomers?
Thinking about whether to delete/revert or not. Comments welcome.
For [PATCH 2/2] doc: advise batching patch rerolls:
- Add a trailer to thank Patrick.
Suggestions from Junio:
- Mention that waiting between rerolls gives reviewers across time
zones a fair chance to participate.
- Mention that waiting also encourages authors to polish patches
before sending them.
[a] <ai_7Wh7hrD8PZozg@pks.im>
---
base commit: 700432b2ba (topic flush before -rc1 (batch 1), 2026-06-15)
Weijie Yuan (2):
doc: encourage review replies before rerolling
doc: advise batching patch rerolls
Documentation/MyFirstContribution.adoc | 34 ++++++++++++++++++++++----
Documentation/SubmittingPatches | 23 ++++++++++++++---
2 files changed, 48 insertions(+), 9 deletions(-)
Range-diff against v2:
1: 4bb1efe71d = 1: 4bb1efe71d doc: encourage review replies before rerolling
2: 496a08c74d ! 2: e1050a6ef5 doc: advise batching patch rerolls
@@ Commit message
time to comment regardless of their time zones.
Mention factors that can affect the timing, such as series size, review
- depth, substantial rework, and how close the topic is to being accepted.
- Also point out that avoiding rapid rerolls encourages authors to polish
- each version before sending it, so reviewers can focus on substantial
- issues.
+ depth, and substantial rework. Also point out that avoiding rapid
+ rerolls encourages authors to polish each version before sending it, so
+ reviewers can focus on substantial issues.
Helped-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Weijie Yuan <wy@wyuan.org>
@@ Documentation/MyFirstContribution.adoc: previous one" patches over 2 days), revi
+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.
++still coming. If the comments call for substantial rework, do not rush out an
++updated version before you have reviewed the larger changes carefully. Instead,
++reply to the review that prompted the rewrite, say that you are preparing a
++substantial rework, and mention which parts of the current series will become
++obsolete so reviewers can avoid spending time on them until the updated series
++is ready.
+
[[reviewing]]
@@ Documentation/SubmittingPatches: area.
-input and avoids unnecessary churn from many rapid iterations.
+input, gives reviewers in different time zones a fair chance to comment,
+and avoids unnecessary churn from many rapid iterations. Waiting also
-+encourages you to polish each version before sending it, so reviewers can
-+focus on substantial issues rather than typos or other small mistakes.
++encourages you to polish each version before sending it, so reviewers
++can focus on substantial issues rather than typos or other small
++mistakes. (only changes textwidth here)
++
+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.
++series per day, while considering the size of the series and the depth
++of review.
. These early update iterations are expected to be full replacements,
not incremental updates on top of what you posted already. If you
--
2.54.0
^ permalink raw reply
* [PATCH v3 1/2] doc: encourage review replies before rerolling
From: Weijie Yuan @ 2026-06-21 8:05 UTC (permalink / raw)
To: git; +Cc: gitster, ps
In-Reply-To: <cover.1782028813.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. It also encourages more direct social interaction between
contributors and helps foster a more collaborative review process.
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 b9fdefce02..00704ab91e 100644
--- a/Documentation/MyFirstContribution.adoc
+++ b/Documentation/MyFirstContribution.adoc
@@ -1337,11 +1337,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 f042bb5aaf..6c1e1f6423 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
@@ -613,7 +617,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
* [PATCH v3 2/2] doc: advise batching patch rerolls
From: Weijie Yuan @ 2026-06-21 8:05 UTC (permalink / raw)
To: git; +Cc: gitster, ps
In-Reply-To: <cover.1782028813.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 regardless of their time zones.
Mention factors that can affect the timing, such as series size, review
depth, and substantial rework. Also point out that avoiding rapid
rerolls encourages authors to polish each version before sending it, so
reviewers can focus on substantial issues.
Helped-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Weijie Yuan <wy@wyuan.org>
---
Documentation/MyFirstContribution.adoc | 22 ++++++++++++++++++++++
Documentation/SubmittingPatches | 13 +++++++++++--
2 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/Documentation/MyFirstContribution.adoc b/Documentation/MyFirstContribution.adoc
index 00704ab91e..35105bc3b4 100644
--- a/Documentation/MyFirstContribution.adoc
+++ b/Documentation/MyFirstContribution.adoc
@@ -1330,6 +1330,28 @@ 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, gives reviewers across time zones a fair chance to
+participate, lets you batch feedback together, and gives you time to think
+through the comments you received. Knowing that you should not immediately send
+another version also encourages you to review the patches more carefully before
+sending them, catch small mistakes such as typos and off-by-one errors
+yourself, and let reviewers spend more of their attention on design,
+algorithms, and other substantial issues.
+
+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 call for substantial rework, do not rush out an
+updated version before you have reviewed the larger changes carefully. Instead,
+reply to the review that prompted the rewrite, say that you are preparing a
+substantial rework, and mention which parts of the current series will become
+obsolete so reviewers can avoid spending time on them until the updated series
+is ready.
+
[[reviewing]]
=== Responding to Reviews
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 6c1e1f6423..d89efe0707 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -58,7 +58,15 @@ area.
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.
+input, gives reviewers in different time zones a fair chance to comment,
+and avoids unnecessary churn from many rapid iterations. Waiting also
+encourages you to polish each version before sending it, so reviewers
+can focus on substantial issues rather than typos or other small
+mistakes.
++
+As a rough default, avoid sending more than one new version of the same
+series per day, while considering the size of the series and the depth
+of review.
. These early update iterations are expected to be full replacements,
not incremental updates on top of what you posted already. If you
@@ -619,7 +627,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
* [PATCH 2/1] git-gui: reduce complexity of the quiet msgfmt rule
From: Johannes Sixt @ 2026-06-21 10:33 UTC (permalink / raw)
To: Harald Nordgren; +Cc: git, Harald Nordgren via GitGitGadget
In-Reply-To: <pull.2339.v2.git.git.1781995570677.gitgitgadget@gmail.com>
In non-verbose builds (without V=1) the rule to compile *.po files with
msgfmt captures the output in a shell variable and then strips down the
text produced by --statistics to fit on a 80 column line. The previous
commit removed --statistics output of the msgfmt invocation, so that we
don't get to see anything beyond "MSGFMT po/xx.msg" anymore. Make the
rule as minimal as the other "quiet" rules.
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
Am 21.06.26 um 00:46 schrieb Harald Nordgren via GitGitGadget:
> The catalog rules ran msgfmt with --statistics, whose output went to
> stderr and so survived "make -s" (gitk also echoed "Generating
> catalog").
>
> The statistics are not needed, as in 2f12b31b746c (Makefile: don't
> invoke msgfmt with --statistics, 2021-12-17), and the "Generating
> catalog" line is not needed either. Remove them so a quiet build stays
> quiet.
I split off the git-gui part, adjusted the commit message, and then
applied the patch below to simplify the Makefile further.
Makefile | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/Makefile b/Makefile
index 48d848a59dfb..2e1711adc5a5 100644
--- a/Makefile
+++ b/Makefile
@@ -69,8 +69,7 @@ ifndef V
QUIET = @
QUIET_GEN = $(QUIET)echo ' ' GEN '$@' &&
QUIET_INDEX = $(QUIET)echo ' ' INDEX $(dir $@) &&
- QUIET_MSGFMT0 = $(QUIET)printf ' MSGFMT %12s ' $@ && v=`
- QUIET_MSGFMT1 = 2>&1` && echo "$$v" | sed -e 's/fuzzy translations/fuzzy/' | sed -e 's/ messages*//g'
+ QUIET_MSGFMT = $(QUIET)echo ' ' MSGFMT '$@' &&
INSTALL_D0 = dir=
INSTALL_D1 = && echo ' ' DEST $$dir && $(INSTALL) -d -m 755 "$$dir"
@@ -155,7 +154,7 @@ $(PO_TEMPLATE): $(SCRIPT_SH) $(ALL_LIBFILES)
update-po:: $(PO_TEMPLATE)
$(foreach p, $(ALL_POFILES), echo Updating $p ; msgmerge -U $p $(PO_TEMPLATE) ; )
$(ALL_MSGFILES): %.msg : %.po
- $(QUIET_MSGFMT0)$(MSGFMT) --tcl -l $(basename $(notdir $<)) -d $(dir $@) $< $(QUIET_MSGFMT1)
+ $(QUIET_MSGFMT)$(MSGFMT) --tcl -l $(basename $(notdir $<)) -d $(dir $@) $<
lib/tclIndex: $(ALL_LIBFILES) generate-tclindex.sh GIT-GUI-BUILD-OPTIONS
$(QUIET_INDEX)$(SHELL_PATH) generate-tclindex.sh . ./GIT-GUI-BUILD-OPTIONS $(ALL_LIBFILES)
--
2.55.0.rc0.230.g889306758c
^ permalink raw reply related
* Re: [PATCH v2] gitk, git-gui: drop msgfmt --statistics output
From: Johannes Sixt @ 2026-06-21 13:00 UTC (permalink / raw)
To: Harald Nordgren; +Cc: git, Harald Nordgren via GitGitGadget
In-Reply-To: <pull.2339.v2.git.git.1781995570677.gitgitgadget@gmail.com>
Am 21.06.26 um 00:46 schrieb Harald Nordgren via GitGitGadget:
> From: Harald Nordgren <haraldnordgren@gmail.com>
>
> The catalog rules ran msgfmt with --statistics, whose output went to
> stderr and so survived "make -s" (gitk also echoed "Generating
> catalog").
>
> The statistics are not needed, as in 2f12b31b746c (Makefile: don't
> invoke msgfmt with --statistics, 2021-12-17), and the "Generating
> catalog" line is not needed either. Remove them so a quiet build stays
> quiet.
> diff --git a/gitk-git/Makefile b/gitk-git/Makefile
> index 41116d8a14..0ae083c1ca 100644
> --- a/gitk-git/Makefile
> +++ b/gitk-git/Makefile
> @@ -75,8 +75,7 @@ update-po:: $(PO_TEMPLATE)
> echo; \
> echo " git config filter.gettext-no-location.clean \"msgcat --no-location -\""
> $(ALL_MSGFILES): %.msg : %.po
> - @echo Generating catalog $@
> - $(MSGFMT) --statistics --tcl -l $(basename $(notdir $<)) -d $(dir $@) $<
> + $(MSGFMT) --tcl -l $(basename $(notdir $<)) -d $(dir $@) $<
>
> .PHONY: all install uninstall clean update-po
> .PHONY: FORCE
This Gitk part doesn't make the build silent, yet. It misses the "if -s
is in the flags" bracket.
It do not mind doing both (removing --statistics and make it silent) in
a single patch.
BTW, please write commit message in the usual style, in particular,
describe the status quo in present tense, not past tense.
Please bear in mind that Git, Gitk and Git GUI are tracked in different
repositories. Do not put changes to multiple of these into a single
commit/patch.
-- Hannes
^ permalink raw reply
* Re: [PATCH v2] gitk, git-gui: drop msgfmt --statistics output
From: Harald Nordgren @ 2026-06-21 13:15 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git, Harald Nordgren via GitGitGadget
In-Reply-To: <98718401-9ff4-4b1a-97c7-71f8b6639fea@kdbg.org>
Hi Johannes!
Thanks for the feedback here. What do you want me to do now, should I
update my code or you are taking over the whole thing from me?
Harald
^ permalink raw reply
* Re: [PATCH v2] gitk, git-gui: drop msgfmt --statistics output
From: Johannes Sixt @ 2026-06-21 13:27 UTC (permalink / raw)
To: Harald Nordgren; +Cc: git, Harald Nordgren via GitGitGadget
In-Reply-To: <CAHwyqnWM8GpYWOLdMtaF1YJ9mTRBtK0NCQeZE4AorO==7Mz2tg@mail.gmail.com>
Am 21.06.26 um 15:15 schrieb Harald Nordgren:
> Thanks for the feedback here. What do you want me to do now, should I
> update my code or you are taking over the whole thing from me?
The git-gui part is good. The Gitk part needs more work. Please submit a
new patch for Gitk under the topic "make `make -s` silent". That this
has to remove --statistics from msgfmt invocations is just a part of
this topic.
-- Hannes
^ permalink raw reply
* Re: [PATCH v2] gitk, git-gui: drop msgfmt --statistics output
From: Harald Nordgren @ 2026-06-21 13:32 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git, Harald Nordgren via GitGitGadget
In-Reply-To: <c98bc105-f868-43bd-8268-52eb56e5a7c5@kdbg.org>
Same series or a new series alongside this one?
Harald
^ permalink raw reply
* Re: [PATCH v2] gitk, git-gui: drop msgfmt --statistics output
From: Johannes Sixt @ 2026-06-21 13:47 UTC (permalink / raw)
To: Harald Nordgren; +Cc: git, Harald Nordgren via GitGitGadget
In-Reply-To: <CAHwyqnU45DKGMfhJ1e3FmaebRUWkYb39pojPU2TBgOEDvgv-DQ@mail.gmail.com>
Am 21.06.26 um 15:32 schrieb Harald Nordgren:
> Same series or a new series alongside this one?
Don't start a new thread. Since you are using Gitgitgadget, I think this
means that it should be the "same series", whatever the means for you.
-- Hannes
^ permalink raw reply
* [PATCH v3 0/2] Silence po catalog output under "make -s"
From: Harald Nordgren via GitGitGadget @ 2026-06-21 14:56 UTC (permalink / raw)
To: git; +Cc: Harald Nordgren
In-Reply-To: <pull.2339.v2.git.git.1781995570677.gitgitgadget@gmail.com>
The gitk and git-gui are noisy despite "make -s", quiet the builds.
Changes in v3:
* Split the single combined commit into two, one per Makefile (gitk,
git-gui)
* gitk: gate the quiet helpers on -s in MAKEFLAGS and give the catalog rule
a QUIET_MSGFMT prefix, so a silent build emits no MSGFMT/GEN lines
* git-gui: replace the QUIET_MSGFMT0/QUIET_MSGFMT1 pair with a single
QUIET_MSGFMT, since with --statistics gone there is no output left to
reformat
Changes in v2:
* Reworked from conditionally silencing msgfmt output under make -s to just
removing --statistics outright, following 2f12b31b74 (Makefile: don't
invoke msgfmt with --statistics, 2021-12-17)
* Also drop gitk's Generating catalog echo, which is not needed either
Harald Nordgren (2):
gitk: make "make -s" silent
git-gui: silence statistics under "make -s"
git-gui/Makefile | 5 ++---
gitk-git/Makefile | 6 ++++--
2 files changed, 6 insertions(+), 5 deletions(-)
base-commit: 8d96f09e9245ddf80c1981476fcbac8c4bb4125f
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2339%2FHaraldNordgren%2Fsilence-catalog-output-under-make-s-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2339/HaraldNordgren/silence-catalog-output-under-make-s-v3
Pull-Request: https://github.com/git/git/pull/2339
Range-diff vs v2:
1: ee57c25009 ! 1: 4d977d6f3f gitk, git-gui: drop msgfmt --statistics output
@@ Metadata
Author: Harald Nordgren <haraldnordgren@gmail.com>
## Commit message ##
- gitk, git-gui: drop msgfmt --statistics output
+ gitk: make "make -s" silent
- The catalog rules ran msgfmt with --statistics, whose output went to
- stderr and so survived "make -s" (gitk also echoed "Generating
- catalog").
+ The catalog rule runs msgfmt with --statistics, whose output goes to
+ stderr and so survives "make -s", and the rule also echoes "Generating
+ catalog". The Gitk Makefile guards its quiet helpers on V alone, so a
+ silent build still prints these and the GEN line.
The statistics are not needed, as in 2f12b31b746c (Makefile: don't
- invoke msgfmt with --statistics, 2021-12-17), and the "Generating
- catalog" line is not needed either. Remove them so a quiet build stays
- quiet.
+ invoke msgfmt with --statistics, 2021-12-17). Drop them, suppress the
+ quiet helpers when "s" is among the make flags, and give the catalog
+ rule a quiet prefix so a quiet build stays quiet.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
- ## git-gui/Makefile ##
-@@ git-gui/Makefile: $(PO_TEMPLATE): $(SCRIPT_SH) $(ALL_LIBFILES)
- update-po:: $(PO_TEMPLATE)
- $(foreach p, $(ALL_POFILES), echo Updating $p ; msgmerge -U $p $(PO_TEMPLATE) ; )
- $(ALL_MSGFILES): %.msg : %.po
-- $(QUIET_MSGFMT0)$(MSGFMT) --statistics --tcl -l $(basename $(notdir $<)) -d $(dir $@) $< $(QUIET_MSGFMT1)
-+ $(QUIET_MSGFMT0)$(MSGFMT) --tcl -l $(basename $(notdir $<)) -d $(dir $@) $< $(QUIET_MSGFMT1)
-
- lib/tclIndex: $(ALL_LIBFILES) generate-tclindex.sh GIT-GUI-BUILD-OPTIONS
- $(QUIET_INDEX)$(SHELL_PATH) generate-tclindex.sh . ./GIT-GUI-BUILD-OPTIONS $(ALL_LIBFILES)
-
## gitk-git/Makefile ##
+@@ gitk-git/Makefile: PO_TEMPLATE = po/gitk.pot
+ ALL_POFILES = $(wildcard po/*.po)
+ ALL_MSGFILES = $(subst .po,.msg,$(ALL_POFILES))
+
++ifneq ($(findstring s,$(firstword -$(MAKEFLAGS))),s)
+ ifndef V
+ QUIET = @
+ QUIET_GEN = $(QUIET)echo ' ' GEN $@ &&
++ QUIET_MSGFMT = $(QUIET)echo ' ' MSGFMT $@ &&
++endif
+ endif
+
+ all:: gitk-wish $(ALL_MSGFILES)
@@ gitk-git/Makefile: update-po:: $(PO_TEMPLATE)
echo; \
echo " git config filter.gettext-no-location.clean \"msgcat --no-location -\""
$(ALL_MSGFILES): %.msg : %.po
- @echo Generating catalog $@
- $(MSGFMT) --statistics --tcl -l $(basename $(notdir $<)) -d $(dir $@) $<
-+ $(MSGFMT) --tcl -l $(basename $(notdir $<)) -d $(dir $@) $<
++ $(QUIET_MSGFMT)$(MSGFMT) --tcl -l $(basename $(notdir $<)) -d $(dir $@) $<
.PHONY: all install uninstall clean update-po
.PHONY: FORCE
-: ---------- > 2: b613d4ac4a git-gui: silence statistics under "make -s"
--
gitgitgadget
^ permalink raw reply
* [PATCH v3 1/2] gitk: make "make -s" silent
From: Harald Nordgren via GitGitGadget @ 2026-06-21 14:56 UTC (permalink / raw)
To: git; +Cc: Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2339.v3.git.git.1782053803.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
The catalog rule runs msgfmt with --statistics, whose output goes to
stderr and so survives "make -s", and the rule also echoes "Generating
catalog". The Gitk Makefile guards its quiet helpers on V alone, so a
silent build still prints these and the GEN line.
The statistics are not needed, as in 2f12b31b746c (Makefile: don't
invoke msgfmt with --statistics, 2021-12-17). Drop them, suppress the
quiet helpers when "s" is among the make flags, and give the catalog
rule a quiet prefix so a quiet build stays quiet.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
gitk-git/Makefile | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/gitk-git/Makefile b/gitk-git/Makefile
index 41116d8a14..dd87f501e5 100644
--- a/gitk-git/Makefile
+++ b/gitk-git/Makefile
@@ -43,9 +43,12 @@ PO_TEMPLATE = po/gitk.pot
ALL_POFILES = $(wildcard po/*.po)
ALL_MSGFILES = $(subst .po,.msg,$(ALL_POFILES))
+ifneq ($(findstring s,$(firstword -$(MAKEFLAGS))),s)
ifndef V
QUIET = @
QUIET_GEN = $(QUIET)echo ' ' GEN $@ &&
+ QUIET_MSGFMT = $(QUIET)echo ' ' MSGFMT $@ &&
+endif
endif
all:: gitk-wish $(ALL_MSGFILES)
@@ -75,8 +78,7 @@ update-po:: $(PO_TEMPLATE)
echo; \
echo " git config filter.gettext-no-location.clean \"msgcat --no-location -\""
$(ALL_MSGFILES): %.msg : %.po
- @echo Generating catalog $@
- $(MSGFMT) --statistics --tcl -l $(basename $(notdir $<)) -d $(dir $@) $<
+ $(QUIET_MSGFMT)$(MSGFMT) --tcl -l $(basename $(notdir $<)) -d $(dir $@) $<
.PHONY: all install uninstall clean update-po
.PHONY: FORCE
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 2/2] git-gui: silence statistics under "make -s"
From: Harald Nordgren via GitGitGadget @ 2026-06-21 14:56 UTC (permalink / raw)
To: git; +Cc: Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2339.v3.git.git.1782053803.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
The catalog rule runs msgfmt with --statistics, whose output goes to
stderr and so survives "make -s". In non-verbose builds the rule also
captures the output in a shell variable to strip it to an 80 column
line.
The statistics are not needed, as in 2f12b31b746c (Makefile: don't
invoke msgfmt with --statistics, 2021-12-17). Remove them, and with
nothing left to format make the rule as minimal as the other quiet
rules, so a quiet build stays quiet.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
git-gui/Makefile | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/git-gui/Makefile b/git-gui/Makefile
index d33204e875..2e1711adc5 100644
--- a/git-gui/Makefile
+++ b/git-gui/Makefile
@@ -69,8 +69,7 @@ ifndef V
QUIET = @
QUIET_GEN = $(QUIET)echo ' ' GEN '$@' &&
QUIET_INDEX = $(QUIET)echo ' ' INDEX $(dir $@) &&
- QUIET_MSGFMT0 = $(QUIET)printf ' MSGFMT %12s ' $@ && v=`
- QUIET_MSGFMT1 = 2>&1` && echo "$$v" | sed -e 's/fuzzy translations/fuzzy/' | sed -e 's/ messages*//g'
+ QUIET_MSGFMT = $(QUIET)echo ' ' MSGFMT '$@' &&
INSTALL_D0 = dir=
INSTALL_D1 = && echo ' ' DEST $$dir && $(INSTALL) -d -m 755 "$$dir"
@@ -155,7 +154,7 @@ $(PO_TEMPLATE): $(SCRIPT_SH) $(ALL_LIBFILES)
update-po:: $(PO_TEMPLATE)
$(foreach p, $(ALL_POFILES), echo Updating $p ; msgmerge -U $p $(PO_TEMPLATE) ; )
$(ALL_MSGFILES): %.msg : %.po
- $(QUIET_MSGFMT0)$(MSGFMT) --statistics --tcl -l $(basename $(notdir $<)) -d $(dir $@) $< $(QUIET_MSGFMT1)
+ $(QUIET_MSGFMT)$(MSGFMT) --tcl -l $(basename $(notdir $<)) -d $(dir $@) $<
lib/tclIndex: $(ALL_LIBFILES) generate-tclindex.sh GIT-GUI-BUILD-OPTIONS
$(QUIET_INDEX)$(SHELL_PATH) generate-tclindex.sh . ./GIT-GUI-BUILD-OPTIONS $(ALL_LIBFILES)
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH] meson: wire up USE_NSEC build knob
From: D. Ben Knoble @ 2026-06-21 16:41 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, brian m . carlson, Jeff King, Patrick Steinhardt,
Ramsay Jones
In-Reply-To: <xmqq5x3cg10a.fsf@gitster.g>
On Sat, Jun 20, 2026 at 9:01 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "D. Ben Knoble" <ben.knoble+github@gmail.com> writes:
>
> > Autotools-style builds permit enabling USE_NSEC for cases where that's
> > desired; the equivalent knob is missing from meson-based builds.
>
> With or without autoconf, Makefile based build can use USE_NSEC.
Thanks. I almost wrote "Make-based," but I wasn't sure how we
preferred to describe it.
> It
> is a welcome addition to the other side of thw world. I do not know
> if 'meson setup -Dnanosec=true' is a name that is easy to discover,
> though.
>
> Will queue. Thanks.
Agreed for the name. Alternatives welcome.
^ permalink raw reply
* Re: [PATCH v3 0/2] completion: hide dotfiles for selected path completion
From: D. Ben Knoble @ 2026-06-21 16:46 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Zakariyah Ali via GitGitGadget, git, Zakariyah Ali
In-Reply-To: <xmqq1pe0g08t.fsf@gitster.g>
[Small typo correction that may affect how the message is read]
On Sat, Jun 20, 2026 at 9:18 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Zakariyah Ali via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > The completion helper for index paths uses git ls-files rather than shell
> > filename completion. As a result, leading-dot paths such as a tracked
> > .gitignore were offered even when the user had not started the path with ..
> >
> > Hide leading-dot path components for git rm, git mv, and git ls-files when
> > completing an empty path component. Explicit dot completion is still
> > preserved, so git rm . can still complete .gitignore.
> >
> > This removes the existing TODO expectations in t/t9902-completion.sh and
> > adds coverage for explicit dot completion.
>
> OK.
>
> > Validation:
> >
> > * git diff --check -- contrib/completion/git-completion.bash
> > t/t9902-completion.sh
> > * bash -n contrib/completion/git-completion.bash
> > * ./t9902-completion.sh
>
> I am not sure what you wanted to say with these lines. If you did
> the above to build confidence that your patch works, that would be
> great. Or are you telling readers to do these things and when they
> do not see any issues consider your patch perfect?
>
> What is missing around here in this cover letter is a description of
> how this iteration is different from the previous one. And ...
>
> > Zakariyah Ali (2):
> > completion: hide dotfiles for selected path completion
> > completion: hide dotfiles by default for path completion
> >
> > contrib/completion/git-completion.bash | 53 +++++++++++++++-----------
> > t/t9902-completion.sh | 19 ++++-----
> > 2 files changed, 40 insertions(+), 32 deletions(-)
> >
> >
> > base-commit: 9b7fa37559a1b95ee32e32858b0d038b4cf583e5
> > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2311%2Falibaba0010%2Fcompletion-hide-dotfiles-v3
> > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2311/alibaba0010/completion-hide-dotfiles-v3
> > Pull-Request: https://github.com/git/git/pull/2311
> >
> > Range-diff vs v2:
> >
> > 1: 056e239e06 = 1: 056e239e06 completion: hide dotfiles for selected path completion
> > -: ---------- > 2: 7482ee4645 completion: hide dotfiles by default for path completion
>
> ... I find this range diff very troubling. If we look at patch 2,
> it seems that it redoes some part of what is done in patch 1 saying
> "oops that was wrong, so let's do it better this time". Such a
> drunken-mans' walk that goes in one direction in an earlier step,
> only to be corrected to move to a different course, is now how we
"is not" :)
> want a new topic to be presented.
>
> The end result may be much easier to read, mostly thanks to updated
> loop in the awk script, so if we really want to pretend this as two
> patches for "small pieces are easier to digest" value, perhaps have
> [PATCH 1/2] that updates the awk script (without doing anything
> related to hide-dotfiles theme) to make it easier to read by not
> having multiple "print pfx p" in it, and then build on top of that
> improved base, have [PATCH 2/2] that adds the support to hide
> dotfiles, perhaps?
>
> Since the initial iteration was quite a while ago, I no longer
> remember the details of the review I gave, but I recall having hard
> time telling which callers of the complete-index-file helper hide
> dotfiles from their output and which callers do not hide them, and
> how the patch decided to choose which ones should and should not
> hide. Has it been improved and if so how? That is something we
> expect the cover letter to tell, too.
>
> Thanks.
>
--
D. Ben Knoble
^ permalink raw reply
* Re: git-diff in a worktree is an order of magnitude slower?
From: Jeff King @ 2026-06-21 17:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: D. Ben Knoble, Git
In-Reply-To: <xmqqa4sog1e9.fsf@gitster.g>
On Sat, Jun 20, 2026 at 05:53:02PM -0700, Junio C Hamano wrote:
> > which dates to aecbf914c4 (git-diff: resurrect the traditional empty
> > "diff --git" behaviour, 2007-08-31). On my system that comparison is
> > false because the double-negation produces 1
> > (diff_auto_refresh_index=1 or the result of git_config_bool).
>
> Not quite. It was false because double-negation initializes the
> member to 1, which causes a call to diffcore_skip_stat_unmatch()
> be made, *and* the diffcore_skip_stat_unmatch() function did not
> find any ghost changes, i.e., paths that were only stat-dirty hence
> needed a call to refresh_index_quietly().
I think this is the core of the issue. These entries are "racy git
dirty" in the sense that their mtimes are the same as the index mtime,
and so we double-check the contents. This is the first bullet point
under the "Racy Git" section of Documentation/technical/racy-git.adoc.
But diffcore_skip_stat_unmatch() doesn't count them as dirty, so we
don't increment the counter, and thus top-level git-diff won't write out
the new index. And thus every subsequent diff repeats the same
expensive double-check.
But I'm not sure where the blame lies. Either:
1. diffcore_skip_stat_unmatch() should be counting these in its
"dirty" counter; or
2. the index should be marking these differently. The second bullet
point of that Racy Git section says:
When the index file is updated that contains racily clean
entries, cached `st_size` information is truncated to zero
before writing a new version of the index file.
Should the index be written out with a 0 size field here, so that
we know they are dirty and should be updated? I guess that would be
user-visible, though, because commands that _don't_ update the
index (like plumbing diff-files) would generate a spurious diff
there rather than doing the content-level comparison.
I dunno. You had solved most of the racy git stuff before I came along,
so I never gave it too much thought (and what little thought I did was
many years ago).
> > So… has that conditional been quietly dead all this time? I can't
> > imagine that's right, but…
>
> I initially thought it was an embarrassing thinko, but after seeing
> how .skip_stat_unmatch is used as a 1-based counter (i.e., if the
> member says 42, it means it saw 41 paths that were stat-dirty but
> without actual content change), I do not think so.
>
> Now, it is a different matter if such a "dual" purpose "more than a
> simple boolean" counter is a good idea. Apparently it confused both
> of us in this case ;-).
Make that three of us. ;)
-Peff
^ permalink raw reply
* Re: git-diff in a worktree is an order of magnitude slower?
From: Jeff King @ 2026-06-21 17:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: D. Ben Knoble, Git
In-Reply-To: <20260621172432.GA2206349@coredump.intra.peff.net>
On Sun, Jun 21, 2026 at 01:24:32PM -0400, Jeff King wrote:
> I think this is the core of the issue. These entries are "racy git
> dirty" in the sense that their mtimes are the same as the index mtime,
> and so we double-check the contents. This is the first bullet point
> under the "Racy Git" section of Documentation/technical/racy-git.adoc.
>
> But diffcore_skip_stat_unmatch() doesn't count them as dirty, so we
> don't increment the counter, and thus top-level git-diff won't write out
> the new index. And thus every subsequent diff repeats the same
> expensive double-check.
>
> But I'm not sure where the blame lies. Either:
>
> 1. diffcore_skip_stat_unmatch() should be counting these in its
> "dirty" counter; or
BTW, I don't think diffcore actually has the information it would need
to do so. The racy stuff is handled under the hood in ie_match_stat(),
which returns only a set of "changed" flags. So the caller cannot tell
the difference between the two cases:
1. We checked ce_match_stat_basic() which said "no change", and then
is_racy_timestamp() was false, so that was good enough.
2. is_racy_timestamp() is true, so we further did a content check,
found nothing, and returned the same "no change"
Obviously we could pass back another flag, but that would disrupt the
other callers. Hmm. It looks like we could pass in a flag to say "assume
racy entries are modified". And then they come back to the diff code,
diffcore_skip_stat_unmatch() sees they're not real diffs and suppresses
them, but we _do_ count them as stat-dirty.
Like this:
diff --git a/builtin/diff.c b/builtin/diff.c
index 4b46e394ce..4d36b5c1e0 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -271,6 +271,9 @@ static void builtin_diff_files(struct rev_info *revs, int argc, const char **arg
argv++; argc--;
}
+ if (revs->diffopt.skip_stat_unmatch)
+ options |= DIFF_RACY_IS_MODIFIED;
+
/*
* "diff --base" should not combine merges because it was not
* asked to. "diff -c" should not densify (if the user wants
That seems to work, in the sense that "git diff" does refresh the index
afterwards. But the timings are a bit funny.
In my working tree of linux.git with many racy entries it was ~500ms to
do the first diff (and the second, and so on, because we never updated
the index). After the patch above, it is 1800ms to do the first diff,
and then fast (~30ms) after.
I could believe it takes twice as long when we refresh the index
(because I don't think we use the stat-cleanliness we collected from the
diff, but rather just do a from-scratch index refresh). But that would
imply it should take ~1000ms. Where does the extra 800ms go? I guess
that somehow the content-check done by diffcore_skip_stat_unmatch() is
slower than the one done by ie_match_stat(). I think the individual
functions are respectively diff_filespec_check_stat_unmatch() and
ce_modified_check_fs().
I don't know if any of this is really worth digging too far. This feels
like a case we could do a bit better at, but I wonder how much it
matters in practice. As soon as you do any index-refresh (including "git
status"), the racy entries are cleared and everything is faster. It
just seems kind of lame that we write out the initial working tree with
so many racy entries.
-Peff
^ permalink raw reply related
* Re: [PATCH] meson: wire up USE_NSEC build knob
From: Jeff King @ 2026-06-21 17:49 UTC (permalink / raw)
To: D. Ben Knoble
Cc: git, brian m . carlson, Junio C Hamano, Patrick Steinhardt,
Ramsay Jones
In-Reply-To: <c4c5ade901ff95b0f95939ea818870e4f3d59da1.1781971201.git.ben.knoble+github@gmail.com>
On Sat, Jun 20, 2026 at 12:00:24PM -0400, D. Ben Knoble wrote:
> Autotools-style builds permit enabling USE_NSEC for cases where that's
> desired; the equivalent knob is missing from meson-based builds.
Seems reasonable. This is not changing the defaults at all, but just
bringing meson's options to parity with the Makefile.
I'm not still not sure if turning on USE_NSEC is a good idea. There's
some discussion in Documentation/technical/racy-git.adoc:
With `USE_NSEC`
compile-time option, `st_mtim.tv_nsec` and `st_ctim.tv_nsec`
members are also compared. On Linux, this is not enabled by default
because in-core timestamps can have finer granularity than
on-disk timestamps, resulting in meaningless changes when an
inode is evicted from the inode cache. See commit 8ce13b0
of git://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git
([PATCH] Sync in core time granularity with filesystems,
2005-01-04). This patch is included in kernel 2.6.11 and newer, but
only fixes the issue for file systems with exactly 1 ns or 1 s
resolution. Other file systems are still broken in current Linux
kernels (e.g. CEPH, CIFS, NTFS, UDF), see
https://lore.kernel.org/lkml/5577240D.7020309@gmail.com/
That's the most succinct description of the problem I've seen, but I
have no idea how widely it still applies. Kernel 2.6.11 is quite old
now, but I could believe that other filesystems (especially network
ones) still exhibit the issue.
So I guess if we wanted to go further it would take some digging as to
how each platform behaves, and then flipping the config.make.uname knob
for ones where it can be argued that the behavior is always reasonable.
But that's all outside the scope of your patch here.
-Peff
^ permalink raw reply
* Re: [PATCH v5 2/2] graph: indent visual root in graph
From: Jeff King @ 2026-06-21 18:05 UTC (permalink / raw)
To: Kristofer Karlsson
Cc: Pablo Sabater, git, ayu.chandekar, chandrapratap3519,
christian.couder, gitster, jltobler, karthik.188, phillip.wood,
siddharthasthana31
In-Reply-To: <CAL71e4MAtD4MqE-22UyYaNFVYcFgYmffngihhovEChVfHLmEdA@mail.gmail.com>
On Fri, Jun 19, 2026 at 09:34:16AM +0200, Kristofer Karlsson wrote:
> On Thu, 18 Jun 2026 at 18:05, Jeff King <peff@peff.net> wrote:
> >
> > Thanks for looking into it. I meant to also cc the Kristofer, the author
> > of dd4bc01c0a, for any thoughts (adding him now).
> >
>
> Thanks for the CC. I took a look at how this interacts with my
> change.
>
> dd4bc01c0a doesn't hurt here I think, but future followup changes
> might. From what I can tell --graph triggers topo_order, so
> the walk mode is either REV_WALK_TOPO or REV_WALK_LIMITED
> and the prio_queue change only applies to REV_WALK_STREAMING.
I'm not so sure. If I merge 53967f242a (graph: indent visual root in
graph, 2026-06-13) into master (so that it has both your commit_queue
changes and Pablo's topic), and then apply this:
diff --git a/graph.c b/graph.c
index e0d1e2a510..8a5f17a089 100644
--- a/graph.c
+++ b/graph.c
@@ -926,6 +926,10 @@ static void graph_peek_next_visible(struct git_graph *graph,
flags->is_next_visual_root = 0;
flags->next_has_column = 0;
+ warning("peeking at visible commits: %d in list, %d in queue",
+ commit_list_count(graph->revs->commits),
+ (int)graph->revs->commit_queue.nr);
+
for (cl = graph->revs->commits; cl; cl = cl->next) {
if (get_commit_action(graph->revs, cl->item) != commit_show)
continue;
and run:
./git log --graph -- Makefile
then we always see exactly one commit in the list, but an
ever-increasing number in the queue (up to ~4000). We do seem to be in
REV_WALK_TOPO mode, so I think we'd never return the commits via
get_revision(), but it is weird that we are sticking them in the queue
at all.
Looks like that happens via rewrite_parents(), which always writes into
commit_queue. I guess it doesn't matter because in topo mode we are
always pulling off of the topo_walk_info queue anyway? It does make me
wonder if there is a lurking bug around history simplification and
--topo-order, though.
> That said, graph_peek_next_visible() reaching directly into
> revs->commits feels fragile -- especially if we drop revs->commits
> in the future. One option would be to add a thin abstraction in
> revision.c that dispatches per walk mode, something like:
>
> int revision_has_more_commits(struct rev_info *revs)
> {
> if (revs->topo_walk_info)
> return revs->topo_walk_info->topo_queue.nr > 0;
> return revs->commits != NULL;
> }
>
> struct commit *revision_peek_next_commit(struct rev_info *revs)
> {
> if (revs->topo_walk_info)
> return prio_queue_peek(&revs->topo_walk_info->topo_queue);
> if (revs->commits)
> return revs->commits->item;
> return NULL;
> }
>
> That way graph.c does not need to know which data structure the
> walker uses, and if the internals change later the API adapts in
> one place.
Yeah, I agree some abstraction would help. I think it would have to be
full iteration, though; the graph code wants to know if there is any
commit that is actually going to be shown, not just a potential single
next one. So we at least need to be able to iterate in arbitrary order.
> As for the multi-element peek question, I think I would either opt
> for draining into a buffer if it's really needed, though when looking
> at the code here I think multi-element peeking is not truly needed.
> It seems like the logic just checks if there is at least another
> element after the peek, but it does not try to read the actual value,
> so we can just check the queue size instead.
We do look at some characteristics of the commit we find by peeking, but
I'm not sure how much it matters if we get the _next_ commit that will
be shown, or if any arbitrary commit is OK.
-Peff
^ permalink raw reply related
* Re: [PATCH v14 4/6] branch: add --prune-merged <branch>
From: Harald Nordgren @ 2026-06-21 18:46 UTC (permalink / raw)
To: Junio C Hamano
Cc: Phillip Wood, Harald Nordgren via GitGitGadget, git,
Kristoffer Haugsbakk, Johannes Sixt
In-Reply-To: <CAHwyqnWt59h2HO5EJbFswYr7QEA7oNZKdBt_vTk5axNbWFZbpA@mail.gmail.com>
Looking into this more and attempting to implement the logic for
re-assigning the upstream, it becomes quite a lot of code.
Maybe an easier way forward now is to avoid deleting these cases. We
can always add the re-assigning logic later on without breaking
backward compatibility.
Harald
^ 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