Re: path-restricted log vs. subtree merges

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Mar 16, 2025, 16:32 UTC
Message-ID: <2d53bebd-e85e-7414-c7ba-bd1f31810cea@gmx.de>
In-Reply-To: <Z9WSbUQw4VBQiOqi@ugly>

Hi Oswald,

On Sat, 15 Mar 2025, Oswald Buddenhagen wrote:

Show 12 quoted lines

> in my quest to prepare for hacking git-gui, i ran
>
>   cd .../git/git-gui
>   git log .
>
> the result is ... unhelpful. apart from a few commits that were done
> directly in the git repo, there are only merges. notably, there is no
> trace of the commits that are being merged (and the way gitk presents
> this partial log is even more unhelpful).
>
> i'm speculating that this is the result of git log not handling the
> subtree merges in any way, with somewhat predictable consequences.

That sounds highly plausible.

> i suppose there is overlap with the fact that --follow works only for a
> single file, which is also unfortunate. (a recent test case in the repo
> would be templates/hooks.)

I just ran `git log --follow --raw upstream/seen -- git-gui/` and it does not error out, but as you say, it is unhelpful.

It's probably not easy to fix this.

I had a quick look at the relevant function (`try_to_follow_renames()` in `tree-diff.c`), and my intuition told me that

- `choice = q->queue[0];` would need to be replaced with a loop that adds
  the `q->queue` elements to a `strset`,

- `!strcmp(p->two->path, opt->pathspec.items[0].match)` would need to be
  scrapped in favor of testing in a loop whether `p->two->path` is matched
  by any of `opt->pathspec.items` (because there would now be more than
  1),

- `const char *path[2];` would need to be replaced by a `strvec`, and the
  entire `pathspec` replacing could only be done after the `for (i = 0; i
  < q->nr; i++)` loop completed,

- The `break;` out of the loop could only be done once all of the input
  paths were accounted for.

However, the devil lies in more details. For example, I expected that `git log --follow -- git-gui/` would error out because `--follow` requires a single file to work. This suggests to me that the logic would have to be modified to expand such implicit pathspec into a full list of files.

And I imagine that the performance of this new logic may be quite horrible, what with adding a nested loop for the `pathspec` matching inside `try_to_follow_renames()` existing loop.

Of course, the fact that Git silently accepts `--follow -- <dir>/` despite the fact that it cannot handle `<dir>` renames at all is a bummer.

Ciao, Johannes