Re: [PATCH v3 5/5] builtin/stash: merge index in-core
- From
D. Ben Knoble <ben.knoble@gmail.com>
- Date
- Sep 28, 2026, 12:02 UTC
- Message-ID
- <CALnO6CBTsfMsPrkSMHj6bRMqHc3vEfNEJnh6vz=2+_qCf_26Sg@mail.gmail.com>
- In-Reply-To
- <xmqqo6dir04i.fsf@gitster.g>
On Sun, Sep 27, 2026 at 2:59 PM Junio C Hamano <gitster@pobox.com> wrote:
Show 34 quoted lines
>
> "D. Ben Knoble" <ben.knoble@gmail.com> writes:
>
> > @@ -671,29 +627,27 @@ static enum stash_apply_result do_apply_stash(const char *prefix,
> > oideq(&c_tree, &info->i_tree)) {
> > has_index = 0;
> > } else {
> > - struct strbuf out = STRBUF_INIT;
> > + struct merge_result result = { 0 };
> >
> > - if (diff_tree_binary(&out, &info->w_commit)) {
> > - strbuf_release(&out);
> > - return error(_("could not generate diff %s^!."),
> > - oid_to_hex(&info->w_commit));
> > - }
> > + o.branch1 = "Current index";
> > + o.branch2 = "Stashed index changes";
> > + o.ancestor = "Stash base";
> >
> > - ret = apply_cached(&out);
> > - strbuf_release(&out);
> > - if (ret)
> > + o.verbosity = 0;
>
> We realize that 'o' is a struct merge_options defined on the stack
> for this function, initialized with init_ui_merge_options() fairly
> early on. It would have initialized '.verbosity' to the default
> verbosity, the merge.verbosity configuration variable, or the
> GIT_MERGE_VERBOSITY environment variable.
>
> You drop the verbosity here, presumably because you want to match
> the previous implementation 'diff-tree | apply --cached' (which I
> guess was fairly quiet, but I do not use 'stash pop --index'
> myself).Yes, the original piped "apply --cached" output to a strbuf and discarded it.
Show 22 quoted lines
> > +
> > + if (!result.clean)
> > return error(_("conflicts in index. "
> > "Try without --index."));
> > -
> > - discard_index(the_repository->index);
> > - repo_read_index(the_repository);
> > - if (write_index_as_tree(&index_tree, the_repository->index,
> > - repo_get_index_file(the_repository), 0, NULL))
> > - return error(_("could not save index tree"));
> > -
> > - reset_head();
> > - discard_index(the_repository->index);
> > - repo_read_index(the_repository);
> > }
> > }
>
>
> But the thing is, this is not the end of the function, or the last
> call to the merge machinery using 'o'. We then use the same 'o' to
> drive another three-way merge. Yet nobody restores '.verbosity'
> that was unconditionally turned off above for that second merge.But you're right, we should restore the verbosity (which is not what the sketch patch does exactly). Will fix.