* Re: [PATCH/RFC 1/5] replay: support replaying 2-parent merges
From: Johannes Schindelin @ 2026-05-17 14:32 UTC (permalink / raw)
To: Phillip Wood
Cc: Johannes Schindelin via GitGitGadget, git, Elijah Newren,
Patrick Steinhardt
In-Reply-To: <72901ee2-1212-46cd-b752-f451cce6e1ff@gmail.com>
Hi Phillip,
On Fri, 8 May 2026, Phillip Wood wrote:
> On 06/05/2026 23:43, Johannes Schindelin via GitGitGadget wrote:
> >
> > Elijah Newren spelled out a way to lift this limitation in his
> > replay-design-notes [1] and prototyped it in a 2022
> > work-in-progress sketch [2]. The idea is that a merge commit M on
> > parents (P1, P2) records both an automatic merge of those parents
> > AND any manual layer the author put on top of that automatic merge
> > (textual conflict resolution and any semantic edit outside conflict
> > markers). Replaying M onto rewritten parents (P1', P2') must
> > preserve that manual layer, but the rewritten parents change the
> > automatic merge, so a simple cherry-pick is wrong: the manual layer
> > would be re-introduced on top of stale auto-merge text.
> >
> > What works instead is a three-way merge of three trees the existing
> > infrastructure already knows how to compute. Let R be the recursive
> > auto-merge of (P1, P2), O be M's actual tree and N be the recursive
> > auto-merge of (P1', P2'). Then `git diff R O` is morally
> > `git show --remerge-diff M`: it captures exactly what the author
> > added on top of the automatic merge. A non-recursive 3-way merge
> > with R as the merge base, O as side 1 and N as side 2 layers that
> > manual contribution onto the freshly auto-merged rewritten parents
> > (N) and produces the replayed tree.
>
> So we cherry-pick the difference between the user's conflict resolution O and
> the auto-merge M of the original parents onto the auto-merge N of the replayed
> parents. If we have a topology that looks like
>
> |
> A
> /|\
> / B \
> E | D
> C /
> |/
> O
>
> then running
>
> git replay --onto E --ancestry-path B..O
>
> will replay C and O onto E. If the changes in E and D conflict but those
> conflicts do not overlap with the conflicts in M that were resolved to create
> O then the replayed version of O will contain conflict markers from the
> conflicting changes in E and D. Because the previous conflict resolution
> applies to N without conflicts we do not recognize that there are still
> conflicts in N that need to be resolved.
Very good point, and exactly the kind of feedback I was hoping for when I
marked this as an RFC. Thank you!
> Having realized this I went to look at Elijah's notes and they recognize
> this possibility and suggest extending the xdiff merge code to detect
> when N has conflicts that do not correspond to the conflicts in M. That
> sounds like quite a lot of work. I've not put much effort into coming up
> with a counterexample but think that because "git replay" and "git
> history" do not yet allow the commits in the merged branches to be
> edited we may be able to safely use the implementation proposed in this
> series if both merge parents have been rebased (or we might want all the
> merge bases of the new merge to be a descendants of "--onto"). In the
> example above if both the parents were rebased onto E then any new
> conflicts would happen when picking D rather than when recreating the
> merge.
Right. I have to admit that I missed this corner-case when I looked at the
original notes.
And while `git history`'s `reword` and `split` subcommands won't be
affected, the upcoming `fixup` subcommand _will_ be affected.
I am reworking the patches as we speak, loosely following Elijah's notes.
So far, I'm confident that this will address that problem.
What I am not confident at all so far (because I'm still trying to get the
actual algorithm to work, and haven't had a chance to test this on
real-world scenarios) is that the _conflict output_ is helpful. That is,
whether the conflict markers in case of corner-cases (merge conflicts in
R overlapping with merge conflicts in N, but not being identical, for
example) are clear enough to act upon, or will only lead to despair in the
keen reader.
For example, I noticed that a merge conflict resolution in O that is no
longer necessary in N leads to a quite unhelpful output...
I know that `git replay` is not designed as an interactive tool, but `git
history` is, and will ultimately _have_ to find ways to surface such merge
conflicts and help the user resolve them and then continue the replay.
For now, however, I do agree that we need to capture the error modes
correctly.
Ciao,
Johannes
>
> Thanks
>
> Phillip
>
> > Implement `pick_merge_commit()` along those lines and dispatch to it
> > from `replay_revisions()` when the commit being replayed has exactly
> > two parents. Two specific points (learned the hard way) keep
> > non-trivial cases working where the WIP sketch [2] bailed out.
> > First, R and N use identical `merge_options.branch1` and `branch2`
> > labels ("ours"/"theirs"). When the original parents conflicted on a
> > region of a file, both R and N produce textually identical conflict
> > markers; the outer non-recursive merge then sees N == R in that
> > region and the user's manual resolution from O wins cleanly. Without
> > this, the conflict-marker text would differ between R and N (because
> > the inner merges would label the conflicts differently), and the
> > outer merge would itself be unclean even when the user did supply a
> > clean resolution. Second, an unclean inner merge
> > (`result.clean == 0`) is _not_ fatal: the tree merge-ort produces in
> > that case still has well-defined contents (with conflict markers in
> > the conflicted files) and is a valid input to the outer
> > non-recursive merge. Only a real error (`< 0`) propagates as
> > failure.
> >
> > The replay propagates the textual diffs the user actually made in M;
> > it does _not_ extrapolate symbol-level intent. If rewriting the
> > parents pulls in genuinely new content (for example, a brand-new
> > caller of a function that the merge renamed), that new content stays
> > as the rewritten parents have it. Symbol-aware refactoring is out of
> > scope here, just as it is for plain rebase.
> >
> > Octopus merges (more than two parents) and revert-of-merge are not
> > supported and are surfaced as explicit errors at the dispatch point.
> > The "split" sub-command of `git history` continues to refuse when
> > the targeted commit is itself a merge: split semantics do not apply
> > to merges. The pre-walk gate in `builtin/history.c` that previously
> > rejected any merge in the rewrite path now only rejects octopus
> > merges; rename it accordingly.
> >
> > A small refactor in `create_commit()` makes the merge case possible:
> > the helper now takes a `struct commit_list *parents` rather than a
> > single parent pointer and takes ownership of the list. The single
> > existing caller in `pick_regular_commit()` builds and passes a
> > one-element list; the new `pick_merge_commit()` builds a two-element
> > list, with the order of the `from` and `merge` parents preserved.
> >
> > Update the negative expectations in t3451, t3452 and t3650 that were
> > asserting the now-retired "not supported yet" message, replacing
> > them with positive coverage where it fits. Octopus rejection and
> > revert-of-merge rejection are covered by new positive tests in
> > t3650. A dedicated test script with merge-replay scenarios driven by
> > a new test-tool fixture builder will follow in a subsequent commit.
> >
> > [1] https://github.com/newren/git/blob/replay/replay-design-notes.txt
> > [2]
> > https://github.com/newren/git/commit/4c45e8955ef9bf7d01fd15d9106b3bdb8ea91b45
> >
> > Helped-by: Elijah Newren <newren@gmail.com>
> > Assisted-by: Claude Opus 4.7
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> > builtin/history.c | 16 ++-
> > replay.c | 209 ++++++++++++++++++++++++++++++++++++--
> > t/t3451-history-reword.sh | 21 ++--
> > t/t3452-history-split.sh | 6 +-
> > t/t3650-replay-basics.sh | 46 ++++++++-
> > 5 files changed, 269 insertions(+), 29 deletions(-)
> >
> > diff --git a/builtin/history.c b/builtin/history.c
> > index 9526938085..00097b2226 100644
> > --- a/builtin/history.c
> > +++ b/builtin/history.c
> > @@ -195,15 +195,15 @@ static int parse_ref_action(const struct option *opt,
> > const char *value, int uns
> > return 0;
> > }
> >
> > -static int revwalk_contains_merges(struct repository *repo,
> > - const struct strvec *revwalk_args)
> > +static int revwalk_contains_octopus_merges(struct repository *repo,
> > + const struct strvec *revwalk_args)
> > {
> > struct strvec args = STRVEC_INIT;
> > struct rev_info revs;
> > int ret;
> >
> > strvec_pushv(&args, revwalk_args->v);
> > - strvec_push(&args, "--min-parents=2");
> > + strvec_push(&args, "--min-parents=3");
> >
> > repo_init_revisions(repo, &revs, NULL);
> > @@ -217,7 +217,7 @@ static int revwalk_contains_merges(struct repository
> > *repo,
> > }
> >
> > if (get_revision(&revs)) {
> > - ret = error(_("replaying merge commits is not supported
> > yet!"));
> > + ret = error(_("replaying octopus merges is not supported"));
> > goto out;
> > }
> > @@ -289,7 +289,7 @@ static int setup_revwalk(struct repository *repo,
> > strvec_push(&args, "HEAD");
> > }
> > - ret = revwalk_contains_merges(repo, &args);
> > + ret = revwalk_contains_octopus_merges(repo, &args);
> > if (ret < 0)
> > goto out;
> > @@ -482,6 +482,9 @@ static int cmd_history_reword(int argc,
> > if (ret < 0) {
> > ret = error(_("failed replaying descendants"));
> > goto out;
> > + } else if (ret) {
> > + ret = error(_("conflict during replay; some descendants were
> > not rewritten"));
> > + goto out;
> > }
> >
> > ret = 0;
> > @@ -721,6 +724,9 @@ static int cmd_history_split(int argc,
> > if (ret < 0) {
> > ret = error(_("failed replaying descendants"));
> > goto out;
> > + } else if (ret) {
> > + ret = error(_("conflict during replay; some descendants were
> > not rewritten"));
> > + goto out;
> > }
> >
> > ret = 0;
> > diff --git a/replay.c b/replay.c
> > index f96f1f6551..3dbce095f9 100644
> > --- a/replay.c
> > +++ b/replay.c
> > @@ -1,6 +1,7 @@
> > #define USE_THE_REPOSITORY_VARIABLE
> >
> > #include "git-compat-util.h"
> > +#include "commit-reach.h"
> > #include "environment.h"
> > #include "hex.h"
> > #include "merge-ort.h"
> > @@ -77,15 +78,21 @@ static void generate_revert_message(struct strbuf *msg,
> > repo_unuse_commit_buffer(repo, commit, message);
> > }
> >
> > +/*
> > + * Build a new commit with the given tree and parent list, copying author,
> > + * extra headers and (for pick mode) the commit message from `based_on`.
> > + *
> > + * Takes ownership of `parents`: it will be freed before returning, even on
> > + * error. Parent order is preserved as supplied by the caller.
> > + */
> > static struct commit *create_commit(struct repository *repo,
> > struct tree *tree,
> > struct commit *based_on,
> > - struct commit *parent,
> > + struct commit_list *parents,
> > enum replay_mode mode)
> > {
> > struct object_id ret;
> > struct object *obj = NULL;
> > - struct commit_list *parents = NULL;
> > char *author = NULL;
> > char *sign_commit = NULL; /* FIXME: cli users might want to sign again */
> > struct commit_extra_header *extra = NULL;
> > @@ -96,7 +103,6 @@ static struct commit *create_commit(struct repository
> > *repo,
> > const char *orig_message = NULL;
> > const char *exclude_gpgsig[] = { "gpgsig", "gpgsig-sha256", NULL };
> > - commit_list_insert(parent, &parents);
> > extra = read_commit_extra_headers(based_on, exclude_gpgsig);
> > if (mode == REPLAY_MODE_REVERT) {
> > generate_revert_message(&msg, based_on, repo);
> > @@ -273,6 +279,7 @@ static struct commit *pick_regular_commit(struct
> > repository *repo,
> > {
> > struct commit *base, *replayed_base;
> > struct tree *pickme_tree, *base_tree, *replayed_base_tree;
> > + struct commit_list *parents = NULL;
> >
> > if (pickme->parents) {
> > base = pickme->parents->item;
> > @@ -327,7 +334,143 @@ static struct commit *pick_regular_commit(struct
> > repository *repo,
> > if (oideq(&replayed_base_tree->object.oid, &result->tree->object.oid) &&
> > !oideq(&pickme_tree->object.oid, &base_tree->object.oid))
> > return replayed_base;
> > - return create_commit(repo, result->tree, pickme, replayed_base, mode);
> > + commit_list_insert(replayed_base, &parents);
> > + return create_commit(repo, result->tree, pickme, parents, mode);
> > +}
> > +
> > +/*
> > + * Replay a 2-parent merge commit by composing three calls into merge-ort:
> > + *
> > + * R = recursive merge of pickme's two original parents (auto-remerge of
> > + * the original merge, accepting any conflicts)
> > + * N = recursive merge of the (possibly rewritten) parents
> > + * O = pickme's tree (the user's actual merge, including any manual
> > + * resolutions)
> > + *
> > + * The picked tree comes from a non-recursive merge using R as the base,
> > + * O as side1 and N as side2. `git diff R O` is morally `git show
> > + * --remerge-diff $oldmerge`, so this layers the user's original manual
> > + * resolution on top of the freshly auto-merged rewritten parents (see
> > + * `replay-design-notes.txt` on the `replay` branch of newren/git).
> > + *
> > + * If the outer 3-way merge is unclean, propagate the conflict status to
> > + * the caller via `result->clean = 0` and return NULL. The two inner
> > + * merges (R and N) being unclean is _not_ fatal: the conflict-markered
> > + * trees they produce are valid inputs to the outer merge, and using
> > + * identical labels for both inner merges keeps the marker text
> > + * byte-equal between R and N so the user's resolution recorded in O
> > + * collapses the conflict cleanly there. Octopus merges (more than two
> > + * parents) and revert-of-merge are rejected by the caller before this
> > + * function is invoked.
> > + */
> > +static struct commit *pick_merge_commit(struct repository *repo,
> > + struct commit *pickme,
> > + kh_oid_map_t *replayed_commits,
> > + struct merge_options *merge_opt,
> > + struct merge_result *result)
> > +{
> > + struct commit *parent1, *parent2;
> > + struct commit *replayed_par1, *replayed_par2;
> > + struct tree *pickme_tree;
> > + struct merge_options remerge_opt = { 0 };
> > + struct merge_options new_merge_opt = { 0 };
> > + struct merge_result remerge_res = { 0 };
> > + struct merge_result new_merge_res = { 0 };
> > + struct commit_list *parent_bases = NULL;
> > + struct commit_list *replayed_bases = NULL;
> > + struct commit_list *parents;
> > + struct commit *picked = NULL;
> > + char *ancestor_name = NULL;
> > +
> > + parent1 = pickme->parents->item;
> > + parent2 = pickme->parents->next->item;
> > +
> > + /*
> > + * Map the merge's parents to their replayed counterparts. With the
> > + * boundary commits pre-seeded into `replayed_commits`, every parent
> > + * either has an explicit mapping (rewritten or boundary -> onto) or
> > + * sits outside the rewrite range entirely; the latter must stay at
> > + * the original parent commit, so use `parent` itself as the fallback
> > + * for both sides.
> > + */
> > + replayed_par1 = mapped_commit(replayed_commits, parent1, parent1);
> > + replayed_par2 = mapped_commit(replayed_commits, parent2, parent2);
> > +
> > + /*
> > + * R: auto-remerge of the original parents.
> > + *
> > + * Use the same branch labels for the inner merges that compute R
> > + * and N so conflict markers (if any) are textually identical
> > + * between the two; the outer non-recursive merge can then collapse
> > + * the manual resolution from O against them.
> > + */
> > + init_basic_merge_options(&remerge_opt, repo);
> > + remerge_opt.show_rename_progress = 0;
> > + remerge_opt.branch1 = "ours";
> > + remerge_opt.branch2 = "theirs";
> > + if (repo_get_merge_bases(repo, parent1, parent2, &parent_bases) < 0) {
> > + result->clean = -1;
> > + goto out;
> > + }
> > + merge_incore_recursive(&remerge_opt, parent_bases,
> > + parent1, parent2, &remerge_res);
> > + parent_bases = NULL; /* consumed by merge_incore_recursive */
> > + if (remerge_res.clean < 0) {
> > + result->clean = remerge_res.clean;
> > + goto out;
> > + }
> > +
> > + /* N: fresh merge of the (possibly rewritten) parents. */
> > + init_basic_merge_options(&new_merge_opt, repo);
> > + new_merge_opt.show_rename_progress = 0;
> > + new_merge_opt.branch1 = "ours";
> > + new_merge_opt.branch2 = "theirs";
> > + if (repo_get_merge_bases(repo, replayed_par1, replayed_par2,
> > + &replayed_bases) < 0) {
> > + result->clean = -1;
> > + goto out;
> > + }
> > + merge_incore_recursive(&new_merge_opt, replayed_bases,
> > + replayed_par1, replayed_par2, &new_merge_res);
> > + replayed_bases = NULL; /* consumed by merge_incore_recursive */
> > + if (new_merge_res.clean < 0) {
> > + result->clean = new_merge_res.clean;
> > + goto out;
> > + }
> > +
> > + /*
> > + * Outer non-recursive merge: base=R, side1=O (pickme), side2=N.
> > + */
> > + pickme_tree = repo_get_commit_tree(repo, pickme);
> > + ancestor_name = xstrfmt("auto-remerge of %s",
> > + oid_to_hex(&pickme->object.oid));
> > + merge_opt->ancestor = ancestor_name;
> > + merge_opt->branch1 = short_commit_name(repo, pickme);
> > + merge_opt->branch2 = "merge of replayed parents";
> > + merge_incore_nonrecursive(merge_opt,
> > + remerge_res.tree,
> > + pickme_tree,
> > + new_merge_res.tree,
> > + result);
> > + merge_opt->ancestor = NULL;
> > + merge_opt->branch1 = NULL;
> > + merge_opt->branch2 = NULL;
> > + if (!result->clean)
> > + goto out;
> > +
> > + parents = NULL;
> > + commit_list_insert(replayed_par2, &parents);
> > + commit_list_insert(replayed_par1, &parents);
> > + picked = create_commit(repo, result->tree, pickme, parents,
> > + REPLAY_MODE_PICK);
> > +
> > +out:
> > + free(ancestor_name);
> > + free_commit_list(parent_bases);
> > + free_commit_list(replayed_bases);
> > + merge_finalize(&remerge_opt, &remerge_res);
> > + merge_finalize(&new_merge_opt, &new_merge_res);
> > + return picked;
> > }
> >
> > void replay_result_release(struct replay_result *result)
> > @@ -407,17 +550,63 @@ int replay_revisions(struct rev_info *revs,
> > merge_opt.show_rename_progress = 0;
> > last_commit = onto;
> > replayed_commits = kh_init_oid_map();
> > +
> > + /*
> > + * Seed the rewritten-commit map with each negative-side ("BOTTOM")
> > + * cmdline entry pointing at `onto`. This matters for merge replay:
> > + * a 2-parent merge whose first parent is the boundary (e.g. the
> > + * commit being reworded) must replay onto the rewritten boundary,
> > + * yet pick_merge_commit uses a self fallback so the second parent
> > + * (a side branch outside the rewrite range) is preserved as-is.
> > + * Pre-seeding the boundary disambiguates the two: in the map ->
> > + * rewritten, missing -> kept as-is.
> > + *
> > + * Only do this for the pick path; revert mode chains reverts
> > + * through last_commit and a pre-seeded boundary would short-circuit
> > + * that chain.
> > + */
> > + if (mode == REPLAY_MODE_PICK) {
> > + for (size_t i = 0; i < revs->cmdline.nr; i++) {
> > + struct rev_cmdline_entry *e = &revs->cmdline.rev[i];
> > + struct commit *boundary;
> > + khint_t pos;
> > + int hr;
> > +
> > + if (!(e->flags & BOTTOM))
> > + continue;
> > + boundary = lookup_commit_reference_gently(revs->repo,
> > +
> > &e->item->oid, 1);
> > + if (!boundary)
> > + continue;
> > + pos = kh_put_oid_map(replayed_commits,
> > + boundary->object.oid, &hr);
> > + if (hr != 0)
> > + kh_value(replayed_commits, pos) = onto;
> > + }
> > + }
> > +
> > while ((commit = get_revision(revs))) {
> > const struct name_decoration *decoration;
> > khint_t pos;
> > int hr;
> > - if (commit->parents && commit->parents->next)
> > - die(_("replaying merge commits is not supported
> > yet!"));
> > -
> > - last_commit = pick_regular_commit(revs->repo, commit,
> > replayed_commits,
> > - mode == REPLAY_MODE_REVERT ?
> > last_commit : onto,
> > - &merge_opt, &result, mode);
> > + if (commit->parents && commit->parents->next) {
> > + if (commit->parents->next->next) {
> > + ret = error(_("replaying octopus merges is not
> > supported"));
> > + goto out;
> > + }
> > + if (mode == REPLAY_MODE_REVERT) {
> > + ret = error(_("reverting merge commits is not
> > supported"));
> > + goto out;
> > + }
> > + last_commit = pick_merge_commit(revs->repo, commit,
> > + replayed_commits,
> > + &merge_opt, &result);
> > + } else {
> > + last_commit = pick_regular_commit(revs->repo, commit,
> > replayed_commits,
> > + mode ==
> > REPLAY_MODE_REVERT ? last_commit : onto,
> > + &merge_opt, &result,
> > mode);
> > + }
> > if (!last_commit)
> > break;
> > diff --git a/t/t3451-history-reword.sh b/t/t3451-history-reword.sh
> > index de7b357685..d103f866a2 100755
> > --- a/t/t3451-history-reword.sh
> > +++ b/t/t3451-history-reword.sh
> > @@ -201,12 +201,21 @@ test_expect_success 'can reword a merge commit' '
> > git switch - &&
> > git merge theirs &&
> > - # It is not possible to replay merge commits embedded in the
> > - # history (yet).
> > - test_must_fail git -c core.editor=false history reword HEAD~
> > 2>err &&
> > - test_grep "replaying merge commits is not supported yet" err
> > &&
> > + # Reword a non-merge commit whose descendants include the
> > + # merge: replay carries the merge through.
> > + reword_with_message HEAD~ <<-EOF &&
> > + ours reworded
> > + EOF
> > + expect_graph <<-EOF &&
> > + * Merge tag ${SQ}theirs${SQ}
> > + |\\
> > + | * theirs
> > + * | ours reworded
> > + |/
> > + * base
> > + EOF
> > - # But it is possible to reword a merge commit directly.
> > + # And reword a merge commit directly.
> > reword_with_message HEAD <<-EOF &&
> > Reworded merge commit
> > EOF
> > @@ -214,7 +223,7 @@ test_expect_success 'can reword a merge commit' '
> > * Reworded merge commit
> > |\
> > | * theirs
> > - * | ours
> > + * | ours reworded
> > |/
> > * base
> > EOF
> > diff --git a/t/t3452-history-split.sh b/t/t3452-history-split.sh
> > index 8ed0cebb50..ad6309f98b 100755
> > --- a/t/t3452-history-split.sh
> > +++ b/t/t3452-history-split.sh
> > @@ -36,7 +36,7 @@ expect_tree_entries () {
> > test_cmp expect actual
> > }
> >
> > -test_expect_success 'refuses to work with merge commits' '
> > +test_expect_success 'refuses to split a merge commit' '
> > test_when_finished "rm -rf repo" &&
> > git init repo &&
> > (
> > @@ -49,9 +49,7 @@ test_expect_success 'refuses to work with merge commits' '
> > git switch - &&
> > git merge theirs &&
> > test_must_fail git history split HEAD 2>err &&
> > - test_grep "cannot split up merge commit" err &&
> > - test_must_fail git history split HEAD~ 2>err &&
> > - test_grep "replaying merge commits is not supported yet" err
> > + test_grep "cannot split up merge commit" err
> > )
> > '
> >
> > diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh
> > index 3353bc4a4d..368b1b0f9a 100755
> > --- a/t/t3650-replay-basics.sh
> > +++ b/t/t3650-replay-basics.sh
> > @@ -103,10 +103,48 @@ test_expect_success 'cannot advance target ...
> > ordering would be ill-defined' '
> > test_cmp expect actual
> > '
> >
> > -test_expect_success 'replaying merge commits is not supported yet' '
> > - echo "fatal: replaying merge commits is not supported yet!" >expect &&
> > - test_must_fail git replay --advance=main main..topic-with-merge
> > 2>actual &&
> > - test_cmp expect actual
> > +test_expect_success 'using replay to rebase a 2-parent merge' '
> > + # main..topic-with-merge contains a 2-parent merge (P) introduced
> > + # via test_merge. Use --ref-action=print so this test does not
> > + # mutate state for subsequent tests in this file.
> > + git replay --ref-action=print --onto main main..topic-with-merge
> > >result &&
> > + test_line_count = 1 result &&
> > +
> > + new_tip=$(cut -f 3 -d " " result) &&
> > +
> > + # Result is still a 2-parent merge.
> > + git cat-file -p $new_tip >cat &&
> > + grep -c "^parent " cat >count &&
> > + echo 2 >expect &&
> > + test_cmp expect count &&
> > +
> > + # Merge subject is preserved.
> > + echo P >expect &&
> > + git log -1 --format=%s $new_tip >actual &&
> > + test_cmp expect actual &&
> > +
> > + # The replayed merge sits on top of main: walking back via the
> > + # first-parent chain reaches main.
> > + git merge-base --is-ancestor main $new_tip
> > +'
> > +
> > +test_expect_success 'replaying an octopus merge is rejected' '
> > + # Build an octopus side-branch so the rest of the test state stays
> > + # untouched.
> > + test_when_finished "git update-ref -d refs/heads/octopus-tip" &&
> > + octopus_tip=$(git commit-tree -p topic4 -p topic1 -p topic3 \
> > + -m "octopus" $(git rev-parse topic4^{tree})) &&
> > + git update-ref refs/heads/octopus-tip "$octopus_tip" &&
> > +
> > + test_must_fail git replay --ref-action=print --onto main \
> > + topic4..octopus-tip 2>actual &&
> > + test_grep "octopus merges" actual
> > +'
> > +
> > +test_expect_success 'reverting a merge commit is rejected' '
> > + test_must_fail git replay --ref-action=print --revert=topic-with-merge
> > \
> > + topic4..topic-with-merge 2>actual &&
> > + test_grep "reverting merge commits" actual
> > '
> >
> > test_expect_success 'using replay to rebase two branches, one on top of
> > other' '
>
>
^ permalink raw reply
* [PATCH v2] config: retry acquiring config.lock, configurable via core.configLockTimeout
From: Joerg Thalheim @ 2026-05-17 13:21 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
Jörg Thalheim
In-Reply-To: <409d05a5-235b-6b19-5a33-a4e613dd447c@gmx.de>
From: Jörg Thalheim <joerg@thalheim.io>
Concurrent config writers race for the ".lock" file, which is taken
with open(O_EXCL) and no retry, so the losers fail right away with
"could not lock config file".
This shows up with parallel "git worktree add -b" against the same
repository: each one writes a couple of branch.* keys and the losers
fail at random. Worse, "git worktree add" doesn't propagate that
failure to its exit code, so the tracking config is silently dropped.
(The swallowed error is a separate bug.)
Retry instead of giving up on the first EEXIST. The lock is only held
while rewriting a small file, so the loser only has to wait out the
other writers. Same approach as 4ff0f01cb7 (refs: retry acquiring
reference locks for 100ms, 2017-08-21).
On the semantics: the on-disk config is read only after the lock is
taken, so writers touching different keys can't lose each other's
change. Writers touching the same key still get last-writer-wins, but
that is already the case today and would need a compare-and-swap config
API to fix. The retry only turns hard failures into successes.
Default to 1000ms, like core.packedRefsTimeout: same shape of problem,
one shared file everyone serializes through. A larger timeout only
costs anything when a stale lock is left behind by a crash, which is
rare; a smaller one fails spuriously on slow filesystems (NTFS has
been seen needing more than 100ms). Make it configurable as
core.configLockTimeout. There is no chicken-and-egg problem: we read
the config before we lock it.
microsoft/git carries a similar patch (core.configWriteLockTimeoutMS,
default off) for Scalar's tests. Defaulting to non-zero here because
the worktree case fails silently.
Helped-by: Patrick Steinhardt <ps@pks.im>
Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Jörg Thalheim <joerg@thalheim.io>
---
Thanks for the review and for poking me, this had fallen off my radar.
v1 -> v2:
- added core.configLockTimeout. Johannes is right that there is no
chicken-and-egg problem (config is read before the lock), so no env
var needed.
- default bumped to 1000ms; packed-refs is the closer precedent and it
keeps NTFS out of trouble.
- commit message now covers the read-after-lock / last-writer-wins
semantics Patrick asked about.
- added tests; existing stale-lock tests in t3200/t5505 now pass
-c core.configLockTimeout=0 so they still fail fast.
I matched the core.filesRefLockTimeout naming rather than reusing
microsoft/git's core.configWriteLockTimeoutMS, but can switch if the
downstream compat matters more.
Documentation/config/core.adoc | 8 ++++++++
config.c | 24 ++++++++++++++++++++++--
t/t1300-config.sh | 17 +++++++++++++++++
t/t3200-branch.sh | 6 ++++--
t/t5505-remote.sh | 3 ++-
5 files changed, 53 insertions(+), 5 deletions(-)
diff --git a/Documentation/config/core.adoc b/Documentation/config/core.adoc
index a0ebf03e2e..340329edc3 100644
--- a/Documentation/config/core.adoc
+++ b/Documentation/config/core.adoc
@@ -589,6 +589,14 @@ core.packedRefsTimeout::
all; -1 means to try indefinitely. Default is 1000 (i.e.,
retry for 1 second).
+core.configLockTimeout::
+ The length of time, in milliseconds, to retry when trying to
+ lock a configuration file for writing. Value 0 means not to
+ retry at all; -1 means to try indefinitely. Default is 1000
+ (i.e., retry for 1 second). This is read from the configuration
+ that is already on disk before the lock is taken, so it can be
+ set persistently like any other option.
+
core.pager::
Text viewer for use by Git commands (e.g., 'less'). The value
is meant to be interpreted by the shell. The order of preference
diff --git a/config.c b/config.c
index 156f2a24fa..ac9563781b 100644
--- a/config.c
+++ b/config.c
@@ -2903,6 +2903,24 @@ char *git_config_prepare_comment_string(const char *comment)
return prepared;
}
+/*
+ * How long to retry acquiring config.lock when another process holds
+ * it. Default matches core.packedRefsTimeout; override via
+ * core.configLockTimeout.
+ */
+static long config_lock_timeout_ms(struct repository *r)
+{
+ static int configured;
+ static int timeout_ms = 1000;
+
+ if (!configured) {
+ repo_config_get_int(r, "core.configlocktimeout", &timeout_ms);
+ configured = 1;
+ }
+
+ return timeout_ms;
+}
+
static void validate_comment_string(const char *comment)
{
size_t leading_blanks;
@@ -2986,7 +3004,8 @@ int repo_config_set_multivar_in_file_gently(struct repository *r,
* The lock serves a purpose in addition to locking: the new
* contents of .git/config will be written into it.
*/
- fd = hold_lock_file_for_update(&lock, config_filename, 0);
+ fd = hold_lock_file_for_update_timeout(&lock, config_filename, 0,
+ config_lock_timeout_ms(r));
if (fd < 0) {
error_errno(_("could not lock config file %s"), config_filename);
ret = CONFIG_NO_LOCK;
@@ -3331,7 +3350,8 @@ static int repo_config_copy_or_rename_section_in_file(
if (!config_filename)
config_filename = filename_buf = repo_git_path(r, "config");
- out_fd = hold_lock_file_for_update(&lock, config_filename, 0);
+ out_fd = hold_lock_file_for_update_timeout(&lock, config_filename, 0,
+ config_lock_timeout_ms(r));
if (out_fd < 0) {
ret = error(_("could not lock config file %s"), config_filename);
goto out;
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 128971ee12..12a1cb8580 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -2939,4 +2939,21 @@ test_expect_success 'writing value with trailing CR not stripped on read' '
test_cmp expect actual
'
+test_expect_success 'writing config fails immediately with core.configLockTimeout=0' '
+ test_when_finished "rm -f .git/config.lock" &&
+ >.git/config.lock &&
+ test_must_fail git -c core.configLockTimeout=0 config foo.bar baz 2>err &&
+ test_grep "could not lock config file" err
+'
+
+test_expect_success 'writing config retries until lock is released' '
+ test_when_finished "rm -f .git/config.lock" &&
+ >.git/config.lock &&
+ {
+ ( sleep 1 && rm -f .git/config.lock ) &
+ } &&
+ git -c core.configLockTimeout=5000 config retried.key value &&
+ test "$(git config retried.key)" = value
+'
+
test_done
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index e7829c2c4b..5f8a31c21d 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -1037,7 +1037,8 @@ test_expect_success '--set-upstream-to fails on locked config' '
test_when_finished "rm -f .git/config.lock" &&
>.git/config.lock &&
git branch locked &&
- test_must_fail git branch --set-upstream-to locked 2>err &&
+ test_must_fail git -c core.configLockTimeout=0 \
+ branch --set-upstream-to locked 2>err &&
test_grep "could not lock config file .git/config" err
'
@@ -1068,7 +1069,8 @@ test_expect_success '--unset-upstream should fail if config is locked' '
test_when_finished "rm -f .git/config.lock" &&
git branch --set-upstream-to locked &&
>.git/config.lock &&
- test_must_fail git branch --unset-upstream 2>err &&
+ test_must_fail git -c core.configLockTimeout=0 \
+ branch --unset-upstream 2>err &&
test_grep "could not lock config file .git/config" err
'
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index e592c0bcde..aea9222649 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -1327,7 +1327,8 @@ test_expect_success 'remote set-url with locked config' '
test_when_finished "rm -f .git/config.lock" &&
git config --get-all remote.someremote.url >expect &&
>.git/config.lock &&
- test_must_fail git remote set-url someremote baz &&
+ test_must_fail git -c core.configLockTimeout=0 \
+ remote set-url someremote baz &&
git config --get-all remote.someremote.url >actual &&
cmp expect actual
'
--
2.53.0
^ permalink raw reply related
* Re: [PATCH/RFC 4/5] test-tool: add a "historian" subcommand for building merge fixtures
From: Johannes Schindelin @ 2026-05-17 11:40 UTC (permalink / raw)
To: Toon Claes
Cc: Johannes Schindelin via GitGitGadget, git, Elijah Newren,
Patrick Steinhardt
In-Reply-To: <87lddooq2t.fsf@toon--20250203-5JQV3.mail-host-address-is-not-set>
Hi Toon,
On Tue, 12 May 2026, Toon Claes wrote:
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > diff --git a/t/helper/test-historian.c b/t/helper/test-historian.c
> > new file mode 100644
> > index 0000000000..2250d420c0
> > --- /dev/null
> > +++ b/t/helper/test-historian.c
> > @@ -0,0 +1,189 @@
> > +/*
> > + * Build a small history out of a tiny declarative input. Used by tests
> > + * that need specific merge topologies without long sequences of
> > + * plumbing commands or fragile shell helpers.
> > + *
> > + * The historian reads stdin line by line and emits an equivalent
> > + * stream to a `git fast-import` child process. It also allocates marks
> > + * for named objects so tests can refer to commits and blobs by name.
>
> Really appreciate you're introducing this command. I'm actually
> surprised no else did before.
Heh. I am not surprised, though. Given that it is _really_ hard to come up
with a decent Domain-Specific Language to describe commit history for
defining test fixtures (testament to which are your comments below), I was
debating whether to go for range-diff's/libgit2's approach and simply
adding a bare repository or a fast-import script as a fixture. But that
would make the test code even harder to follow! And I did not want to add
to the amount of hard-to-follow test code.
> > + *
> > + * Input directives (one per line, shell-style quoting):
> > + *
> > + * blob NAME LINE1 LINE2 ...
> > + * Each LINE becomes a content line in the blob; lines are
> > + * joined with '\n' and the blob ends with a final '\n'. With
> > + * no LINEs, the blob is empty.
> > + *
> > + * commit NAME BRANCH SUBJECT [from=PARENT] [merge=PARENT]... [PATH=BLOB]...
>
> I'm not sure how I feel about mixing named arguments (like `from=PARENT`) with
> the `PATH=BLOB` arguments? Obviously this tool isn't made for anything
> that's even close to production, but still feels strange. How about
> putting a double dash (`--`) before the paths, or using the `PATH:BLOB`
> syntax instead?
Okay, I can see that's a better design. To be honest, I did not really
optimize for unambiguity here, but for precision of defining a test
fixture. As such, I am mostly interested in keeping the definition as
small as possible without losing readability. Changing that `=` to a `:`
to disambiguate keeps the same length while clarifying the structure. I
like it!
> > + * Creates a commit on refs/heads/BRANCH using the listed
> > + * file=blob mappings as the entire tree (no inheritance from
> > + * parents). Up to one `from=` and any number of `merge=`
> > + * parents may be given. `from=` defaults to the current branch
> > + * tip; if BRANCH has no tip yet, the commit becomes a root.
>
> At GitLab in our Gitaly suite we have a similar tool as what you're
> introducing here, but there you have to specify the parent(s) for each
> commit and if you want to assign a ref to a commit, you have to be
> explicit about it. So I would replace `from=` and `merge=` with
> `parent=` and allow that to be occur zero or more times (this would also
> allow creating unrelated histories). And remove the mandatory argument
> BRANCH, and instead allow the command to accept a `branch=` argument.
>
> If we'd take an example from the follow-up commit:
>
> # Setup:
> # A (a) --- C (a, h) ----+--- M (a, g, h)
> # \ /
> # +-- B (a, g) ------+
> #
> # Topic touches `g` only; main touches `h` only. The auto-merge
> # at M is clean.
> blob a "shared content"
> blob g guarded
> blob h host
> commit A main "A" a=a
> commit B topic "B (introduces g)" from=A a=a g=g
> commit C main "C (introduces h)" a=a h=h
> commit M main "Merge topic" merge=B a=a g=g h=h
>
> I would suggest to rewrite that to:
>
> blob a "shared content"
> blob g guarded
> blob h host
> commit A "A" a:a
> commit B "B (introduces g)" parent=A branch=topic a:a g:g
> commit C "C (introduces h)" parent=A a:a h:h
> commit M "Merge topic" parent=A parent=B ref=main a:a g:g h:h
>
> I realize this is less alike to git-fast-import(1), so I'd understand if
> you'd reject this idea.
That makes sense to me!
I'm not interested in keeping the `historian` code as small as possible, I
am interested in reducing the cognitive load required to read and write
those test fixtures. Therefore, the `parent=` way is much preferable.
Thank you!
Johannes
^ permalink raw reply
* Re: [PATCH/RFC 0/5] replay: support replaying 2-parent merges
From: Johannes Schindelin @ 2026-05-17 11:33 UTC (permalink / raw)
To: Ben Knoble
Cc: Johannes Schindelin via GitGitGadget, git, Elijah Newren,
Patrick Steinhardt
In-Reply-To: <21A507D3-1B0D-4404-8AF5-9485B01E63A6@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3157 bytes --]
Hi Ben,
On Thu, 7 May 2026, Ben Knoble wrote:
> > Le 7 mai 2026 à 11:06, Johannes Schindelin <johannes.schindelin@gmx.de> a écrit :
> >
> >> On Thu, 7 May 2026, D. Ben Knoble wrote:
> >>
> >>> On Wed, May 6, 2026 at 6:44 PM Johannes Schindelin via GitGitGadget
> >>> <gitgitgadget@gmail.com> wrote:
> >>>
> >>> [...]
> >>>
> >>> While I was at it, git history reword had a pre-existing
> >>> silent-success bug: a positive return from replay_revisions() (which
> >>> means "conflict, no updates queued") was treated as success. Obviously
> >>> this should never occur, as a reword simply does not change any of the
> >>> file contents, but bugs do happen. The merge-replay work is complex
> >>> enough to make that class of bugs more likely, therefore I introduce
> >>> error messages for those instances.
> >>
> >> Fixing this bug sounded interesting; I had a hard time spotting it
> >> while skimming the first 2 patches.
> >
> > It's this part:
> >
> > @@ -482,6 +482,9 @@ static int cmd_history_reword(int argc,
> > if (ret < 0) {
> > ret = error(_("failed replaying descendants"));
> > goto out;
> > + } else if (ret) {
> > + ret = error(_("conflict during replay; some descendants were not rewritten"));
> > + goto out;
> > }
> >
> > ret = 0;
> > @@ -721,6 +724,9 @@ static int cmd_history_split(int argc,
> > if (ret < 0) {
> > ret = error(_("failed replaying descendants"));
> > goto out;
> > + } else if (ret) {
> > + ret = error(_("conflict during replay; some descendants were not rewritten"));
> > + goto out;
> > }
> >
> > ret = 0;
>
> Thanks, super helpful.
>
> (Perhaps later) if we can say _which_ descendants weren’t rewritten, that might be good.
I am afraid that that particular information is lost at this point, all we
have to work with is an `int ret`.
Ciao,
Johannes
> >> Did I just miss it? Is it worth splitting that fix out to a separate patch?
> >
> > Well, you _could_ argue that they were not bugs at all: a `git history
> > reword` isn't supposed to be able to result in merge conflicts, nor is
> > `git history split` because they leave the respective commits tree-same
> > (in the `split` case, the second commit).
>
> I seem to recall Patrick making a similar argument, but don’t let me put
> words in anyone’s mouth.
>
> > I could see the point were anybody to suggest using `BUG()` instead of
> > `error()` here, but erred on the "nicer to the user" side.
> >
> > The only way this _might_ be triggered before this patch series is most
> > likely by playing games with replace objects. Or maybe you cannot trigger
> > it at all.
> >
> > With the changes in this here patch series, I wasn't so certain that I had
> > covered all the edge cases (an early iteration of the quick short-cut in
> > patch 2/5 keyed only on the parent commits' trees, and forgot to verify
> > the merge _bases_' trees, for example). That's why I think it matters more
> > now than it did before.
> >
> > Ciao,
> > Johannes
>
> Makes sense, thanks.
^ permalink raw reply
* Re: [PATCH] config: retry acquiring config.lock for 100ms
From: Johannes Schindelin @ 2026-05-17 11:27 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: Jörg Thalheim, Junio C Hamano, git
In-Reply-To: <agGo9Prt8Hs2gbic@pks.im>
[-- Attachment #1: Type: text/plain, Size: 6855 bytes --]
Hi Patrick & Jörg,
On Mon, 11 May 2026, Patrick Steinhardt wrote:
> On Mon, May 11, 2026 at 09:06:00AM +0000, Jörg Thalheim wrote:
> > May 11, 2026 at 4:32 AM, "Junio C Hamano" <gitster@pobox.com
> > mailto:gitster@pobox.com?to=%22Junio%20C%20Hamano%22%20%3Cgitster%40pobox.com%3E
> > > wrote:
> > > Patrick Steinhardt <ps@pks.im> writes:
> > > > > This bites in practice when running `git worktree add -b` concurrently
> > > > > against the same repository. Each invocation makes several writes to
> > > > > ".git/config" to set up branch tracking, and tooling that creates
> > > > > worktrees in parallel sees intermittent failures. Worse, `git worktree
> > > > > add` does not propagate the failed config write to its exit code: the
> > > > > worktree is created and the command exits 0, but tracking
> > > > > configuration is silently dropped.
> > > > >
> > > > This very much sounds like a bug that is worth fixing independently.
> > > >
> > > > >
> > > > > The lock is held only for the duration of rewriting a small file, so
> > > > > retrying for 100 ms papers over any realistic contention while still
> > > > > failing fast if a stale lock has been left behind by a crashed
> > > > > process. This mirrors what we already do for individual reference
> > > > > locks (4ff0f01cb7 (refs: retry acquiring reference locks for 100ms,
> > > > > 2017-08-21)).
> > > > >
> > > > Famous last words :) Experience tells me that any timeout value that
> > > > isn't excessive will eventually be hit in some production system. Which
> > > > raises the question whether we want to make the timeout configurable,
> > > > similar to "core.filesRefLockTimeout" and "core.packedRefsTimeout".
> > > > ...
> > > > Honestly though, I'm not really sure what to make with this. We could
> > > > of course also add some validation that the configuration we want to set
> > > > hasn't been modified meanwhile. But that would now lead to a situation
> > > > where we have to update every single caller in our tree to make use of
> > > > the new mechanism, which would be a bunch of work.
> > > >
> > > > And adding the timeout doesn't really change the status quo, either. We
> > > > already have the case that we'll happily overwrite changes made by
> > > > concurrent processes. The only thing that changes is that we make it
> > > > more likely for concurrent changes to succeed.
> > > >
> > > We haven't heard any response to these points raised in the message
> > > I am responding to. Should I still keep the patch in my tree,
> > > hoping that a responses may come some day? I am tempted to discard
> > > the topic as it has been quite a while since we last looked at it.
> >
> > I am not really sure what you want me to do here.
>
> In general, the idea here is to engage in a discussion that can
> ultimately lead to one of two outcomes:
>
> - The discussion surfaces an area the author hasn't thought about, so
> the patch is adapted accordingly.
>
> - The discussion shows that the author already did think about the
> issue, but hasn't documented the assumptions. In this case, it
> should be the commit message that gets adapted.
For what it's worth, I meant to chime in earlier, but obligations kept
preventing me from setting aside the time to do so. Well, better late than
never.
> > I don't see how git can have this value configurable, given it's about
> > reading the configuration itself. Is the user supposed via command
> > line?
>
> This is a fair point indeed. But if it's not possible to change via the
> configuration itself, then the next-best thing might be to introduce an
> environment variable that allows configuring it.
Well, given that the config is read first before it's written, it is
totally possible to configure a timeout via the config, and I have some
real-world proof that this works as intended (see below).
> The other aspect that wasn't discussed in the commit message is how
> concurrent writes are handled, both when they are non-conflicting
> (updating different keys) and when they are conflicting (updating the
> same key). After spending some more time in the code I think it's
> ultimately nothing we have to worry about too much, as we only start
> reading the configuration after we've locked it.
Correct. I had performed this analysis myself when writing a similar patch
to fix problems in Scalar's Functional Test suite, which wants to register
_many_ Scalar repositories with ~/.gitconfig concurrently. The current
iteration of the patch can be found here:
https://github.com/microsoft/git/commit/a1c2d97cb61bc3697086d1749de848586df2ec54
It does include the config setting, leaving the default as "off" (but I
missed the separate code path to rename sections, which has _independent_
code that also wants to lock the config file, which your patch did not
miss). The subsequent child commit
https://github.com/microsoft/git/commit/5d365c1f332b8d2214ae9c44970d6370ed9caffc
configures it to 150ms in Scalar repositories only. This is notably larger
than the 100ms you suggested, and it is rooted in the fact that NTFS I/O
characteristics are unfortunately in need of a wider margin. In other
words, the optimal value depends on the operating system (and the CPU
load, as Junio had pointed out).
For the record, feel free to adopt whatever you want from my patches for
your next iteration (but also feel free to ignore all of it).
> So in the semantically non-conflicting case there isn't really much of a
> race, because things already work as expected. But in the semantically
> conflicting case it's a bit different, as the latter writer will
> overwrite the result of the former one. In theory it would be possible
> to detect such conflicts by:
>
> - Reading the configuration file.
>
> - Taking the lock.
>
> - Rereading the configuration to check for conflicts.
>
> But even that is racy as the first writer might have succeeded before we
> read the configuration the first time. So I'm not sure whether we can do
> anything about that in the first place, as the race basically exists in
> the outer loop controlled by the caller.
>
> So there probably isn't much we can do about that, and unless I missed
> something I think your timeout is sensible. But ideally, such nuances
> would be discussed as part of the commit message so that reviewers and
> future readers are made aware of them.
I agree. Complex cases like this would require a sort of transactional
support to be added to `git config`, and that would in and of itself open
a can of worms I'm not sure we should open unless there is a concrete use
case that bites enough real-world scenarios to require acting upon.
Ciao,
Johannes
^ permalink raw reply
* git rebase --continue segfault
From: Alex Naidenkov @ 2026-05-17 9:19 UTC (permalink / raw)
To: git
Hi, ive encountered on segfault when ran `git rebase --continue`.
Hopefully this would help
- i was in the middle of big rebase
- entered pin for signing commit
- segfault happened
```
Debuginfo Build ID: d98c557aaa4baa2e6da8a12cf5a76d241c5af104
```
```
gef➤ bt
#0 repo_parse_tree_gently (r=0x557e86b43780 <the_repo.lto_priv>,
item=0x0, quiet_on_missing=0x0) at /usr/src/debug/git/git/tree.c:193
#1 0x0000557e869161bd in repo_parse_tree (r=<optimized out>,
item=<optimized out>) at /usr/src/debug/git/git/tree.h:28
#2 collect_merge_info (opt=0x557e9bf793b0, merge_base=<optimized out>,
side1=<optimized out>, side2=<optimized out>) at
/usr/src/debug/git/git/merge-ort.c:1745
#3 merge_ort_nonrecursive_internal (opt=opt@entry=0x7ffe20ae9eb0,
merge_base=<optimized out>, merge_base@entry=0x557e9bf793b0,
side1=side1@entry=0x557e9bf79430, side2=<optimized out>,
side2@entry=0x0, result=result@entry=0x7ffe20ae9e80) at
/usr/src/debug/git/git/merge-ort.c:5256
#4 0x0000557e8691a6b8 in merge_incore_nonrecursive (opt=0x7ffe20ae9eb0,
merge_base=0x557e9bf793b0, side1=0x557e9bf79430, side2=0x0,
result=0x7ffe20ae9e80)
at /usr/src/debug/git/git/merge-ort.c:5419
#5 0x0000557e869e0e8a in do_recursive_merge (r=r@entry=0x557e86b43780
<the_repo.lto_priv>, base=base@entry=0x557e9bf8b800,
next=next@entry=0x557e9bf8b850,
base_label=base_label@entry=0x557e9bf627e0 "parent of db33c0f
(fix)", next_label=next_label@entry=0x557e9bfab130 "db33c0f (fix)",
head=head@entry=0x7ffe20aea160,
msgbuf=0x557e9bf1b710, opts=0x7ffe20aeb990) at
/usr/src/debug/git/git/sequencer.c:782
#6 0x0000557e869e355e in do_pick_commit (r=0x557e86b43780
<the_repo.lto_priv>, item=<optimized out>, opts=0x7ffe20aeb990,
final_fixup=0x0, check_todo=0x7ffe20aea35c)
at /usr/src/debug/git/git/sequencer.c:2445
#7 0x0000557e869ebe75 in pick_one_commit (r=<optimized out>,
todo_list=0x7ffe20aeb330, opts=<optimized out>,
check_todo=0x7ffe20aea35c, reschedule=<synthetic pointer>)
at /usr/src/debug/git/git/sequencer.c:4921
#8 pick_commits (r=0x557e86b43780 <the_repo.lto_priv>,
todo_list=<optimized out>, opts=0x7ffe20aeb990) at
/usr/src/debug/git/git/sequencer.c:5030
#9 0x0000557e869ef336 in sequencer_continue (r=<optimized out>,
opts=<optimized out>) at /usr/src/debug/git/git/sequencer.c:5487
#10 0x0000557e867d536e in run_sequencer_rebase (opts=0x7ffe20aeb7a0) at
builtin/rebase.c:376
#11 run_specific_rebase (opts=0x7ffe20aeb7a0) at builtin/rebase.c:755
#12 cmd_rebase (argc=<optimized out>, argv=<optimized out>,
prefix=<optimized out>, repo=<optimized out>) at builtin/rebase.c:1910
#13 0x0000557e866e9e65 in run_builtin (p=0x557e86b35530
<commands.lto_priv+2352>, argc=<optimized out>, argv=<optimized out>,
repo=0x557e86b43780 <the_repo.lto_priv>)
at /usr/src/debug/git/git/git.c:506
#14 handle_builtin (args=args@entry=0x7ffe20aed760) at
/usr/src/debug/git/git/git.c:780
#15 0x0000557e866eb30c in run_argv (args=0x7ffe20aed760) at
/usr/src/debug/git/git/git.c:863
#16 cmd_main (argc=<optimized out>, argv=<optimized out>) at
/usr/src/debug/git/git/git.c:984
#17 0x0000557e866e77e4 in main (argc=0x3, argv=0x7ffe20aeda58) at
/usr/src/debug/git/git/common-main.c:9
```
```
[System Info]
git version:
git version 2.54.0
cpu: x86_64
built from commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
rust: enabled
gettext: enabled
libcurl: 8.19.0
OpenSSL: OpenSSL 3.6.2 7 Apr 2026
zlib-ng: 2.3.3
SHA-1: SHA1_DC
SHA-256: SHA256_BLK
default-ref-format: files
default-hash: sha1
uname: Linux 7.0.5-arch1-1 #1 SMP PREEMPT_DYNAMIC Fri, 08 May 2026
09:29:23 +0000 x86_64
compiler info: gnuc: 15.2
libc info: glibc: 2.43
$SHELL (typically, interactive shell): /usr/bin/zsh
[Enabled Hooks]
pre-commit
```
^ permalink raw reply
* Re: [GSoC RFC PATCH 0/1] graph: add indentation for commits preceded by a root
From: Chandra Pratap @ 2026-05-17 6:31 UTC (permalink / raw)
To: phillip.wood
Cc: Pablo Sabater, git, gitster, christian.couder, karthik.188,
jltobler, ayu.chandekar, siddharthasthana31
In-Reply-To: <2e8b9b1b-6a69-4e94-95ea-7f587435bfce@gmail.com>
Hi all,
On Fri, 15 May 2026 at 15:03, Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> On 14/05/2026 18:45, Pablo Sabater wrote:
> > El jue, 14 may 2026 a las 17:15, Phillip Wood
> > (<phillip.wood123@gmail.com>) escribió:
> >> On 02/04/2026 22:17, Pablo Sabater wrote:
> >>> When having a history with multiple root commits and drawing the history
> >>> near the roots, the graphing engine renders the commit one below the other,
> >>> seeming that they are related, which makes the graph confusing.
> >>>
> >>> This issue was reported by Junio at:
> >>> https://lore.kernel.org/git/xmqqikaawrpx.fsf@gitster.g/
> >>>
> >>> e.g.:
> >>>
> >>> * root-B
> >>> * child-A2
> >>> * child-A1
> >>> * root-A
> >>>
> >>> [...]
> >> >
> >>> * root-B
> >>> * child-A2
> >>> /
> >>> * child-A1
> >>> * root-A
> >>
> >> I'm rather late to the party here, but personally I find the indentation
> >> a bit confusing, it would be clearer to me if we had a blank line after
> >> a root commit
> >
> > Hi,
> >
> >>
> >> * root-B
> >>
> >> * child-A2
> >> * child-A1
> >> * root-A
> >>
> >> It takes the same amount of vertical space but keeps the children of
> >> root-A together.
> >
> > I have mixed feelings about which approach to choose.
> > The idea of a blank line was thought at
> > https://lore.kernel.org/git/xmqq8s8vvw9m.fsf@gitster.c.googlers.com/
> > but Junio argued against it for having an extra row because the
> > indentation he proposed didn't collapse, however I find indentation +
> > no collapse the most confusing one.
> > I'd say that I'm fine with both approaches, blank line or indentation
> > + collapse.
>
> I'm afraid I don't understand this - what does it mean for the
> indentation to collapse, or not collapse. Looking at the examples Junio
> gave they look quite nice to me, though I'd find it clearer if
>
>
> | | * 12345678 2021-01-14 merge xxxxx@xxxx into the history
> | | |\
> | | | \
> | | * \ 23456789 2021-01-12 merge citest into the main history
> | | |\ * 5505e019c2 2014-07-09 initial xxxxxx@xxxx
> | | | * 3e658f4085 2019-09-10 (wiki/wip-citest, origin/wip-citest)
> Added defau
> | | | * ad148aafe6 2019-09-10 Added default CI/CD Jenkinsfile (from
> f7daf088)
>
> was rendered as
>
>
> | | * 12345678 2021-01-14 merge xxxxx@xxxx into the history
> | | |\
> | | | * 5505e019c2 2014-07-09 initial xxxxxx@xxxx
> | | * 23456789 2021-01-12 merge citest into the main history
> | | |\
> | | | * 3e658f4085 2019-09-10 (wiki/wip-citest, origin/wip-citest)
> Added defau
> | | | * ad148aafe6 2019-09-10 Added default CI/CD Jenkinsfile (from
> f7daf088)
It probably *does* look clearer here, but I have the same reservations
against this as Junio: the break won't be as noticeable when --graph is
*not* used with --oneline.
> >>> without the patch:
> >>>
> >>> * A root
> >>> * B root
> >>> * C root
> >>> * D1 child
> >>> * D root
> >>>
> >>> with the patch, the indentation cascades:
> >>>
> >>> * A root
> >>> * B root
> >>> * C root
> >>> * D1 child
> >>> _ /
> >>> /
> >>> /
> >>> * D root
> >
> > * A root
> >
> > * B root
> >
> > * C root
> >
> > * D1 child
> >
> > * D root
> >
> > Here I think a blank line looks worse, too much space for just 5
> > commits and becomes one extra line which if this were like up to 7 or
> > more parentless commits one after the other would be more noticeable.
>
> But there shouldn't be a blank line between D and D1 so the two
> alternatives take up the same amount of vertical space, the main
> difference being whether D1 appears next to D
>
> * A root * A root
> * B root
> * B root * C root
> * D1 child
> * C root _/
> /
> * D1 child /
> * D root * D root
>
> Of course if the indentation was smarter it would take up less room and
> look better than having blank lines
>
> * A root
> * B root
> * C root
> * D1 child
> * D root
Right, this would be ideal but that would require too much change to the
existing graphing logic, and should be its own patch.
> > But there are cases that blank line might be better:
> >
> > * 10_A2
> > * 10_A1
> > * 10_A
> > * 10_M
> > /|\
> > | | * 10_D
> > | * 10_C
> > * 10_B
> >
> > Feels like a shower of commits instead of an indented merge.
>
> Yes, that is a bit confusing. I think the thing I find confusing with
> this approach is that we're treating the commit rendered below the root
> commit specially, rather than treating the root commit itself specially.
> To me it is the root commit that's the odd one out because it does not
> have any parents, but we treat the commit that's rendered below as the
> odd one by indenting it relative to its parents.
I guess that would make the examples look something like this:
* A root
* B root
* C root
* D1 child
* D root
No cascading, and no need for that massive _ / collapse line.
* 10_A2
* 10_A1
\
* 10_A
* 10_M
| \ \
| | * 10_D
| * 10_C
* 10_B
I say it looks better than the alternatives, but I'm not sure if this will
be easy to implement. The diagonal connection line (\) will need to
be printed before printing the actual root commit, which will require
lookahead logic.
I'd prefer to avoid major surgery on the codebase.
> > Pro to the blank line, the parentless check is the same and it's just
> > printing a '\n' at the right spot, while indent i'm mimicking like if
> > there was a commit there.
> > Anyways, I think in the majority of the cases the indentation +
> > collapsing looks better.
> > Sorry for the brief reply, I'm busy today.
>
> No need to apologize, it seemed quite comprehensive to me
>
> Thanks
>
> Phillip
>
> > Regards,
> >
> > --
> > Pablo
> >
> >>
> >> Thanks
> >>
> >> Phillip
> >>
> >>> This is done by adding a is_placeholder flag to the columns, the root commit
> >>> is actually there but marked as a placeholder
> >>>
> >>> e.g.:
> >>>
> >>> * root-B
> >>> (B) * child-A2
> >>> /
> >>> * child-A1
> >>> * root-A
> >>>
> >>> (B) would be root-B column with the placeholder flag active.
> >>>
> >>> Then teaching the rendering function to print a padding ' ' when meeting a
> >>> placeholder column outputs the second example.
> >>>
> >>> There could also be the case where there are multiple roots
> >>>
> >>> without the patch:
> >>>
> >>> * A root
> >>> * B root
> >>> * C root
> >>> * D1 child
> >>> * D root
> >>>
> >>> with the patch, the indentation cascades:
> >>>
> >>> * A root
> >>> * B root
> >>> * C root
> >>> * D1 child
> >>> _ /
> >>> /
> >>> /
> >>> * D root
> >>>
> >>> the _ / might look weird but that's how the collapsing rendering does it
> >>> for big gaps, this case being from the 4th column to the 0th column.
> >>> Another patch could change the collapsing rendering for placeholders ?
> >>> I haven't done it to keep it minimal, but a follow up could make it
> >>> to be straight '/'. This would make it bigger but easier for the eye to follow.
> >>> IMO is not worth it, but opinions are welcome.
> >>>
> >>> The patch also adds tests for different cases like a root preceding multiple
> >>> parents merges and the examples above.
> >>>
> >>> There could be some edge cases still so any testing is very welcome.
> >>>
> >>> Pablo Sabater (1):
> >>> graph: add indentation for commits preceded by a root
> >>>
> >>> graph.c | 68 ++++++++++++++++--
> >>> t/t4215-log-skewed-merges.sh | 136 +++++++++++++++++++++++++++++++++++
> >>> 2 files changed, 198 insertions(+), 6 deletions(-)
> >>>
> >>>
> >>> base-commit: 256554692df0685b45e60778b08802b720880c50
> >>
>
Thanks,
Chandra.
^ permalink raw reply
* [PATCH v2] stash: add coverage for show --include-untracked
From: Pushkar Singh @ 2026-05-16 18:33 UTC (permalink / raw)
To: pushkarkumarsingh1970; +Cc: git, gitster, peff, ps
In-Reply-To: <20260505103332.43702-2-pushkarkumarsingh1970@gmail.com>
Add a test for 'git stash show --include-untracked' to
cover the case where untracked files saved in the stash
are included in the output.
While stash creation and restoration of untracked files
are already tested, there is currently no explicit test
covering the output behavior of 'stash show
--include-untracked'.
Signed-off-by: Pushkar Singh <pushkarkumarsingh1970@gmail.com>
---
Changes in v2:
- Clarify in the commit message that the patch fills a gap
in existing test coverage for 'stash show
--include-untracked'
t/t3903-stash.sh | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 70879941c2..d4867536b9 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -1790,4 +1790,21 @@ test_expect_success 'stash.index=false overridden by --index' '
test_cmp expect file
'
+test_expect_success 'stash show --include-untracked includes untracked files' '
+ git reset --hard &&
+
+ echo tracked >tracked &&
+ git add tracked &&
+ git commit -m "base" &&
+
+ echo change >>tracked &&
+ echo untracked >untracked &&
+
+ git stash push --include-untracked &&
+ test_path_is_missing untracked &&
+
+ git stash show --include-untracked >actual &&
+ test_grep "untracked" actual
+'
+
test_done
--
2.53.0.582.gca1db8a0f7
^ permalink raw reply related
* [PATCH v3 2/2] t6600: add tests for duplicate tips in tips_reachable_from_bases()
From: Kristofer Karlsson via GitGitGadget @ 2026-05-16 15:59 UTC (permalink / raw)
To: git
Cc: Jeff King, Kristofer Karlsson, Derrick Stolee, Kristofer Karlsson,
Kristofer Karlsson
In-Reply-To: <pull.2116.v3.git.1778947182.gitgitgadget@gmail.com>
From: Kristofer Karlsson <krka@spotify.com>
When multiple refs point to the same commit, the reachability check
must handle them correctly. Add three tests:
- duplicate tips, all reachable
- duplicate tips, none reachable
- duplicate tips at the minimum generation (exercises the
early-termination advancement logic)
Suggested-by: Derrick Stolee <stolee@gmail.com>
Signed-off-by: Kristofer Karlsson <krka@spotify.com>
---
t/t6600-test-reach.sh | 45 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 45 insertions(+)
diff --git a/t/t6600-test-reach.sh b/t/t6600-test-reach.sh
index dc0421ed2f..9486002866 100755
--- a/t/t6600-test-reach.sh
+++ b/t/t6600-test-reach.sh
@@ -612,6 +612,51 @@ test_expect_success 'for-each-ref merged:none' '
--format="%(refname)" --stdin
'
+test_expect_success 'for-each-ref merged:duplicate, all reachable' '
+ git branch dup-a commit-3-3 &&
+ git branch dup-b commit-3-3 &&
+ cat >input <<-\EOF &&
+ refs/heads/commit-1-1
+ refs/heads/dup-a
+ refs/heads/dup-b
+ EOF
+ cat >expect <<-\EOF &&
+ refs/heads/commit-1-1
+ refs/heads/dup-a
+ refs/heads/dup-b
+ EOF
+ run_all_modes git for-each-ref --merged=commit-5-5 \
+ --format="%(refname)" --stdin
+'
+
+test_expect_success 'for-each-ref merged:duplicate, none reachable' '
+ cat >input <<-\EOF &&
+ refs/heads/dup-a
+ refs/heads/dup-b
+ refs/heads/commit-9-9
+ EOF
+ >expect &&
+ run_all_modes git for-each-ref --merged=commit-2-2 \
+ --format="%(refname)" --stdin
+'
+
+test_expect_success 'for-each-ref merged:duplicate at min generation' '
+ git branch dup-c commit-1-1 &&
+ git branch dup-d commit-1-1 &&
+ cat >input <<-\EOF &&
+ refs/heads/dup-c
+ refs/heads/dup-d
+ refs/heads/commit-5-5
+ EOF
+ cat >expect <<-\EOF &&
+ refs/heads/commit-5-5
+ refs/heads/dup-c
+ refs/heads/dup-d
+ EOF
+ run_all_modes git for-each-ref --merged=commit-5-5 \
+ --format="%(refname)" --stdin
+'
+
# For get_branch_base_for_tip, we only care about
# first-parent history. Here is the test graph with
# second parents removed:
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 1/2] commit-reach: use object flags for tips_reachable_from_bases()
From: Kristofer Karlsson via GitGitGadget @ 2026-05-16 15:59 UTC (permalink / raw)
To: git
Cc: Jeff King, Kristofer Karlsson, Derrick Stolee, Kristofer Karlsson,
Kristofer Karlsson
In-Reply-To: <pull.2116.v3.git.1778947182.gitgitgadget@gmail.com>
From: Kristofer Karlsson <krka@spotify.com>
tips_reachable_from_bases() walks the commit graph from a set of base
commits to find which tip commits are reachable. The inner loop does
a linear scan over the tips array to check whether each visited commit
is a tip, making the overall cost O(C * T) where C is commits walked
and T is the number of tips.
Use the RESULT object flag to mark tip commits, replacing the linear
scan with a single flag test per visited commit. This reduces the
per-commit tip check from O(T) to O(1) and the overall cost from
O(C * T) to O(C + T).
When multiple refs point to the same commit, the shared object gets
the flag once, so all duplicates are handled automatically. The
early-termination advancement loop checks the flag on the sorted
commits array directly, which naturally handles duplicates since the
flag is on the shared commit object.
This also removes the index field from struct commit_and_index, since
the indirection through the original tips array is no longer needed.
This function is called by `git for-each-ref --merged` and
`git branch/tag --contains/--no-contains` via reach_filter() in
ref-filter.c.
Benchmark on a merge-heavy monorepo (2.3M commits, 10,000 refs):
Command Before After Speedup
for-each-ref --merged HEAD 6.57s 1.59s 4.1x
for-each-ref --no-merged HEAD 6.67s 1.66s 4.0x
branch --merged HEAD 0.68s 0.61s 10%
branch --no-merged HEAD 0.65s 0.61s 8%
tag --merged HEAD 0.12s 0.12s -
On linux.git with 10,000 synthetic branches at the root commit (worst
case for the DFS walk):
Command Before After Speedup
for-each-ref --merged HEAD 1.35s 0.35s 3.9x
for-each-ref --no-merged HEAD 1.82s 0.31s 5.9x
The large speedup for for-each-ref is because it checks all 10,000
refs as tips, making the O(T) inner loop expensive. The branch
subcommand only checks local branches (fewer tips), so the improvement
is smaller.
Signed-off-by: Kristofer Karlsson <krka@spotify.com>
---
commit-reach.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/commit-reach.c b/commit-reach.c
index d3a9b3ed6f..82614d2409 100644
--- a/commit-reach.c
+++ b/commit-reach.c
@@ -1125,7 +1125,6 @@ void ahead_behind(struct repository *r,
struct commit_and_index {
struct commit *commit;
- unsigned int index;
timestamp_t generation;
};
@@ -1165,7 +1164,6 @@ void tips_reachable_from_bases(struct repository *r,
for (size_t i = 0; i < tips_nr; i++) {
commits[i].commit = tips[i];
- commits[i].index = i;
commits[i].generation = commit_graph_generation(tips[i]);
}
@@ -1173,6 +1171,9 @@ void tips_reachable_from_bases(struct repository *r,
QSORT(commits, tips_nr, compare_commit_and_index_by_generation);
min_generation = commits[0].generation;
+ for (size_t i = 0; i < tips_nr; i++)
+ commits[i].commit->object.flags |= RESULT;
+
while (bases) {
repo_parse_commit(r, bases->item);
commit_list_insert(bases->item, &stack);
@@ -1183,20 +1184,16 @@ void tips_reachable_from_bases(struct repository *r,
int explored_all_parents = 1;
struct commit_list *p;
struct commit *c = stack->item;
- timestamp_t c_gen = commit_graph_generation(c);
/* Does it match any of our tips? */
- for (size_t j = min_generation_index; j < tips_nr; j++) {
- if (c_gen < commits[j].generation)
- break;
-
- if (commits[j].commit == c) {
- tips[commits[j].index]->object.flags |= mark;
+ {
+ if (c->object.flags & RESULT) {
+ c->object.flags |= mark;
- if (j == min_generation_index) {
- unsigned int k = j + 1;
+ if (commits[min_generation_index].commit->object.flags & mark) {
+ unsigned int k = min_generation_index + 1;
while (k < tips_nr &&
- (tips[commits[k].index]->object.flags & mark))
+ (commits[k].commit->object.flags & mark))
k++;
/* Terminate early if all found. */
@@ -1232,6 +1229,8 @@ void tips_reachable_from_bases(struct repository *r,
}
done:
+ for (size_t i = 0; i < tips_nr; i++)
+ commits[i].commit->object.flags &= ~RESULT;
free(commits);
repo_clear_commit_marks(r, SEEN);
commit_list_free(stack);
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 0/2] commit-reach: use object flags for tips_reachable_from_bases()
From: Kristofer Karlsson via GitGitGadget @ 2026-05-16 15:59 UTC (permalink / raw)
To: git; +Cc: Jeff King, Kristofer Karlsson, Derrick Stolee, Kristofer Karlsson
In-Reply-To: <pull.2116.v2.git.1778922993480.gitgitgadget@gmail.com>
This replaces the O(C*T) linear scan in tips_reachable_from_bases() with an
O(1) flag check using the RESULT object flag.
The function is called by git for-each-ref --merged and git branch/tag
--contains/--no-contains via reach_filter() in ref-filter.c.
Benchmarks on a merge-heavy monorepo (2.3M commits, 10,000 refs):
* for-each-ref --merged HEAD: 6.6s → 1.6s (4.1x)
* for-each-ref --no-merged HEAD: 6.7s → 1.7s (4.0x)
On linux.git with 10,000 synthetic branches at the root commit:
* for-each-ref --merged HEAD: 1.35s → 0.35s (3.9x)
* for-each-ref --no-merged HEAD: 1.82s → 0.31s (5.9x)
v2 of this patch, addressing Jeff King's feedback:
* Replaced the decoration hash with the RESULT object flag (simpler, no
extra data structure, handles duplicate tips naturally)
* Fixed early-termination bug when multiple refs point to the same commit
(the decoration API overwrites on duplicate keys)
* Removed the now-unused index field from struct commit_and_index
* Diff is +11/-12 lines
Kristofer Karlsson (2):
commit-reach: use object flags for tips_reachable_from_bases()
t6600: add tests for duplicate tips in tips_reachable_from_bases()
commit-reach.c | 23 +++++++++++-----------
t/t6600-test-reach.sh | 45 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 56 insertions(+), 12 deletions(-)
base-commit: 59ff4886a579f4bc91e976fe18590b9ae02c7a08
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2116%2Fspkrka%2Ftips-reachable-minimal-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2116/spkrka/tips-reachable-minimal-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/2116
Range-diff vs v2:
1: 7399a12518 = 1: 7399a12518 commit-reach: use object flags for tips_reachable_from_bases()
-: ---------- > 2: 4d11ebb79e t6600: add tests for duplicate tips in tips_reachable_from_bases()
--
gitgitgadget
^ permalink raw reply
* Re: [PATCH v1 11/11] git-gui: add gui and pick as explicit subcommands
From: Mark Levedahl @ 2026-05-16 15:42 UTC (permalink / raw)
To: Johannes Sixt; +Cc: egg_mushroomcow, bootaina702, git
In-Reply-To: <e336544b-941d-43ed-890f-2b8950dbaf88@kdbg.org>
On 5/16/26 4:18 AM, Johannes Sixt wrote:
> Am 14.05.26 um 16:33 schrieb Mark Levedahl:
>> git-gui accepts subcommands blame | browser | citool, and assumes the
>> subcommand is 'gui' if none is actually given, But, git gui also has a
>> repository picker (choose_repository::pick) that can create a new
>> repository + worktree, or choose an existing one, switch to that, and
>> the run the gui. The user has no direct control over invoking the
>> picker, instead the picker is triggered by failure in the repository /
>> worktree discover process: this includes being started in a directory
>> not controlled by git, which is probably the intended use case.
>>
>> The picker can appear when the user has no intention of creating a new
>> worktree, and the user cannot use the picker to create a new worktree
>> inside another.
>>
>> So, add two new explicit subcommands:
>> gui - Run the gui if repository/worktree discovery succeeds, or die
>> with an error message, but never run the picker.
>> pick - First run the picker, regardless, then start the gui in
>> the chosen worktree.
>>
>> Nothing in this changes the prior behavior, the alternates above must be
>> explicitly selected to see any change.
> OK.
>
>> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
>> ---
>> git-gui.sh | 22 ++++++++++++++++++++--
>> 1 file changed, 20 insertions(+), 2 deletions(-)
>>
>> diff --git a/git-gui.sh b/git-gui.sh
>> index 3a83dd5..c56aeef 100755
>> --- a/git-gui.sh
>> +++ b/git-gui.sh
>> @@ -1021,6 +1021,7 @@ proc load_config {include_global} {
>> ##
>> ## feature option selection
>>
>> +set run_picker_on_error 1
>> if {[regexp {^git-(.+)$} [file tail $argv0] _junk subcommand]} {
>> unset _junk
>> } else {
>> @@ -1030,6 +1031,7 @@ if {$subcommand eq {gui.sh}} {
>> set subcommand gui
>> }
>> if {$subcommand eq {gui} && [llength $argv] > 0} {
>> + set run_picker_on_error 0
>> set subcommand [lindex $argv 0]
>> set argv [lrange $argv 1 end]
>> }
>> @@ -1047,6 +1049,7 @@ blame {
>> disable_option multicommit
>> disable_option branch
>> disable_option transport
>> + set run_picker_on_error 0
>> }
>> citool {
>> enable_option singlecommit
>> @@ -1055,6 +1058,7 @@ citool {
>> disable_option multicommit
>> disable_option branch
>> disable_option transport
>> + set run_picker_on_error 0
>>
>> while {[llength $argv] > 0} {
>> set a [lindex $argv 0]
> Can we please use the available disable_option and enable_option feature
> instead of a new variable. Just for consistency around repository discovery.
>
>> @@ -1162,14 +1166,28 @@ proc pick_repo {} {
>> set picked 1
>> }
>>
>> +# run repository picker if explicitly requested
>> +switch -- $subcommand {
>> + pick {
>> + pick_repo
>> + set subcommand gui
>> + set run_picker_on_error 0
>> + }
>> +}
>> +
> It just feels wrong to have a new pick_repo call before repository
> discovery. Can we not treat this case below as if regular repository
> discovery failed and then end up in the existing call of pick_repo?
So, your suggestion is to create an error inside the catch clause, assure GIT_VAR and
GIT_WORK_TREE are unset so we don't throw and error message and abort, and then fall
through to the existing pick_repo clause?
I think this makes code less understandable and more complicated. As written, the user
selects the repo, and now the standard discovery runs except that the picker cannot be re-run.
I like the current structure better.
>> # find repository.
>> if {[catch {
>> set _gitdir [git rev-parse --absolute-git-dir]
>> } err]} {
>> if {[is_gitvars_error $err]} {
>> exit 1
>> - } else {
>> + }
>> + if {$run_picker_on_error} {
>> pick_repo
>> + } else {
>> + catch {wm withdraw .}
>> + error_popup [strcat [mc "Git directory not found:"] "\n\n$err"]
>> + exit 1
>> }
>> }
>>
>> @@ -3051,7 +3069,7 @@ gui {
>> # fall through to setup UI for commits
>> }
>> default {
>> - set err "[mc usage:] $argv0 \[{blame|browser|citool}\]"
>> + set err "[mc usage:] $argv0 \[{blame|browser|citool|gui|pick}\]"
>> if {[tk windowingsystem] eq "win32"} {
>> wm withdraw .
>> tk_messageBox -icon error -message $err \
> We don't need to switch on the new subcommands?
>
> As a follow-up to my comment on 04/11: How relevant is it that variable
> $picked is set in a 'git gui pick' invocation?
>
> -- Hannes
>
Repeating, I need to understand what "picked" really does in the gui. Will definitely
address your question.
Mark
^ permalink raw reply
* Re: [PATCH v1 10/11] git-gui: improve worktree discovery
From: Mark Levedahl @ 2026-05-16 15:28 UTC (permalink / raw)
To: Johannes Sixt; +Cc: egg_mushroomcow, bootaina702, git
In-Reply-To: <8b8feffa-1651-41aa-ac76-d2721d656b45@kdbg.org>
On 5/16/26 4:16 AM, Johannes Sixt wrote:
> Am 14.05.26 um 16:33 schrieb Mark Levedahl:
>> git gui's worktree discovery needs update based upon prior work in this
>> series. In the normal case, all information we need comes directly from
>> git rev-parse (--show-toplevel, and --show-prefix). Should this work, we
>> have a valid worktree and all git gui commands can run.
>>
>> If not, we need to consider:
>> - if GIT_DIR or GIT_WORK_TREE are in the environment, just stop as we
>> the input configuration was wrong, the user must fix that.
>> - if we have a browser or blame subcommand, no worktree is needed so
>> git-gui can run without.
>> - using the git repository's parent is a valid worktree (if possible),
>> restoring prior behavior.
>>
>> The current directory should be either the root of the worktree, if one
>> is found, or the top-level of the git repository.
> I disagree in the case where no working tree is found. Then there is no
> point in changing the current directory.
git-gui always changes to root of the worktree if that is found.
Failing to cd to the root of the repository when operating with no worktree opens the
possibility th
>
>> Make it so. Also, make worktree discover directly follow repository
>> discovery, reducing the locations that might need error trapping to
>> catch configuration issues.
> Good!
>
>> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
> BTW, please make the subject line more descriptive. The word "improve"
> does not convey anything of importance.
>
>> ---
>> git-gui.sh | 56 ++++++++++++++++++++++--------------------------------
>> 1 file changed, 23 insertions(+), 33 deletions(-)
>>
>> diff --git a/git-gui.sh b/git-gui.sh
>> index e326401..3a83dd5 100755
>> --- a/git-gui.sh
>> +++ b/git-gui.sh
>> @@ -1173,6 +1173,28 @@ if {[catch {
>> }
>> }
>>
>> +# find worktree, continue without if not required
>> +if {[catch {
>> + set _gitworktree [git rev-parse --show-toplevel]
>> + set _prefix [git rev-parse --show-prefix]
>> + cd $_gitworktree
>> +} err]} {
> We have three commands, each with their own possible failure sources.
> One of the outcomes of an error is that we proceed anyway. I think that
> this is incorrect if the 'cd' fails: we must not proceed if it fails.
> Therefore, we must handle its failure separately.
>
>> + if {[is_gitvars_error $err]} {
>> + exit 1
>> + }
>> + set _gitworktree {}
>> + set _prefix {}
>> + if {[is_enabled bare]} {
>> + cd $_gitdir
> Why change the directory here? If we run `git gui browser master dir` we
> do not want to change the directory in an uncontrolled manner. The
> argument parser will want to check for the existence of files, and then
> we do not want to operate from a random directory.
>
> Also, I think that the check must be for [is_bare] and not [is_enabled
> bare].
[is_enabled_bare] is correct. This code handles the case:
- neither the startup directory nor GIT_WORK_TREE are useable worktrees, so [is_bare]
is currently true.
- the command given is browser or blame so a worktree is not needed. We can proceed.
The bigger question is whether to change directory at all: git-gui should never touch
files that are neither in the worktree nor in the repository. Leaving the current
directory as neither of those could be troublesome. I have no strong feeling here though,
will delete this.
>> + } elseif {![is_parent_worktree]} {
>> + catch {wm withdraw .}
>> + error_popup [strcat [mc "Cannot use bare repository:"] "\n\n" $_gitdir]
>> + exit 1
>> + }
>> +}
>> +
>> +# repository and worktree config are complete, export them
>> +set_gitdir_vars
>>
>> # Use object format as hash algorithm (either "sha1" or "sha256")
>> set hashalgorithm [git rev-parse --show-object-format]
> This moves code around. In particular, we see load_config and
> apply_config in the context below, which now happens only after these
> calls. How certain are we that these have no effect on the code that
> runs now earlier?
We need to load the system and user global config before running the repository picker. We
(re-) load the full config including the repository after we have a repository. I think
this is correct: git-config explicitly lists worktree dependent includeif statements,
meaning the config can be worktree dependent, and we must not load the final config until
repository and worktree discovery are complete.
git rev-parse, etc., perform discovery and config file loading each time they are invoked,
those are unaffected by git-gui's internal config.
I will clarify this explicitly in the commit message.
>> @@ -1189,37 +1211,8 @@ if {$hashalgorithm eq "sha1"} {
>> load_config 0
>> apply_config
>>
>> -set _gitworktree [git rev-parse --show-toplevel]
>>
>> -if {$_prefix ne {}} {
>> - if {$_gitworktree eq {}} {
>> - regsub -all {[^/]+/} $_prefix ../ cdup
>> - } else {
>> - set cdup $_gitworktree
>> - }
>> - if {[catch {cd $cdup} err]} {
>> - catch {wm withdraw .}
>> - error_popup [strcat [mc "Cannot move to top of working directory:"] "\n\n$err"]
>> - exit 1
>> - }
>> - set _gitworktree [pwd]
>> - unset cdup
>> -} elseif {![is_enabled bare]} {
>> - if {[is_bare]} {
>> - catch {wm withdraw .}
>> - error_popup [strcat [mc "Cannot use bare repository:"] "\n\n$_gitdir"]
>> - exit 1
>> - }
>> - if {$_gitworktree eq {}} {
>> - set _gitworktree [file dirname $_gitdir]
>> - }
>> - if {[catch {cd $_gitworktree} err]} {
>> - catch {wm withdraw .}
>> - error_popup [strcat [mc "No working directory"] " $_gitworktree:\n\n$err"]
>> - exit 1
>> - }
>> - set _gitworktree [pwd]
>> -}
>> +# Derive a human-readable repository name
>> set _reponame [file split [file normalize $_gitdir]]
>> if {[lindex $_reponame end] eq {.git}} {
>> set _reponame [lindex $_reponame end-1]
>> @@ -1227,9 +1220,6 @@ if {[lindex $_reponame end] eq {.git}} {
>> set _reponame [lindex $_reponame end]
>> }
>>
>> -# Export the final paths
>> -set_gitdir_vars
>> -
>> ######################################################################
>> ##
>> ## global init
> -- Hannes
>
^ permalink raw reply
* [PATCH v4 3/4] approxidate: make "specials" respect fixed day-of-month
From: Tuomas Ahola @ 2026-05-16 15:15 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano, Tuomas Ahola
In-Reply-To: <20260516151540.9611-1-taahol@utu.fi>
The special approxidate time formats, "noon" and "tea" differ from
"12pm" and "5pm" by having the feature of wrapping to the previous day
if the current time is before those hours:
now -> 2026-05-13 11:00:00 +0000
12pm -> 2026-05-13 12:00:00 +0000
5pm -> 2026-05-13 17:00:00 +0000
noon -> 2026-05-12 12:00:00 +0000
tea -> 2026-05-12 17:00:00 +0000
However, that logic carries too far. Even when the date is specified,
the behavior of the "specials" depends on the current time. Assuming
the same time as above, we get:
today at noon -> 2026-05-12 12:00:00 +0000 (should be 13 May)
13 May at tea -> 2026-05-12 17:00:00 +0000
or, using an example mentioned in date-formats.adoc:
last Friday at noon -> 2026-05-07 12:00:00 +0000 (should be 8 May)
The quirk seems to be rather old. Already in 2006, Linus Torvalds
remarked that the date yielded by "one year ago yesterday at tea-time"
was "just silly and not even correct". Indeed, even today it gives:
One year ago yesterday at tea-time -> 2025-05-11 17:00:00 +0000
(should be 12 May)
Let's fix all of those with a simple patch. Check whether we already
have a specified day-of-month in `tm->tm_mday` and make `date_time()`
stick to it. Ensure the correct behavior with relevant tests.
Links:
1. https://lore.kernel.org/git/Pine.LNX.4.64.0610101102560.3952@g5.osdl.org/
Signed-off-by: Tuomas Ahola <taahol@utu.fi>
---
date.c | 6 +++++-
t/t0006-date.sh | 4 ++++
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/date.c b/date.c
index 343d6aab6f..7a458f3cac 100644
--- a/date.c
+++ b/date.c
@@ -1132,7 +1132,11 @@ static void date_yesterday(struct tm *tm, struct tm *now, int *num)
static void date_time(struct tm *tm, struct tm *now, int hour)
{
- if (tm->tm_hour < hour)
+ /*
+ * If we do not yet have a specified day, we'll use the most recent
+ * version of "hour" relative to now. But that may be yesterday.
+ */
+ if (tm->tm_mday < 0 && tm->tm_hour < hour)
update_tm(tm, now, 24*60*60);
tm->tm_hour = hour;
tm->tm_min = 0;
diff --git a/t/t0006-date.sh b/t/t0006-date.sh
index 15fbc12861..7358903046 100755
--- a/t/t0006-date.sh
+++ b/t/t0006-date.sh
@@ -209,8 +209,12 @@ check_approxidate '6pm yesterday' '2009-08-29 18:00:00'
check_approxidate '3:00' '2009-08-30 03:00:00'
check_approxidate '15:00' '2009-08-30 15:00:00'
check_approxidate 'noon today' '2009-08-30 12:00:00'
+check_approxidate 'today at noon' '2009-08-30 12:00:00' '-12 hours'
check_approxidate 'noon yesterday' '2009-08-29 12:00:00'
+check_approxidate 'last Friday at noon' '2009-08-28 12:00:00'
+check_approxidate 'last Friday at noon' '2009-08-28 12:00:00' '-12 hours'
check_approxidate 'January 5th noon pm' '2009-01-05 12:00:00'
+check_approxidate 'January 5th noon pm' '2009-01-05 12:00:00' '-12 hours'
check_approxidate '10am noon' '2009-08-29 12:00:00'
check_approxidate 'January 5th yesterday' '2009-01-29 19:20:00'
check_approxidate 'January 5th yesterday' '2008-12-31 19:20:00' '+2 days'
--
2.30.2
^ permalink raw reply related
* [PATCH v4 1/4] approxidate: make "today" wrap to midnight
From: Tuomas Ahola @ 2026-05-16 15:15 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano, Tuomas Ahola
In-Reply-To: <20260516151540.9611-1-taahol@utu.fi>
Although some commands do reject invalid approxidate expressions,
in other cases those are simply evaluated as the current time.
Oftentimes that is a perfectly good compromise to handle silly
requests, but it isn't without rough edges.
Because of the silent acceptance, it is easy to forget that
"today" isn't actually a valid approxidate format. That is
a bit awkward because while the fallback logic of using the
current time does make some sense, there is no deliberative
decision behind such behavior of "today". Indeed, whatever
(non-)action "today" currently has, is just an accidental
side effect.
That means "git log --since=today" is currently unlikely to
print anything at all as it tries to list commits dated with
*future* timestamps. Arguably it would be more useful to
list the commits of the current day---i.e. those made since
midnight.
On the other hand, "git log --until=today" doesn't really
filter commits at all. Changing the definition of "today"
would make it return the commits made before the current day.
That isn't without problems though---running "git log
--until=today" in the late afternoon could reasonably include
the work done earlier that day (as the command currently
does do).
Still the utility of no-op "--until=today" is debatable and
perhaps outweighed by the pros of having "--since=today" to
mean "--since=midnight". The thing is that the approxidate
machinery doesn't know about its consumers, so the meaning
of "today" has to be the same for "--since" and "--until".
In fact, "git log --until=" is documented as
`--until=<date>`::
`--before=<date>`::
Show commits older than _<date>_,
so excluding commits made today would actually match the
documentation more closely.
Moreover, a revision parameter "@{today}" is currently outright
rejected. Making "today" a valid approxidate time format could
make a natural way to specify the state of the ref at the start
of the current day.
Bind "today" to new function `date_today()` as an approxidate
special. Make it return the last midnight if no specific time
is given; i.e. retain the old behavior of "noon today" and such.
Document the new behavior of "git log --since=today" in
rev-list-options.adoc.
Signed-off-by: Tuomas Ahola <taahol@utu.fi>
---
Documentation/rev-list-options.adoc | 3 ++-
date.c | 10 ++++++++++
t/t0006-date.sh | 2 ++
3 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/Documentation/rev-list-options.adoc b/Documentation/rev-list-options.adoc
index 2d195a1474..a5abadf689 100644
--- a/Documentation/rev-list-options.adoc
+++ b/Documentation/rev-list-options.adoc
@@ -23,7 +23,8 @@ ordering and formatting options, such as `--reverse`.
`--since=<date>`::
`--after=<date>`::
- Show commits more recent than _<date>_.
+ Show commits more recent than _<date>_. As a special case,
+ 'today' means the last midnight.
`--since-as-filter=<date>`::
Show all commits more recent than _<date>_. This visits
diff --git a/date.c b/date.c
index 17a95077cf..343d6aab6f 100644
--- a/date.c
+++ b/date.c
@@ -1192,6 +1192,15 @@ static void date_never(struct tm *tm, struct tm *now UNUSED, int *num)
*num = 0;
}
+static void date_today(struct tm *tm, struct tm *now, int *num UNUSED)
+{
+ if (tm->tm_hour == now->tm_hour &&
+ tm->tm_min == now->tm_min &&
+ tm->tm_sec == now->tm_sec)
+ date_time(tm, now, 0);
+ update_tm(tm, now, 0);
+}
+
static const struct special {
const char *name;
void (*fn)(struct tm *, struct tm *, int *);
@@ -1204,6 +1213,7 @@ static const struct special {
{ "AM", date_am },
{ "never", date_never },
{ "now", date_now },
+ { "today", date_today },
{ NULL }
};
diff --git a/t/t0006-date.sh b/t/t0006-date.sh
index 53ced36df4..07bf6115ab 100755
--- a/t/t0006-date.sh
+++ b/t/t0006-date.sh
@@ -164,6 +164,7 @@ check_approxidate() {
}
check_approxidate now '2009-08-30 19:20:00'
+check_approxidate today '2009-08-30 00:00:00'
check_approxidate '5 seconds ago' '2009-08-30 19:19:55'
check_approxidate 5.seconds.ago '2009-08-30 19:19:55'
check_approxidate 10.minutes.ago '2009-08-30 19:10:00'
@@ -187,6 +188,7 @@ check_approxidate 'last tuesday' '2009-08-25 19:20:00'
check_approxidate 'July 5th' '2009-07-05 19:20:00'
check_approxidate '06/05/2009' '2009-06-05 19:20:00'
check_approxidate '06.05.2009' '2009-05-06 19:20:00'
+check_approxidate 'Jan 5 today' '2009-01-30 00:00:00'
check_approxidate 'Jun 6, 5AM' '2009-06-06 05:00:00'
check_approxidate '5AM Jun 6' '2009-06-06 05:00:00'
--
2.30.2
^ permalink raw reply related
* [PATCH v4 0/4] approxidate: tweak special date formats
From: Tuomas Ahola @ 2026-05-16 15:15 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano, Tuomas Ahola
In-Reply-To: <20260514115520.6660-1-taahol@utu.fi>
The approxidate system is an endless source of absurdities. Let's make the
usual "eh, that's crazy, let's do better with this input" type of fix[1], and
tweak some sharp edge cases, including one noticed by Linus back in 2006[2].
After this series, "tea" and "noon" will work predictably with all kinds of
date formats (today, yesterday, last Friday, January 5th, one year ago
yesterday...) regardless of the current time of day.
More importantly, approxidate is taught about "today", meaning by default
the last midnight, as discussed in the RFC thread.[3]
Links:
1. https://lore.kernel.org/git/20181115144854.GB16450@sigill.intra.peff.net/
2. https://lore.kernel.org/git/Pine.LNX.4.64.0610101102560.3952@g5.osdl.org/
3. https://lore.kernel.org/git/20260515205803.26211-1-taahol@utu.fi/
Tuomas Ahola (4):
approxidate: make "today" wrap to midnight
t0006: add support for approxidate test date adjustment
approxidate: make "specials" respect fixed day-of-month
approxidate: use deferred mday adjustments for "specials"
Documentation/rev-list-options.adoc | 3 +-
date.c | 45 ++++++++++++++++++++++-------
t/t0006-date.sh | 43 ++++++++++++++++++++++++++-
3 files changed, 79 insertions(+), 12 deletions(-)
Intervall-diff mot v3:
-: ---------- > 1: 86bcb70ac2 approxidate: make "today" wrap to midnight
1: 7ea9c9967b = 2: 9863f359a1 t0006: add support for approxidate test date adjustment
2: 3a21727dbe < -: ---------- approxidate: alias "today" to "now"
3: d1992d23d0 = 3: 830de02f74 approxidate: make "specials" respect fixed day-of-month
4: 0b1a10305c ! 4: d33195dc91 approxidate: use deferred mday adjustments for "specials"
@@ Commit message
field for deferred date adjustments which can be easily reverted, so
that the default logic of the special formats only applies if we don't
get any explicit date (mday) specification. In particular, overwrite
- the field with -1 in "now" and "yesterday", so that those formats will
+ the field with -1 in "today" and "yesterday", so that those formats will
be relative to the current date. That makes specifications like "tea
yesterday" behave more sensibly: instead of going backwards to the
last tea-time and then a day back, Git will now understand that as the
@@ date.c: void datestamp(struct strbuf *out)
if (tm->tm_mon < 0)
tm->tm_mon = now->tm_mon;
if (tm->tm_year < 0) {
-@@ date.c: static void pending_number(struct tm *tm, int *num)
- static void date_now(struct tm *tm, struct tm *now, int *num)
- {
- *num = 0;
-+ tm->tm_mday = -1;
- update_tm(tm, now, 0);
- }
-
+@@ date.c: static void date_now(struct tm *tm, struct tm *now, int *num)
static void date_yesterday(struct tm *tm, struct tm *now, int *num)
{
*num = 0;
@@ date.c: static void pending_number(struct tm *tm, int *num)
}
static void date_pm(struct tm *tm, struct tm *now UNUSED, int *num)
+@@ date.c: static void date_today(struct tm *tm, struct tm *now, int *num UNUSED)
+ if (tm->tm_hour == now->tm_hour &&
+ tm->tm_min == now->tm_min &&
+ tm->tm_sec == now->tm_sec)
+- date_time(tm, now, 0);
++ date_time(tm, 0);
++ tm->tm_mday = -1;
+ update_tm(tm, now, 0);
+ }
+
## t/t0006-date.sh ##
@@ t/t0006-date.sh: check_approxidate '3:00' '2009-08-30 03:00:00'
base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
--
2.30.2
^ permalink raw reply
* [PATCH v4 2/4] t0006: add support for approxidate test date adjustment
From: Tuomas Ahola @ 2026-05-16 15:15 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano, Tuomas Ahola
In-Reply-To: <20260516151540.9611-1-taahol@utu.fi>
t0006 uses a hard-coded test date and provides no convenient
way to override it temporarily. Add an optional parameter to
check_approxidate to adjust the time as needed, and demonstrate
the feature with a new test.
Signed-off-by: Tuomas Ahola <taahol@utu.fi>
---
t/t0006-date.sh | 33 ++++++++++++++++++++++++++++++++-
1 file changed, 32 insertions(+), 1 deletion(-)
diff --git a/t/t0006-date.sh b/t/t0006-date.sh
index 07bf6115ab..15fbc12861 100755
--- a/t/t0006-date.sh
+++ b/t/t0006-date.sh
@@ -155,12 +155,41 @@ check_parse '2100-00-00 00:00:00 -11' bad
check_parse '2100-00-00 00:00:00 +11' bad
REQUIRE_64BIT_TIME=
+add_time_offset() {
+ case "$3" in
+ hours)
+ unit=$(( 60*60 ))
+ ;;
+ days)
+ unit=$(( 24*60*60 ))
+ ;;
+ esac
+ offset=$(( $2 * unit ))
+ echo $(( $1 + offset ))
+}
+
check_approxidate() {
+ old_date=$GIT_TEST_DATE_NOW
+ if test "$3" = "failure"
+ then
+ expection="$3"
+ else
+ expection=${4:-success}
+ offset="$3"
+ fi
+ if test -n "$offset"
+ then
+ GIT_TEST_DATE_NOW=$(add_time_offset $old_date $offset)
+ caption="$1; offset $offset"
+ else
+ caption=$1
+ fi
echo "$1 -> $2 +0000" >expect
- test_expect_${3:-success} "parse approxidate ($1)" "
+ test_expect_$expection "parse approxidate ($caption)" "
test-tool date approxidate '$1' >actual &&
test_cmp expect actual
"
+ GIT_TEST_DATE_NOW=$old_date
}
check_approxidate now '2009-08-30 19:20:00'
@@ -183,6 +212,8 @@ check_approxidate 'noon today' '2009-08-30 12:00:00'
check_approxidate 'noon yesterday' '2009-08-29 12:00:00'
check_approxidate 'January 5th noon pm' '2009-01-05 12:00:00'
check_approxidate '10am noon' '2009-08-29 12:00:00'
+check_approxidate 'January 5th yesterday' '2009-01-29 19:20:00'
+check_approxidate 'January 5th yesterday' '2008-12-31 19:20:00' '+2 days'
check_approxidate 'last tuesday' '2009-08-25 19:20:00'
check_approxidate 'July 5th' '2009-07-05 19:20:00'
--
2.30.2
^ permalink raw reply related
* [PATCH v4 4/4] approxidate: use deferred mday adjustments for "specials"
From: Tuomas Ahola @ 2026-05-16 15:15 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano, Tuomas Ahola
In-Reply-To: <20260516151540.9611-1-taahol@utu.fi>
There are cases where the "wrap-to-yesterday" behavior of "tea" and
"noon" should be reverted later on down the line, so that "today tea"
and "tea today" won't yield different results. However, the logic of
approxidate doesn't seem to lend itself particularly well to
such cases.
Start tackling the issue by reusing negative values of `tm->tm_mday`
field for deferred date adjustments which can be easily reverted, so
that the default logic of the special formats only applies if we don't
get any explicit date (mday) specification. In particular, overwrite
the field with -1 in "today" and "yesterday", so that those formats will
be relative to the current date. That makes specifications like "tea
yesterday" behave more sensibly: instead of going backwards to the
last tea-time and then a day back, Git will now understand that as the
tea-time of yesterday.
Replace the call of `update_tm()` in `date_time()` with the assignment
`tm->tm_mday = -2`. Add the corresponding code to handle that in
`update_tm()`, wrapping to the previous day if the field still holds
such assignment, meaning that we haven't seen any better specification
for the day-of-month. On the other hand, `mday=-3` would mean going
two days back and so on. Even though such functionality isn't
actually needed by this patch, it won't add much complexity in the
code and is rather natural way to handle such values.
As `date_time()` won't no longer need the `now` struct, mark the
associated function parameters as unused. The parameters themselves
have to stay, however, as those functions are called through pointers
in `approxidate_alpha`. Add relevant tests to cover the changes.
Signed-off-by: Tuomas Ahola <taahol@utu.fi>
---
date.c | 31 +++++++++++++++++++++----------
t/t0006-date.sh | 4 ++++
2 files changed, 25 insertions(+), 10 deletions(-)
diff --git a/date.c b/date.c
index 7a458f3cac..6e7cf907da 100644
--- a/date.c
+++ b/date.c
@@ -1071,13 +1071,22 @@ void datestamp(struct strbuf *out)
/*
* Relative time update (eg "2 days ago"). If we haven't set the time
* yet, we need to set it from current time.
+ *
+ * The tm->tm_mday field has an additional logic of using negative values
+ * for date adjustments: -2 means yesterday and -3 the day before that,
+ * and so on. The idea is to deref such adjustments until we are sure
+ * there's no explicit mday specification in the approxidate string.
*/
static time_t update_tm(struct tm *tm, struct tm *now, time_t sec)
{
time_t n;
- if (tm->tm_mday < 0)
+ if (tm->tm_mday < 0) {
+ int offset = tm->tm_mday + 1;
+ if (sec == 0 && offset < 0)
+ sec = -offset * 24*60*60;
tm->tm_mday = now->tm_mday;
+ }
if (tm->tm_mon < 0)
tm->tm_mon = now->tm_mon;
if (tm->tm_year < 0) {
@@ -1127,38 +1136,39 @@ static void date_now(struct tm *tm, struct tm *now, int *num)
static void date_yesterday(struct tm *tm, struct tm *now, int *num)
{
*num = 0;
+ tm->tm_mday = -1;
update_tm(tm, now, 24*60*60);
}
-static void date_time(struct tm *tm, struct tm *now, int hour)
+static void date_time(struct tm *tm, int hour)
{
/*
* If we do not yet have a specified day, we'll use the most recent
* version of "hour" relative to now. But that may be yesterday.
*/
if (tm->tm_mday < 0 && tm->tm_hour < hour)
- update_tm(tm, now, 24*60*60);
+ tm->tm_mday = -2; /* eventually handled by update_tm() */
tm->tm_hour = hour;
tm->tm_min = 0;
tm->tm_sec = 0;
}
-static void date_midnight(struct tm *tm, struct tm *now, int *num)
+static void date_midnight(struct tm *tm, struct tm *now UNUSED, int *num)
{
pending_number(tm, num);
- date_time(tm, now, 0);
+ date_time(tm, 0);
}
-static void date_noon(struct tm *tm, struct tm *now, int *num)
+static void date_noon(struct tm *tm, struct tm *now UNUSED, int *num)
{
pending_number(tm, num);
- date_time(tm, now, 12);
+ date_time(tm, 12);
}
-static void date_tea(struct tm *tm, struct tm *now, int *num)
+static void date_tea(struct tm *tm, struct tm *now UNUSED, int *num)
{
pending_number(tm, num);
- date_time(tm, now, 17);
+ date_time(tm, 17);
}
static void date_pm(struct tm *tm, struct tm *now UNUSED, int *num)
@@ -1201,7 +1211,8 @@ static void date_today(struct tm *tm, struct tm *now, int *num UNUSED)
if (tm->tm_hour == now->tm_hour &&
tm->tm_min == now->tm_min &&
tm->tm_sec == now->tm_sec)
- date_time(tm, now, 0);
+ date_time(tm, 0);
+ tm->tm_mday = -1;
update_tm(tm, now, 0);
}
diff --git a/t/t0006-date.sh b/t/t0006-date.sh
index 7358903046..b187b1bfc4 100755
--- a/t/t0006-date.sh
+++ b/t/t0006-date.sh
@@ -210,9 +210,13 @@ check_approxidate '3:00' '2009-08-30 03:00:00'
check_approxidate '15:00' '2009-08-30 15:00:00'
check_approxidate 'noon today' '2009-08-30 12:00:00'
check_approxidate 'today at noon' '2009-08-30 12:00:00' '-12 hours'
+check_approxidate 'noon today' '2009-09-01 12:00:00' '+36 hours'
check_approxidate 'noon yesterday' '2009-08-29 12:00:00'
check_approxidate 'last Friday at noon' '2009-08-28 12:00:00'
check_approxidate 'last Friday at noon' '2009-08-28 12:00:00' '-12 hours'
+check_approxidate 'noon yesterday' '2009-08-29 12:00:00' '-12 hours'
+check_approxidate 'tea last saturday' '2009-08-29 17:00:00'
+check_approxidate 'tea last saturday' '2009-08-29 17:00:00' '-12 hours'
check_approxidate 'January 5th noon pm' '2009-01-05 12:00:00'
check_approxidate 'January 5th noon pm' '2009-01-05 12:00:00' '-12 hours'
check_approxidate '10am noon' '2009-08-29 12:00:00'
--
2.30.2
^ permalink raw reply related
* Re: [PATCH v1 09/11] git-gui: support using repository parent dir as a worktree
From: Mark Levedahl @ 2026-05-16 14:48 UTC (permalink / raw)
To: Johannes Sixt; +Cc: egg_mushroomcow, bootaina702, git
In-Reply-To: <4d25544d-1a7e-4407-9191-1fb05ff55244@kdbg.org>
On 5/16/26 4:14 AM, Johannes Sixt wrote:
> Am 14.05.26 um 16:33 schrieb Mark Levedahl:
>> git-gui, since 87cd09f43e ("git-gui: work from the .git dir",
>> 2010-01-23), has had the intent to allow starting from inside a
>> repository, then switching to the parent directory if that is a valid
>> worktree.
>>
>> This certainly hasn't worked since 2d92ab32fd ("rev-parse: make
>> --show-toplevel without a worktree an error", 2019-11-19) in git, but
>> breaking this git-gui feature was unintentional.
>>
>> Add a proc to test if the parent of the git repository is a valid
>> worktree, and set that directory as the worktree if so. Use invocations
>> of git rev-parse to assure all validity and safety checks included in
>> git-core are executed.
> BTW, missing sign-off.
>
>> ---
>> git-gui.sh | 17 +++++++++++++++++
>> 1 file changed, 17 insertions(+)
>>
>> diff --git a/git-gui.sh b/git-gui.sh
>> index a03eaa7..e326401 100755
>> --- a/git-gui.sh
>> +++ b/git-gui.sh
>> @@ -1100,6 +1100,23 @@ unset argv0dir
>> ##
>> ## repository setup
>>
>> +proc is_parent_worktree {} {
>> + # Directory 'parent' of a repository named 'parent/.git' might be the worktree
>> + set ok 0
>> + if {[file tail $::_gitdir] eq {.git}} {
>> + set gitdir_parent [file join $::_gitdir {..}]
>> + set expected_worktree [file normalize $gitdir_parent]
> We have [file dirname ...]. Is there a reason to avoid it?
>
>> + catch {set git_worktree [git -C $gitdir_parent rev-parse --show-toplevel]}
>> + if {[string compare $expected_worktree $git_worktree] == 0} {
> The purpose of this check should be explained in a comment. I think it is:
>
> For a repository with the database in a directory named .git we assume
> that the working tree is the directory containing .git. But
> configuration may point to a different worktree. Then we do not want to
> hold on to our assumption.
>
> However, whether [git -C elsewhere ...] uses the same gitdir that we
> have discovered so far cannot be told from this piece of code alone.
> Therefore, I think it is wrong to extract this check into a function.
>
> Also, I don't think we can use string comparison here. On Windows, the
> command returns the Windows style path, but Tcl my operate with a POSIX
> style path.
As you have correctly inferred, am trying to unambiguously establish that git running in
the parent directory is using the child .git as the repository. I think this actually
requires two calls to git-revparse (--absolute-git-dir and --show-toplevel).
- the current git repo is valid to support a worktree. Will rework.
>> + set ::_prefix {}
>> + set ::_gitworktree $git_worktree
>> + cd $git_worktree
> So many side-effects in a function whose name suggests that it only does
> some checks. Please, don't do that.
>
>> + set ok 1
>> + }
>> + }
>> + return $ok
>> +}
>> +
>> proc is_gitvars_error {err} {
>> set havevars 0
>> set GIT_DIR {}
> In general, I am not a fan of commits that add new functions, but no
> call sites. Please squash this into 10/11. Ditto for is_gitvars_error in
> 06/11.
>
> -- Hannes
>
Next round should address all of your comments.
Mark
^ permalink raw reply
* Re: [PATCH v1 07/11] git-gui: use rev-parse exclusively to find a repository
From: Mark Levedahl @ 2026-05-16 14:38 UTC (permalink / raw)
To: Johannes Sixt; +Cc: egg_mushroomcow, bootaina702, git
In-Reply-To: <d8844726-0b08-4035-946e-c5ada0759f32@kdbg.org>
On 5/15/26 12:06 PM, Johannes Sixt wrote:
> Am 14.05.26 um 16:33 schrieb Mark Levedahl:
>> git-gui attempts to use env(GIT_DIR) directly as the git repository,
>> accepting GIT_DIR if it is a directory. Only if that fails is git
>> rev-parse used to discover the repository. But, this avoids all of
>> git-core's validity checking on a repository, thus possibly deferring an
>> error to a later step, possibly unexpected. Repository validation should
>> be part of initial setup so that later processing does not need error
>> trapping for configuration errors.
> OK. If the user gave us GIT_DIR with our without GIT_WORK_TREE, then
> that combination better be workable.
>
>> Let's just invoke rev-parse so all error checking is done. Stop here if
>> the user set GIT_DIR or GIT_WORK_TREE. Otherwise, continue the existing
>> behavior and show the repository picker.
> OK. But the paragraph is confusing, because a big "If an error occurs"
> is missing after the first sentence.
will fix.
>> Also, remove a later check on whether _gitdir is a directory: that code
>> cannot be reached without rev-parse having validating the repository.
> Good.
>
>> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
>> ---
>> git-gui.sh | 24 +++++++++---------------
>> 1 file changed, 9 insertions(+), 15 deletions(-)
>>
>> diff --git a/git-gui.sh b/git-gui.sh
>> index 2e2ddc0..81789dd 100755
>> --- a/git-gui.sh
>> +++ b/git-gui.sh
>> @@ -374,6 +374,7 @@ set _gitdir {}
>> set _gitworktree {}
>> set _isbare {}
>> set _githtmldir {}
>> +set _prefix {}
>> set _reponame {}
>> set _shellpath {@@SHELL_PATH@@}
>>
>> @@ -1167,19 +1168,18 @@ proc pick_repo {} {
>> set picked 1
>> }
>>
>> +# find repository.
>> if {[catch {
>> - set _gitdir $env(GIT_DIR)
>> - set _prefix {}
>> - }]
>> - && [catch {
>> - # beware that from the .git dir this sets _gitdir to .
>> - # and _prefix to the empty string
>> - set _gitdir [git rev-parse --absolute-git-dir]
>> - set _prefix [git rev-parse --show-prefix]
>> - } err]} {
>> + set _gitdir [git rev-parse --absolute-git-dir]
> Please do also set _prefix. It should fix the bug that the file chooser
> uses an empty prefix after
>
> cd lib
> GIT_DIR=$PWD/../.git GIT_WORK_TREE=$PWD/.. ../git-gui.sh browser master .
>
> (this is an old bug.)
>
> Please keep the additional indentation of the catch body.
>
>> +} err]} {
>> + if {[is_gitvars_error $err]} {
>> + exit 1
>> + } else {
>> pick_repo
>> + }
> Treat the 'if' as an early exist without an else, and we don't need the
> previously strange indentation of 'pick_repo'.
>
>> }
>>
>> +
>> # Use object format as hash algorithm (either "sha1" or "sha256")
>> set hashalgorithm [git rev-parse --show-object-format]
>> if {$hashalgorithm eq "sha1"} {
>> @@ -1191,12 +1191,6 @@ if {$hashalgorithm eq "sha1"} {
>> exit 1
>> }
>>
>> -if {![file isdirectory $_gitdir]} {
>> - catch {wm withdraw .}
>> - error_popup [strcat [mc "Git directory not found:"] "\n\n$_gitdir"]
>> - exit 1
>> -}
>> -
>> # _gitdir exists, so try loading the config
>> load_config 0
>> apply_config
will fix all.
Mark
^ permalink raw reply
* Re: [PATCH v1 05/11] git-gui: use --absolute-git-dir
From: Mark Levedahl @ 2026-05-16 14:33 UTC (permalink / raw)
To: Johannes Sixt; +Cc: egg_mushroomcow, bootaina702, git
In-Reply-To: <948f9f9f-8225-4bfe-be7d-e9b03c912aeb@kdbg.org>
On 5/15/26 12:00 PM, Johannes Sixt wrote:
> Am 14.05.26 um 16:33 schrieb Mark Levedahl:
>> git-gui uses git rev-parse --git-dir to get the pathname of the
>> discovered git repository. The returned value can be relative, and is
>> '.' if the current directory is the top of the repository directory
>> itself. git-gui has code to change '.' to [pwd] in this case so that
>> subsequent logic runs.
>>
>> But, git rev-parse supports --absolute-git-dir from fac60b8925
>> ("rev-parse: add option for absolute or relative path formatting",
>> 2020-12-13), and included in git 2.31. git-gui requires git >= 2.36, so
>> this more useful form is always available. Use --absolute-git-dir to
>> always get an absolute path, avoiding the need for other checks.
> Nice!
>
> However, the patch is incomplete. We set _gitdir also from
> lib/choose_repository.tcl. I think it would be best to swap this patch
> with patch 4/11, remove the _gitdir setters from the picker
> implementation, and call `rev-parse --absolute-git-dir` like you did in
> 4/11. This depends on that the picker sets the current directory to the
> top-level of the working tree with the embeded .git directory.
>
> BTW, missing sign-off.
I will change the interface to the picker so that success / failure is a returned value
rather than _gitdir being non-empty, then rework order and content of these patches.
Mark
^ permalink raw reply
* Re: [PATCH v1 04/11] git-gui: put choose_repository::pick in a proc
From: Mark Levedahl @ 2026-05-16 14:29 UTC (permalink / raw)
To: Johannes Sixt; +Cc: egg_mushroomcow, bootaina702, git
In-Reply-To: <7544deeb-163c-4444-833a-7b840a7caa4a@kdbg.org>
On 5/15/26 11:59 AM, Johannes Sixt wrote:
> Am 14.05.26 um 16:33 schrieb Mark Levedahl:
>> git-gui includes a 'repository picker', which allows creating a new
>> repository + worktree, or selecting a worktree from a recent list.
>> git-gui runs the picker when a valid git repository is not found. All of
>> the code for this is embedded in the discovery process block, making the
>> latter more difficult to read, and also making things more difficult if
>> we want to have an explicit 'pick' subcommand to force this to run.
> OK, let's see how useful this becomes.
>
>> Let's move this invocation and supporting code to a separate proc,
>> aiding in subsequent refactoring. Assure GIT_DIR and GIT_WORK_TREE are
>> unset, configuration is loaded, ant that _gitdir is correctly set
> s/ant/and/
will fix
>> afterwards. As this is invoked before worktree discovery, later code
>> will set that anyway so need not be included here.
>>
>> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
>> ---
>> git-gui.sh | 18 +++++++++++-------
>> 1 file changed, 11 insertions(+), 7 deletions(-)
>>
>> diff --git a/git-gui.sh b/git-gui.sh
>> index 387cad6..0b73c35 100755
>> --- a/git-gui.sh
>> +++ b/git-gui.sh
>> @@ -1139,6 +1139,16 @@ proc unset_gitdir_vars {} {
>> }
>>
>> set picked 0
>> +proc pick_repo {} {
>> + unset_gitdir_vars
>> + load_config 1
>> + apply_config
>> + choose_repository::pick
>> + set _gitdir [git rev-parse --absolute-git-dir]
>> + set _prefix {}
>> + set picked 1
>> +}
>> +
> So, this isn't intended as a plain move of code? Since we set _gitdir
> here, we could remove the corresonding lines from lib/choose_repository.tcl.
it should be a plain move.
> Is the variable "picked" only needed for this particular picker
> invocation? Then it should not be set in the function, but at the call site.
I need to better understand how "picked" is used to decide... will do before an update.
>> if {[catch {
>> set _gitdir $env(GIT_DIR)
>> set _prefix {}
>> @@ -1149,13 +1159,7 @@ if {[catch {
>> set _gitdir [git rev-parse --git-dir]
>> set _prefix [git rev-parse --show-prefix]
>> } err]} {
>> - load_config 1
>> - apply_config
>> - choose_repository::pick
>> - if {![file isdirectory $_gitdir]} {
>> - exit 1
>> - }
>> - set picked 1
>> + pick_repo
> The indentation is off here.
will fix.
Mark
^ permalink raw reply
* Re: [PATCH v1 03/11] git-gui: guard set/unset of GIT_DIR and GIT_WORK_TREE
From: Mark Levedahl @ 2026-05-16 14:25 UTC (permalink / raw)
To: Johannes Sixt; +Cc: egg_mushroomcow, bootaina702, git
In-Reply-To: <d9af8b60-e354-4cfc-87b3-a3e708b362da@kdbg.org>
On 5/15/26 11:58 AM, Johannes Sixt wrote:
> Am 14.05.26 um 16:33 schrieb Mark Levedahl:
>> git-gui unconditionally exports GIT_DIR and GIT_WORK_TREE to the
>> environment, and furthmore unconditionally unsets these in many places.
>> But, GIT_WORK_TREE should be set only if it is not {} as the empty
>> value, really meaning no work-tree is found, causes git to throw fatal
>> errors (git-gui gets the error from branch --show-current). Fixing this
>> is required to allow blame and browser to operate from a repository
>> without a worktree.
>>
>> Establish a pair of functions to remove GIT_DIR and GIT_WORK_TREE from
>> the environment, avoiding any error if they do not exist. Also, add a
>> function to export these, but export GIT_WORK_TREE only if not empty.
> Good. But as I said in a parallel thread, I actually concur with your
> assessment in the coverletter of this patch series that GIT_WORK_TREE
> should be not set at all. At least in the modes that require a working tree.
>
>> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
>> ---
>> git-gui.sh | 32 ++++++++++++++++++++++----------
>> 1 file changed, 22 insertions(+), 10 deletions(-)
>>
>> diff --git a/git-gui.sh b/git-gui.sh
>> index a951fcd..387cad6 100755
>> --- a/git-gui.sh
>> +++ b/git-gui.sh
>> @@ -1122,6 +1122,22 @@ unset argv0dir
>> ##
>> ## repository setup
>>
>> +proc set_gitdir_vars {} {
>> + global _gitdir _gitworktree env
>> + if {$_gitdir ne {}} {
>> + set env(GIT_DIR) $_gitdir
>> + }
>> + if {$_gitworktree ne {}} {
>> + set env(GIT_WORK_TREE) $_gitworktree
>> + }
>> +}
>> +
>> +proc unset_gitdir_vars {} {
>> + global env
>> + catch {unset env(GIT_DIR)}
>> + catch {unset env(GIT_WORK_TREE)}
>> +}
>> +
>> set picked 0
>> if {[catch {
>> set _gitdir $env(GIT_DIR)
>> @@ -1207,8 +1223,8 @@ if {[lindex $_reponame end] eq {.git}} {
>> set _reponame [lindex $_reponame end]
>> }
>>
>> -set env(GIT_DIR) $_gitdir
>> -set env(GIT_WORK_TREE) $_gitworktree
>> +# Export the final paths
>> +set_gitdir_vars
>>
>> ######################################################################
>> ##
>> @@ -2050,13 +2066,11 @@ proc do_gitk {revs {is_submodule false}} {
>> # TODO we could make life easier (start up faster?) for gitk
>> # by setting these to the appropriate values to allow gitk
>> # to skip the heuristics to find their proper value
>> - unset env(GIT_DIR)
>> - unset env(GIT_WORK_TREE)
>> + unset_gitdir_vars
>> }
>> safe_exec_bg [concat $cmd $revs "--" "--"]
>>
>> - set env(GIT_DIR) $_gitdir
>> - set env(GIT_WORK_TREE) $_gitworktree
>> + set_gitdir_vars
>> cd $pwd
>>
>> if {[info exists main_status]} {
>> @@ -2084,16 +2098,14 @@ proc do_git_gui {} {
>>
>> # see note in do_gitk about unsetting these vars when
>> # running tools in a submodule
>> - unset env(GIT_DIR)
>> - unset env(GIT_WORK_TREE)
>> + unset_gitdir_vars
>>
>> set pwd [pwd]
>> cd $current_diff_path
>>
>> safe_exec_bg [concat $exe gui]
>>
>> - set env(GIT_DIR) $_gitdir
>> - set env(GIT_WORK_TREE) $_gitworktree
>> + set_gitdir_vars
>> cd $pwd
>>
>> set status_operation [$::main_status \
> After these changes, a 'global env' probably becomes stale and could be
> removed.
>
> -- Hannes
>
Will update to only ever set GIT_DIR, will still remove GIT_WORK_TREE on unset.
Mark
^ permalink raw reply
* Re: [PATCH v1 02/11] git-gui: refactor browser / blame argument parsing
From: Mark Levedahl @ 2026-05-16 14:21 UTC (permalink / raw)
To: Johannes Sixt; +Cc: egg_mushroomcow, bootaina702, git
In-Reply-To: <cb3012ab-1a97-4197-bc57-34eb3fa472a2@kdbg.org>
On 5/15/26 11:56 AM, Johannes Sixt wrote:
snip...
> The check for the existence of files is actually necessary to
> disambiguate the meaning of the argument. If a file "maint" exists, then
> the argument is to be interpreted as path, not as the ref "maint", even
> if that exists, too.
>
> I suggest to protect the "file exists" calls with ($_gitworktree ne {}
> && ...) or (![is_bare] && ...) to handle being invoked from a bare
> repository. That is, in a bare repository we treat arguments the same as
> files that do not exist in the currently checked-out branch.
Let me start over, addressing only the use-case in a bare repository.
Mark
^ permalink raw reply
* Re: [PATCH v2] approxidate: make "today" wrap to midnight
From: Tuomas Ahola @ 2026-05-16 13:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King
In-Reply-To: <xmqqik8ncw98.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> wrote:
> Tuomas Ahola <taahol@utu.fi> writes:
>
> > Although some commands do reject invalid approxidate expressions,
> > in other cases those are simply evaluated as the current time.
> > Oftentimes that is a perfectly good compromise to handle silly
> > requests, but it isn't without rough edges.
> > ...
> > Bind "today" to new function `date_today()` as an approxidate
> > special. Make it return the last midnight if no specific time
> > is given; i.e. retain the old behavior of "noon today" and such.
> >
> > Document the new behavior of "git log --since=today" in
> > rev-list-options.adoc.
> >
> > Signed-off-by: Tuomas Ahola <taahol@utu.fi>
> > ---
>
> I like this construction of argument.
>
> How does this patch mesh with your earlier effort to make "noon" and
> "tea" more sensible? Should we eject the "today is now" step from
> that series and instead queue this patch in its place?
>
> Thanks.
Yes. I have already rebased the series on top of this patch. I will
soon(ish) post v4 without "today is now" step.
--Tuomas
^ 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