* Re: [PATCH RFC 2/2] builtin/history: print feedback after successful reword
From: Ben Knoble @ 2026-06-08 16:47 UTC (permalink / raw)
To: Pablo Sabater; +Cc: Junio C Hamano, git, Patrick Steinhardt, Kaartic Sivaraam
In-Reply-To: <CAN5EUNQNj86Q+hi6PouOZNWo1T4QTQ6sE5Hs9USZXWpkTedTcw@mail.gmail.com>
> Le 8 juin 2026 à 09:29, Pablo Sabater <pabloosabaterr@gmail.com> a écrit :
>
> El lun, 8 jun 2026 a las 14:16, Junio C Hamano (<gitster@pobox.com>) escribió:
>>
>> Pablo Sabater <pabloosabaterr@gmail.com> writes:
>>
>>> Unlike `git commit --amend` and `git rebase -i`, `git history reword`
>>> doesn't print anything, this makes it feel empty for a porcelain command
>>> and hard to tell if the command did anything without using other
>>> commands like `git log <commit>` to check if the reword was done.
>>>
>>> Print a message on successful rewords so the user has feedback about it.
>>>
>>> Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
>>> ---
>>> builtin/history.c | 4 ++++
>>> t/t3451-history-reword.sh | 14 ++++++++++++++
>>> 2 files changed, 18 insertions(+)
>>>
>>> diff --git a/builtin/history.c b/builtin/history.c
>>> index 51a22a9a1c..0f1ba3b531 100644
>>> --- a/builtin/history.c
>>> +++ b/builtin/history.c
>>> @@ -739,6 +739,10 @@ static int cmd_history_reword(int argc,
>>> goto out;
>>> }
>>>
>>> + fprintf(stderr, _("Successfully reworded commit %s to %s\n"),
>>> + repo_find_unique_abbrev(repo, &original->object.oid, DEFAULT_ABBREV),
>>> + repo_find_unique_abbrev(repo, &rewritten->object.oid, DEFAULT_ABBREV));
>>> +
>>> ret = 0;
>>>
>>> out:
>>
>> Do other commands in "git history" (split is in 'master', drop and
>> fixup are cooking) behave with similar verbosity? Consistency within
>> the same "history" umbrella matters more than being similar with
>> other commands that can be used for similar purposes.
>
> They do not, they are thought with the rule of silence in mind.
> However I think that this output is valuable information I might have
> explained myself better at [1] but my thought is:
>
> git history reword aabb
>
> Now that I have my commit aabb rewritten I want to check it again just
> to make sure I did what I wanted correctly,
Some thoughts:
- If the rewritten commit is an ancestor of HEAD, look at the log of HEAD@{1} or the log between HEAD and the aforementioned reflog entry. (git-range-diff may also be helpful there.)
- Similarly, if the rewritten commit is reachable from some ref R, check R@{1} etc.
> but git log aabb is still
> the old commit, the rewritten one has a different hash which I do not
> know unless I search for it, if it's far from HEAD I'd have to git log
> --oneline, get the hash and then git log new_hash. I think that git
> history reword that does have the information about the new hash
> should print it to avoid this search.
> What I want is something like:
>
> git history reword aabb
> Successfully reworded aabb to ccdd
>
> So I can just git log ccdd without having to search.
>
> I want to say I haven't looked as much as I'd like to split, drop and
> fixup, but I think it would be a good addition for them also. On [1]
> Patrick wrote about a --verbose for git history, I think that the
> basic information i.e. at reword which is the new hash should be
> always printed but if it's preferred it could go there.
>
> For split it can print the hashes of the new commits like:
> "...split into ccdd and eeff."
> For fixup the commit hash also changes, so the same as reword.
> The one that will have more friction would be drop is the one that
> doesn't end up with new commits.
>
> [1]: https://lore.kernel.org/git/CAN5EUNSAOMRvmLGVfzQiwWoOn9VGNVU5rVMZizOryn_q2fbCNA@mail.gmail.com/
^ permalink raw reply
* Re: [PATCH RFC 1/2] builtin/history: abort reword on unchanged message
From: Ben Knoble @ 2026-06-08 16:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Pablo Sabater, git, Patrick Steinhardt, Kaartic Sivaraam
In-Reply-To: <xmqqmrx5z0po.fsf@gitster.g>
> Le 8 juin 2026 à 08:23, Junio C Hamano <gitster@pobox.com> a écrit :
>
[snip]
> Having said that, I personally think that the current behaviour of
> `commit --amend` and `history reword` are both _wrong_ [*2*].
>
> You may start `git commit --amend`, and after staring at the
> existing commit log message for some time in your editor, it is
> quite natural for you to decide that leaving the commit as-is is the
> right thing [*3*] in your situation. It may have been a better
> design for the system to notice this situation and leave the commit
> as-is, with an override option `--force` to allow users to forcibly
> update the committer ident and timestamp in the commit header. I am
> not a `history reword` user (yet), but from the motivation you
> described for this patch, I sense that the story is the same there.
FWIW, in this situation I abort my editor (:cquit in Vim) so that the amend gets an error-valued exit code from the subprocess and aborts itself.
Perhaps there could/should be a better side-channel for communicating that, though? I do not know how easy it is to tell other editors to « quit with errors ».
> [Footnote]
>
> *1* Besides, doesn't "--update-refs" in "rebase -i" allow you to
> adjust the branches?
>
> *2* But it is an established behaviour people _rely_ on, so even
> though it may have been better if these commands behaved
> differently, it probably is a bit too late to change it now.
>
> *3* This includes the case where the original author is especially
> difficult to work with and would complain any change to their
> commits, even if the only change you made for them is a
> typofix. Fixing a small typo/grammo may not be worth your time
> and unpleasant exchanges with them after touching their commit.
^ permalink raw reply
* Re: [PATCH RFC 1/2] builtin/history: abort reword on unchanged message
From: Ben Knoble @ 2026-06-08 16:37 UTC (permalink / raw)
To: Pablo Sabater; +Cc: git, Patrick Steinhardt, Kaartic Sivaraam, Pablo Sabater
In-Reply-To: <20260607-ps-history-reword-v1-1-ba43a3cbb81b@gmail.com>
I don’t have any strong opinions on the rest…
> Le 7 juin 2026 à 16:08, Pablo Sabater <pabloosabaterr@gmail.com> a écrit :
>
> When using `git history reword` if the new message is the same as the
> original it continues anyway creating a new commit with the same
> message and updates its descendants, modifying the history after this
> 'reworded' commit even though there was no actual change.
>
> `git commit --amend` and `git rebase -i` + reword share this behavior,
> however `git history reword` is different:
> 1. Works in-memory without touching the index or the worktree [1], so
> there are no side effects like staged files that could justify
> rewriting the history when the commit message is the same.
> 2. `git history` by default updates all the branches [2] that contain the
> original commit making it more costly than `git rebase -i` that only
> updates the current branch.
>
> Add a check if the original commit message is the same as the new one
> and abort if so.
>
> [1]: https://lore.kernel.org/git/20260113-b4-pks-history-builtin-v11-8-e74ebfa2652d@pks.im/
> [2]: https://git-scm.com/docs/git-history#_description
>
> Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
> ---
> builtin/history.c | 10 ++++++++++
> t/t3451-history-reword.sh | 20 ++++++++++++++++++++
> 2 files changed, 30 insertions(+)
>
> diff --git a/builtin/history.c b/builtin/history.c
> index 0fc06fb204..51a22a9a1c 100644
> --- a/builtin/history.c
> +++ b/builtin/history.c
> @@ -135,6 +135,13 @@ static int commit_tree_ext(struct repository *repo,
> original_body, action, &commit_message);
> if (ret < 0)
> goto out;
> +
> + if (!strcmp(original_body, commit_message.buf)) {
> + fprintf(stderr, _("Message unchanged,"
> + " aborting reword.\n"));
> + ret = 1;
> + goto out;
> + }
> } else {
> strbuf_addstr(&commit_message, original_body);
> }
> @@ -718,6 +725,9 @@ static int cmd_history_reword(int argc,
> if (ret < 0) {
> ret = error(_("failed writing reworded commit"));
> goto out;
> + } else if (ret == 1) {
> + ret = 0;
> + goto out;
> }
>
> strbuf_addf(&reflog_msg, "reword: updating %s", argv[0]);
> diff --git a/t/t3451-history-reword.sh b/t/t3451-history-reword.sh
> index de7b357685..54ea8a7207 100755
> --- a/t/t3451-history-reword.sh
> +++ b/t/t3451-history-reword.sh
> @@ -396,4 +396,24 @@ test_expect_success 'retains changes in the worktree and index' '
> )
> '
>
> +test_expect_success 'aborts if the commit message is the same' '
> + test_when_finished "rm -rf repo" &&
> + git init repo &&
> + (
> + cd repo &&
> + test_commit first &&
> + test_commit second &&
> +
> + git rev-parse HEAD >oid-before &&
> + write_script fake-editor.sh <<-\EOF &&
> + true
> + EOF
> + test_set_editor "$(pwd)"/fake-editor.sh &&
> + git history reword HEAD 2>err &&
> + git rev-parse HEAD >oid-after &&
> + test_cmp oid-before oid-after &&
> + test_grep "Message unchanged" err
> + )
…but I think this test case could do something like "GIT_EDITOR=true git history reword HEAD" and avoid the script?
> +'
> +
> test_done
>
> --
> 2.54.0
Best,
Ben
^ permalink raw reply
* Re: [PATCH 00/16] odb: make packed object source a proper `struct odb_source`
From: Karthik Nayak @ 2026-06-08 16:15 UTC (permalink / raw)
To: Patrick Steinhardt, git
In-Reply-To: <20260604-pks-odb-source-packed-v1-0-2e7ab31b4b5c@pks.im>
[-- Attachment #1: Type: text/plain, Size: 1271 bytes --]
Patrick Steinhardt <ps@pks.im> writes:
> Hi,
>
> this patch series converts the "packed" source into a proper `struct
> odb_source`. It's thus the equivalent to [1], which did the same thing
> for the "loose" source.
>
> This series here is unfortunately a bit bigger, mostly because I'm also
> renaming `struct packfile_store` to `struct odb_source_packed`. Back
> when I introduced the packfile store I didn't yet have the full vision
> of how the final layout will look like, so I didn't have the foresight
> yet to call it `struct odb_source_packed`. But now that the layout has
> materialized I think it's sensible to adjust its naming to match all the
> other sources that we have.
>
> Also: I don't have anything else in the pipeline anymore that moves
> around large pieces of our code in the vicinity of the object database.
> So after this series got merged, subsequent changes should be of a more
> incremental nature.
>
> This series is built on top of 9ac3f193c0 (The 11th batch, 2026-06-02)
> with ps/odb-source-loose at ef4778bcba (odb/source-loose: drop pointer
> to the "files" source, 2026-06-01) merged into it.
This was a good read. The commits towards the end are mostly simple code
movements. Overall the series looks to be in good shape.
[snip]
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
* Re: [PATCH] worktree: record creation time and free-form note
From: Kiesel, Norbert @ 2026-06-08 16:12 UTC (permalink / raw)
To: git, Junio C Hamano; +Cc: phillip.wood, Chris Torek, kristofferhaugsbakk
In-Reply-To: <CAPx1Gvegc0KvE8zb90n7vLJLKx6EkmBvCWW=NPf+nwiZc+oWdQ@mail.gmail.com>
Hi team,
I updated my proposed extension in a couple of ways you suggested, and
also added some more test code.
Best,
Norbert
diff --git Documentation/git-worktree.adoc Documentation/git-worktree.adoc
index fbf8426cd9..1cdbdc8dbe 100644
--- Documentation/git-worktree.adoc
+++ Documentation/git-worktree.adoc
@@ -10,8 +10,11 @@ SYNOPSIS
--------
[synopsis]
git worktree add [-f] [--detach] [--checkout] [--lock [--reason <string>]]
+ [--description <string>]
[--orphan] [(-b | -B) <new-branch>] <path> [<commit-ish>]
-git worktree list [-v | --porcelain [-z]]
+git worktree describe <worktree> [<description>]
+git worktree list [-v | --porcelain [-z]] [--show-created]
+ [--show-updated] [--show-description] [--sort=<key>]
git worktree lock [--reason <string>] <worktree>
git worktree move <worktree> <new-path>
git worktree prune [-n] [-v] [--expire <expire>]
@@ -106,6 +109,16 @@ passed to the command. In the event the
repository has a remote and
command fails with a warning reminding the user to fetch from their remote
first (or override by using `-f`/`--force`).
+`describe <worktree> [<description>]`::
+
+Set, replace, or clear a free-form description on a linked worktree.
+Useful for recording what a worktree was created for so it can be identified
+later. With _<description>_, the worktree's description is set or replaced;
+without a description argument, the existing description is cleared. The
+description for a worktree may also be set at creation time with
+`git worktree add --description <description>`. The main worktree cannot be
+described.
+
`list`::
List details of each worktree. The main worktree is listed first,
@@ -114,6 +127,28 @@ whether the worktree is bare, the revision
currently checked out, the
branch currently checked out (or "detached HEAD" if none), "locked" if
the worktree is locked, "prunable" if the worktree can be pruned by the
`prune` command.
++
+Each worktree's creation timestamp is recorded when it is created with
+`git worktree add`. Worktrees created before this feature existed have no
+recorded creation timestamp; for them, `list` reports `created: unknown`
+in human output and omits the `created` line in `--porcelain` output. Pass
+`--show-created` to include creation timestamps in human output. Worktrees
+without a recorded timestamp sort last (or first when reversed) with
+`--sort=created`.
++
+Pass `--show-updated` to include each worktree's last-updated timestamp,
+which is the modification time of the worktree's `HEAD` file and so
+reflects checkouts, commits, resets, rebases, and similar Git operations.
++
+Pass `--show-description` to include any user-provided description in human
+output. In `--porcelain` output, the `created`, `updated`, and
+`description` lines are emitted whenever the underlying data is available.
++
+Use `--sort=<key>` (where _<key>_ is `path`, `created`, or `updated`,
+optionally prefixed with `-` to reverse) to order the linked worktrees;
+the main worktree always remains first. Sorting by `created` or `updated`
+implies the matching `--show-created` / `--show-updated` flag so the order
+is visible alongside the data.
`lock`::
@@ -286,6 +321,46 @@ _<time>_.
With `lock` or with `add --lock`, an explanation why the worktree
is locked.
+`--description <string>`::
+ With `add`, attach a free-form description to the new worktree.
+ The description is stored alongside the worktree's administrative
+ files and can be displayed with `git worktree list --show-description`
+ or in `--porcelain` output. It can be changed later with
+ `git worktree describe`.
+
+`--show-created`::
+ With `list`, include each worktree's creation timestamp in the
+ human-readable output. Worktrees with no recorded creation time are
+ shown as `created: unknown`. In `--porcelain` output, the creation
+ timestamp is always included (when available) on a `created` line.
+
+`--show-updated`::
+ With `list`, include each linked worktree's last-updated timestamp in
+ the human-readable output, derived from the modification time of the
+ worktree's `HEAD` file. Linked worktrees whose `HEAD` cannot be read
+ are shown as `updated: unknown`. The main worktree is not annotated
+ with an updated timestamp. In `--porcelain` output, the timestamp is
+ included on an `updated` line whenever it is available (and the
+ worktree is not the main worktree).
+
+`--show-description`::
+ With `list`, include each worktree's description (if set) in the
+ human-readable output. In `--porcelain` output, the description is
+ always included (when set) on a `description` line.
+
+`--sort=<key>`::
+ With `list`, sort linked worktrees by _<key>_, which is one of
+ `path`, `created`, or `updated`. Prefix with `-` to reverse the order,
+ e.g. `--sort=-created` lists newest first. The main worktree is always
+ listed first regardless of sort order. For `created`, worktrees with no
+ recorded creation timestamp sort after those that have one (or before,
+ when reversed). For `updated`, ordering is by the modification time of
+ each worktree's `HEAD` file (a proxy for when the worktree was last
+ touched by checkout, commit, reset or rebase); worktrees whose `HEAD`
+ cannot be read sort after those that can. Sorting by `created` or
+ `updated` implies the matching `--show-created` / `--show-updated`
+ option so the values driving the order appear in human output.
+
_<worktree>_::
Worktrees can be identified by path, either relative or absolute.
+
@@ -462,7 +537,10 @@ are terminated with NUL rather than a newline.
Attributes are listed with a
label and value separated by a single space. Boolean attributes (like `bare`
and `detached`) are listed as a label only, and are present only
if the value is true. Some attributes (like `locked`) can be listed as a label
-only or with a value depending upon whether a reason is available. The first
+only or with a value depending upon whether a reason is available. Optional
+valued attributes (like `created`, `updated`, and `description`) appear
+only when the corresponding metadata has been recorded for that worktree.
+The first
attribute of a worktree is always `worktree`, an empty line indicates the
end of the record. For example:
@@ -474,10 +552,15 @@ bare
worktree /path/to/linked-worktree
HEAD abcd1234abcd1234abcd1234abcd1234abcd1234
branch refs/heads/master
+created 2026-06-01T12:34:56Z
+updated 2026-06-04T17:20:11Z
+description investigating login bug
worktree /path/to/other-linked-worktree
HEAD 1234abc1234abc1234abc1234abc1234abc1234a
detached
+created 2026-05-28T08:15:00Z
+updated 2026-05-30T09:42:08Z
worktree /path/to/linked-worktree-locked-no-reason
HEAD 5678abc5678abc5678abc5678abc5678abc5678c
diff --git builtin/worktree.c builtin/worktree.c
index d21c43fde3..132de668e3 100644
--- builtin/worktree.c
+++ builtin/worktree.c
@@ -27,13 +27,17 @@
#include "utf8.h"
#include "worktree.h"
#include "quote.h"
+#include "date.h"
#define BUILTIN_WORKTREE_ADD_USAGE \
N_("git worktree add [-f] [--detach] [--checkout] [--lock [--reason
<string>]]\n" \
+ " [--description <string>]\n" \
" [--orphan] [(-b | -B) <new-branch>] <path>
[<commit-ish>]")
#define BUILTIN_WORKTREE_LIST_USAGE \
- N_("git worktree list [-v | --porcelain [-z]]")
+ N_("git worktree list [-v | --porcelain [-z]] [--show-created]\n" \
+ " [--show-updated] [--show-description]\n" \
+ " [--sort=<key>]")
#define BUILTIN_WORKTREE_LOCK_USAGE \
N_("git worktree lock [--reason <string>] <worktree>")
#define BUILTIN_WORKTREE_MOVE_USAGE \
@@ -46,6 +50,8 @@
N_("git worktree repair [<path>...]")
#define BUILTIN_WORKTREE_UNLOCK_USAGE \
N_("git worktree unlock <worktree>")
+#define BUILTIN_WORKTREE_DESCRIBE_USAGE \
+ N_("git worktree describe <worktree> [<description>]")
#define WORKTREE_ADD_DWIM_ORPHAN_INFER_TEXT \
_("No possible source branch, inferring '--orphan'")
@@ -66,6 +72,7 @@
static const char * const git_worktree_usage[] = {
BUILTIN_WORKTREE_ADD_USAGE,
+ BUILTIN_WORKTREE_DESCRIBE_USAGE,
BUILTIN_WORKTREE_LIST_USAGE,
BUILTIN_WORKTREE_LOCK_USAGE,
BUILTIN_WORKTREE_MOVE_USAGE,
@@ -116,6 +123,11 @@ static const char * const git_worktree_unlock_usage[] = {
NULL
};
+static const char * const git_worktree_describe_usage[] = {
+ BUILTIN_WORKTREE_DESCRIBE_USAGE,
+ NULL
+};
+
struct add_opts {
int force;
int detach;
@@ -124,6 +136,7 @@ struct add_opts {
int orphan;
int relative_paths;
const char *keep_locked;
+ const char *description;
};
static int show_only;
@@ -131,6 +144,9 @@ static int verbose;
static int guess_remote;
static int use_relative_paths;
static timestamp_t expire;
+static int show_created = -1;
+static int show_updated = -1;
+static int show_description;
static int git_worktree_config(const char *var, const char *value,
const struct config_context *ctx, void *cb)
@@ -544,6 +560,16 @@ static int add_worktree(const char *path, const
char *refname,
strbuf_addf(&sb, "%s/commondir", sb_repo.buf);
write_file(sb.buf, "../..");
+ strbuf_reset(&sb);
+ strbuf_addf(&sb, "%s/created", sb_repo.buf);
+ write_file(sb.buf, "%"PRItime, (timestamp_t) time(NULL));
+
+ if (opts->description && *opts->description) {
+ strbuf_reset(&sb);
+ strbuf_addf(&sb, "%s/description", sb_repo.buf);
+ write_file(sb.buf, "%s", opts->description);
+ }
+
/*
* Set up the ref store of the worktree and create the HEAD reference.
*/
@@ -815,6 +841,8 @@ static int add(int ac, const char **av, const char *prefix,
OPT_BOOL(0, "lock", &keep_locked, N_("keep the new working tree locked")),
OPT_STRING(0, "reason", &lock_reason, N_("string"),
N_("reason for locking")),
+ OPT_STRING(0, "description", &opts.description, N_("string"),
+ N_("attach a free-form description to the worktree")),
OPT__QUIET(&opts.quiet, N_("suppress progress reporting")),
OPT_PASSTHRU(0, "track", &opt_track, NULL,
N_("set up tracking mode (see git-branch(1))"),
@@ -963,6 +991,8 @@ static int add(int ac, const char **av, const char *prefix,
static void show_worktree_porcelain(struct worktree *wt, int line_terminator)
{
const char *reason;
+ const char *description;
+ timestamp_t created;
printf("worktree %s%c", wt->path, line_terminator);
if (wt->is_bare)
@@ -975,6 +1005,26 @@ static void show_worktree_porcelain(struct
worktree *wt, int line_terminator)
printf("branch %s%c", wt->head_ref, line_terminator);
}
+ created = worktree_created_at(wt);
+ if (created)
+ printf("created %s%c",
+ show_date(created, 0, DATE_MODE(ISO8601_STRICT)),
+ line_terminator);
+
+ {
+ timestamp_t updated = worktree_updated_at(wt);
+ if (updated)
+ printf("updated %s%c",
+ show_date(updated, 0, DATE_MODE(ISO8601_STRICT)),
+ line_terminator);
+ }
+
+ description = worktree_description(wt);
+ if (description && *description) {
+ fputs("description ", stdout);
+ write_name_quoted(description, stdout, line_terminator);
+ }
+
reason = worktree_lock_reason(wt);
if (reason) {
fputs("locked", stdout);
@@ -1034,6 +1084,32 @@ static void show_worktree(struct worktree *wt,
struct worktree_display *display,
else if (reason)
strbuf_addstr(&sb, " prunable");
+ if (show_created > 0 || verbose) {
+ timestamp_t created = worktree_created_at(wt);
+ struct date_mode mode = { .type = DATE_ISO8601, .local = 1 };
+ if (created)
+ strbuf_addf(&sb, "\n\tcreated: %s",
+ show_date(created, 0, mode));
+ else if (show_created > 0 && !is_main_worktree(wt))
+ strbuf_addstr(&sb, "\n\tcreated: unknown");
+ }
+
+ if (show_updated > 0 || verbose) {
+ timestamp_t updated = worktree_updated_at(wt);
+ struct date_mode mode = { .type = DATE_ISO8601, .local = 1 };
+ if (updated)
+ strbuf_addf(&sb, "\n\tupdated: %s",
+ show_date(updated, 0, mode));
+ else if (show_updated > 0 && !is_main_worktree(wt))
+ strbuf_addstr(&sb, "\n\tupdated: unknown");
+ }
+
+ if (show_description || verbose) {
+ const char *description = worktree_description(wt);
+ if (description && *description)
+ strbuf_addf(&sb, "\n\tdescription: %s", description);
+ }
+
printf("%s\n", sb.buf);
strbuf_release(&sb);
}
@@ -1068,6 +1144,48 @@ static int pathcmp(const void *a_, const void *b_)
return fspathcmp((*a)->path, (*b)->path);
}
+static int createdcmp(const void *a_, const void *b_)
+{
+ struct worktree *const *a = a_;
+ struct worktree *const *b = b_;
+ timestamp_t ta = worktree_created_at(*a);
+ timestamp_t tb = worktree_created_at(*b);
+
+ /* Worktrees without a recorded timestamp (legacy) sort after those
with one. */
+ if (!ta && !tb)
+ return fspathcmp((*a)->path, (*b)->path);
+ if (!ta)
+ return 1;
+ if (!tb)
+ return -1;
+ if (ta < tb)
+ return -1;
+ if (ta > tb)
+ return 1;
+ return 0;
+}
+
+static int updatedcmp(const void *a_, const void *b_)
+{
+ struct worktree *const *a = a_;
+ struct worktree *const *b = b_;
+ timestamp_t ta = worktree_updated_at(*a);
+ timestamp_t tb = worktree_updated_at(*b);
+
+ /* Worktrees whose HEAD mtime can't be read sort after those that can. */
+ if (!ta && !tb)
+ return fspathcmp((*a)->path, (*b)->path);
+ if (!ta)
+ return 1;
+ if (!tb)
+ return -1;
+ if (ta < tb)
+ return -1;
+ if (ta > tb)
+ return 1;
+ return 0;
+}
+
static void pathsort(struct worktree **wt)
{
int n = 0;
@@ -1078,11 +1196,45 @@ static void pathsort(struct worktree **wt)
QSORT(wt, n, pathcmp);
}
+static int sort_worktrees(struct worktree **wt, const char *key)
+{
+ int n = 0, reverse = 0;
+ struct worktree **p = wt;
+ int (*cmp)(const void *, const void *);
+
+ if (*key == '-') {
+ reverse = 1;
+ key++;
+ }
+ if (!strcmp(key, "path"))
+ cmp = pathcmp;
+ else if (!strcmp(key, "created"))
+ cmp = createdcmp;
+ else if (!strcmp(key, "updated"))
+ cmp = updatedcmp;
+ else
+ return -1;
+
+ while (*p++)
+ n++;
+ QSORT(wt, n, cmp);
+ if (reverse) {
+ int i;
+ for (i = 0; i < n / 2; i++) {
+ struct worktree *tmp = wt[i];
+ wt[i] = wt[n - 1 - i];
+ wt[n - 1 - i] = tmp;
+ }
+ }
+ return 0;
+}
+
static int list(int ac, const char **av, const char *prefix,
struct repository *repo UNUSED)
{
int porcelain = 0;
int line_terminator = '\n';
+ const char *sort_key = NULL;
struct option options[] = {
OPT_BOOL(0, "porcelain", &porcelain, N_("machine-readable output")),
@@ -1091,6 +1243,14 @@ static int list(int ac, const char **av, const
char *prefix,
N_("add 'prunable' annotation to missing worktrees older than <time>")),
OPT_SET_INT('z', NULL, &line_terminator,
N_("terminate records with a NUL character"), '\0'),
+ OPT_BOOL(0, "show-created", &show_created,
+ N_("show worktree creation timestamps")),
+ OPT_BOOL(0, "show-updated", &show_updated,
+ N_("show worktree last-updated timestamps")),
+ OPT_BOOL(0, "show-description", &show_description,
+ N_("show worktree descriptions")),
+ OPT_STRING(0, "sort", &sort_key, N_("key"),
+ N_("sort worktrees by key (path, created, updated); prefix with -
to reverse")),
OPT_END()
};
@@ -1107,8 +1267,27 @@ static int list(int ac, const char **av, const
char *prefix,
int path_maxwidth = 0, abbrev = DEFAULT_ABBREV, i;
struct worktree_display *display = NULL;
- /* sort worktrees by path but keep main worktree at top */
- pathsort(worktrees + 1);
+ /* sort worktrees but keep main worktree at top */
+ if (sort_key) {
+ const char *bare_key = sort_key;
+ if (*bare_key == '-')
+ bare_key++;
+ /*
+ * Sorting by a timestamp without showing it would
+ * leave the user guessing why the order is what it
+ * is, so opt in the matching display by default.
+ * An explicit --show-* / --no-show-* still wins.
+ */
+ if (!strcmp(bare_key, "created") && show_created < 0)
+ show_created = 1;
+ else if (!strcmp(bare_key, "updated") && show_updated < 0)
+ show_updated = 1;
+
+ if (sort_worktrees(worktrees + 1, sort_key))
+ die(_("unknown sort key '%s'"), sort_key);
+ } else {
+ pathsort(worktrees + 1);
+ }
if (!porcelain)
measure_widths(worktrees, &abbrev,
@@ -1200,6 +1379,32 @@ static int unlock_worktree(int ac, const char
**av, const char *prefix,
return ret;
}
+static int describe_worktree(int ac, const char **av, const char *prefix,
+ struct repository *repo UNUSED)
+{
+ struct option options[] = {
+ OPT_END()
+ };
+ struct worktree **worktrees, *wt;
+ int ret;
+
+ ac = parse_options(ac, av, prefix, options, git_worktree_describe_usage, 0);
+ if (ac < 1 || ac > 2)
+ usage_with_options(git_worktree_describe_usage, options);
+
+ worktrees = get_worktrees();
+ wt = find_worktree(worktrees, prefix, av[0]);
+ if (!wt)
+ die(_("'%s' is not a working tree"), av[0]);
+ if (is_main_worktree(wt))
+ die(_("The main working tree cannot be described"));
+
+ ret = set_worktree_description(wt, ac == 2 ? av[1] : NULL);
+
+ free_worktrees(worktrees);
+ return ret;
+}
+
static void validate_no_submodules(const struct worktree *wt)
{
struct index_state istate = INDEX_STATE_INIT(the_repository);
@@ -1469,6 +1674,7 @@ int cmd_worktree(int ac,
parse_opt_subcommand_fn *fn = NULL;
struct option options[] = {
OPT_SUBCOMMAND("add", &fn, add),
+ OPT_SUBCOMMAND("describe", &fn, describe_worktree),
OPT_SUBCOMMAND("prune", &fn, prune),
OPT_SUBCOMMAND("list", &fn, list),
OPT_SUBCOMMAND("lock", &fn, lock_worktree),
diff --git t/meson.build t/meson.build
index 2af8d01279..7b6e8435d7 100644
--- t/meson.build
+++ t/meson.build
@@ -308,6 +308,7 @@ integration_tests = [
't2405-worktree-submodule.sh',
't2406-worktree-repair.sh',
't2407-worktree-heads.sh',
+ 't2410-worktree-metadata.sh',
't2500-untracked-overwriting.sh',
't2501-cwd-empty.sh',
't3000-ls-files-others.sh',
diff --git t/t2402-worktree-list.sh t/t2402-worktree-list.sh
index e0c6abd2f5..fb1f4b1d3c 100755
--- t/t2402-worktree-list.sh
+++ t/t2402-worktree-list.sh
@@ -71,7 +71,8 @@ test_expect_success '"list" all worktrees --porcelain' '
echo "HEAD $(git rev-parse HEAD)" >>expect &&
echo "detached" >>expect &&
echo >>expect &&
- git worktree list --porcelain >actual &&
+ git worktree list --porcelain >actual.raw &&
+ grep -Ev "^(created|updated) " actual.raw >actual &&
test_cmp expect actual
'
@@ -86,7 +87,7 @@ test_expect_success '"list" all worktrees --porcelain -z' '
"$(git -C here rev-parse --show-toplevel)" \
"$(git rev-parse HEAD)" >>expect &&
git worktree list --porcelain -z >_actual &&
- nul_to_q <_actual >actual &&
+ nul_to_q <_actual | tr Q "\n" | grep -Ev "^(created|updated) " | tr
"\n" Q >actual &&
test_cmp expect actual
'
@@ -220,7 +221,7 @@ test_expect_success '"list" all worktrees from bare main' '
'
test_expect_success '"list" all worktrees --porcelain from bare main' '
- test_when_finished "rm -rf there actual expect && git -C bare1
worktree prune" &&
+ test_when_finished "rm -rf there actual actual.raw expect && git -C
bare1 worktree prune" &&
git -C bare1 worktree add --detach ../there main &&
echo "worktree $(pwd)/bare1" >expect &&
echo "bare" >>expect &&
@@ -229,7 +230,8 @@ test_expect_success '"list" all worktrees
--porcelain from bare main' '
echo "HEAD $(git -C there rev-parse HEAD)" >>expect &&
echo "detached" >>expect &&
echo >>expect &&
- git -C bare1 worktree list --porcelain >actual &&
+ git -C bare1 worktree list --porcelain >actual.raw &&
+ grep -Ev "^(created|updated) " actual.raw >actual &&
test_cmp expect actual
'
diff --git t/t2410-worktree-metadata.sh t/t2410-worktree-metadata.sh
new file mode 100755
index 0000000000..e1ecb1c1bf
--- /dev/null
+++ t/t2410-worktree-metadata.sh
@@ -0,0 +1,245 @@
+#!/bin/sh
+
+test_description='git worktree creation timestamp and description metadata'
+
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+ test_commit init
+'
+
+test_expect_success 'add writes created file' '
+ test_when_finished "git worktree remove -f wt1 && git worktree prune" &&
+ git worktree add wt1 &&
+ test_path_is_file .git/worktrees/wt1/created &&
+ # contents should be a positive integer (unix timestamp)
+ created=$(cat .git/worktrees/wt1/created) &&
+ test "$created" -gt 0
+'
+
+test_expect_success 'add --description writes description file' '
+ test_when_finished "git worktree remove -f wt2 && git worktree prune" &&
+ git worktree add --description "investigating bug" wt2 &&
+ test_path_is_file .git/worktrees/wt2/description &&
+ echo "investigating bug" >expect &&
+ test_cmp expect .git/worktrees/wt2/description
+'
+
+test_expect_success 'add without --description does not create
description file' '
+ test_when_finished "git worktree remove -f wt3 && git worktree prune" &&
+ git worktree add wt3 &&
+ test_path_is_missing .git/worktrees/wt3/description
+'
+
+test_expect_success 'describe sets a description on an existing worktree' '
+ test_when_finished "git worktree remove -f wt4 && git worktree prune" &&
+ git worktree add wt4 &&
+ git worktree describe wt4 "later description" &&
+ echo "later description" >expect &&
+ test_cmp expect .git/worktrees/wt4/description
+'
+
+test_expect_success 'describe replaces an existing description' '
+ test_when_finished "git worktree remove -f wt5 && git worktree prune" &&
+ git worktree add --description "old" wt5 &&
+ git worktree describe wt5 "new" &&
+ echo "new" >expect &&
+ test_cmp expect .git/worktrees/wt5/description
+'
+
+test_expect_success 'describe with no text clears the description' '
+ test_when_finished "git worktree remove -f wt6 && git worktree prune" &&
+ git worktree add --description "to delete" wt6 &&
+ test_path_is_file .git/worktrees/wt6/description &&
+ git worktree describe wt6 &&
+ test_path_is_missing .git/worktrees/wt6/description
+'
+
+test_expect_success 'describe refuses to operate on the main worktree' '
+ test_must_fail git worktree describe . "should fail" 2>err &&
+ grep -i "main working tree" err
+'
+
+test_expect_success 'list --show-description displays description in
human output' '
+ test_when_finished "git worktree remove -f wt7 && git worktree prune" &&
+ git worktree add --description "release branch" wt7 &&
+ git worktree list --show-description >actual &&
+ grep "description: release branch" actual
+'
+
+test_expect_success 'list --show-created displays created timestamp' '
+ test_when_finished "git worktree remove -f wt8 && git worktree prune" &&
+ git worktree add wt8 &&
+ git worktree list --show-created >actual &&
+ grep "created: " actual
+'
+
+test_expect_success 'list --show-created shows unknown for legacy worktrees' '
+ test_when_finished "git worktree remove -f wt9 && git worktree prune" &&
+ git worktree add wt9 &&
+ rm .git/worktrees/wt9/created &&
+ git worktree list --show-created >actual &&
+ grep "created: unknown" actual
+'
+
+test_expect_success 'list --show-updated displays updated timestamp' '
+ test_when_finished "git worktree remove -f wt8u && git worktree prune" &&
+ git worktree add wt8u &&
+ git worktree list --show-updated >actual &&
+ grep "updated: " actual
+'
+
+test_expect_success 'list --porcelain always includes created,
updated, and description' '
+ test_when_finished "git worktree remove -f wtp && git worktree prune" &&
+ git worktree add --description "porcelain test" wtp &&
+ git worktree list --porcelain >actual &&
+ grep "^created " actual &&
+ grep "^updated " actual &&
+ grep "^description porcelain test" actual
+'
+
+test_expect_success 'list --sort=created orders by creation time' '
+ test_when_finished "git worktree remove -f a && git worktree remove
-f b && git worktree remove -f c && git worktree prune" &&
+ git worktree add a &&
+ git worktree add b &&
+ git worktree add c &&
+ echo 1000 >.git/worktrees/a/created &&
+ echo 2000 >.git/worktrees/b/created &&
+ echo 3000 >.git/worktrees/c/created &&
+ git worktree list --sort=created --porcelain >actual &&
+ grep "^worktree " actual | sed -n "2,4p" >linked &&
+ awk "NR==1" linked | grep -q "/a$" &&
+ awk "NR==2" linked | grep -q "/b$" &&
+ awk "NR==3" linked | grep -q "/c$"
+'
+
+test_expect_success 'list --sort=-created reverses order' '
+ test_when_finished "git worktree remove -f a && git worktree remove
-f b && git worktree remove -f c && git worktree prune" &&
+ git worktree add a &&
+ git worktree add b &&
+ git worktree add c &&
+ echo 1000 >.git/worktrees/a/created &&
+ echo 2000 >.git/worktrees/b/created &&
+ echo 3000 >.git/worktrees/c/created &&
+ git worktree list --sort=-created --porcelain >actual &&
+ grep "^worktree " actual | sed -n "2,4p" >linked &&
+ awk "NR==1" linked | grep -q "/c$" &&
+ awk "NR==2" linked | grep -q "/b$" &&
+ awk "NR==3" linked | grep -q "/a$"
+'
+
+test_expect_success 'list --sort=created places legacy worktrees last' '
+ test_when_finished "git worktree remove -f early && git worktree
remove -f legacy && git worktree prune" &&
+ git worktree add early &&
+ echo 1000 >.git/worktrees/early/created &&
+ git worktree add legacy &&
+ rm .git/worktrees/legacy/created &&
+ git worktree list --sort=created --porcelain >actual &&
+ grep "^worktree " actual | sed -n "2,3p" >linked &&
+ awk "NR==1" linked | grep -q "/early$" &&
+ awk "NR==2" linked | grep -q "/legacy$"
+'
+
+test_expect_success 'list --sort=updated orders by HEAD mtime' '
+ test_when_finished "git worktree remove -f u1 && git worktree remove
-f u2 && git worktree remove -f u3 && git worktree prune" &&
+ git worktree add u1 &&
+ git worktree add u2 &&
+ git worktree add u3 &&
+ # Force a known ordering: u2 oldest, u1 middle, u3 newest.
+ test-tool chmtime =1000 .git/worktrees/u2/HEAD &&
+ test-tool chmtime =2000 .git/worktrees/u1/HEAD &&
+ test-tool chmtime =3000 .git/worktrees/u3/HEAD &&
+ git worktree list --sort=updated --porcelain >actual &&
+ grep "^worktree " actual | sed -n "2,4p" >linked &&
+ awk "NR==1" linked | grep -q "/u2$" &&
+ awk "NR==2" linked | grep -q "/u1$" &&
+ awk "NR==3" linked | grep -q "/u3$"
+'
+
+test_expect_success 'list --sort=-updated reverses order' '
+ test_when_finished "git worktree remove -f u1 && git worktree remove
-f u2 && git worktree remove -f u3 && git worktree prune" &&
+ git worktree add u1 &&
+ git worktree add u2 &&
+ git worktree add u3 &&
+ test-tool chmtime =1000 .git/worktrees/u2/HEAD &&
+ test-tool chmtime =2000 .git/worktrees/u1/HEAD &&
+ test-tool chmtime =3000 .git/worktrees/u3/HEAD &&
+ git worktree list --sort=-updated --porcelain >actual &&
+ grep "^worktree " actual | sed -n "2,4p" >linked &&
+ awk "NR==1" linked | grep -q "/u3$" &&
+ awk "NR==2" linked | grep -q "/u1$" &&
+ awk "NR==3" linked | grep -q "/u2$"
+'
+
+test_expect_success 'list --sort=created auto-shows created timestamp' '
+ test_when_finished "git worktree remove -f autoc && git worktree prune" &&
+ git worktree add autoc &&
+ git worktree list --sort=created >actual &&
+ grep "created: " actual
+'
+
+test_expect_success 'list --sort=-created auto-shows created timestamp' '
+ test_when_finished "git worktree remove -f autocr && git worktree prune" &&
+ git worktree add autocr &&
+ git worktree list --sort=-created >actual &&
+ grep "created: " actual
+'
+
+test_expect_success 'list --sort=updated auto-shows updated timestamp' '
+ test_when_finished "git worktree remove -f autou && git worktree prune" &&
+ git worktree add autou &&
+ git worktree list --sort=updated >actual &&
+ grep "updated: " actual
+'
+
+test_expect_success 'list --sort=-updated auto-shows updated timestamp' '
+ test_when_finished "git worktree remove -f autour && git worktree prune" &&
+ git worktree add autour &&
+ git worktree list --sort=-updated >actual &&
+ grep "updated: " actual
+'
+
+test_expect_success 'list --sort=path does not auto-show timestamps' '
+ test_when_finished "git worktree remove -f autop && git worktree prune" &&
+ git worktree add autop &&
+ git worktree list --sort=path >actual &&
+ ! grep "created: " actual &&
+ ! grep "updated: " actual
+'
+
+test_expect_success 'list --sort with unknown key fails' '
+ test_must_fail git worktree list --sort=bogus 2>err &&
+ grep -i "unknown sort key" err
+'
+
+test_expect_success 'list --sort=updated --no-show-updated suppresses
auto-show' '
+ test_when_finished "git worktree remove -f noshowu && git worktree prune" &&
+ git worktree add noshowu &&
+ git worktree list --sort=updated --no-show-updated >actual &&
+ ! grep "updated: " actual
+'
+
+test_expect_success 'list --sort=created --no-show-created suppresses
auto-show' '
+ test_when_finished "git worktree remove -f noshowc && git worktree prune" &&
+ git worktree add noshowc &&
+ git worktree list --sort=created --no-show-created >actual &&
+ ! grep "created: " actual
+'
+
+test_expect_success 'list --show-updated formats human output in
local timezone' '
+ test_when_finished "git worktree remove -f tz && git worktree prune" &&
+ git worktree add tz &&
+ # Pin HEAD mtime to a fixed unix time outside any DST transition
+ # so the rendered offset is deterministic in PST8PDT (-0700 in July).
+ test-tool chmtime =1500000000 .git/worktrees/tz/HEAD &&
+ TZ=PST8PDT git worktree list --show-updated >human &&
+ grep "updated: 2017-07-13 19:40:00 -0700" human &&
+ # Porcelain stays in UTC ISO-8601 strict form regardless of TZ.
+ TZ=PST8PDT git worktree list --porcelain >porcelain &&
+ grep "^updated 2017-07-14T02:40:00Z$" porcelain
+'
+
+test_done
diff --git worktree.c worktree.c
index 97eddc3916..4b019a532b 100644
--- worktree.c
+++ worktree.c
@@ -14,6 +14,8 @@
#include "dir.h"
#include "wt-status.h"
#include "config.h"
+#include "date.h"
+#include "wrapper.h"
void free_worktree(struct worktree *worktree)
{
@@ -24,6 +26,7 @@ void free_worktree(struct worktree *worktree)
free(worktree->head_ref);
free(worktree->lock_reason);
free(worktree->prune_reason);
+ free(worktree->description);
free(worktree);
}
@@ -324,6 +327,100 @@ const char *worktree_lock_reason(struct worktree *wt)
return wt->lock_reason;
}
+timestamp_t worktree_created_at(struct worktree *wt)
+{
+ if (is_main_worktree(wt))
+ return 0;
+
+ if (!wt->created_at_valid) {
+ struct strbuf path = STRBUF_INIT;
+ struct strbuf buf = STRBUF_INIT;
+
+ strbuf_addstr(&path, worktree_git_path(wt, "created"));
+ if (file_exists(path.buf) &&
+ strbuf_read_file(&buf, path.buf, 0) >= 0) {
+ char *end;
+ timestamp_t t;
+ strbuf_trim(&buf);
+ t = parse_timestamp(buf.buf, &end, 10);
+ if (end != buf.buf && *end == '\0')
+ wt->created_at = t;
+ }
+ wt->created_at_valid = 1;
+ strbuf_release(&path);
+ strbuf_release(&buf);
+ }
+
+ return wt->created_at;
+}
+
+timestamp_t worktree_updated_at(struct worktree *wt)
+{
+ struct stat st;
+ char *git_dir;
+ char *head_path;
+ timestamp_t result = 0;
+
+ if (is_main_worktree(wt))
+ return 0;
+
+ git_dir = get_worktree_git_dir(wt);
+ head_path = xstrfmt("%s/HEAD", git_dir);
+ if (!stat(head_path, &st))
+ result = (timestamp_t) st.st_mtime;
+ free(head_path);
+ free(git_dir);
+ return result;
+}
+
+const char *worktree_description(struct worktree *wt)
+{
+ if (is_main_worktree(wt))
+ return NULL;
+
+ if (!wt->description_valid) {
+ struct strbuf path = STRBUF_INIT;
+
+ strbuf_addstr(&path, worktree_git_path(wt, "description"));
+ if (file_exists(path.buf)) {
+ struct strbuf description = STRBUF_INIT;
+ if (strbuf_read_file(&description, path.buf, 0) < 0)
+ die_errno(_("failed to read '%s'"), path.buf);
+ strbuf_trim_trailing_newline(&description);
+ wt->description = strbuf_detach(&description, NULL);
+ } else
+ wt->description = NULL;
+ wt->description_valid = 1;
+ strbuf_release(&path);
+ }
+
+ return wt->description;
+}
+
+int set_worktree_description(struct worktree *wt, const char *text)
+{
+ char *path;
+ int ret = 0;
+
+ if (is_main_worktree(wt))
+ return error(_("cannot set description on the main worktree"));
+
+ path = repo_common_path(wt->repo, "worktrees/%s/description", wt->id);
+ if (!text || !*text) {
+ if (file_exists(path) && unlink(path))
+ ret = error_errno(_("failed to remove '%s'"), path);
+ } else {
+ write_file(path, "%s", text);
+ }
+
+ /* invalidate cache so a follow-up worktree_description() re-reads */
+ FREE_AND_NULL(wt->description);
+ wt->description_valid = 0;
+
+ free(path);
+ return ret;
+}
+
const char *worktree_prune_reason(struct worktree *wt, timestamp_t expire)
{
struct strbuf reason = STRBUF_INIT;
diff --git worktree.h worktree.h
index 1075409f9a..2568830237 100644
--- worktree.h
+++ worktree.h
@@ -13,12 +13,16 @@ struct worktree {
char *head_ref; /* NULL if HEAD is broken or detached */
char *lock_reason; /* private - use worktree_lock_reason */
char *prune_reason; /* private - use worktree_prune_reason */
+ char *description; /* private - use worktree_description */
struct object_id head_oid;
+ timestamp_t created_at; /* private - use worktree_created_at; 0 if unknown */
int is_detached;
int is_bare;
int is_current; /* does `path` match `repo->worktree` */
int lock_reason_valid; /* private */
int prune_reason_valid; /* private */
+ int description_valid; /* private */
+ int created_at_valid; /* private */
};
/*
@@ -96,6 +100,34 @@ int is_main_worktree(const struct worktree *wt);
*/
const char *worktree_lock_reason(struct worktree *wt);
+/*
+ * Return the worktree's recorded creation timestamp, or 0 if no timestamp
+ * was recorded (e.g. a worktree created before this metadata existed, or
+ * the main worktree which never carries the file).
+ */
+timestamp_t worktree_created_at(struct worktree *wt);
+
+/*
+ * Return the modification time of the worktree's HEAD file as an
+ * approximation of "when was this worktree last touched by Git" (checkout,
+ * commit, reset, rebase, etc.). Returns 0 for the main worktree, and 0 if
+ * HEAD cannot be stat'd.
+ */
+timestamp_t worktree_updated_at(struct worktree *wt);
+
+/*
+ * Return the user-supplied description for the given worktree, or NULL
+ * if none was set.
+ */
+const char *worktree_description(struct worktree *wt);
+
+/*
+ * Write or replace the worktree's description. Pass NULL or "" to delete
+ * the description. Returns 0 on success, -1 on failure. Not valid for the
+ * main worktree.
+ */
+int set_worktree_description(struct worktree *wt, const char *text);
+
/*
* Return the reason string if the given worktree should be pruned, otherwise
* NULL if it should not be pruned. `expire` defines a grace period to prune
On Fri, Jun 5, 2026 at 9:57 AM Chris Torek <chris.torek@gmail.com> wrote:
>
> On Fri, Jun 5, 2026 at 8:31 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
> > Isn't "what is the worktree for" a property of the branch that's checked
> > out, not the worktree itself?
>
> I don't think it is.
>
> A lot of things within Git have, shall way say, "less than optimal"
> names, with "branch" (with at least three different meanings),
> "HEAD", and "index" being examples of this. (This is just an
> observation, not a complaint: we know from studies that
> oddities in names don't matter that much after a bit of usage
> of some system. They're just minor stumbling blocks when
> getting started.)
>
> Work-tree or working tree is not one of them, though. It's
> concise and pointed: a working tree is where you do work.
>
> As such, the *purpose* of a working tree is exactly as general
> as the purpose of doing work! That's a wide-open set.
>
> Git's internal constraint, of requiring each working tree that
> is using a branch name to have a unique-to-that-tree branch
> name, is a property specific to branch names, not to branching
> in general (an example of the ambiguity of "branch" here).
> And of course, as you note, any working tree can be on
> a detached HEAD.
>
> Exactly what properties any given working tree should
> have, and the weird entanglement Git has between the
> "primary" working tree (the one created by any non-bare
> clone) and all "secondary" working trees, is a mere (ahem)
> matter of implementation. Descriptions, creation times,
> modification times, etc., are all potentially useful.
>
> I think, had Git initially made all repositories effectively
> bare, with separate working trees added later, this might
> all be a little clearer, but of course that ship sailed,
> crossed *all* the oceans, sank, was refloated and refitted,
> and sailed for another decade already. :-)
>
> Chris
--
Norbert Kiesel | Staff Software Engineer | Credit Karma
norbert.kiesel@creditkarma.com | www.creditkarma.com
This email may contain confidential and privileged information. Any
review, use, distribution, or disclosure by anyone other than the
intended recipient(s) is prohibited. If you are not the intended
recipient, please contact the sender by reply email and delete all
copies of this message.
^ permalink raw reply related
* Re: [PATCH 11/16] odb/source-packed: wire up `count_objects()` callback
From: Karthik Nayak @ 2026-06-08 16:12 UTC (permalink / raw)
To: Patrick Steinhardt, git
In-Reply-To: <20260604-pks-odb-source-packed-v1-11-2e7ab31b4b5c@pks.im>
[-- Attachment #1: Type: text/plain, Size: 1768 bytes --]
Patrick Steinhardt <ps@pks.im> writes:
[snip]
> diff --git a/odb/source-packed.c b/odb/source-packed.c
> index a61c809c8c..013d8a50f8 100644
> --- a/odb/source-packed.c
> +++ b/odb/source-packed.c
> @@ -338,6 +338,39 @@ static int odb_source_packed_for_each_object(struct odb_source *source,
> return ret;
> }
>
> +static int odb_source_packed_count_objects(struct odb_source *source,
> + enum odb_count_objects_flags flags UNUSED,
> + unsigned long *out)
> +{
> + struct odb_source_packed *packed = odb_source_packed_downcast(source);
> + struct packfile_list_entry *e;
> + struct multi_pack_index *m;
> + unsigned long count = 0;
> + int ret;
> +
> + m = get_multi_pack_index(&packed->files->base);
> + if (m)
> + count += m->num_objects + m->num_objects_in_base;
> +
> + for (e = packfile_store_get_packs(packed); e; e = e->next) {
> + if (e->pack->multi_pack_index)
> + continue;
> + if (open_pack_index(e->pack)) {
> + ret = -1;
> + goto out;
> + }
> +
> + count += e->pack->num_objects;
> + }
> +
> + *out = count;
> + ret = 0;
> +
> +out:
> + return ret;
> +}
> +
> +
Nit: extra newline.
> void (*report_garbage)(unsigned seen_bits, const char *path);
>
> static void report_helper(const struct string_list *list,
> @@ -549,6 +582,7 @@ struct odb_source_packed *odb_source_packed_new(struct odb_source_files *parent)
> packed->base.read_object_info = odb_source_packed_read_object_info;
> packed->base.read_object_stream = odb_source_packed_read_object_stream;
> packed->base.for_each_object = odb_source_packed_for_each_object;
> + packed->base.count_objects = odb_source_packed_count_objects;
>
> if (!is_absolute_path(parent->base.path))
> chdir_notify_register(NULL, odb_source_packed_reparent, packed);
[snip]
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
* Re: [PATCH 07/16] packfile: use higher-level interface to implement `has_object_pack()`
From: Karthik Nayak @ 2026-06-08 16:07 UTC (permalink / raw)
To: Patrick Steinhardt, git
In-Reply-To: <20260604-pks-odb-source-packed-v1-7-2e7ab31b4b5c@pks.im>
[-- Attachment #1: Type: text/plain, Size: 1705 bytes --]
Patrick Steinhardt <ps@pks.im> writes:
> In `has_object_pack()` we're checking whether a specific object exists
> as part of a packfile. This is done by calling the low-level function
> `find_pack_entry()`, but this function will eventually be moved into
> "odb/source-packed.c" and made file-local.
>
> Refactor the code to use `packfile_store_read_object_info()` instead.
> This refactoring is functionally equivalent as that function will call
> `find_pack_entry()` itself and then return immediately when it ain't got
> no object info pointer as parameter.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> packfile.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/packfile.c b/packfile.c
> index 902b7f70f2..3ee71d7f71 100644
> --- a/packfile.c
> +++ b/packfile.c
> @@ -2132,14 +2132,12 @@ struct packed_git **packfile_store_get_kept_pack_cache(struct odb_source_packed
> int has_object_pack(struct repository *r, const struct object_id *oid)
> {
> struct odb_source *source;
> - struct pack_entry e;
>
> odb_prepare_alternates(r->objects);
> for (source = r->objects->sources; source; source = source->next) {
> struct odb_source_files *files = odb_source_files_downcast(source);
> - int ret = find_pack_entry(files->packed, oid, &e);
> - if (ret)
> - return ret;
> + if (!packfile_store_read_object_info(files->packed, oid, NULL, 0))
> + return 1;
> }
>
I was wondering if there would be an added cost of actually obtaining
the object-info. Seems like there isn't, because we pass `NULL` for the
`struct object_info *oi`, which means it will return before reading the
object info.
> return 0;
>
> --
> 2.54.0.1064.gd145956f57.dirty
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
* Re: [PATCH] describe: limit default ref iteration to tags
From: Tamir Duberstein @ 2026-06-08 15:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King, Patrick Steinhardt
In-Reply-To: <xmqqecihyzse.fsf@gitster.g>
On Mon, Jun 8, 2026 at 5:36 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Tamir Duberstein <tamird@gmail.com> writes:
>
> [jc: Removing Shawn from CC who passed away quite a while ago, RIP].
>
> > Unless --all is given, get_name() rejects every ref outside refs/tags/.
> > The rejection happens only after the ref backend has enumerated the ref,
> > so repositories with many other refs spend most of a simple describe
> > invocation visiting refs which cannot affect its result.
> > ...
> > Both revisions were built with -O3, -mcpu=native, and ThinLTO using
> > Apple clang 21.0.0 on macOS 26.5. The machine was a MacBook Pro
> > (Mac16,6) with a 16-core Apple M4 Max (12 performance and four
> > efficiency cores) and 128 GB RAM.
> >
> > Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> > ---
> > builtin/describe.c | 3 +++
> > t/perf/p6100-describe.sh | 20 ++++++++++++++++++++
> > 2 files changed, 23 insertions(+)
>
> Interesting. How would this relate to and work well with
> <20260601233727.43558-1-jacob.e.keller@intel.com>?
They are orthogonal. That patch changes the argument construction
inside the `contains` block, which invokes `cmd_name_rev()` and
returns. This patch changes the ref iterator used after that block, so
it only affects the ordinary, non-`--contains` path.
>
> > +test_lazy_prereq PERF_REFFILES '
> > + test "$(git rev-parse --show-ref-format)" = files
> > +'
> > +
> > +ref_count=10000
> > +
> > # clear out old tags and give us a known state
> > test_expect_success 'set up tags' '
> > git for-each-ref --format="delete %(refname)" refs/tags >to-delete &&
> > @@ -27,4 +33,18 @@ test_perf 'describe HEAD with one tag' '
> > git describe --match=new HEAD
> > '
> >
> > +test_expect_success PERF_REFFILES 'set up many unrelated refs' '
> > + git tag -m tip tip HEAD &&
> > + for i in $(test_seq $ref_count)
> > + do
> > + printf "create refs/heads/describe-perf/%05d HEAD\n" $i ||
> > + return 1
> > + done >instructions &&
> > + git update-ref --stdin <instructions
> > +'
> > +
> > +test_perf 'describe exact tag with many loose refs' --prereq PERF_REFFILES '
> > + git describe --exact-match HEAD
> > +'
> > +
>
> Is there a strong reason to guard this new test behind
> `PERF_REFFILES`?
>
> Even though the penalty of enumerating 10,000 unrelated loose
> references may be most pronounced in the `files` backend, skipping
> unnecessary reference enumeration is an architectural win for other
> backends (like `reftable` or a fully packed repository) as well.
>
> If we drop `PERF_REFFILES` and retitle the test to "describe exact
> tag with many unrelated refs", we could run it unconditionally to
> benchmark the improvement across all storage formats.
Yeah, there's no good reason - and Patrick made the same observation.
In v2 I will remove the prerequisite and rename the case to refer to
unrelated rather than loose refs.
^ permalink raw reply
* Re: [PATCH] describe: limit default ref iteration to tags
From: Tamir Duberstein @ 2026-06-08 15:46 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Shawn O. Pearce, Junio C Hamano, Jeff King
In-Reply-To: <aiZoYE8koq1UKlWq@pks.im>
On Sun, Jun 7, 2026 at 11:59 PM Patrick Steinhardt <ps@pks.im> wrote:
>
> On Sun, Jun 07, 2026 at 04:51:53PM -0400, Tamir Duberstein wrote:
> > Unless --all is given, get_name() rejects every ref outside refs/tags/.
> > The rejection happens only after the ref backend has enumerated the ref,
> > so repositories with many other refs spend most of a simple describe
> > invocation visiting refs which cannot affect its result.
>
> Right. The relevant block is this one:
>
> if (skip_prefix(ref->name, "refs/tags/", &path_to_match)) {
> is_tag = 1;
> } else if (all) {
> if ((exclude_patterns.nr || patterns.nr) &&
> !skip_prefix(ref->name, "refs/heads/", &path_to_match) &&
> !skip_prefix(ref->name, "refs/remotes/", &path_to_match)) {
> /* Only accept reference of known type if there are match/exclude patterns */
> return 0;
> }
> } else {
> /* Reject anything outside refs/tags/ unless --all */
> return 0;
> }
>
> So we really only use tags unless "--all" is given.
>
> > Commit 8a5a1884e9 (Avoid accessing non-tag refs in git-describe unless
> > --all is requested, 2008-02-24) moved this rejection before object
> > lookup, but left iteration unscoped. Pass the existing refs/tags/
> > restriction to the iterator unless --all is given so the backend can
> > avoid unrelated refs.
> >
> > On a checkout with 124,357 refs, of which 330 were tags, I ran the
> > following command with the parent and patched binaries:
> >
> > hyperfine --warmup 3 --runs 15 \
> > 'git describe --always --long --abbrev=40 HEAD'
> >
> > The results were:
> >
> > parent this commit
> > elapsed 196.2 ms 63.3 ms
> > user 69.5 ms 48.0 ms
> > system 123.0 ms 12.0 ms
>
> It's a bit curious that you don't post the hyperfine(1) results as-is
> here.
Agreed, will include that in v2. For reference:
Benchmark 1: parent
Time (mean ± σ): 171.7 ms ± 18.5 ms [User: 23.9 ms,
System: 133.6 ms]
Range (min … max): 142.3 ms … 198.3 ms 15 runs
Benchmark 2: this commit
Time (mean ± σ): 9.9 ms ± 1.1 ms [User: 3.3 ms,
System: 4.7 ms]
Range (min … max): 8.8 ms … 13.1 ms 15 runs
>
> > The wall-time standard deviations were 13.2 ms and 2.6 ms, respectively,
> > for a 3.10x speedup.
>
> Makes sense that this would result in a sizeable speedup, depending of
> course on the shape of the existing refs in the repository.
>
> > diff --git a/builtin/describe.c b/builtin/describe.c
> > index 1c47d7c0b7..3532c8ff22 100644
> > --- a/builtin/describe.c
> > +++ b/builtin/describe.c
> > @@ -740,6 +740,9 @@ int cmd_describe(int argc,
> > return ret;
> > }
> >
> > + if (!all)
> > + for_each_ref_opts.prefix = "refs/tags/";
> > +
> > hashmap_init(&names, commit_name_neq, NULL, 0);
> > refs_for_each_ref_ext(get_main_ref_store(the_repository),
> > get_name, NULL, &for_each_ref_opts);
>
> Another performance optimization that we could do here is to wire up the
> exclude patterns via `for_each_ref_opts.exclude_patterns`. But that's
> outside the scope of this patch series, and also much less likely to
> help many use cases out there.
I tried this and have a separate patch prepared.
The patterns cannot be passed through verbatim: `git describe
--exclude=foo` excludes the exact name `foo`, while the refs API would
treat `foo` as a directory prefix and also skip `foo/*`. The patch
therefore passes only patterns consisting of a literal prefix followed
by trailing asterisks, adds back the applicable ref namespace, and
retains the existing callback filtering.
With 30,000 packed remote-tracking refs under an excluded prefix, the
perf test invokes `git describe` ten times per run:
```
master patched
describe excluding many refs 0.16(0.07+0.05) 0.12(0.04+0.05)
```
That is a 25% wall-time reduction, with user CPU falling from 0.07 to
0.04 seconds.
I also tested a larger checkout with 62,170 refs under
`refs/remotes/origin/`:
```
git describe --all --exact-match --exclude='origin/*' HEAD
```
This improved from 176.7 ms to 161.3 ms, or about 10%. Startup work
unrelated to ref iteration dominates more of that repository's runtime.
>
> > diff --git a/t/perf/p6100-describe.sh b/t/perf/p6100-describe.sh
> > index 069f91ce49..dfcaf59e90 100755
> > --- a/t/perf/p6100-describe.sh
> > +++ b/t/perf/p6100-describe.sh
> > @@ -5,6 +5,12 @@ test_description='performance of git-describe'
> >
> > test_perf_default_repo
> >
> > +test_lazy_prereq PERF_REFFILES '
> > + test "$(git rev-parse --show-ref-format)" = files
> > +'
> > +
> > +ref_count=10000
>
> Let's not declare this variable outside of tests.
Done in v2.
>
> > @@ -27,4 +33,18 @@ test_perf 'describe HEAD with one tag' '
> > git describe --match=new HEAD
> > '
> >
> > +test_expect_success PERF_REFFILES 'set up many unrelated refs' '
> > + git tag -m tip tip HEAD &&
> > + for i in $(test_seq $ref_count)
> > + do
> > + printf "create refs/heads/describe-perf/%05d HEAD\n" $i ||
> > + return 1
> > + done >instructions &&
> > + git update-ref --stdin <instructions
> > +'
>
> Why is this limited to the "files" backend, only? The logic should work
> for both backends as-is.
You're right, fixed in v2.
>
> Thanks!
Thanks for the quick review!
^ permalink raw reply
* Re: [PATCH 05/16] odb/source-packed: wire up `close()` callback
From: Karthik Nayak @ 2026-06-08 15:31 UTC (permalink / raw)
To: Patrick Steinhardt, git
In-Reply-To: <20260604-pks-odb-source-packed-v1-5-2e7ab31b4b5c@pks.im>
[-- Attachment #1: Type: text/plain, Size: 2420 bytes --]
Patrick Steinhardt <ps@pks.im> writes:
> Wire up a new `close()` callback for the packed source and call it from
> the "files" source via the generic `odb_source_close()` interface.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> odb/source-files.c | 2 +-
> odb/source-packed.c | 16 ++++++++++++++++
> packfile.c | 12 ------------
> packfile.h | 6 ------
> 4 files changed, 17 insertions(+), 19 deletions(-)
>
> diff --git a/odb/source-files.c b/odb/source-files.c
> index 3608808e7c..9b0fa9ccdc 100644
> --- a/odb/source-files.c
> +++ b/odb/source-files.c
> @@ -38,7 +38,7 @@ static void odb_source_files_close(struct odb_source *source)
> {
> struct odb_source_files *files = odb_source_files_downcast(source);
> odb_source_close(&files->loose->base);
> - packfile_store_close(files->packed);
> + odb_source_close(&files->packed->base);
> }
>
> static void odb_source_files_reprepare(struct odb_source *source)
> diff --git a/odb/source-packed.c b/odb/source-packed.c
> index f81a990cbd..74805be1dd 100644
> --- a/odb/source-packed.c
> +++ b/odb/source-packed.c
> @@ -1,6 +1,7 @@
> #include "git-compat-util.h"
> #include "abspath.h"
> #include "chdir-notify.h"
> +#include "midx.h"
> #include "odb/source-packed.h"
> #include "packfile.h"
>
> @@ -16,6 +17,20 @@ static void odb_source_packed_reparent(const char *name UNUSED,
> packed->base.path = path;
> }
>
> +static void odb_source_packed_close(struct odb_source *source)
> +{
> + struct odb_source_packed *packed = odb_source_packed_downcast(source);
> +
> + for (struct packfile_list_entry *e = packed->packs.head; e; e = e->next) {
> + if (e->pack->do_not_close)
> + BUG("want to close pack marked 'do-not-close'");
> + close_pack(e->pack);
> + }
> + if (packed->midx)
> + close_midx(packed->midx);
> + packed->midx = NULL;
> +}
> +
>
Most of my ODB understandings is coming from reviewing your patches. But
I really like how we can map the current workings to the ODB interface.
> static void odb_source_packed_free(struct odb_source *source)
> {
> struct odb_source_packed *packed = odb_source_packed_downcast(source);
> @@ -42,6 +57,7 @@ struct odb_source_packed *odb_source_packed_new(struct odb_source_files *parent)
> strmap_init(&packed->packs_by_path);
>
> packed->base.free = odb_source_packed_free;
> + packed->base.close = odb_source_packed_close;
>
This is what I mean :)
[snip]
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
* Re: [PATCH 04/16] odb/source-packed: start converting to a proper `struct odb_source`
From: Karthik Nayak @ 2026-06-08 15:29 UTC (permalink / raw)
To: Patrick Steinhardt, git
In-Reply-To: <20260604-pks-odb-source-packed-v1-4-2e7ab31b4b5c@pks.im>
[-- Attachment #1: Type: text/plain, Size: 2816 bytes --]
Patrick Steinhardt <ps@pks.im> writes:
> Start converting `struct odb_source_packed` into a proper pluggable
> `struct odb_source` by embedding the base struct and assigning it the
> new `ODB_SOURCE_PACKED` type. Furthermore, wire up lifecycle management
> of this source by implementing the `free` callback and taking ownership
> of the chdir notifications.
>
> Note that the packed source is not yet functional as a standalone `struct
> odb_source`, as it's missing all of the callback implementations. These
> will be wired up in subsequent commits.
Okay, so individual commits going on will implement the callbacks.
[snip]
> diff --git a/odb/source-packed.c b/odb/source-packed.c
> index 12e785be48..f81a990cbd 100644
> --- a/odb/source-packed.c
> +++ b/odb/source-packed.c
> @@ -1,11 +1,50 @@
> #include "git-compat-util.h"
> +#include "abspath.h"
> +#include "chdir-notify.h"
> #include "odb/source-packed.h"
> +#include "packfile.h"
> +
> +static void odb_source_packed_reparent(const char *name UNUSED,
> + const char *old_cwd,
> + const char *new_cwd,
> + void *cb_data)
> +{
> + struct odb_source_packed *packed = cb_data;
> + char *path = reparent_relative_path(old_cwd, new_cwd,
> + packed->base.path);
> + free(packed->base.path);
> + packed->base.path = path;
> +}
> +
> +static void odb_source_packed_free(struct odb_source *source)
> +{
> + struct odb_source_packed *packed = odb_source_packed_downcast(source);
> +
> + chdir_notify_unregister(NULL, odb_source_packed_reparent, packed);
> +
> + for (struct packfile_list_entry *e = packed->packs.head; e; e = e->next)
> + free(e->pack);
> + packfile_list_clear(&packed->packs);
> +
> + strmap_clear(&packed->packs_by_path, 0);
> + odb_source_release(&packed->base);
> + free(packed);
> +}
>
> struct odb_source_packed *odb_source_packed_new(struct odb_source_files *parent)
> {
> - struct odb_source_packed *store;
> - CALLOC_ARRAY(store, 1);
> - store->files = parent;
> - strmap_init(&store->packs_by_path);
> - return store;
> + struct odb_source_packed *packed;
> +
Nit: we could've had a better diff if we used `struct odb_source_packed
*packed` from the start. But its tiny and doesn't bother me.
> + CALLOC_ARRAY(packed, 1);
> + odb_source_init(&packed->base, parent->base.odb, ODB_SOURCE_PACKED,
> + parent->base.path, parent->base.local);
> + packed->files = parent;
> + strmap_init(&packed->packs_by_path);
> +
> + packed->base.free = odb_source_packed_free;
> +
> + if (!is_absolute_path(parent->base.path))
> + chdir_notify_register(NULL, odb_source_packed_reparent, packed);
> +
Tangent: seems like no one sets the 'name' field within
`chdir_notify_register()`. It is meant for tracing purposes, but if no
one is using it, we might as well remove it...? Perhaps #leftoverbits
[snip]
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
* Re: [PATCH] log: improve --follow following renames for non-linear history
From: Junio C Hamano @ 2026-06-08 15:10 UTC (permalink / raw)
To: Miklos Vajna; +Cc: Jeff King, git
In-Reply-To: <aiZipugmA7z8oBcd@collabora.com>
Miklos Vajna <vmiklos@collabora.com> writes:
> Have a repo with a subtree merge, do a 'git log --follow prefix/test.c',
> the output only contains history in the outer repo, not commits that
> were merged via a subtree merge.
>
> What happened is that 'git log --follow' used to store the followed path
> only in opt->diffopt.pathspec, so in case the commit history is
> non-linear, and multiple parents had renames to the followed path, then
> the end result wasn't really defined: the first commit that happened to
> be visited in one of the parents updated opt->diffopt.pathspec, and from
> that point, only that updated path was visited.
When describing a problematic symptom you are trying to improve, you
should talk about the current state of the system in the present
tense. "used to store" makes it sound like in ancient times back
when Linus wrote the first version of this feature it was so, but a
few years ago that changed, but that is not what you want to say, is
it?
The above may sound picky, but using the consistent style of
description makes it easier to follow the thought process,
especially when you need to read many commits to understand what is
going on.
> Fix the problem by introducing a commit -> path map
> (follow_pathspec_slab) that stores that will be path to follow when
> visiting that parent. At the top of log_tree_commit(), if the slab has
> an entry for this commit, we replace opt->diffopt.pathspec with it, so
> the correct path is followed, even if an unrelated sub-tree changed the
> path to be followed to something else.
Can a "map" cut it?
If a history forked at commit A, with two children commit B and
commit C, and you started traversing the history from a much later
descendant M that merges these two lines of history (i.e., M^1
contains B, M^2 contains C, and A==B^1==C^1), while traversing down
from M to B you may find that you need to follow path1 and similarly
somewhere between M down to C the path you are following may be
path2. And the traversal meets at A. The slab records path1 for B
and path2 for C. Wouldn't you need to be able to store both path1
and path2 for commit A? What path do you need to pay attention to
when traversing past A to its ancestors?
^ permalink raw reply
* Re: [PATCH 02/16] packfile: move packed source into "odb/" subsystem
From: Karthik Nayak @ 2026-06-08 15:09 UTC (permalink / raw)
To: Patrick Steinhardt, git
In-Reply-To: <20260604-pks-odb-source-packed-v1-2-2e7ab31b4b5c@pks.im>
[-- Attachment #1: Type: text/plain, Size: 1185 bytes --]
Patrick Steinhardt <ps@pks.im> writes:
> In subsequent patches we'll be turning `struct odb_source_packed` into a
> proper `struct odb_source`. As a first step towards this goal, move its
> struct out of "packfile.{c,h}" and into "odb/source-packed.{c,h}".
>
> This detaches the implementation of the packfile object source from the
> generic packfile code, following the same convention already used by the
> "files" and "in-memory" sources.
>
[snip]
> diff --git a/odb/source-packed.h b/odb/source-packed.h
> new file mode 100644
> index 0000000000..c17068a4f1
> --- /dev/null
> +++ b/odb/source-packed.h
> @@ -0,0 +1,80 @@
> +#ifndef ODB_SOURCE_PACKED_H
> +#define ODB_SOURCE_PACKED_H
> +
> +#include "odb/source.h"
> +#include "strmap.h"
> +
> +struct packfile_list {
> + struct packfile_list_entry *head, *tail;
> +};
> +
> +struct packfile_list_entry {
> + struct packfile_list_entry *next;
> + struct packed_git *pack;
> +};
> +
So this is exposed, because outside of the odb, we also use packfiles in
the transport layer. That makes me wonder if these two structures are
better kept alonsigde `struct packed_git` in 'packfile.h'.
[snip]
The rest of the patch looks good.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
* Re: [GSoC PATCH v2 1/4] path: introduce format_path() for centralized path formatting
From: Lucas Seiki Oshiro @ 2026-06-08 15:05 UTC (permalink / raw)
To: K Jayatheerth
Cc: git, a3205153416, gitster, jltobler, kumarayushjha123,
phillip.wood, sandals
In-Reply-To: <20260605163012.181089-2-jayatheerthkulkarni2005@gmail.com>
> +++ b/path.h
> @@ -262,6 +262,36 @@ enum scld_error safe_create_leading_directories_no_share(char *path);
> int safe_create_file_with_leading_directories(struct repository *repo,
> const char *path);
>
> +/**
> + * The formatting strategy to apply when writing a path into a buffer.
> + */
> +enum path_format {
> + /* Output the path exactly as-is without any modifications. */
> + PATH_FORMAT_UNMODIFIED,
> +
> + /* Output a path relative to the provided directory prefix. */
> + PATH_FORMAT_RELATIVE,
> +
> + /* Output a relative path only if the path shares a root with the prefix. */
> + PATH_FORMAT_RELATIVE_IF_SHARED,
> +
> + /* Output a fully resolved, absolute canonical path. */
> + PATH_FORMAT_CANONICAL
> +};
> +
> +/**
> + * Format a path according to the specified formatting strategy and append
> + * the result to the given strbuf.
> + *
> + * `buf` : The string buffer to append the formatted path to.
> + * `path` : The path string that needs to be formatted.
> + * `prefix` : The directory prefix to calculate relative offsets against.
> + * Pass NULL to default to the current working directory where applicable.
> + * `format` : The formatting behavior rule to execute.
> + */
> +void format_path(struct strbuf *buf, const char *path,
> + const char *prefix, enum path_format format);
Nitpick: the documentation is clear to me, but maybe the function name
"format" and the parameter name "buf" can mislead the user to think
that it only formats the path without appending to the existing string
in `buf`. My suggestion is to rename them to something like
`append_formatted_path` and `dest`, respectively.
^ permalink raw reply
* Re: [PATCH GSoC RFC v12 03/12] cat-file: add declaration of variable i inside its for loop
From: Junio C Hamano @ 2026-06-08 14:52 UTC (permalink / raw)
To: Pablo Sabater
Cc: eric.peijian, calvinwan, chriscool, git, jltobler, jonathantanmy,
karthik.188, toon, chandrapratap3519
In-Reply-To: <20260608-ps-eric-work-rebase-v12-3-5338b766e658@gmail.com>
Pablo Sabater <pabloosabaterr@gmail.com> writes:
> From: Eric Ju <eric.peijian@gmail.com>
> Subject: Re: [PATCH GSoC RFC v12 03/12] cat-file: add declaration of variable i inside its for loop
"add" sounds a bit strange, as the existing code wouldn't have
compiled if the variable were never declared. What the patch did
was to move (not add) the declaration of a function scope variable
that is used to control for() loops. Would any of these work?
Subject: [PATCH GSOC v12 03/12] cat-file: narrow scope of loop counter
Subject: [PATCH GSOC v12 03/12] cat-file: declare loop counter inside for()
> Some code used in this series declares variable i and only uses it
> in a for loop, not in any other logic outside the loop.
>
> Change the declaration of i to be inside the for loop for readability.
> While at it, we also change its type from "int" to "size_t" where the latter makes more sense.
Curious single line that is overly long?
> Helped-by: Christian Couder <chriscool@tuxfamily.org>
> Signed-off-by: Eric Ju <eric.peijian@gmail.com>
> Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
> ---
> builtin/cat-file.c | 11 +++--------
> fetch-pack.c | 3 +--
> 2 files changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> index fa45f774d7..c060fd4800 100644
> --- a/builtin/cat-file.c
> +++ b/builtin/cat-file.c
> @@ -726,12 +726,10 @@ static void dispatch_calls(struct batch_options *opt,
> struct queued_cmd *cmd,
> int nr)
> {
> - int i;
> -
> if (!opt->buffer_output)
> die(_("flush is only for --buffer mode"));
>
> - for (i = 0; i < nr; i++)
> + for (size_t i = 0; i < nr; i++)
> cmd[i].fn(opt, cmd[i].line, output, data);
The loop limit "nr" will not become as large as size_t because the
caller passes a platform natural "int" to the function. Wouldn't a
stupid compiler give us warning on comparing unsigned size_t with
signed int here?
> @@ -739,9 +737,7 @@ static void dispatch_calls(struct batch_options *opt,
>
> static void free_cmds(struct queued_cmd *cmd, size_t *nr)
> {
> - size_t i;
> -
> - for (i = 0; i < *nr; i++)
> + for (size_t i = 0; i < *nr; i++)
> FREE_AND_NULL(cmd[i].line);
No type change, so the result is as safe as the original.
> @@ -768,7 +764,6 @@ static void batch_objects_command(struct batch_options *opt,
> size_t alloc = 0, nr = 0;
>
> while (strbuf_getdelim_strip_crlf(&input, stdin, opt->input_delim) != EOF) {
> - int i;
> const struct parse_cmd *cmd = NULL;
> const char *p = NULL, *cmd_end;
> struct queued_cmd call = {0};
> @@ -778,7 +773,7 @@ static void batch_objects_command(struct batch_options *opt,
> if (isspace(*input.buf))
> die(_("whitespace before command: '%s'"), input.buf);
>
> - for (i = 0; i < ARRAY_SIZE(commands); i++) {
> + for (size_t i = 0; i < ARRAY_SIZE(commands); i++) {
> if (!skip_prefix(input.buf, commands[i].name, &cmd_end))
> continue;
ARRAY_SIZE() is some arithmetic over sizeof(*commands) and
sizeof(commands), which is of type size_t, so this is better than
the original. Use of size_t i of course is a natural way to index
into commands[] array, so the result is just fine.
> diff --git a/fetch-pack.c b/fetch-pack.c
> index 120e01f3cf..f13951d154 100644
> --- a/fetch-pack.c
> +++ b/fetch-pack.c
> @@ -1388,9 +1388,8 @@ static void write_fetch_command_and_capabilities(struct strbuf *req_buf,
> if (advertise_sid && server_supports_v2("session-id"))
> packet_buf_write(req_buf, "session-id=%s", trace2_session_id());
> if (server_options && server_options->nr) {
> - int i;
> ensure_server_supports_v2("server-option");
> - for (i = 0; i < server_options->nr; i++)
> + for (size_t i = 0; i < server_options->nr; i++)
> packet_buf_write(req_buf, "server-option=%s",
> server_options->items[i].string);
server_options is a string_list whose .nr member is of type size_t,
so this comparison is perfectly fine. Ditto for ->items[i].string
that is a natural way to index into an array.
> }
v
^ permalink raw reply
* Re: [PATCH 0/2] worktree: copy-on-write creation and shared-branch worktrees
From: Junio C Hamano @ 2026-06-08 14:36 UTC (permalink / raw)
To: Jason Newton via GitGitGadget; +Cc: git, Jason Newton
In-Reply-To: <pull.2317.git.git.1780685368.gitgitgadget@gmail.com>
"Jason Newton via GitGitGadget" <gitgitgadget@gmail.com> writes:
> When many worktrees share one repository -- e .g. a fleet of agents each
> needing an isolated checkout -- "git worktree add" is costly at scale.
> Objects are shared via the common dir, but the working tree is not: each add
> rewrites every tracked file, so N worktrees cost N full checkouts of disk
> and I/O.
Are the "CoW" semantics offered by these underlying mechanisms,
which may differ per operating system and possibly filesystem type,
all meant as mere storage-space optimization, or do some of them
trade potential space saving with some limitation of the features,
i.e., what you can do in the CoW copy and original, or increased
runtime cost, either at the clone time or the time of first
modification?
What I am trying to get at is why should this be even an opt-in
feature. If "cp treeA treeB" at the shell level would make all the
files in treeA under identical names and contents in treeB, and let
you edit/update/delete copies in either tree without affecting the
other tree, then in practice you would not even be able to _tell_ if
CoW is in use, no?
It may tilt the scale if there is a downside associated with the use
of CoW, like at the first modification of one copy, the system may
need to do real copies of other copies, but even such a cost should
not be outrageously worse than the cost of copying everything once
at the worktree creation time.
So I would understand "whenever we say git_copy_file(A, B), we
always use CoW facility under the hood if available, regardless of
the purpose of the operation to copy one file to another location---
it may include, but does not have to be limited to, populating
working file trees in a new worktree", and I think it is a welcome
change.
But I do not quite get "... only if the user gives --reflink
option". Why is it even necessary to offer a choice? Especially
since you seem to have auto-probe, we should be able to implement a
low-level operation to materialize contents identified by a_hash at
a_path in the working tree in two different ways, switching on the
availablity of CoW, e.g.,
if (CoW available && we can find existing path with a_hash) {
copy-cow the found path to a_path;
} else {
write object identified by a_hash to a_path;
}
> And a branch can only be checked out in one worktree.
This is a safety feature that has nothing to do with shared files
across worktrees, no? Your two worktrees may think they have a
checkout of the same branch (thus the same commit), one worktree
makes changes and commits, the other worktree suddenly starts seeing
a totally different output from its "git diff HEAD" that mixes what
it did relative to where it started (which is what we want) plus the
reversion of what was done in the other worktree (which is definitely
not what we want).
^ permalink raw reply
* Re: [PATCH 2/3] config: add GIT_CONFIG_INCLUDES
From: Patrick Steinhardt @ 2026-06-08 14:34 UTC (permalink / raw)
To: Derrick Stolee via GitGitGadget; +Cc: git, gitster, Derrick Stolee
In-Reply-To: <b48fe9f7abe794864ac4470c2620048c2e5e6b53.1780927027.git.gitgitgadget@gmail.com>
On Mon, Jun 08, 2026 at 01:57:05PM +0000, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <stolee@gmail.com>
>
> The config keys 'include.path' and 'includeIf.*' allow users to specify
> config stored in a location outside of the typical list of config files
> (system, global, local, etc.). For example, users who accept the risk
> can specify helpful aliases via a file checked into the repo by pointing
> 'include.path' to the position of that file in the working directory.
> This is dangerous, but people do it.
Huh, I never even considered this use case. But of course, this is
possible, even though it's quite scary.
> What becomes tricky is that this modifies all Git behavior, including
> operations that are intended to be limited in activity or sandboxed in
> some way. These include directives can provide surprising changes to
> behavior, especially when expecting a specific list of allowed file
> accesses. This could lead to failed builds, for instance.
>
> To allow for these user-desired features when they are running commands,
> add a new GIT_CONFIG_INCLUDES environment variable that disables these
> redirections of config when set to zero. This variable can be set by
> automation, such as build tooling, to avoid these strange behaviors.
> This could be considered a recommended option for tools executing Git
> commands, the same as GIT_ADVICE=0.
I don't know about this part though. I could see use cases where the
tools _should_ read the project-relative configuration. It might also be
the case that the user may want to evaluate some includes, but not all
of them.
That raises the question whether we can introduce the configuration in a
way that it allows a bit more flexibility than just "yes"/"no", like for
example an allow-list of locations that should be evaluated. But maybe
I'm overthinking this.
Patrick
^ permalink raw reply
* Re: [PATCH] prio-queue: fold lazy_queue into prio_queue for automatic get+put fusion
From: Junio C Hamano @ 2026-06-08 14:07 UTC (permalink / raw)
To: Kristofer Karlsson
Cc: Kristofer Karlsson via GitGitGadget, git, René Scharfe
In-Reply-To: <CAL71e4MbC+tdTuN6p1HiHtE1XYuS1gBM-KSejFZJ1wbftxNveg@mail.gmail.com>
Kristofer Karlsson <krka@spotify.com> writes:
> Agreed, that's the right fix. I looked for existing ways of marking
> fields as private, internal or hidden but the only thing I found was
> the convention of using a code comment: /* for internal use only */
>
> I will apply a rename and submit a v2. Perhaps something like
> nr_internal to make it look less like a public API.
I think we often use trailing underscore (e.g., "n_") to mark
variables for "not to be used casually, there are better ways to
access this information", which pre-ANSI C people probably used
leading underscore (e.g. "_n") for.
This is often used in callback functions where the types of their
formal parameters are specified by the API and use of them with
casting at every use site is awkward. For example, qsort() and
friends often take a pointer to the location that stores the value
to be compared, but it is awkward, so we do cast just once upfront
like so:
static int compare_callback(const void *a_, const void *b_)
{
const a_type a = *((const a_type *)a_);
const a_type b = *((const a_type *)b_);
... use values 'a' and 'b', without having to cast a_ or b_ ...
return a - b;
}
I think the technique/convention can be used in a similar way for
"this is hidden and unless you can tell if you should be using it
directly, you probably shouldn't" kind of structure members.
So, nr_internal is perfectly fine, but if you find it too long, nr_
is probably just as good.
^ permalink raw reply
* [PATCH 3/3] git: add --no-includes top-level option
From: Derrick Stolee via GitGitGadget @ 2026-06-08 13:57 UTC (permalink / raw)
To: git; +Cc: gitster, Derrick Stolee, Derrick Stolee
In-Reply-To: <pull.2139.git.1780927027.gitgitgadget@gmail.com>
From: Derrick Stolee <stolee@gmail.com>
The previous change added a GIT_CONFIG_INCLUDES=0 override in the
environment, similar to GIT_ADVICE=0. Follow the same model as
--no-advice to add a --no-includes option to the top-level Git options.
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
Documentation/git.adoc | 6 +++++-
git.c | 6 +++++-
t/t1305-config-include.sh | 8 ++++++--
3 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/Documentation/git.adoc b/Documentation/git.adoc
index 8a5cdd3b3d..f220427930 100644
--- a/Documentation/git.adoc
+++ b/Documentation/git.adoc
@@ -12,7 +12,7 @@ SYNOPSIS
'git' [-v | --version] [-h | --help] [-C <path>] [-c <name>=<value>]
[--exec-path[=<path>]] [--html-path] [--man-path] [--info-path]
[-p | --paginate | -P | --no-pager] [--no-replace-objects] [--no-lazy-fetch]
- [--no-optional-locks] [--no-advice] [--bare] [--git-dir=<path>]
+ [--no-optional-locks] [--no-advice] [--no-includes] [--bare] [--git-dir=<path>]
[--work-tree=<path>] [--namespace=<name>] [--config-env=<name>=<envvar>]
<command> [<args>]
@@ -194,6 +194,10 @@ If you just want to run git as if it was started in `<path>` then use
--no-advice::
Disable all advice hints from being printed.
+--no-includes::
+ Disable all `include.path` and `includeIf.*` config directives.
+ See linkgit:git-config[1] for more information.
+
--literal-pathspecs::
Treat pathspecs literally (i.e. no globbing, no pathspec magic).
This is equivalent to setting the `GIT_LITERAL_PATHSPECS` environment
diff --git a/git.c b/git.c
index 36f08891ef..52cfbf0e23 100644
--- a/git.c
+++ b/git.c
@@ -40,7 +40,7 @@ const char git_usage_string[] =
N_("git [-v | --version] [-h | --help] [-C <path>] [-c <name>=<value>]\n"
" [--exec-path[=<path>]] [--html-path] [--man-path] [--info-path]\n"
" [-p | --paginate | -P | --no-pager] [--no-replace-objects] [--no-lazy-fetch]\n"
- " [--no-optional-locks] [--no-advice] [--bare] [--git-dir=<path>]\n"
+ " [--no-optional-locks] [--no-advice] [--no-includes] [--bare] [--git-dir=<path>]\n"
" [--work-tree=<path>] [--namespace=<name>] [--config-env=<name>=<envvar>]\n"
" <command> [<args>]");
@@ -354,6 +354,10 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
setenv(GIT_ADVICE_ENVIRONMENT, "0", 1);
if (envchanged)
*envchanged = 1;
+ } else if (!strcmp(cmd, "--no-includes")) {
+ setenv(CONFIG_INCLUDES_ENVIRONMENT, "0", 1);
+ if (envchanged)
+ *envchanged = 1;
} else {
fprintf(stderr, _("unknown option: %s\n"), cmd);
usage(git_usage_string);
diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh
index 270e4b89ab..b636e5ae7b 100755
--- a/t/t1305-config-include.sh
+++ b/t/t1305-config-include.sh
@@ -409,8 +409,11 @@ test_expect_success 'GIT_CONFIG_INCLUDES=0 disables include.path and includeIf'
git config get foo.baz &&
test_must_fail env GIT_CONFIG_INCLUDES=0 git config get foo.bar &&
test_must_fail env GIT_CONFIG_INCLUDES=0 git config get foo.baz &&
+ test_must_fail git --no-includes config get foo.bar &&
+ test_must_fail git --no-includes config get foo.baz &&
git config get --includes foo.bar &&
- test_must_fail env GIT_CONFIG_INCLUDES=0 git config get --includes foo.bar
+ test_must_fail env GIT_CONFIG_INCLUDES=0 git config get --includes foo.bar &&
+ test_must_fail git --no-includes config get --includes foo.bar
)
'
@@ -423,7 +426,8 @@ test_expect_success 'GIT_CONFIG_INCLUDES=0 blocks included alias override' '
git config set include.path config.inc &&
git config set -f .git/config.inc alias.test status &&
git test &&
- test_must_fail env GIT_CONFIG_INCLUDES=0 git test
+ test_must_fail env GIT_CONFIG_INCLUDES=0 git test &&
+ test_must_fail git --no-includes test
)
'
--
gitgitgadget
^ permalink raw reply related
* [PATCH 2/3] config: add GIT_CONFIG_INCLUDES
From: Derrick Stolee via GitGitGadget @ 2026-06-08 13:57 UTC (permalink / raw)
To: git; +Cc: gitster, Derrick Stolee, Derrick Stolee
In-Reply-To: <pull.2139.git.1780927027.gitgitgadget@gmail.com>
From: Derrick Stolee <stolee@gmail.com>
The config keys 'include.path' and 'includeIf.*' allow users to specify
config stored in a location outside of the typical list of config files
(system, global, local, etc.). For example, users who accept the risk
can specify helpful aliases via a file checked into the repo by pointing
'include.path' to the position of that file in the working directory.
This is dangerous, but people do it.
What becomes tricky is that this modifies all Git behavior, including
operations that are intended to be limited in activity or sandboxed in
some way. These include directives can provide surprising changes to
behavior, especially when expecting a specific list of allowed file
accesses. This could lead to failed builds, for instance.
To allow for these user-desired features when they are running commands,
add a new GIT_CONFIG_INCLUDES environment variable that disables these
redirections of config when set to zero. This variable can be set by
automation, such as build tooling, to avoid these strange behaviors.
This could be considered a recommended option for tools executing Git
commands, the same as GIT_ADVICE=0.
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
Documentation/git-config.adoc | 5 +++++
config.c | 7 ++++++-
environment.h | 6 ++++++
t/t1305-config-include.sh | 31 +++++++++++++++++++++++++++++++
4 files changed, 48 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-config.adoc b/Documentation/git-config.adoc
index 044d776613..c9b5159501 100644
--- a/Documentation/git-config.adoc
+++ b/Documentation/git-config.adoc
@@ -502,6 +502,11 @@ GIT_CONFIG::
historical compatibility; there is generally no reason to use it
instead of the `--file` option.
+GIT_CONFIG_INCLUDES::
+ If GIT_CONFIG_INCLUDES is set to 0, then Git will not follow
+ `include.path` or `includeIf.*.path` directives when reading
+ configuration files.
+
[[EXAMPLES]]
EXAMPLES
--------
diff --git a/config.c b/config.c
index a1b92fe083..85edd05672 100644
--- a/config.c
+++ b/config.c
@@ -1595,9 +1595,14 @@ int config_with_options(config_fn_t fn, void *data,
const struct config_options *opts)
{
struct config_include_data inc = CONFIG_INCLUDE_INIT;
+ int respect_includes = opts->respect_includes;
int ret;
- if (opts->respect_includes) {
+ if (respect_includes &&
+ !git_env_bool(CONFIG_INCLUDES_ENVIRONMENT, 1))
+ respect_includes = 0;
+
+ if (respect_includes) {
inc.fn = fn;
inc.data = data;
inc.opts = opts;
diff --git a/environment.h b/environment.h
index 9eb97b3869..2c57ae2533 100644
--- a/environment.h
+++ b/environment.h
@@ -52,6 +52,12 @@
*/
#define GIT_ADVICE_ENVIRONMENT "GIT_ADVICE"
+/*
+ * Environment variable used to prevent following include.path or includeIf.*
+ * config directives.
+ */
+#define CONFIG_INCLUDES_ENVIRONMENT "GIT_CONFIG_INCLUDES"
+
/*
* Environment variable used in handshaking the wire protocol.
* Contains a colon ':' separated list of keys with optional values
diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh
index f3892578e4..270e4b89ab 100755
--- a/t/t1305-config-include.sh
+++ b/t/t1305-config-include.sh
@@ -396,4 +396,35 @@ test_expect_success 'onbranch without repository but explicit nonexistent Git di
test_must_fail nongit git --git-dir=nonexistent config get foo.bar
'
+test_expect_success 'GIT_CONFIG_INCLUDES=0 disables include.path and includeIf' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ git config set include.path config.inc &&
+ git config set "includeIf.gitdir:*.path" config2.inc &&
+ git config set -f .git/config.inc foo.bar from-include &&
+ git config set -f .git/config2.inc foo.baz from-includeif &&
+ git config get foo.bar &&
+ git config get foo.baz &&
+ test_must_fail env GIT_CONFIG_INCLUDES=0 git config get foo.bar &&
+ test_must_fail env GIT_CONFIG_INCLUDES=0 git config get foo.baz &&
+ git config get --includes foo.bar &&
+ test_must_fail env GIT_CONFIG_INCLUDES=0 git config get --includes foo.bar
+ )
+'
+
+test_expect_success 'GIT_CONFIG_INCLUDES=0 blocks included alias override' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ git config set alias.test false &&
+ git config set include.path config.inc &&
+ git config set -f .git/config.inc alias.test status &&
+ git test &&
+ test_must_fail env GIT_CONFIG_INCLUDES=0 git test
+ )
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH 1/3] git-config.adoc: fix paragraph break
From: Derrick Stolee via GitGitGadget @ 2026-06-08 13:57 UTC (permalink / raw)
To: git; +Cc: gitster, Derrick Stolee, Derrick Stolee
In-Reply-To: <pull.2139.git.1780927027.gitgitgadget@gmail.com>
From: Derrick Stolee <stolee@gmail.com>
The bulletted list about environment variables is missing a '+' between
some paragraphs that belong to the same bullet item. Without it, the
bulletted list is rendered as two separate lists with "See also FILES."
as a normal paragraph between them. Adding '+' unifies the lists.
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
Documentation/git-config.adoc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/git-config.adoc b/Documentation/git-config.adoc
index 00545b2054..044d776613 100644
--- a/Documentation/git-config.adoc
+++ b/Documentation/git-config.adoc
@@ -476,7 +476,7 @@ GIT_CONFIG_SYSTEM::
GIT_CONFIG_NOSYSTEM::
Whether to skip reading settings from the system-wide
$(prefix)/etc/gitconfig file. See linkgit:git[1] for details.
-
++
See also <<FILES>>.
GIT_CONFIG_COUNT::
--
gitgitgadget
^ permalink raw reply related
* [PATCH 0/3] config: allow disabling config includes
From: Derrick Stolee via GitGitGadget @ 2026-06-08 13:57 UTC (permalink / raw)
To: git; +Cc: gitster, Derrick Stolee
This series introduces a new way to ignore config include directives via two
mechanisms:
* GIT_CONFIG_INCLUDES=0 in the environment.
* git --no-includes ... in the command line.
My motivation is from a tricky situation where users want to do the risky
thing and include a repo-tracked file for sharing aliases and other
recommended config. They are then struggling in a later build step that is
running Git commands (under a tool we don't control and can't change) that
then cause filesystem accesses outside of the build system's sandbox.
While git config has a --no-includes option, that doesn't impact the
behavior of other Git commands. We build upon that existing logic for
disabling includes, though.
Having had recent luck recommending GIT_ADVICE=0 when running Git commands
from third-party tools, I thought that a similar environment variable to
disable this functionality would be helpful, too.
One thing I do worry about is whether or not this would cause a significant
break in behavior, or if this is a relatively safe thing to allow.
The three patches are organized as follows:
1. Patch 1 has a small typo fix in the config documentation that messes
with the format of the bulleted list. I include it here because I add to
that list in patch 2.
2. Patch 2 adds the environment variable and tests it via 'git config' and
the use of a Git alias.
3. Patch 3 adds the '--no-includes' option at the top level.
Thanks, -Stolee
Derrick Stolee (3):
git-config.adoc: fix paragraph break
config: add GIT_CONFIG_INCLUDES
git: add --no-includes top-level option
Documentation/git-config.adoc | 7 ++++++-
Documentation/git.adoc | 6 +++++-
config.c | 7 ++++++-
environment.h | 6 ++++++
git.c | 6 +++++-
t/t1305-config-include.sh | 35 +++++++++++++++++++++++++++++++++++
6 files changed, 63 insertions(+), 4 deletions(-)
base-commit: 9ac3f193c05c2237e2b14ebaa1149e9fc8a1abe0
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2139%2Fderrickstolee%2Fconfig-include-override-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2139/derrickstolee/config-include-override-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2139
--
gitgitgadget
^ permalink raw reply
* Re: [PATCH 7/7] odb: use size_t for object_info.sizep and the size APIs
From: Patrick Steinhardt @ 2026-06-08 13:53 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget
Cc: git, Kristofer Karlsson, Johannes Schindelin
In-Reply-To: <f3aeae983ac8b281d6ba54299961e19d16699c94.1780570273.git.gitgitgadget@gmail.com>
On Thu, Jun 04, 2026 at 10:51:12AM +0000, Johannes Schindelin via GitGitGadget wrote:
> diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> index fa45f774d7..fa6e396ddc 100644
> --- a/builtin/cat-file.c
> +++ b/builtin/cat-file.c
> @@ -120,7 +120,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
> struct object_id oid;
> enum object_type type;
> char *buf;
> - unsigned long size;
> + size_t size;
> struct object_context obj_context = {0};
> struct object_info oi = OBJECT_INFO_INIT;
> unsigned flags = OBJECT_INFO_LOOKUP_REPLACE;
> @@ -166,7 +166,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
> if (use_mailmap && (type == OBJ_COMMIT || type == OBJ_TAG)) {
> size_t s = size;
> buf = replace_idents_using_mailmap(buf, &s);
> - size = cast_size_t_to_ulong(s);
> + size = s;
> }
>
> printf("%"PRIuMAX"\n", (uintmax_t)size);
Can't we drop this local variable completely and instead supply `&size`
directly?
> @@ -219,7 +225,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
> if (use_mailmap) {
> size_t s = size;
> buf = replace_idents_using_mailmap(buf, &s);
> - size = cast_size_t_to_ulong(s);
> + size = s;
> }
>
> /* otherwise just spit out the data */
> @@ -266,7 +272,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
> if (use_mailmap) {
> size_t s = size;
> buf = replace_idents_using_mailmap(buf, &s);
> - size = cast_size_t_to_ulong(s);
> + size = s;
> }
> break;
> }
> @@ -446,7 +455,7 @@ static void print_object_or_die(struct batch_options *opt, struct expand_data *d
> if (use_mailmap) {
> size_t s = size;
> contents = replace_idents_using_mailmap(contents, &s);
> - size = cast_size_t_to_ulong(s);
> + size = s;
> }
>
> if (type != data->type)
Likewise for these three instances.
> @@ -555,7 +564,7 @@ static void batch_object_write(const char *obj_name,
> if (!buf)
> die(_("unable to read %s"), oid_to_hex(&data->oid));
> buf = replace_idents_using_mailmap(buf, &s);
> - data->size = cast_size_t_to_ulong(s);
> + data->size = s;
>
> free(buf);
> }
And I think this site here can be adapted, as well.
> diff --git a/diff.c b/diff.c
> index 5a584fa1d5..816b89dc6c 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -4594,8 +4594,9 @@ int diff_populate_filespec(struct repository *r,
> }
> }
> else {
> + size_t size_st = 0;
> struct object_info info = {
> - .sizep = &s->size
> + .sizep = &size_st
> };
>
> if (!(size_only || check_binary))
> @@ -4617,6 +4618,7 @@ int diff_populate_filespec(struct repository *r,
> die("unable to read %s", oid_to_hex(&s->oid));
>
> object_read:
> + s->size = cast_size_t_to_ulong(size_st);
> if (size_only || check_binary) {
> if (size_only)
> return 0;
> @@ -4631,6 +4633,7 @@ object_read:
> if (odb_read_object_info_extended(r->objects, &s->oid, &info,
> OBJECT_INFO_LOOKUP_REPLACE))
> die("unable to read %s", oid_to_hex(&s->oid));
> + s->size = cast_size_t_to_ulong(size_st);
> }
> s->should_free = 1;
> }
The flow in this function is quite weird if you ask me, but that's a
preexisting issue. This does look correct to me, even if it's awkward.
Patrick
^ permalink raw reply
* Re: [PATCH 6/7] packfile,delta: drop the `cast_size_t_to_ulong()` wrappers
From: Patrick Steinhardt @ 2026-06-08 13:53 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget
Cc: git, Kristofer Karlsson, Johannes Schindelin
In-Reply-To: <460d733feeaf2a94fe28d7509cc4128e9c0a7610.1780570273.git.gitgitgadget@gmail.com>
On Thu, Jun 04, 2026 at 10:51:11AM +0000, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> When I started the transition from `unsigned long` to `size_t`, in the
> interest of keeping the patches reviewable, I introduced these calls to
> prevent data type narrowing from silently failing to handle large object
> sizes. I also introduced `*_sz()` variants that would allow most of the
> callers to keep using that `unsigned long` that the 90s kindly asked to
> be returned.
>
> After the preceding commits, the only places that called the narrow
> wrappers either no longer exist or already use the `_sz` form
> internally, so the wrappers just narrow values back through
> `cast_size_t_to_ulong()` for no reason.
>
> Drop them and rename the `_sz` variants back to the natural names.
Aha, so you already address my comment I had on one of the preceding
patches :)
Patrick
^ permalink raw reply
* Re: [PATCH 4/7] packfile: widen unpack_entry()'s size out-parameter to size_t
From: Patrick Steinhardt @ 2026-06-08 13:53 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget
Cc: git, Kristofer Karlsson, Johannes Schindelin
In-Reply-To: <bdebc36f21d1e2a13bc91d72a3ada1db3f7e184e.1780570273.git.gitgitgadget@gmail.com>
On Thu, Jun 04, 2026 at 10:51:09AM +0000, Johannes Schindelin via GitGitGadget wrote:
> diff --git a/builtin/fast-import.c b/builtin/fast-import.c
> index 82bc6dcc00..3dff898c43 100644
> --- a/builtin/fast-import.c
> +++ b/builtin/fast-import.c
> @@ -1239,6 +1239,8 @@ static void *gfi_unpack_entry(
> unsigned long *sizep)
> {
> enum object_type type;
> + size_t size_st = 0;
> + void *data;
> struct packed_git *p = all_packs[oe->pack_id];
> if (p == pack_data && p->pack_size < (pack_size + the_hash_algo->rawsz)) {
> /* The object is stored in the packfile we are writing to
> @@ -1260,7 +1262,10 @@ static void *gfi_unpack_entry(
> */
> p->pack_size = pack_size + the_hash_algo->rawsz;
> }
> - return unpack_entry(the_repository, p, oe->idx.offset, &type, sizep);
> + data = unpack_entry(the_repository, p, oe->idx.offset, &type, &size_st);
> + if (sizep)
> + *sizep = cast_size_t_to_ulong(size_st);
> + return data;
> }
Nit, please feel free to ignore: do we want to add a NEEDSWORK comment
here?
Patrick
^ 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