path-restricted log vs. subtree merges

path-restricted log vs. subtree merges

[Oswald Buddenhagen]

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.

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.)

Re: path-restricted log vs. subtree merges

[Johannes Schindelin]

Hi Oswald,

On Sat, 15 Mar 2025, Oswald Buddenhagen wrote:

> 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

Re: path-restricted log vs. subtree merges

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> 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),

These are needed regardless of "following renames of directory".
The fact that there is one single path being followed globally means
that the current "check-box item" design of --follow-renames would
not work in a mergy history, where the path that holds the contents
being followed is called differently on different branches and given
a name (either taking one of the names from these branches, or a
nname entirely different from any of the ancestors) at the merge, or
a path is renamed differently along the forked history, eventually
getting merged back.  Even in a "follow this single path" mode, you'd
need the notion of "this is the path being followed while we are on
this single strand of commits" that is kept per ancestors whenever
you hit a merge while traversing from child to its parents.

The current "following single path" would need to be done correctly,
and then "following more than one paths from anywhere in the tree"
would fall out as a natural consequence once it is done.  "following
renames of a directory" would mostly be a special case of "following
more than one paths from anywhere in the tree".

Thanks.

Re: path-restricted log vs. subtree merges

[Jeff King]

On Sat, Mar 15, 2025 at 03:45:01PM +0100, Oswald Buddenhagen wrote:

> 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.

Yes, the merged-in paths and the paths in the original commits are
different. So the merge will update path "git-gui/git-gui.sh", etc, but
the individual commits are touching "git-gui.sh" at the root of the tree
(in the git-gui repository).

Leaving aside for a moment how --follow and other tooling could help,
the workaround in this case is:

  git log -- git-gui/ git-gui.sh lib/

You need to ask for the paths you care about at the root-level (since
that's what the individual commits from the git-gui repo will have). But
you also need to include git-gui/; otherwise history simplification will
not go down the side branches from the git-gui repo (you can also use
--full-history, but that would include a lot of irrelevant stuff). And
you need to use "--" before the paths, since the rev/path dwim logic
will only treat an argument as a path if it is present in the working
tree (which the latter two paths are not).

-Peff