From: "Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail•com>
To: "Toon Claes" <toon@iotcl•com>, git@vger•kernel.org
Cc: "Jeff King" <peff@peff•net>, "Taylor Blau" <me@ttaylorr•com>,
"Derrick Stolee" <stolee@gmail•com>,
"Ævar Arnfjörð Bjarmason" <avarab@gmail•com>
Subject: Re: [PATCH RFC v2 0/5] Introduce git-last-modified(1) command
Date: Tue, 01 Jul 2025 22:35:13 +0200 [thread overview]
Message-ID: <f0c508cc-5c6b-4c4b-a3f3-0cdd8d1071e5@app.fastmail.com> (raw)
In-Reply-To: <20250523-toon-new-blame-tree-v2-0-101e4ca4c1c9@iotcl.com>
On Fri, May 23, 2025, at 11:33, Toon Claes wrote:
> This is another attempt to upstream the ~~git-blame-tree(1)~~
> git-last-modified(1) subcommand. After my previous attempt[1] the
> people of GitHub shared their version of the subcommand, and this
> version integrates those changes.
>
> What is different from the series shared by GitHub:
>
> * Renamed the subcommand from `blame-tree` to `last-modified`. There was
> some consensus[4] this name works better, so let's give it a try and
> see how this name feels.
>
> * Patches for --max-depth are excluded. I think it's a separate topic to
> discuss and I'm not sure it needs to be part of series anyway. The
> main patch was submitted in the previous attempt[2] and if people
> consider it valuable, I'm happy to discuss that in a separate patch
> series.
>
> * The patches in 'tb/blame-tree' at Taylor's fork[3] implements a
> caching layer. This feature reads/writes cached results in
> `.git/blame-tree/<hash>.btc`. To keep this series to a reviewable
> size, that feature is excluded from this series. I think it's better
> to submit this as a separate series.
>
> * Squashed various commits together. Like they introduced a flag
> `--go-faster`, which later became the default and only implementation.
> That story was wrapped up in a single commit.
>
> * The last-modified command isn't recursive by default. If you want
> recurse into subtrees, you need to pass `-r`.
>
> * Fixed all memory leaks, and removed the use of
> USE_THE_REPOSITORY_VARIABLE.
>
> I've attempted to reuse commit messages as good as possible, but feel
> free to correct me where you think I didn't give proper credit or messed
> up. Although I have no idea what to do with the Signed-off-by trailers.
>
> I didn't modify the benchmark results in the commit messages, simply
> because I didn't get comparable results. In my benchmarks the difference
> between two implementations was negligible, and even in some scenarios
> the performance was worse in the "improved" implementation. As far as I
> can tell, I didn't break anything in my refactoring, because the version
> in these patches acts similar to Taylor's branch. To be honest, I cannot
> explain why...?
>
> Again thanks to Taylor and the people at GitHub for sharing these
> patches. I hope we can work together to get this upstreamed.
>
> [1]:
> https://lore.kernel.org/git/20250326-toon-blame-tree-v1-0-4173133f3786@iotcl.com/
> [2]:
> https://lore.kernel.org/git/20250326-toon-blame-tree-v1-3-4173133f3786@iotcl.com/
> [3]: git@github•com:ttaylorr/git.git
> [4]: https://lore.kernel.org/git/aCbBKj7O9LjO3SMK@pks.im/
> --
> Cheers,
> Toon
>
> Signed-off-by: Toon Claes <toon@iotcl•com>
> ---
> Changes in v2:
> - The subcommand is renamed from `blame-tree` to `last-modified`
> - Documentation is added. Here we mark the command as experimental.
> - Some test cases are added related to merges.
> - Link to v1:
> https://lore.kernel.org/r/20250422-toon-new-blame-tree-v1-0-fdb51b8a394a@iotcl.com
It feels like the command strays a bit from the usual patterns to me. For paths/files
that is. I like this:
```
$ git last-modified -r refs.c refs.h
062b914c841329a003f74e1340ea5178391274a6 refs.c
47478802daddf3f9916111307f153c6298ffc0bc refs.h
```
I ask for two files and I get those in the output.
But for individual files in subdirectories:
```
$ git last-modified refs.c refs.h Documentation/git-last-modified.adoc Documentation/git-config.adoc
3691fe72d927658ae77ade7fe967544fc6739e67 Documentation
062b914c841329a003f74e1340ea5178391274a6 refs.c
47478802daddf3f9916111307f153c6298ffc0bc refs.h
```
Same as if I ask for `Documentation`:
```
$ git last-modified refs.c refs.h Documentation/
3691fe72d927658ae77ade7fe967544fc6739e67 Documentation
062b914c841329a003f74e1340ea5178391274a6 refs.c
47478802daddf3f9916111307f153c6298ffc0bc refs.h
```
But I didn’t ask for the directory first. I asked for two files.
I have to use `-r` (recurse):
```
$ git last-modified -r refs.c refs.h Documentation/git-last-modified.adoc Documentation/git-config.adoc
3691fe72d927658ae77ade7fe967544fc6739e67 Documentation/git-last-modified.adoc
062b914c841329a003f74e1340ea5178391274a6 refs.c
47478802daddf3f9916111307f153c6298ffc0bc refs.h
0fbe93b36c05bbf4156c157f27998938ce312265 Documentation/git-config.adoc
```
And `-r` with a directory like `Documentation` will recurse through that
directory.
As a user I imagine I want `-r` to control whether to, say, only show
each directory under `Documentation`. But now you seem to get a special
case of allowing directly listing first-level files but not the ones in
subdirectories.
I’m more used to being able to use individual files if I want that
as well as pathspecs for recursion. But now I just get the directory:
```
git last-modified -- 't/*'
532d9a0984e6464deadb6bdb0287fcce2990adc9 t
```
But I can still use pathspec magic which is nice:
```
$ git last-modified -r -- 't/*' ':^t/t1016*' | grep 1016
<empty>
```
I’m imagining that you may want to feed a specific pathspec to the
command and get only the output for those that match. Without having
worry about turning on recursion since that may cover some of the things
you want but also make it do too much. Well maybe that is just as
controllable here but it seems less obvious than for commands where the
pathspec is used more directly (?). I appreciate when these commands
allow me to express things directly without postprocessing (no excessive
pipelining).
Also you get a sort of trailing error if you give a non-existing option:
```
$ git last-modified --recursive
error: unknown last-modified argument: --recursive
fatal: error setting up last-modified traversal
```
next prev parent reply other threads:[~2025-07-01 20:35 UTC|newest]
Thread overview: 135+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-22 17:46 [PATCH RFC 0/5] Introduce git-blame-tree(1) command Toon Claes
2025-04-22 17:46 ` [PATCH RFC 1/5] blame-tree: introduce new subcommand to blame files Toon Claes
2025-04-24 16:19 ` Junio C Hamano
2025-05-07 13:13 ` Toon Claes
2025-04-22 17:46 ` [PATCH RFC 2/5] t/perf: add blame-tree perf script Toon Claes
2025-04-22 17:46 ` [PATCH RFC 3/5] blame-tree: use Bloom filters when available Toon Claes
2025-04-22 17:46 ` [PATCH RFC 4/5] blame-tree: implement faster algorithm Toon Claes
2025-04-22 17:46 ` [PATCH RFC 5/5] blame-tree.c: initialize revision machinery without walk Toon Claes
2025-04-23 13:26 ` [PATCH RFC 0/5] Introduce git-blame-tree(1) command Marc Branchaud
2025-05-07 14:22 ` Toon Claes
2025-05-07 20:23 ` Marc Branchaud
2025-05-07 20:45 ` Junio C Hamano
2025-05-08 13:26 ` Marc Branchaud
2025-05-08 14:26 ` Junio C Hamano
2025-05-08 15:12 ` Marc Branchaud
2025-05-14 14:42 ` Toon Claes
2025-05-14 19:29 ` Junio C Hamano
2025-05-14 21:15 ` Marc Branchaud
2025-05-15 13:29 ` Patrick Steinhardt
2025-05-15 16:39 ` Junio C Hamano
2025-05-15 17:39 ` Marc Branchaud
2025-05-15 19:30 ` Jeff King
2025-05-16 4:38 ` Patrick Steinhardt
2025-05-20 8:49 ` Toon Claes
2025-05-15 17:30 ` Marc Branchaud
2025-05-16 4:30 ` Patrick Steinhardt
2025-05-14 21:15 ` Marc Branchaud
2025-05-07 20:49 ` Kristoffer Haugsbakk
2025-05-08 13:20 ` D. Ben Knoble
2025-05-08 13:26 ` Marc Branchaud
2025-05-08 13:18 ` D. Ben Knoble
2025-05-23 9:33 ` [PATCH RFC v2 0/5] Introduce git-last-modified(1) command Toon Claes
2025-05-23 9:33 ` [PATCH RFC v2 1/5] last-modified: new subcommand to show when files were last modified Toon Claes
2025-05-25 20:07 ` Justin Tobler
2025-06-05 8:32 ` Toon Claes
2025-05-27 10:39 ` Patrick Steinhardt
2025-06-13 9:34 ` Toon Claes
2025-06-13 9:52 ` Kristoffer Haugsbakk
2025-05-23 9:33 ` [PATCH RFC v2 2/5] t/perf: add last-modified perf script Toon Claes
2025-05-23 9:33 ` [PATCH RFC v2 3/5] last-modified: use Bloom filters when available Toon Claes
2025-05-27 10:40 ` Patrick Steinhardt
2025-06-13 11:05 ` Toon Claes
2025-05-23 9:33 ` [PATCH RFC v2 4/5] last-modified: implement faster algorithm Toon Claes
2025-05-27 10:39 ` Patrick Steinhardt
2025-05-23 9:33 ` [PATCH RFC v2 5/5] last-modified: initialize revision machinery without walk Toon Claes
2025-05-27 10:39 ` Patrick Steinhardt
2025-07-01 20:35 ` Kristoffer Haugsbakk [this message]
2025-07-01 21:06 ` [PATCH RFC v2 0/5] Introduce git-last-modified(1) command Junio C Hamano
2025-07-01 21:30 ` Kristoffer Haugsbakk
2025-07-02 13:00 ` Toon Claes
2025-07-09 15:53 ` Toon Claes
2025-07-09 17:00 ` Junio C Hamano
2025-06-30 18:49 ` [PATCH RFC v3 0/3] " Toon Claes
2025-06-30 18:49 ` [PATCH RFC v3 1/3] last-modified: new subcommand to show when files were last modified Toon Claes
2025-07-01 20:20 ` Kristoffer Haugsbakk
2025-07-02 11:51 ` Junio C Hamano
2025-06-30 18:49 ` [PATCH RFC v3 2/3] t/perf: add last-modified perf script Toon Claes
2025-06-30 18:49 ` [PATCH RFC v3 3/3] last-modified: use Bloom filters when available Toon Claes
2025-07-01 23:01 ` [PATCH RFC v3 0/3] Introduce git-last-modified(1) command Junio C Hamano
2025-07-09 15:26 ` [PATCH v4 " Toon Claes
2025-07-09 21:57 ` Junio C Hamano
2025-07-10 18:37 ` Junio C Hamano
2025-07-16 13:32 ` [PATCH v5 0/6] " Toon Claes
2025-07-16 13:35 ` [PATCH v5 1/6] last-modified: new subcommand to show when files were last modified Toon Claes
2025-07-18 0:02 ` Taylor Blau
2025-07-19 6:44 ` Jeff King
2025-07-22 15:50 ` Toon Claes
2025-08-01 9:09 ` Christian Couder
2025-08-01 16:59 ` Junio C Hamano
2025-07-16 13:35 ` [PATCH v5 2/6] t/perf: add last-modified perf script Toon Claes
2025-07-18 0:08 ` Taylor Blau
2025-07-22 15:52 ` Toon Claes
2025-07-16 13:35 ` [PATCH v5 3/6] last-modified: use Bloom filters when available Toon Claes
2025-07-18 0:16 ` Taylor Blau
2025-07-22 16:02 ` Toon Claes
2025-07-16 13:35 ` [PATCH v5 4/6] pretty: allow caller to disable indentation Toon Claes
2025-07-16 15:50 ` Junio C Hamano
2025-07-17 16:31 ` Toon Claes
2025-07-16 13:35 ` [PATCH v5 5/6] last-modified: support --extended format Toon Claes
2025-07-16 16:09 ` Junio C Hamano
2025-07-17 16:31 ` Toon Claes
2025-07-17 22:37 ` Junio C Hamano
2025-07-18 17:36 ` Junio C Hamano
2025-07-22 16:06 ` Toon Claes
2025-07-16 13:42 ` [PATCH v5 6/6] fixup! last-modified: use Bloom filters when available Toon Claes
2025-07-17 23:39 ` [PATCH v5 0/6] Introduce git-last-modified(1) command Taylor Blau
2025-07-22 15:35 ` Toon Claes
2025-07-30 17:59 ` Toon Claes
2025-07-31 7:45 ` Patrick Steinhardt
2025-07-30 17:55 ` [PATCH v6 0/4] " Toon Claes
2025-07-31 18:40 ` Junio C Hamano
2025-07-31 23:57 ` Junio C Hamano
2025-08-05 9:33 ` [PATCH v7 0/3] " Toon Claes
2025-08-05 14:34 ` Patrick Steinhardt
2025-08-05 16:21 ` Junio C Hamano
2025-08-05 16:34 ` Junio C Hamano
2025-08-05 16:55 ` Toon Claes
2025-08-05 17:20 ` Jean-Noël AVILA
2025-08-05 21:46 ` Junio C Hamano
2025-08-06 12:01 ` Toon Claes
2025-08-06 15:38 ` Junio C Hamano
2025-08-28 22:44 ` Junio C Hamano
2025-08-05 18:28 ` Junio C Hamano
2025-08-05 9:33 ` [PATCH v7 1/3] last-modified: new subcommand to show when files were last modified Toon Claes
2025-08-05 9:33 ` [PATCH v7 2/3] t/perf: add last-modified perf script Toon Claes
2025-08-05 9:33 ` [PATCH v7 3/3] last-modified: use Bloom filters when available Toon Claes
2025-07-30 17:55 ` [PATCH v6 1/4] last-modified: new subcommand to show when files were last modified Toon Claes
2025-07-31 6:42 ` Patrick Steinhardt
2025-08-01 16:22 ` Toon Claes
2025-08-01 17:09 ` Junio C Hamano
2025-08-04 6:34 ` Patrick Steinhardt
2025-08-04 17:14 ` Junio C Hamano
2025-08-05 5:35 ` Toon Claes
2025-08-01 20:34 ` Jean-Noël AVILA
2025-08-05 5:36 ` Toon Claes
2025-08-04 6:33 ` Patrick Steinhardt
2025-08-01 10:18 ` Christian Couder
2025-08-01 10:22 ` Patrick Steinhardt
2025-08-01 17:06 ` Junio C Hamano
2025-08-02 8:18 ` Christian Couder
2025-08-02 11:31 ` Christian Couder
2025-08-02 13:38 ` Christian Couder
2025-08-02 16:26 ` Junio C Hamano
2025-08-04 6:35 ` Patrick Steinhardt
2025-07-30 17:55 ` [PATCH v6 2/4] t/perf: add last-modified perf script Toon Claes
2025-07-30 17:55 ` [PATCH v6 3/4] commit-graph: export prepare_commit_graph() Toon Claes
2025-07-31 6:42 ` Patrick Steinhardt
2025-07-30 17:55 ` [PATCH v6 4/4] last-modified: use Bloom filters when available Toon Claes
2025-07-31 6:43 ` Patrick Steinhardt
2025-08-01 16:23 ` Toon Claes
2025-08-04 6:33 ` Patrick Steinhardt
2025-07-09 15:26 ` [PATCH v4 1/3] last-modified: new subcommand to show when files were last modified Toon Claes
2025-07-09 15:26 ` [PATCH v4 2/3] t/perf: add last-modified perf script Toon Claes
2025-07-09 15:26 ` [PATCH v4 3/3] last-modified: use Bloom filters when available Toon Claes
2025-07-16 13:35 ` [PATCH v5 6/6] fixup! " Toon Claes
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=f0c508cc-5c6b-4c4b-a3f3-0cdd8d1071e5@app.fastmail.com \
--to=kristofferhaugsbakk@fastmail$(echo .)com \
--cc=avarab@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=me@ttaylorr$(echo .)com \
--cc=peff@peff$(echo .)net \
--cc=stolee@gmail$(echo .)com \
--cc=toon@iotcl$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox