{"thread":{"id":"63874","subject":"[PATCH 0/3] Teach git-diff-tree(1) option --max-depth","startedAt":"2025-07-29T18:58:07Z","lastAt":"2025-08-15T05:17:26Z","messageCount":16,"participants":["Toon Claes","Patrick Steinhardt","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"522980","messageId":"20250729-toon-max-depth-v1-0-c177e39c40fb@iotcl.com","threadId":"63874","inReplyTo":null,"subject":"[PATCH 0/3] Teach git-diff-tree(1) option --max-depth","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2025-07-29T18:57:41Z","receivedAt":"2025-07-29T18:58:07Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Please consider these patches authored by Peff. They add option\n`--max-depth` to the diff machinery.\n\nDuring the process to upstream the git-blame-tree(1), later named\ngit-last-modified(1), various times[1][2][3] the topic was raised to add\noption `--max-depth` to the diff machinery. In this patch series this\nchange is added as a separate patch series.\n\nThe patches were originally written by Peff[4] and I'm crediting him as\nthe author. I've taken the patches almost verbatim from his fork on\nGitHub, with some minor tweaks in the commit messages. Because only tiny\nchanges were made, I've kept his Signed-off-by trailers, but I can\nremove if disagreed on.\n\nThe goal of the option `--max-depth` is to stop recursively traversing\nthe tree if the given depth is reached from the pathspec.\n\nThese patches add `max_depth` and `max_depth_valid` to `struct\ndiff_options`. This is different from what git-grep(1) does, which uses\n`max_depth` on `struct pathspec` instead. At the moment I'm on the fence\nwhether this is an issue: while it probably makes sense to consolidate\nthem into the same structs, it does not really make sense to reuse these\nthe struct fields if they are used in two separate code paths.\n\n[1]: https://lore.kernel.org/git/20130318121243.GC14789@sigill.intra.peff.net/\n[2]: https://lore.kernel.org/git/20160831054201.ldlwptlmcndjmfwu@sigill.intra.peff.net/\n[3]: https://lore.kernel.org/git/Y+%2FmnnJUz75yfWCN@coredump.intra.peff.net/\n[4]: https://github.com/peff/git/tree/jk/diff-max-depth\n\n---\nJeff King (3):\n combine-diff: zero memory used for callback filepairs\n within_depth: fix return for empty path\n diff: teach tree-diff a max-depth parameter\n\n Documentation/diff-options.adoc | 28 +++++++++++\n combine-diff.c | 2 +-\n diff-lib.c | 5 ++\n diff.c | 19 +++++++\n diff.h | 9 ++++\n dir.c | 2 +-\n t/meson.build | 1 +\n t/t4072-diff-max-depth.sh | 109 ++++++++++++++++++++++++++++++++++++++++\n tree-diff.c | 78 ++++++++++++++++++++++++++--\n 9 files changed, 248 insertions(+), 5 deletions(-)\n---\n\n\n\n---\n\nbase-commit: e813a0200a7121b97fec535f0d0b460b0a33356c\nchange-id: 20250724-toon-max-depth-25d3c19e2607\n\nThanks\n--\nToon\n\n"},{"id":"522981","messageId":"20250729-toon-max-depth-v1-1-c177e39c40fb@iotcl.com","threadId":"63874","inReplyTo":"20250729-toon-max-depth-v1-0-c177e39c40fb@iotcl.com","subject":"[PATCH 1/3] combine-diff: zero memory used for callback filepairs","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2025-07-29T18:57:42Z","receivedAt":"2025-07-29T18:58:10Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"From: Jeff King \n\nIn commit 25e5e2bf85 (combine-diff: support format_callback,\n2011-08-19), the combined-diff code learned how to make a multi-sourced\n`diff_filepair` to pass to a diff callback. When we create each\nfilepair, we do not bother to fill in many of the fields, because they\nwould make no sense (e.g. there can be no rename score or broken_pair\nflag because we do not go through the diffcore filters). However, we did\nnot even bother to zero them, leading to random values. Let's make sure\neverything is blank with xcalloc(), just as the regular diff code does.\n\nWe would potentially want to set the `status` flag to\nsomething non-zero, but it is not clear to what. Possibly a\nnew DIFF_STATUS_COMBINED would make sense, as this is not\nstrictly a modification, nor does it fit any other category.\n\nSince it is not yet clear what callers would want, this\npatch simply leaves it as `0`, the same empty flag that is\nseen when `diffcore_std` is not used at all.\n\nSigned-off-by: Jeff King \nSigned-off-by: Toon Claes \n---\n combine-diff.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/combine-diff.c b/combine-diff.c\nindex 4ea2dc93c4..3878faabe7 100644\n--- a/combine-diff.c\n+++ b/combine-diff.c\n@@ -1315,7 +1315,7 @@ static struct diff_filepair *combined_pair(struct combine_diff_path *p,\n \tstruct diff_filepair *pair;\n \tstruct diff_filespec *pool;\n \n-\tpair = xmalloc(sizeof(*pair));\n+\tCALLOC_ARRAY(pair, 1);\n \tCALLOC_ARRAY(pool, st_add(num_parent, 1));\n \tpair->one = pool + 1;\n \tpair->two = pool;\n\n-- \n2.50.1.327.g047016eb4a\n\n"},{"id":"522982","messageId":"20250729-toon-max-depth-v1-2-c177e39c40fb@iotcl.com","threadId":"63874","inReplyTo":"20250729-toon-max-depth-v1-0-c177e39c40fb@iotcl.com","subject":"[PATCH 2/3] within_depth: fix return for empty path","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2025-07-29T18:57:43Z","receivedAt":"2025-07-29T18:58:14Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"From: Jeff King \n\nThe within_depth() function is used to check whether pathspecs limited\nby a max-depth parameter are acceptable. It takes a path to check, a\nmaximum depth, and a \"base\" depth. It counts the components in the\npath (by counting slashes), adds them to the base, and compare them to\nthe maximum.\n\nHowever, if the base does not have any slashes at all, we always return\n`true`. If the base depth is 0, then this is correct; no matter what the\nmaximum is, we are always within it. However, if the base depth is\ngreater than 0, then we might return an erroneous result.\n\nThis ends up not causing any user-visible bugs in the current code. The\ncall sites in dir.c always pass a base depth of 0, so are unaffected.\nBut tree_entry_interesting() uses this function differently: it will\npass the prefix of the current entry, along with a `1` if the entry is a\ndirectory, in essence checking whether items inside the entry would be\nof interest. It turns out not to make a difference in behavior, but the\nreasoning is complex.\n\nGiven a tree like:\n\n file\n a/file\n a/b/file\n\nwalking the tree and calling tree_entry_interesting() will yield the\nfollowing results:\n\n (with max_depth=0):\n file: yes\n a: yes\n a/file: no\n a/b: no\n\n (with max_depth=1):\n file: yes\n a: yes\n a/file: yes\n a/b: no\n\nSo we have inconsistent behavior in considering directories interesting.\nIf they are at the edge of our depth but at the root, we will recurse\ninto them, but then find all of their entries uninteresting (e.g., in\nthe first case, we will look at \"a\" but find \"a/*\" uninteresting). But\nif they are at the edge of our depth and not at the root, then we will\nnot recurse (in the second example, we do not even bother entering\n\"a/b\").\n\nThis turns out not to matter because the only caller which uses\nmax-depth pathspecs is cmd_grep(), which only cares about blob entries.\nFrom its perspective, it is exactly the same to not recurse into a\nsubtree, or to recurse and find that it contains no matching entries.\nNot recursing is merely an optimization.\n\nIt is debatable whether tree_entry_interesting() should consider such an\nentry interesting. The only caller does not care if it sees the tree\nitself, and can benefit from the optimization. But if we add a\n\"max-depth\" limiter to regular diffs, then a diff with\nDIFF_OPT_TREE_IN_RECURSIVE would probably want to show the tree itself,\nbut not what it contains.\n\nThis patch just fixes within_depth(), which means we consider such\nentries uninteresting (and makes the current caller happy). If we want\nto change that in the future, then this fix is still the correct first\nstep, as the current behavior is simply inconsistent.\n\nSigned-off-by: Jeff King \nSigned-off-by: Toon Claes \n---\n dir.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/dir.c b/dir.c\nindex 02873f59ea..900ee5e559 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -277,7 +277,7 @@ int within_depth(const char *name, int namelen,\n \t\tif (depth > max_depth)\n \t\t\treturn 0;\n \t}\n-\treturn 1;\n+\treturn depth <= max_depth;\n }\n \n /*\n\n-- \n2.50.1.327.g047016eb4a\n\n"},{"id":"522983","messageId":"20250729-toon-max-depth-v1-3-c177e39c40fb@iotcl.com","threadId":"63874","inReplyTo":"20250729-toon-max-depth-v1-0-c177e39c40fb@iotcl.com","subject":"[PATCH 3/3] diff: teach tree-diff a max-depth parameter","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2025-07-29T18:57:44Z","receivedAt":"2025-07-29T18:58:17Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"From: Jeff King \n\nWhen you are doing a tree-diff, there are basically two options: do not\nrecurse into subtrees at all, or recurse indefinitely. While most\ncallers would want to always recurse and see full pathnames, some may\nwant the efficiency of looking only at a particular level of the tree.\nThis is currently easy to do for the top-level (just turn off\nrecursion), but you cannot say \"show me what changed in subdir/, but do\nnot recurse\".\n\nThis patch adds a max-depth parameter which is measured from the closest\npathspec match, so that you can do:\n\n git log --raw --max-depth=1 -- a/b/c\n\nand see the raw output for a/b/c/, but not those of a/b/c/d/\n(instead of the raw output you would see for a/b/c/d).\n\nSigned-off-by: Jeff King \nSigned-off-by: Toon Claes \n---\n Documentation/diff-options.adoc | 28 +++++++++++\n diff-lib.c | 5 ++\n diff.c | 19 +++++++\n diff.h | 9 ++++\n t/meson.build | 1 +\n t/t4072-diff-max-depth.sh | 109 ++++++++++++++++++++++++++++++++++++++++\n tree-diff.c | 78 ++++++++++++++++++++++++++--\n 7 files changed, 246 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/diff-options.adoc b/Documentation/diff-options.adoc\nindex f3a35d8141..46e6ed2d67 100644\n--- a/Documentation/diff-options.adoc\n+++ b/Documentation/diff-options.adoc\n@@ -893,5 +893,33 @@ endif::git-format-patch[]\n \treverted with `--ita-visible-in-index`. Both options are\n \texperimental and could be removed in future.\n \n+--max-depth=::\n+\n+\tLimit diff recursion to `` levels (implies `-r`). The depth\n+\tis measured from the closest pathspec. Given a tree containing\n+\t`foo/bar/baz`, the following list shows the matches generated by\n+\teach set of options:\n++\n+--\n+ - `--max-depth=0 -- foo`: `foo`\n+\n+ - `--max-depth=1 -- foo`: `foo/bar`\n+\n+ - `--max-depth=1 -- foo/bar`: `foo/bar/baz`\n+\n+ - `--max-depth=1 -- foo foo/bar`: `foo/bar/baz`\n+\n+ - `--max-depth=2 -- foo`: `foo/bar/baz`\n+--\n++\n+If no pathspec is given, the depth is measured as if all\n+top-level entries were specified. Note that this is different\n+than measuring from the root, in that `--max-depth=0` would\n+still return `foo`. This allows you to still limit depth while\n+asking for a subset of the top-level entries.\n++\n+Note that this option is only supported for diffs between tree objects,\n+not against the index or working tree.\n+\n For more detailed explanation on these common options, see also\n linkgit:gitdiffcore[7].\ndiff --git a/diff-lib.c b/diff-lib.c\nindex 244468dd1a..fa7c24c267 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -115,6 +115,9 @@ void run_diff_files(struct rev_info *revs, unsigned int option)\n \tuint64_t start = getnanotime();\n \tstruct index_state *istate = revs->diffopt.repo->index;\n \n+\tif (revs->diffopt.max_depth_valid)\n+\t\tdie(\"max-depth is not supported for worktree diffs\");\n+\n \tdiff_set_mnemonic_prefix(&revs->diffopt, \"i/\", \"w/\");\n \n \trefresh_fsmonitor(istate);\n@@ -560,6 +563,8 @@ static int diff_cache(struct rev_info *revs,\n \topts.dst_index = NULL;\n \topts.pathspec = &revs->diffopt.pathspec;\n \topts.pathspec->recursive = 1;\n+\tif (revs->diffopt.max_depth_valid)\n+\t\tdie(\"max-depth is not supported for index diffs\");\n \n \tinit_tree_desc(&t, &tree->object.oid, tree->buffer, tree->size);\n \treturn unpack_trees(1, &t, &opts);\ndiff --git a/diff.c b/diff.c\nindex dca87e164f..c03a59ac3b 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -4988,6 +4988,9 @@ void diff_setup_done(struct diff_options *options)\n \t\t\toptions->filter = ~filter_bit[DIFF_STATUS_FILTER_AON];\n \t\toptions->filter &= ~options->filter_not;\n \t}\n+\n+\tif (options->pathspec.has_wildcard && options->max_depth_valid)\n+\t\tdie(\"max-depth cannot be used with wildcard pathspecs\");\n }\n \n int parse_long_opt(const char *opt, const char **argv,\n@@ -5622,6 +5625,18 @@ static int diff_opt_rotate_to(const struct option *opt, const char *arg, int uns\n \treturn 0;\n }\n \n+static int diff_opt_max_depth(const struct option *opt,\n+\t\t\t const char *arg, int unset)\n+{\n+\tstruct diff_options *options = opt->value;\n+\n+\tBUG_ON_OPT_NEG(unset);\n+\toptions->flags.recursive = 1;\n+\toptions->max_depth = strtol(arg, NULL, 10);\n+\toptions->max_depth_valid = 1;\n+\treturn 0;\n+}\n+\n /*\n * Consider adding new flags to __git_diff_common_options\n * in contrib/completion/git-completion.bash\n@@ -5894,6 +5909,10 @@ struct option *add_diff_options(const struct option *opts,\n \t\tOPT_CALLBACK_F(0, \"diff-filter\", options, N_(\"[(A|C|D|M|R|T|U|X|B)...[*]]\"),\n \t\t\t N_(\"select files by diff type\"),\n \t\t\t PARSE_OPT_NONEG, diff_opt_diff_filter),\n+\t\tOPT_CALLBACK_F(0, \"max-depth\", options, N_(\"\"),\n+\t\t\t N_(\"maximum tree depth to recurse\"),\n+\t\t\t PARSE_OPT_NONEG, diff_opt_max_depth),\n+\n \t\t{\n \t\t\t.type = OPTION_CALLBACK,\n \t\t\t.long_name = \"output\",\ndiff --git a/diff.h b/diff.h\nindex 62e5768a9a..e3767df237 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -404,6 +404,15 @@ struct diff_options {\n \tstruct strmap *additional_path_headers;\n \n \tint no_free;\n+\n+\t/*\n+\t * The extra \"valid\" flag is a slight hack. The value \"0\" is perfectly\n+\t * valid for max-depth. We would normally use -1 to indicate \"not set\",\n+\t * but there are many code paths which assume that assume that just\n+\t * zero-ing out a diff_options is enough to initialize it.\n+\t */\n+\tint max_depth;\n+\tint max_depth_valid;\n };\n \n unsigned diff_filter_bit(char status);\ndiff --git a/t/meson.build b/t/meson.build\nindex 660d780dcc..11908dad6f 100644\n--- a/t/meson.build\n+++ b/t/meson.build\n@@ -501,6 +501,7 @@ integration_tests = [\n 't4069-remerge-diff.sh',\n 't4070-diff-pairs.sh',\n 't4071-diff-minimal.sh',\n+ 't4072-diff-max-depth.sh',\n 't4100-apply-stat.sh',\n 't4101-apply-nonl.sh',\n 't4102-apply-rename.sh',\ndiff --git a/t/t4072-diff-max-depth.sh b/t/t4072-diff-max-depth.sh\nnew file mode 100755\nindex 0000000000..1545ebb869\n--- /dev/null\n+++ b/t/t4072-diff-max-depth.sh\n@@ -0,0 +1,109 @@\n+#!/bin/sh\n+\n+test_description='check that diff --max-depth will limit recursion'\n+. ./test-lib.sh\n+\n+make_dir() {\n+\tmkdir -p \"$1\" &&\n+\techo \"$2\" >\"$1/file\"\n+}\n+\n+make_files() {\n+\techo \"$1\" >file &&\n+\tmake_dir one \"$1\" &&\n+\tmake_dir one/two \"$1\" &&\n+\tmake_dir one/two/three \"$1\"\n+}\n+\n+test_expect_success 'setup' '\n+\tgit commit --allow-empty -m empty &&\n+\tgit tag empty &&\n+\tmake_files added &&\n+\tgit add . &&\n+\tgit commit -m added &&\n+\tmake_files modified &&\n+\tgit add . &&\n+\tgit commit -m modified &&\n+\tmake_files index &&\n+\tgit add . &&\n+\tmake_files worktree\n+'\n+\n+test_expect_success '--max-depth is disallowed with wildcard pathspecs' '\n+\ttest_must_fail git diff-tree --max-depth=0 HEAD^ HEAD -- \"f*\"\n+'\n+\n+check_one() {\n+\ttype=$1; shift\n+\targs=$1; shift\n+\tpath=$1; shift\n+\tdepth=$1; shift\n+\ttest_expect_${expect:-success} \"diff-$type $args, path=$path, depth=$depth\" \"\n+\t\tfor i in $*; do echo \\$i; done >expect &&\n+\t\tgit diff-$type --max-depth=$depth --name-only $args -- $path >actual &&\n+\t\ttest_cmp expect actual\n+\t\"\n+}\n+\n+# For tree comparisons, we expect to see subtrees at the boundary\n+# get their own entry.\n+check_trees() {\n+\tcheck_one tree \"$*\" '' 0 file one\n+\tcheck_one tree \"$*\" '' 1 file one/file one/two\n+\tcheck_one tree \"$*\" '' 2 file one/file one/two/file one/two/three\n+\tcheck_one tree \"$*\" '' 3 file one/file one/two/file one/two/three/file\n+\tcheck_one tree \"$*\" one 0 one\n+\tcheck_one tree \"$*\" one 1 one/file one/two\n+\tcheck_one tree \"$*\" one 2 one/file one/two/file one/two/three\n+\tcheck_one tree \"$*\" one 3 one/file one/two/file one/two/three/file\n+\tcheck_one tree \"$*\" one/two 0 one/two\n+\tcheck_one tree \"$*\" one/two 1 one/two/file one/two/three\n+\tcheck_one tree \"$*\" one/two 2 one/two/file one/two/three/file\n+\tcheck_one tree \"$*\" one/two/three 0 one/two/three\n+\tcheck_one tree \"$*\" one/two/three 1 one/two/three/file\n+}\n+\n+# But for index comparisons, we do not store subtrees at all, so we do not\n+# expect them.\n+check_index() {\n+\tcheck_one \"$@\" '' 0 file\n+\tcheck_one \"$@\" '' 1 file one/file\n+\tcheck_one \"$@\" '' 2 file one/file one/two/file\n+\tcheck_one \"$@\" '' 3 file one/file one/two/file one/two/three/file\n+\tcheck_one \"$@\" one 0\n+\tcheck_one \"$@\" one 1 one/file\n+\tcheck_one \"$@\" one 2 one/file one/two/file\n+\tcheck_one \"$@\" one 3 one/file one/two/file one/two/three/file\n+\tcheck_one \"$@\" one/two 0\n+\tcheck_one \"$@\" one/two 1 one/two/file\n+\tcheck_one \"$@\" one/two 2 one/two/file one/two/three/file\n+\tcheck_one \"$@\" one/two/three 0\n+\tcheck_one \"$@\" one/two/three 1 one/two/three/file\n+}\n+\n+# Check as a modification...\n+check_trees HEAD^ HEAD\n+# ...and as an addition...\n+check_trees empty HEAD\n+# ...and as a deletion.\n+check_trees HEAD empty\n+\n+# We currently only implement max-depth for trees.\n+expect=failure\n+# Check index against a tree\n+check_index index \"--cached HEAD\"\n+# and index against the worktree\n+check_index files \"\"\n+expect=\n+\n+test_expect_success 'find shortest path within embedded pathspecs' '\n+\tcat >expect <<-\\EOF &&\n+\tone/file\n+\tone/two/file\n+\tone/two/three/file\n+\tEOF\n+\tgit diff-tree --max-depth=2 --name-only HEAD^ HEAD -- one one/two >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_done\ndiff --git a/tree-diff.c b/tree-diff.c\nindex e00fc2f450..acd302500f 100644\n--- a/tree-diff.c\n+++ b/tree-diff.c\n@@ -13,6 +13,7 @@\n #include \"tree-walk.h\"\n #include \"environment.h\"\n #include \"repository.h\"\n+#include \"dir.h\"\n \n /*\n * Some mode bits are also used internally for computations.\n@@ -48,6 +49,73 @@\n \t\tfree((x)); \\\n } while(0)\n \n+/* Returns true if and only if \"dir\" is a leading directory of \"path\" */\n+static int is_dir_prefix(const char *path, const char *dir, int dirlen)\n+{\n+\treturn !strncmp(path, dir, dirlen) &&\n+\t\t(!path[dirlen] || path[dirlen] == '/');\n+}\n+\n+static int check_recursion_depth(struct strbuf *name,\n+\t\t\t\t const struct pathspec *ps,\n+\t\t\t\t int max_depth)\n+{\n+\tint i;\n+\n+\tif (!ps->nr)\n+\t\treturn within_depth(name->buf, name->len, 1, max_depth);\n+\n+\t/*\n+\t * We look through the pathspecs in reverse-sorted order, because we\n+\t * want to find the longest match first (e.g., \"a/b\" is better for\n+\t * checking depth than \"a/b/c\").\n+\t */\n+\tfor (i = ps->nr - 1; i >= 0; i--) {\n+\t\tconst struct pathspec_item *item = ps->items+i;\n+\n+\t\t/*\n+\t\t * If the name to match is longer than the pathspec, then we\n+\t\t * are only interested if the pathspec matches and we are\n+\t\t * within the allowed depth.\n+\t\t */\n+\t\tif (name->len >= item->len) {\n+\t\t\tif (!is_dir_prefix(name->buf, item->match, item->len))\n+\t\t\t\tcontinue;\n+\t\t\treturn within_depth(name->buf + item->len,\n+\t\t\t\t\t name->len - item->len,\n+\t\t\t\t\t 1, max_depth);\n+\t\t}\n+\n+\t\t/*\n+\t\t * Otherwise, our name is shorter than the pathspec. We need to\n+\t\t * check if it is a prefix of the pathspec; if so, we must\n+\t\t * always recurse in order to process further (the resulting\n+\t\t * paths we find might or might not match our pathspec, but we\n+\t\t * cannot know until we recurse).\n+\t\t */\n+\t\tif (is_dir_prefix(item->match, name->buf, name->len))\n+\t\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n+static int should_recurse(struct strbuf *name, struct diff_options *opt)\n+{\n+\tif (!opt->flags.recursive)\n+\t\treturn 0;\n+\tif (!opt->max_depth_valid)\n+\t\treturn 1;\n+\n+\t/*\n+\t * We catch this during diff_setup_done, but let's double-check\n+\t * against any internal munging.\n+\t */\n+\tif (opt->pathspec.has_wildcard)\n+\t\tdie(\"BUG: wildcard pathspecs are incompatible with max-depth\");\n+\n+\treturn check_recursion_depth(name, &opt->pathspec, opt->max_depth);\n+}\n+\n static void ll_diff_tree_paths(\n \tstruct combine_diff_path ***tail, const struct object_id *oid,\n \tconst struct object_id **parents_oid, int nparent,\n@@ -170,9 +238,13 @@ static void emit_path(struct combine_diff_path ***tail,\n \t\tmode = 0;\n \t}\n \n-\tif (opt->flags.recursive && isdir) {\n-\t\trecurse = 1;\n-\t\temitthis = opt->flags.tree_in_recursive;\n+\tif (isdir) {\n+\t\tstrbuf_add(base, path, pathlen);\n+\t\tif (should_recurse(base, opt)) {\n+\t\t\trecurse = 1;\n+\t\t\temitthis = opt->flags.tree_in_recursive;\n+\t\t}\n+\t\tstrbuf_setlen(base, old_baselen);\n \t}\n \n \tif (emitthis) {\n\n-- \n2.50.1.327.g047016eb4a\n\n"},{"id":"522994","messageId":"aIm5vMeTLSLD-6Fz@pks.im","threadId":"63874","inReplyTo":"20250729-toon-max-depth-v1-2-c177e39c40fb@iotcl.com","subject":"Re: [PATCH 2/3] within_depth: fix return for empty path","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-07-30T06:20:44Z","receivedAt":"2025-07-30T06:20:56Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Jul 29, 2025 at 08:57:43PM +0200, Toon Claes wrote:\n> From: Jeff King \n> \n> The within_depth() function is used to check whether pathspecs limited\n> by a max-depth parameter are acceptable. It takes a path to check, a\n> maximum depth, and a \"base\" depth. It counts the components in the\n> path (by counting slashes), adds them to the base, and compare them to\n\ns/compare/&s/\n\n> the maximum.\n> \n> However, if the base does not have any slashes at all, we always return\n> `true`. If the base depth is 0, then this is correct; no matter what the\n> maximum is, we are always within it. However, if the base depth is\n> greater than 0, then we might return an erroneous result.\n> \n> This ends up not causing any user-visible bugs in the current code. The\n> call sites in dir.c always pass a base depth of 0, so are unaffected.\n> But tree_entry_interesting() uses this function differently: it will\n> pass the prefix of the current entry, along with a `1` if the entry is a\n> directory, in essence checking whether items inside the entry would be\n> of interest. It turns out not to make a difference in behavior, but the\n> reasoning is complex.\n> \n> Given a tree like:\n> \n> file\n> a/file\n> a/b/file\n> \n> walking the tree and calling tree_entry_interesting() will yield the\n> following results:\n> \n> (with max_depth=0):\n> file: yes\n> a: yes\n> a/file: no\n> a/b: no\n> \n> (with max_depth=1):\n> file: yes\n> a: yes\n> a/file: yes\n> a/b: no\n> \n> So we have inconsistent behavior in considering directories interesting.\n> If they are at the edge of our depth but at the root, we will recurse\n> into them, but then find all of their entries uninteresting (e.g., in\n> the first case, we will look at \"a\" but find \"a/*\" uninteresting). But\n> if they are at the edge of our depth and not at the root, then we will\n> not recurse (in the second example, we do not even bother entering\n> \"a/b\").\n> \n> This turns out not to matter because the only caller which uses\n> max-depth pathspecs is cmd_grep(), which only cares about blob entries.\n> From its perspective, it is exactly the same to not recurse into a\n> subtree, or to recurse and find that it contains no matching entries.\n> Not recursing is merely an optimization.\n\nOkay, well-explained.\n\n> It is debatable whether tree_entry_interesting() should consider such an\n> entry interesting. The only caller does not care if it sees the tree\n> itself, and can benefit from the optimization. But if we add a\n> \"max-depth\" limiter to regular diffs, then a diff with\n> DIFF_OPT_TREE_IN_RECURSIVE would probably want to show the tree itself,\n> but not what it contains.\n\nI haven't yet read the cover letter, but I guess that the scenario where\nwe'll care about this is in git-last-modified(1) if we want to teach\nthat command a `--max-depth` parameter?\n\n> This patch just fixes within_depth(), which means we consider such\n> entries uninteresting (and makes the current caller happy). If we want\n> to change that in the future, then this fix is still the correct first\n> step, as the current behavior is simply inconsistent.\n\nSo... do we end up with this now?\n\n (with max_depth=1):\n file: yes\n a: yes\n a/file: no\n a/b: no\n\nI think it would be nice to include that in the message to show the\nchange in behaviour at a glance.\n\n> Signed-off-by: Jeff King \n> Signed-off-by: Toon Claes \n> ---\n> dir.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/dir.c b/dir.c\n> index 02873f59ea..900ee5e559 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -277,7 +277,7 @@ int within_depth(const char *name, int namelen,\n> \t\tif (depth > max_depth)\n> \t\t\treturn 0;\n> \t}\n> -\treturn 1;\n> +\treturn depth <= max_depth;\n> }\n\nJust for my own understanding: the only difference is when we don't have\neven a single matching slash, as we don't verify `depth > max_depth` in\nthat case. So in theory, we could modify the function to the following\nequivalent:\n\n\tint within_depth(const char *name, int namelen,\n\t\t\t\tint depth, int max_depth)\n\t{\n\t\tconst char *cp = name, *cpe = name + namelen;\n\n\t\tif (depth > max_depth)\n\t\t\treturn 0;\n\n\t\twhile (cp < cpe) {\n\t\t\tif (*cp++ != '/')\n\t\t\t\tcontinue;\n\t\t\tdepth++;\n\t\t\tif (depth > max_depth)\n\t\t\t\treturn 0;\n\t\t}\n\t\treturn 1;\n\t}\n\n(Not saying we should, I'm just double checking my understanding).\n\nPatrick\n"},{"id":"522995","messageId":"aIm5x4kah8608Ba5@pks.im","threadId":"63874","inReplyTo":"20250729-toon-max-depth-v1-3-c177e39c40fb@iotcl.com","subject":"Re: [PATCH 3/3] diff: teach tree-diff a max-depth parameter","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-07-30T06:20:55Z","receivedAt":"2025-07-30T06:21:01Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Jul 29, 2025 at 08:57:44PM +0200, Toon Claes wrote:\n> From: Jeff King \n> \n> When you are doing a tree-diff, there are basically two options: do not\n> recurse into subtrees at all, or recurse indefinitely. While most\n> callers would want to always recurse and see full pathnames, some may\n> want the efficiency of looking only at a particular level of the tree.\n> This is currently easy to do for the top-level (just turn off\n> recursion), but you cannot say \"show me what changed in subdir/, but do\n> not recurse\".\n> \n> This patch adds a max-depth parameter which is measured from the closest\n> pathspec match, so that you can do:\n> \n> git log --raw --max-depth=1 -- a/b/c\n> \n> and see the raw output for a/b/c/, but not those of a/b/c/d/\n> (instead of the raw output you would see for a/b/c/d).\n\nHm, okay. So what happens if I pass both \"a/b\" and \"a/b/c\"? Would I see\nthe contents of both trees?\n\n> diff --git a/Documentation/diff-options.adoc b/Documentation/diff-options.adoc\n> index f3a35d8141..46e6ed2d67 100644\n> --- a/Documentation/diff-options.adoc\n> +++ b/Documentation/diff-options.adoc\n> @@ -893,5 +893,33 @@ endif::git-format-patch[]\n> \treverted with `--ita-visible-in-index`. Both options are\n> \texperimental and could be removed in future.\n> \n> +--max-depth=::\n> +\n> +\tLimit diff recursion to `` levels (implies `-r`). The depth\n> +\tis measured from the closest pathspec. Given a tree containing\n> +\t`foo/bar/baz`, the following list shows the matches generated by\n> +\teach set of options:\n> ++\n> +--\n> + - `--max-depth=0 -- foo`: `foo`\n> +\n> + - `--max-depth=1 -- foo`: `foo/bar`\n> +\n> + - `--max-depth=1 -- foo/bar`: `foo/bar/baz`\n> +\n> + - `--max-depth=1 -- foo foo/bar`: `foo/bar/baz`\n\nThis partially answers my question, as we talk about the scenario where\nwe have \"foo/bar/baz\", but no \"foo/qux\". If we had the latter, would\nthat also be displayed here because it's 1 level deep from the closest\nmatching pathspec (\"foo\")?\n\n> + - `--max-depth=2 -- foo`: `foo/bar/baz`\n> +--\n> ++\n> +If no pathspec is given, the depth is measured as if all\n> +top-level entries were specified. Note that this is different\n> +than measuring from the root, in that `--max-depth=0` would\n> +still return `foo`. This allows you to still limit depth while\n> +asking for a subset of the top-level entries.\n> ++\n> +Note that this option is only supported for diffs between tree objects,\n> +not against the index or working tree.\n\nLet's also note that wildcard pathspecs aren't supported.\n\n> For more detailed explanation on these common options, see also\n> linkgit:gitdiffcore[7].\n> diff --git a/diff-lib.c b/diff-lib.c\n> index 244468dd1a..fa7c24c267 100644\n> --- a/diff-lib.c\n> +++ b/diff-lib.c\n> @@ -115,6 +115,9 @@ void run_diff_files(struct rev_info *revs, unsigned int option)\n> \tuint64_t start = getnanotime();\n> \tstruct index_state *istate = revs->diffopt.repo->index;\n> \n> +\tif (revs->diffopt.max_depth_valid)\n> +\t\tdie(\"max-depth is not supported for worktree diffs\");\n\nThis and the following error messages should be made translatable.\n\n> diff --git a/diff.c b/diff.c\n> index dca87e164f..c03a59ac3b 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -5622,6 +5625,18 @@ static int diff_opt_rotate_to(const struct option *opt, const char *arg, int uns\n> \treturn 0;\n> }\n> \n> +static int diff_opt_max_depth(const struct option *opt,\n> +\t\t\t const char *arg, int unset)\n> +{\n> +\tstruct diff_options *options = opt->value;\n> +\n> +\tBUG_ON_OPT_NEG(unset);\n> +\toptions->flags.recursive = 1;\n> +\toptions->max_depth = strtol(arg, NULL, 10);\n\nWe're missing error handling in case the argument is not an integer. We\nshould probably use `git_parse_unsigned()` instead.\n\n> @@ -5894,6 +5909,10 @@ struct option *add_diff_options(const struct option *opts,\n> \t\tOPT_CALLBACK_F(0, \"diff-filter\", options, N_(\"[(A|C|D|M|R|T|U|X|B)...[*]]\"),\n> \t\t\t N_(\"select files by diff type\"),\n> \t\t\t PARSE_OPT_NONEG, diff_opt_diff_filter),\n> +\t\tOPT_CALLBACK_F(0, \"max-depth\", options, N_(\"\"),\n> +\t\t\t N_(\"maximum tree depth to recurse\"),\n> +\t\t\t PARSE_OPT_NONEG, diff_opt_max_depth),\n> +\n> \t\t{\n> \t\t\t.type = OPTION_CALLBACK,\n> \t\t\t.long_name = \"output\",\n\nOkay. We don't use `OPT_UNSIGNED()` because we also want to impliy the\n`recursive` flag. Wouldn't it be simpler though to use `OPT_UNSIGNED()`\nand then set the flag in `diff_setup_done()` like we already do for a\ncouple of other options?\n\n> diff --git a/diff.h b/diff.h\n> index 62e5768a9a..e3767df237 100644\n> --- a/diff.h\n> +++ b/diff.h\n> @@ -404,6 +404,15 @@ struct diff_options {\n> \tstruct strmap *additional_path_headers;\n> \n> \tint no_free;\n> +\n> +\t/*\n> +\t * The extra \"valid\" flag is a slight hack. The value \"0\" is perfectly\n> +\t * valid for max-depth. We would normally use -1 to indicate \"not set\",\n> +\t * but there are many code paths which assume that assume that just\n\nDouble \"assume that\".\n\n> +\t * zero-ing out a diff_options is enough to initialize it.\n> +\t */\n> +\tint max_depth;\n> +\tint max_depth_valid;\n\nSo in that case, shouldn't we convert `max_depth` to be unsigned and\n`max_depth_valid` to be a boolean?\n\n> diff --git a/tree-diff.c b/tree-diff.c\n> index e00fc2f450..acd302500f 100644\n> --- a/tree-diff.c\n> +++ b/tree-diff.c\n> @@ -48,6 +49,73 @@\n> \t\tfree((x)); \\\n> } while(0)\n> \n> +/* Returns true if and only if \"dir\" is a leading directory of \"path\" */\n> +static int is_dir_prefix(const char *path, const char *dir, int dirlen)\n> +{\n> +\treturn !strncmp(path, dir, dirlen) &&\n> +\t\t(!path[dirlen] || path[dirlen] == '/');\n> +}\n> +\n> +static int check_recursion_depth(struct strbuf *name,\n\nLet's mark the `name` parameter as `const` both here and in\n`should_recurse()` so that it becomes clear that it shouldn't be\nmodified.\n\n> +\t\t\t\t const struct pathspec *ps,\n> +\t\t\t\t int max_depth)\n> +{\n> +\tint i;\n> +\n> +\tif (!ps->nr)\n> +\t\treturn within_depth(name->buf, name->len, 1, max_depth);\n> +\n> +\t/*\n> +\t * We look through the pathspecs in reverse-sorted order, because we\n> +\t * want to find the longest match first (e.g., \"a/b\" is better for\n> +\t * checking depth than \"a/b/c\").\n> +\t */\n> +\tfor (i = ps->nr - 1; i >= 0; i--) {\n\n`nr` is of type `int` indeed, so the loop index is correct even though\nit feels wrong. But that's nothing we have to fix as part of this patch\nseries. We could inline the declaration though and make it loop-local.\n\n> +\t\tconst struct pathspec_item *item = ps->items+i;\n> +\n> +\t\t/*\n> +\t\t * If the name to match is longer than the pathspec, then we\n> +\t\t * are only interested if the pathspec matches and we are\n> +\t\t * within the allowed depth.\n> +\t\t */\n> +\t\tif (name->len >= item->len) {\n> +\t\t\tif (!is_dir_prefix(name->buf, item->match, item->len))\n> +\t\t\t\tcontinue;\n> +\t\t\treturn within_depth(name->buf + item->len,\n> +\t\t\t\t\t name->len - item->len,\n> +\t\t\t\t\t 1, max_depth);\n> +\t\t}\n> +\n> +\t\t/*\n> +\t\t * Otherwise, our name is shorter than the pathspec. We need to\n> +\t\t * check if it is a prefix of the pathspec; if so, we must\n> +\t\t * always recurse in order to process further (the resulting\n> +\t\t * paths we find might or might not match our pathspec, but we\n> +\t\t * cannot know until we recurse).\n> +\t\t */\n> +\t\tif (is_dir_prefix(item->match, name->buf, name->len))\n> +\t\t\treturn 1;\n> +\t}\n> +\treturn 0;\n> +}\n> +\n> +static int should_recurse(struct strbuf *name, struct diff_options *opt)\n> +{\n> +\tif (!opt->flags.recursive)\n> +\t\treturn 0;\n> +\tif (!opt->max_depth_valid)\n> +\t\treturn 1;\n> +\n> +\t/*\n> +\t * We catch this during diff_setup_done, but let's double-check\n> +\t * against any internal munging.\n> +\t */\n> +\tif (opt->pathspec.has_wildcard)\n> +\t\tdie(\"BUG: wildcard pathspecs are incompatible with max-depth\");\n\nLet's use `BUG()` instead. This patch series must be old, the function\nhas been introduced 8 years ago in d8193743e0 (usage.c: add BUG()\nfunction, 2017-05-12).\n\nPatrick\n"},{"id":"523658","messageId":"87ms8cttdj.fsf@iotcl.com","threadId":"63874","inReplyTo":"aIm5vMeTLSLD-6Fz@pks.im","subject":"Re: [PATCH 2/3] within_depth: fix return for empty path","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2025-08-06T14:30:32Z","receivedAt":"2025-08-06T14:30:50Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Patrick Steinhardt writes:\n\n> On Tue, Jul 29, 2025 at 08:57:43PM +0200, Toon Claes wrote:\n>> From: Jeff King \n>> \n>> The within_depth() function is used to check whether pathspecs limited\n>> by a max-depth parameter are acceptable. It takes a path to check, a\n>> maximum depth, and a \"base\" depth. It counts the components in the\n>> path (by counting slashes), adds them to the base, and compare them to\n>\n> s/compare/&s/\n>\n>> the maximum.\n>> \n>> However, if the base does not have any slashes at all, we always return\n>> `true`. If the base depth is 0, then this is correct; no matter what the\n>> maximum is, we are always within it. However, if the base depth is\n>> greater than 0, then we might return an erroneous result.\n>> \n>> This ends up not causing any user-visible bugs in the current code. The\n>> call sites in dir.c always pass a base depth of 0, so are unaffected.\n>> But tree_entry_interesting() uses this function differently: it will\n>> pass the prefix of the current entry, along with a `1` if the entry is a\n>> directory, in essence checking whether items inside the entry would be\n>> of interest. It turns out not to make a difference in behavior, but the\n>> reasoning is complex.\n>> \n>> Given a tree like:\n>> \n>> file\n>> a/file\n>> a/b/file\n>> \n>> walking the tree and calling tree_entry_interesting() will yield the\n>> following results:\n>> \n>> (with max_depth=0):\n>> file: yes\n>> a: yes\n>> a/file: no\n>> a/b: no\n>> \n>> (with max_depth=1):\n>> file: yes\n>> a: yes\n>> a/file: yes\n>> a/b: no\n>> \n>> So we have inconsistent behavior in considering directories interesting.\n>> If they are at the edge of our depth but at the root, we will recurse\n>> into them, but then find all of their entries uninteresting (e.g., in\n>> the first case, we will look at \"a\" but find \"a/*\" uninteresting). But\n>> if they are at the edge of our depth and not at the root, then we will\n>> not recurse (in the second example, we do not even bother entering\n>> \"a/b\").\n>> \n>> This turns out not to matter because the only caller which uses\n>> max-depth pathspecs is cmd_grep(), which only cares about blob entries.\n>> From its perspective, it is exactly the same to not recurse into a\n>> subtree, or to recurse and find that it contains no matching entries.\n>> Not recursing is merely an optimization.\n>\n> Okay, well-explained.\n>\n>> It is debatable whether tree_entry_interesting() should consider such an\n>> entry interesting. The only caller does not care if it sees the tree\n>> itself, and can benefit from the optimization. But if we add a\n>> \"max-depth\" limiter to regular diffs, then a diff with\n>> DIFF_OPT_TREE_IN_RECURSIVE would probably want to show the tree itself,\n>> but not what it contains.\n>\n> I haven't yet read the cover letter, but I guess that the scenario where\n> we'll care about this is in git-last-modified(1) if we want to teach\n> that command a `--max-depth` parameter?\n\nWell, yes git-last-modified(1) will be able to use this option as well.\nAnd it will be useful there. But those patches are still in flight in a\nseparate series.\n\nIn the commit following this one we see `--max-depth` being used in\ngit-diff-tree(1). Maybe I can do a better job explaining how this\nchanges affects that patch. Let me see how I can phrase this.\n\n>> This patch just fixes within_depth(), which means we consider such\n>> entries uninteresting (and makes the current caller happy). If we want\n>> to change that in the future, then this fix is still the correct first\n>> step, as the current behavior is simply inconsistent.\n>\n> So... do we end up with this now?\n>\n> (with max_depth=1):\n> file: yes\n> a: yes\n> a/file: no\n> a/b: no\n>\n> I think it would be nice to include that in the message to show the\n> change in behaviour at a glance.\n\nThe change in behavior happens when max_depth equals zero. In that case\nthe example would be:\n\n (with max_depth=0):\n file: yes\n a: yes\n a/file: no\n a/b: no\n\nSo no change actually. But that's due to how tree_entry_interesting()\ncalls within_depth(). It doesn't pass the path of the entry, but the\nbase of it. I'm not sure using tree_entry_interesting() here in the\ncommit message to explain the change is the best idea, but it's the\ncallsite that /might/ be affected. But it isn't. So it's pretty involved\ntrying to put this change into words.\n\n>> Signed-off-by: Jeff King \n>> Signed-off-by: Toon Claes \n>> ---\n>> dir.c | 2 +-\n>> 1 file changed, 1 insertion(+), 1 deletion(-)\n>> \n>> diff --git a/dir.c b/dir.c\n>> index 02873f59ea..900ee5e559 100644\n>> --- a/dir.c\n>> +++ b/dir.c\n>> @@ -277,7 +277,7 @@ int within_depth(const char *name, int namelen,\n>> \t\tif (depth > max_depth)\n>> \t\t\treturn 0;\n>> \t}\n>> -\treturn 1;\n>> +\treturn depth <= max_depth;\n>> }\n>\n> Just for my own understanding: the only difference is when we don't have\n> even a single matching slash, as we don't verify `depth > max_depth` in\n> that case. So in theory, we could modify the function to the following\n> equivalent:\n>\n> \tint within_depth(const char *name, int namelen,\n> \t\t\t\tint depth, int max_depth)\n> \t{\n> \t\tconst char *cp = name, *cpe = name + namelen;\n>\n> \t\tif (depth > max_depth)\n> \t\t\treturn 0;\n>\n> \t\twhile (cp < cpe) {\n> \t\t\tif (*cp++ != '/')\n> \t\t\t\tcontinue;\n> \t\t\tdepth++;\n> \t\t\tif (depth > max_depth)\n> \t\t\t\treturn 0;\n> \t\t}\n> \t\treturn 1;\n> \t}\n>\n> (Not saying we should, I'm just double checking my understanding).\n\nTo be honest, I wasn't sure no more. I decided to see if I can write a\nunit test. This is what I came up with:\n\n void test_dir__within_depth(void)\n {\n \tstruct test_case {\n \t\tconst char *path;\n \t\tint depth;\n \t\tint max_depth;\n \t\tint expect;\n \t} test_cases[] = {\n \t\t/* depth = 0; max_depth = 0 */\n \t\t{ \"\", 0, 0, 1 },\n \t\t{ \"file\", 0, 0, 1 },\n \t\t{ \"a\", 0, 0, 1 },\n \t\t{ \"a/file\", 0, 0, 0 },\n \t\t{ \"a/b\", 0, 0, 0 },\n \t\t{ \"a/b/file\", 0, 0, 0 },\n\n \t\t/* depth = 0; max_depth = 1 */\n \t\t{ \"\", 0, 1, 1 },\n \t\t{ \"file\", 0, 1, 1 },\n \t\t{ \"a\", 0, 1, 1 },\n \t\t{ \"a/file\", 0, 1, 1 },\n \t\t{ \"a/b\", 0, 1, 1 },\n \t\t{ \"a/b/file\", 0, 1, 0 },\n\n \t\t/* depth = 1; max_depth = 1 */\n \t\t{ \"\", 1, 1, 1 },\n \t\t{ \"file\", 1, 1, 1 },\n \t\t{ \"a\", 1, 1, 1 },\n \t\t{ \"a/file\", 1, 1, 0 },\n \t\t{ \"a/b\", 1, 1, 0 },\n \t\t{ \"a/b/file\", 1, 1, 0 },\n\n \t\t/* depth = 1; max_depth = 0 */\n \t\t{ \"\", 1, 0, 0 },\n \t\t{ \"file\", 1, 0, 0 },\n \t\t{ \"a\", 1, 0, 0 },\n \t\t{ \"a/file\", 1, 0, 0 },\n \t\t{ \"a/b\", 1, 0, 0 },\n \t\t{ \"a/b/file\", 1, 0, 0 },\n \t};\n\n \tfor (size_t i = 0; i < ARRAY_SIZE(test_cases); i++) {\n \t\tint result = within_depth(test_cases[i].path, strlen(test_cases[i].path),\n \t\t\t\t\t test_cases[i].depth, test_cases[i].max_depth);\n \t\tcl_assert_equal_b(result, test_cases[i].expect);\n \t}\n }\n\nThe change in this patch affect the last batch of tests (the batch with\n'depth = 1; max_depth = 0'). Running the test on both this patch and\nyour suggestion gives the same results, so yes your suggestion has the\nsame output.\n\nDo you think it's worth to add such unit test?\n\nI did add `\"\"` because I've seen cmd_grep() might be passing an empty\npath.\n\n-- \nCheers,\nToon\n"},{"id":"523659","messageId":"87jz3gtshx.fsf@iotcl.com","threadId":"63874","inReplyTo":"aIm5x4kah8608Ba5@pks.im","subject":"Re: [PATCH 3/3] diff: teach tree-diff a max-depth parameter","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2025-08-06T14:49:30Z","receivedAt":"2025-08-06T14:49:42Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Patrick Steinhardt writes:\n\n> On Tue, Jul 29, 2025 at 08:57:44PM +0200, Toon Claes wrote:\n>> From: Jeff King \n>> \n>> When you are doing a tree-diff, there are basically two options: do not\n>> recurse into subtrees at all, or recurse indefinitely. While most\n>> callers would want to always recurse and see full pathnames, some may\n>> want the efficiency of looking only at a particular level of the tree.\n>> This is currently easy to do for the top-level (just turn off\n>> recursion), but you cannot say \"show me what changed in subdir/, but do\n>> not recurse\".\n>> \n>> This patch adds a max-depth parameter which is measured from the closest\n>> pathspec match, so that you can do:\n>> \n>> git log --raw --max-depth=1 -- a/b/c\n>> \n>> and see the raw output for a/b/c/, but not those of a/b/c/d/\n>> (instead of the raw output you would see for a/b/c/d).\n>\n> Hm, okay. So what happens if I pass both \"a/b\" and \"a/b/c\"? Would I see\n> the contents of both trees?\n\nYes, the max-depth applies to both pathspecs and entries that match\neither of them are shown.\n\n>\n>> diff --git a/Documentation/diff-options.adoc b/Documentation/diff-options.adoc\n>> index f3a35d8141..46e6ed2d67 100644\n>> --- a/Documentation/diff-options.adoc\n>> +++ b/Documentation/diff-options.adoc\n>> @@ -893,5 +893,33 @@ endif::git-format-patch[]\n>> \treverted with `--ita-visible-in-index`. Both options are\n>> \texperimental and could be removed in future.\n>> \n>> +--max-depth=::\n>> +\n>> +\tLimit diff recursion to `` levels (implies `-r`). The depth\n>> +\tis measured from the closest pathspec. Given a tree containing\n>> +\t`foo/bar/baz`, the following list shows the matches generated by\n>> +\teach set of options:\n>> ++\n>> +--\n>> + - `--max-depth=0 -- foo`: `foo`\n>> +\n>> + - `--max-depth=1 -- foo`: `foo/bar`\n>> +\n>> + - `--max-depth=1 -- foo/bar`: `foo/bar/baz`\n>> +\n>> + - `--max-depth=1 -- foo foo/bar`: `foo/bar/baz`\n>\n> This partially answers my question, as we talk about the scenario where\n> we have \"foo/bar/baz\", but no \"foo/qux\". If we had the latter, would\n> that also be displayed here because it's 1 level deep from the closest\n> matching pathspec (\"foo\")?\n\n\"foo/qux\" is indeed within 1 level deep from \"foo\", so yes \"foo/qux\"\nwill be shown. \"foo/qux/duck\" not obviously.\n\n>\n>> + - `--max-depth=2 -- foo`: `foo/bar/baz`\n>> +--\n>> ++\n>> +If no pathspec is given, the depth is measured as if all\n>> +top-level entries were specified. Note that this is different\n>> +than measuring from the root, in that `--max-depth=0` would\n>> +still return `foo`. This allows you to still limit depth while\n>> +asking for a subset of the top-level entries.\n>> ++\n>> +Note that this option is only supported for diffs between tree objects,\n>> +not against the index or working tree.\n>\n> Let's also note that wildcard pathspecs aren't supported.\n\nAh yes, good point. Will add.\n\n>> For more detailed explanation on these common options, see also\n>> linkgit:gitdiffcore[7].\n>> diff --git a/diff-lib.c b/diff-lib.c\n>> index 244468dd1a..fa7c24c267 100644\n>> --- a/diff-lib.c\n>> +++ b/diff-lib.c\n>> @@ -115,6 +115,9 @@ void run_diff_files(struct rev_info *revs, unsigned int option)\n>> \tuint64_t start = getnanotime();\n>> \tstruct index_state *istate = revs->diffopt.repo->index;\n>> \n>> +\tif (revs->diffopt.max_depth_valid)\n>> +\t\tdie(\"max-depth is not supported for worktree diffs\");\n>\n> This and the following error messages should be made translatable.\n\nTrue. Will do.\n\n>> diff --git a/diff.c b/diff.c\n>> index dca87e164f..c03a59ac3b 100644\n>> --- a/diff.c\n>> +++ b/diff.c\n>> @@ -5622,6 +5625,18 @@ static int diff_opt_rotate_to(const struct option *opt, const char *arg, int uns\n>> \treturn 0;\n>> }\n>> \n>> +static int diff_opt_max_depth(const struct option *opt,\n>> +\t\t\t const char *arg, int unset)\n>> +{\n>> +\tstruct diff_options *options = opt->value;\n>> +\n>> +\tBUG_ON_OPT_NEG(unset);\n>> +\toptions->flags.recursive = 1;\n>> +\toptions->max_depth = strtol(arg, NULL, 10);\n>\n> We're missing error handling in case the argument is not an integer. We\n> should probably use `git_parse_unsigned()` instead.\n\nAbsolutely. Well, TIL!\n\n>\n>> @@ -5894,6 +5909,10 @@ struct option *add_diff_options(const struct option *opts,\n>> \t\tOPT_CALLBACK_F(0, \"diff-filter\", options, N_(\"[(A|C|D|M|R|T|U|X|B)...[*]]\"),\n>> \t\t\t N_(\"select files by diff type\"),\n>> \t\t\t PARSE_OPT_NONEG, diff_opt_diff_filter),\n>> +\t\tOPT_CALLBACK_F(0, \"max-depth\", options, N_(\"\"),\n>> +\t\t\t N_(\"maximum tree depth to recurse\"),\n>> +\t\t\t PARSE_OPT_NONEG, diff_opt_max_depth),\n>> +\n>> \t\t{\n>> \t\t\t.type = OPTION_CALLBACK,\n>> \t\t\t.long_name = \"output\",\n>\n> Okay. We don't use `OPT_UNSIGNED()` because we also want to impliy the\n> `recursive` flag. Wouldn't it be simpler though to use `OPT_UNSIGNED()`\n> and then set the flag in `diff_setup_done()` like we already do for a\n> couple of other options?\n\nBut how would you determine `max_depth_valid`?\n\nAnd, without realizing, you touch a good point here. Looking at the\ngit-grep(1) docs, it says:\n\n For each given on command line, descend at most \n levels of directories. A value of -1 means no limit.\n\nAnd:\n\n -r::\n --recursive::\n \tSame as `--max-depth=-1`; this is the default.\n\nWe should handle -1 the same, that's currently not the case.\n\n>> diff --git a/diff.h b/diff.h\n>> index 62e5768a9a..e3767df237 100644\n>> --- a/diff.h\n>> +++ b/diff.h\n>> @@ -404,6 +404,15 @@ struct diff_options {\n>> \tstruct strmap *additional_path_headers;\n>> \n>> \tint no_free;\n>> +\n>> +\t/*\n>> +\t * The extra \"valid\" flag is a slight hack. The value \"0\" is perfectly\n>> +\t * valid for max-depth. We would normally use -1 to indicate \"not set\",\n>> +\t * but there are many code paths which assume that assume that just\n>\n> Double \"assume that\".\n\nThanks!\n\n>> +\t * zero-ing out a diff_options is enough to initialize it.\n>> +\t */\n>> +\tint max_depth;\n>> +\tint max_depth_valid;\n>\n> So in that case, shouldn't we convert `max_depth` to be unsigned and\n> `max_depth_valid` to be a boolean?\n\nAs I mentioned above, -1 should mean \"No limit\", but should enable\nrecursion. Now, that doesn't mean we can't make the changes you suggest,\nwe just have to take that into account.\n\n>> diff --git a/tree-diff.c b/tree-diff.c\n>> index e00fc2f450..acd302500f 100644\n>> --- a/tree-diff.c\n>> +++ b/tree-diff.c\n>> @@ -48,6 +49,73 @@\n>> \t\tfree((x)); \\\n>> } while(0)\n>> \n>> +/* Returns true if and only if \"dir\" is a leading directory of \"path\" */\n>> +static int is_dir_prefix(const char *path, const char *dir, int dirlen)\n>> +{\n>> +\treturn !strncmp(path, dir, dirlen) &&\n>> +\t\t(!path[dirlen] || path[dirlen] == '/');\n>> +}\n>> +\n>> +static int check_recursion_depth(struct strbuf *name,\n>\n> Let's mark the `name` parameter as `const` both here and in\n> `should_recurse()` so that it becomes clear that it shouldn't be\n> modified.\n\n:+1:\n\n>> +\t\t\t\t const struct pathspec *ps,\n>> +\t\t\t\t int max_depth)\n>> +{\n>> +\tint i;\n>> +\n>> +\tif (!ps->nr)\n>> +\t\treturn within_depth(name->buf, name->len, 1, max_depth);\n>> +\n>> +\t/*\n>> +\t * We look through the pathspecs in reverse-sorted order, because we\n>> +\t * want to find the longest match first (e.g., \"a/b\" is better for\n>> +\t * checking depth than \"a/b/c\").\n>> +\t */\n>> +\tfor (i = ps->nr - 1; i >= 0; i--) {\n>\n> `nr` is of type `int` indeed, so the loop index is correct even though\n> it feels wrong. But that's nothing we have to fix as part of this patch\n> series. We could inline the declaration though and make it loop-local.\n\nAck.\n\n>> +\t\tconst struct pathspec_item *item = ps->items+i;\n>> +\n>> +\t\t/*\n>> +\t\t * If the name to match is longer than the pathspec, then we\n>> +\t\t * are only interested if the pathspec matches and we are\n>> +\t\t * within the allowed depth.\n>> +\t\t */\n>> +\t\tif (name->len >= item->len) {\n>> +\t\t\tif (!is_dir_prefix(name->buf, item->match, item->len))\n>> +\t\t\t\tcontinue;\n>> +\t\t\treturn within_depth(name->buf + item->len,\n>> +\t\t\t\t\t name->len - item->len,\n>> +\t\t\t\t\t 1, max_depth);\n>> +\t\t}\n>> +\n>> +\t\t/*\n>> +\t\t * Otherwise, our name is shorter than the pathspec. We need to\n>> +\t\t * check if it is a prefix of the pathspec; if so, we must\n>> +\t\t * always recurse in order to process further (the resulting\n>> +\t\t * paths we find might or might not match our pathspec, but we\n>> +\t\t * cannot know until we recurse).\n>> +\t\t */\n>> +\t\tif (is_dir_prefix(item->match, name->buf, name->len))\n>> +\t\t\treturn 1;\n>> +\t}\n>> +\treturn 0;\n>> +}\n>> +\n>> +static int should_recurse(struct strbuf *name, struct diff_options *opt)\n>> +{\n>> +\tif (!opt->flags.recursive)\n>> +\t\treturn 0;\n>> +\tif (!opt->max_depth_valid)\n>> +\t\treturn 1;\n>> +\n>> +\t/*\n>> +\t * We catch this during diff_setup_done, but let's double-check\n>> +\t * against any internal munging.\n>> +\t */\n>> +\tif (opt->pathspec.has_wildcard)\n>> +\t\tdie(\"BUG: wildcard pathspecs are incompatible with max-depth\");\n>\n> Let's use `BUG()` instead. This patch series must be old, the function\n> has been introduced 8 years ago in d8193743e0 (usage.c: add BUG()\n> function, 2017-05-12).\n\nGood catch. Even looking at your suggestion, at first I didn't realize\nit wasn't using BUG(). ;)\n\n-- \nCheers,\nToon\n"},{"id":"523705","messageId":"aJQ_x787aceaudef@pks.im","threadId":"63874","inReplyTo":"87jz3gtshx.fsf@iotcl.com","subject":"Re: [PATCH 3/3] diff: teach tree-diff a max-depth parameter","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-08-07T05:55:19Z","receivedAt":"2025-08-07T05:55:27Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Aug 06, 2025 at 04:49:30PM +0200, Toon Claes wrote:\n> Patrick Steinhardt writes:\n> > On Tue, Jul 29, 2025 at 08:57:44PM +0200, Toon Claes wrote:\n> >> diff --git a/diff.c b/diff.c\n> >> index dca87e164f..c03a59ac3b 100644\n> >> --- a/diff.c\n> >> +++ b/diff.c\n> >> @@ -5894,6 +5909,10 @@ struct option *add_diff_options(const struct option *opts,\n> >> \t\tOPT_CALLBACK_F(0, \"diff-filter\", options, N_(\"[(A|C|D|M|R|T|U|X|B)...[*]]\"),\n> >> \t\t\t N_(\"select files by diff type\"),\n> >> \t\t\t PARSE_OPT_NONEG, diff_opt_diff_filter),\n> >> +\t\tOPT_CALLBACK_F(0, \"max-depth\", options, N_(\"\"),\n> >> +\t\t\t N_(\"maximum tree depth to recurse\"),\n> >> +\t\t\t PARSE_OPT_NONEG, diff_opt_max_depth),\n> >> +\n> >> \t\t{\n> >> \t\t\t.type = OPTION_CALLBACK,\n> >> \t\t\t.long_name = \"output\",\n> >\n> > Okay. We don't use `OPT_UNSIGNED()` because we also want to impliy the\n> > `recursive` flag. Wouldn't it be simpler though to use `OPT_UNSIGNED()`\n> > and then set the flag in `diff_setup_done()` like we already do for a\n> > couple of other options?\n> \n> But how would you determine `max_depth_valid`?\n\nIdeally we wouldn't need it as we could just initialize with a default\nvalue.\n\n> And, without realizing, you touch a good point here. Looking at the\n> git-grep(1) docs, it says:\n> \n> For each given on command line, descend at most \n> levels of directories. A value of -1 means no limit.\n> \n> And:\n> \n> -r::\n> --recursive::\n> \tSame as `--max-depth=-1`; this is the default.\n> \n> We should handle -1 the same, that's currently not the case.\n\nTrue. Do we have the same default? If so, couldn't we drop\n`max_depth_valid` and initialize it with -1 from the start?\n\nIf that's not easily possible we can just stick with what you have.\n\nPatrick\n"},{"id":"523706","messageId":"aJREivx80NI08BEU@pks.im","threadId":"63874","inReplyTo":"87ms8cttdj.fsf@iotcl.com","subject":"Re: [PATCH 2/3] within_depth: fix return for empty path","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-08-07T06:15:38Z","receivedAt":"2025-08-07T06:15:45Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Aug 06, 2025 at 04:30:32PM +0200, Toon Claes wrote:\n> Patrick Steinhardt writes:\n> > Just for my own understanding: the only difference is when we don't have\n> > even a single matching slash, as we don't verify `depth > max_depth` in\n> > that case. So in theory, we could modify the function to the following\n> > equivalent:\n> >\n> > \tint within_depth(const char *name, int namelen,\n> > \t\t\t\tint depth, int max_depth)\n> > \t{\n> > \t\tconst char *cp = name, *cpe = name + namelen;\n> >\n> > \t\tif (depth > max_depth)\n> > \t\t\treturn 0;\n> >\n> > \t\twhile (cp < cpe) {\n> > \t\t\tif (*cp++ != '/')\n> > \t\t\t\tcontinue;\n> > \t\t\tdepth++;\n> > \t\t\tif (depth > max_depth)\n> > \t\t\t\treturn 0;\n> > \t\t}\n> > \t\treturn 1;\n> > \t}\n> >\n> > (Not saying we should, I'm just double checking my understanding).\n> \n> To be honest, I wasn't sure no more. I decided to see if I can write a\n> unit test. This is what I came up with:\n> \n> void test_dir__within_depth(void)\n> {\n> \tstruct test_case {\n> \t\tconst char *path;\n> \t\tint depth;\n> \t\tint max_depth;\n> \t\tint expect;\n> \t} test_cases[] = {\n> \t\t/* depth = 0; max_depth = 0 */\n> \t\t{ \"\", 0, 0, 1 },\n> \t\t{ \"file\", 0, 0, 1 },\n> \t\t{ \"a\", 0, 0, 1 },\n> \t\t{ \"a/file\", 0, 0, 0 },\n> \t\t{ \"a/b\", 0, 0, 0 },\n> \t\t{ \"a/b/file\", 0, 0, 0 },\n> \n> \t\t/* depth = 0; max_depth = 1 */\n> \t\t{ \"\", 0, 1, 1 },\n> \t\t{ \"file\", 0, 1, 1 },\n> \t\t{ \"a\", 0, 1, 1 },\n> \t\t{ \"a/file\", 0, 1, 1 },\n> \t\t{ \"a/b\", 0, 1, 1 },\n> \t\t{ \"a/b/file\", 0, 1, 0 },\n> \n> \t\t/* depth = 1; max_depth = 1 */\n> \t\t{ \"\", 1, 1, 1 },\n> \t\t{ \"file\", 1, 1, 1 },\n> \t\t{ \"a\", 1, 1, 1 },\n> \t\t{ \"a/file\", 1, 1, 0 },\n> \t\t{ \"a/b\", 1, 1, 0 },\n> \t\t{ \"a/b/file\", 1, 1, 0 },\n> \n> \t\t/* depth = 1; max_depth = 0 */\n> \t\t{ \"\", 1, 0, 0 },\n> \t\t{ \"file\", 1, 0, 0 },\n> \t\t{ \"a\", 1, 0, 0 },\n> \t\t{ \"a/file\", 1, 0, 0 },\n> \t\t{ \"a/b\", 1, 0, 0 },\n> \t\t{ \"a/b/file\", 1, 0, 0 },\n> \t};\n> \n> \tfor (size_t i = 0; i < ARRAY_SIZE(test_cases); i++) {\n> \t\tint result = within_depth(test_cases[i].path, strlen(test_cases[i].path),\n> \t\t\t\t\t test_cases[i].depth, test_cases[i].max_depth);\n> \t\tcl_assert_equal_b(result, test_cases[i].expect);\n> \t}\n> }\n> \n> The change in this patch affect the last batch of tests (the batch with\n> 'depth = 1; max_depth = 0'). Running the test on both this patch and\n> your suggestion gives the same results, so yes your suggestion has the\n> same output.\n> \n> Do you think it's worth to add such unit test?\n\nSure, if we can easily test this via such a unit test I think it would\nbe a good addition.\n\nPatrick\n"},{"id":"523773","messageId":"20250807-toon-max-depth-v2-0-50b7e5c81665@iotcl.com","threadId":"63874","inReplyTo":"20250729-toon-max-depth-v1-0-c177e39c40fb@iotcl.com","subject":"[PATCH v2 0/3] Teach git-diff-tree(1) option --max-depth","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2025-08-07T20:52:55Z","receivedAt":"2025-08-07T20:53:14Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Please consider these patches authored by Peff. They add option\n`--max-depth` to the diff machinery.\n\nDuring the process to upstream the git-blame-tree(1), later named\ngit-last-modified(1), various times[1][2][3] the topic was raised to add\noption `--max-depth` to the diff machinery. In this patch series this\nchange is added as a separate patch series.\n\nThe patches were originally written by Peff[4] and I'm crediting him as\nthe author. I've taken the patches almost verbatim from his fork on\nGitHub, with some minor tweaks in the commit messages. Because only tiny\nchanges were made, I've kept his Signed-off-by trailers, but I can\nremove if disagreed on.\n\nThe goal of the option `--max-depth` is to stop recursively traversing\nthe tree if the given depth is reached from the pathspec.\n\nThese patches add `max_depth` and `max_depth_valid` to `struct\ndiff_options`. This is different from what git-grep(1) does, which uses\n`max_depth` on `struct pathspec` instead. At the moment I'm on the fence\nwhether this is an issue: while it probably makes sense to consolidate\nthem into the same structs, it does not really make sense to reuse these\nthe struct fields if they are used in two separate code paths.\n\n[1]: https://lore.kernel.org/git/20130318121243.GC14789@sigill.intra.peff.net/\n[2]: https://lore.kernel.org/git/20160831054201.ldlwptlmcndjmfwu@sigill.intra.peff.net/\n[3]: https://lore.kernel.org/git/Y+%2FmnnJUz75yfWCN@coredump.intra.peff.net/\n[4]: https://github.com/peff/git/tree/jk/diff-max-depth\n\n---\nChanges in v2:\n- Added unit-tests for within_depth() in dir.c. Originally the patch\n was a oneline change by Peff, but I've added a bunch of code and\n extended the commit message, so I've set myself as the author and set\n a Based-on-patch-by trailer for Peff. I hope that's okay?\n- Added support for --max-depth=-1 and extended code comments why we\n need the max_depth_valid flag. With these modification it did no\n longer feel appropriate to keep Peff's Signed-off-by trailer.\n- Made die() messages translatable.\n- Small tweaks to the docs.\n- Added some const-correctness.\n- Switched from `die(\"BUG: ...\")` to `BUG(...)`.\n- Link to v1: https://lore.kernel.org/r/20250729-toon-max-depth-v1-0-c177e39c40fb@iotcl.com\n\n---\nJeff King (2):\n combine-diff: zero memory used for callback filepairs\n diff: teach tree-diff a max-depth parameter\n\nToon Claes (1):\n within_depth: fix return for empty path\n\n Documentation/diff-options.adoc | 28 ++++++++++\n Makefile | 1 +\n combine-diff.c | 2 +-\n diff-lib.c | 5 ++\n diff.c | 24 +++++++++\n diff.h | 8 +++\n dir.c | 2 +-\n t/meson.build | 2 +\n t/t4072-diff-max-depth.sh | 116 ++++++++++++++++++++++++++++++++++++++++\n t/unit-tests/u-dir.c | 47 ++++++++++++++++\n tree-diff.c | 78 +++++++++++++++++++++++++--\n 11 files changed, 308 insertions(+), 5 deletions(-)\n---\n\nRange-diff versus v1:\n\n1: b16919a12d = 1: f551775c58 combine-diff: zero memory used for callback filepairs\n2: c67ed5edee ! 2: 739374a2f8 within_depth: fix return for empty path\n @@\n ## Metadata ##\n -Author: Jeff King \n +Author: Toon Claes \n \n ## Commit message ##\n within_depth: fix return for empty path\n @@ Commit message\n The within_depth() function is used to check whether pathspecs limited\n by a max-depth parameter are acceptable. It takes a path to check, a\n maximum depth, and a \"base\" depth. It counts the components in the\n - path (by counting slashes), adds them to the base, and compare them to\n + path (by counting slashes), adds them to the base, and compares them to\n the maximum.\n \n However, if the base does not have any slashes at all, we always return\n @@ Commit message\n to change that in the future, then this fix is still the correct first\n step, as the current behavior is simply inconsistent.\n \n - Signed-off-by: Jeff King \n + This has the effect the function tree_entry_interesting() now behaves\n + like following on the first example:\n +\n + (with max_depth=0):\n + file: yes\n + a: no\n + a/file: no\n + a/b: no\n +\n + Meaning we won't step in \"a/\" no more to realize all \"a/*\" entries are\n + uninterested, but we stop at the tree entry itself.\n +\n + Based-on-patch-by: Jeff King \n Signed-off-by: Toon Claes \n \n + ## Makefile ##\n +@@ Makefile: THIRD_PARTY_SOURCES += $(UNIT_TEST_DIR)/clar/%\n + THIRD_PARTY_SOURCES += $(UNIT_TEST_DIR)/clar/clar/%\n + \n + CLAR_TEST_SUITES += u-ctype\n ++CLAR_TEST_SUITES += u-dir\n + CLAR_TEST_SUITES += u-example-decorate\n + CLAR_TEST_SUITES += u-hash\n + CLAR_TEST_SUITES += u-hashmap\n +\n ## dir.c ##\n @@ dir.c: int within_depth(const char *name, int namelen,\n \t\tif (depth > max_depth)\n @@ dir.c: int within_depth(const char *name, int namelen,\n }\n \n /*\n +\n + ## t/meson.build ##\n +@@\n + clar_test_suites = [\n + 'unit-tests/u-ctype.c',\n ++ 'unit-tests/u-dir.c',\n + 'unit-tests/u-example-decorate.c',\n + 'unit-tests/u-hash.c',\n + 'unit-tests/u-hashmap.c',\n +\n + ## t/unit-tests/u-dir.c (new) ##\n +@@\n ++#include \"unit-test.h\"\n ++#include \"dir.h\"\n ++\n ++#define TEST_WITHIN_DEPTH(path, depth, max_depth, expect) do { \\\n ++\t\tint actual = within_depth(path, strlen(path), \\\n ++\t\t\t\t\t depth, max_depth); \\\n ++\t\tif (actual != expect) \\\n ++\t\t\tcl_failf(\"path '%s' with depth '%d' and max-depth '%d': expected %d, got %d\", \\\n ++\t\t\t\t path, depth, max_depth, expect, actual); \\\n ++\t} while (0)\n ++\n ++void test_dir__within_depth(void)\n ++{\n ++\t/* depth = 0; max_depth = 0 */\n ++\tTEST_WITHIN_DEPTH(\"\", 0, 0, 1);\n ++\tTEST_WITHIN_DEPTH(\"file\", 0, 0, 1);\n ++\tTEST_WITHIN_DEPTH(\"a\", 0, 0, 1);\n ++\tTEST_WITHIN_DEPTH(\"a/file\", 0, 0, 0);\n ++\tTEST_WITHIN_DEPTH(\"a/b\", 0, 0, 0);\n ++\tTEST_WITHIN_DEPTH(\"a/b/file\", 0, 0, 0);\n ++\n ++\t/* depth = 0; max_depth = 1 */\n ++\tTEST_WITHIN_DEPTH(\"\", 0, 1, 1);\n ++\tTEST_WITHIN_DEPTH(\"file\", 0, 1, 1);\n ++\tTEST_WITHIN_DEPTH(\"a\", 0, 1, 1);\n ++\tTEST_WITHIN_DEPTH(\"a/file\", 0, 1, 1);\n ++\tTEST_WITHIN_DEPTH(\"a/b\", 0, 1, 1);\n ++\tTEST_WITHIN_DEPTH(\"a/b/file\", 0, 1, 0);\n ++\n ++\t/* depth = 1; max_depth = 1 */\n ++\tTEST_WITHIN_DEPTH(\"\", 1, 1, 1);\n ++\tTEST_WITHIN_DEPTH(\"file\", 1, 1, 1);\n ++\tTEST_WITHIN_DEPTH(\"a\", 1, 1, 1);\n ++\tTEST_WITHIN_DEPTH(\"a/file\", 1, 1, 0);\n ++\tTEST_WITHIN_DEPTH(\"a/b\", 1, 1, 0);\n ++\tTEST_WITHIN_DEPTH(\"a/b/file\", 1, 1, 0);\n ++\n ++\t/* depth = 1; max_depth = 0 */\n ++\tTEST_WITHIN_DEPTH(\"\", 1, 0, 0);\n ++\tTEST_WITHIN_DEPTH(\"file\", 1, 0, 0);\n ++\tTEST_WITHIN_DEPTH(\"a\", 1, 0, 0);\n ++\tTEST_WITHIN_DEPTH(\"a/file\", 1, 0, 0);\n ++\tTEST_WITHIN_DEPTH(\"a/b\", 1, 0, 0);\n ++\tTEST_WITHIN_DEPTH(\"a/b/file\", 1, 0, 0);\n ++\n ++\n ++}\n3: b3eb4863c2 ! 3: 2987dbc007 diff: teach tree-diff a max-depth parameter\n @@ Commit message\n and see the raw output for a/b/c/, but not those of a/b/c/d/\n (instead of the raw output you would see for a/b/c/d).\n \n - Signed-off-by: Jeff King \n + Co-authored-by: Toon Claes \n Signed-off-by: Toon Claes \n \n ## Documentation/diff-options.adoc ##\n @@ Documentation/diff-options.adoc: endif::git-format-patch[]\n \treverted with `--ita-visible-in-index`. Both options are\n \texperimental and could be removed in future.\n \n -+--max-depth=::\n -+\n -+\tLimit diff recursion to `` levels (implies `-r`). The depth\n -+\tis measured from the closest pathspec. Given a tree containing\n -+\t`foo/bar/baz`, the following list shows the matches generated by\n -+\teach set of options:\n ++--max-depth=::\n ++\tFor each pathspec given on command line, descend at most ``\n ++\tlevels of directories. A value of `-1` means no limit.\n ++\tCannot be combined with wildcards in the pathspec.\n ++\tGiven a tree containing `foo/bar/baz`, the following list shows the\n ++\tmatches generated by each set of options:\n ++\n +--\n + - `--max-depth=0 -- foo`: `foo`\n @@ diff-lib.c: void run_diff_files(struct rev_info *revs, unsigned int option)\n \tstruct index_state *istate = revs->diffopt.repo->index;\n \n +\tif (revs->diffopt.max_depth_valid)\n -+\t\tdie(\"max-depth is not supported for worktree diffs\");\n ++\t\tdie(_(\"max-depth is not supported for worktree diffs\"));\n +\n \tdiff_set_mnemonic_prefix(&revs->diffopt, \"i/\", \"w/\");\n \n @@ diff-lib.c: static int diff_cache(struct rev_info *revs,\n \topts.pathspec = &revs->diffopt.pathspec;\n \topts.pathspec->recursive = 1;\n +\tif (revs->diffopt.max_depth_valid)\n -+\t\tdie(\"max-depth is not supported for index diffs\");\n ++\t\tdie(_(\"max-depth is not supported for index diffs\"));\n \n \tinit_tree_desc(&t, &tree->object.oid, tree->buffer, tree->size);\n \treturn unpack_trees(1, &t, &opts);\n @@ diff.c: static int diff_opt_rotate_to(const struct option *opt, const char *arg,\n +\tstruct diff_options *options = opt->value;\n +\n +\tBUG_ON_OPT_NEG(unset);\n ++\n ++\tif (!git_parse_int(arg, &options->max_depth))\n ++\t\treturn error(_(\"invalid value for '%s': '%s'\"),\n ++\t\t\t \"--max-depth\", arg);\n ++\n +\toptions->flags.recursive = 1;\n -+\toptions->max_depth = strtol(arg, NULL, 10);\n -+\toptions->max_depth_valid = 1;\n ++\toptions->max_depth_valid = options->max_depth >= 0;\n ++\n +\treturn 0;\n +}\n +\n @@ diff.h: struct diff_options {\n \tint no_free;\n +\n +\t/*\n -+\t * The extra \"valid\" flag is a slight hack. The value \"0\" is perfectly\n -+\t * valid for max-depth. We would normally use -1 to indicate \"not set\",\n -+\t * but there are many code paths which assume that assume that just\n -+\t * zero-ing out a diff_options is enough to initialize it.\n ++\t * The value '0' is a valid max-depth (for no recursion), and value '-1'\n ++\t * also (for unlimited recursion), so the extra \"valid\" flag is used to\n ++\t * determined whether the user specified option --max-depth.\n +\t */\n +\tint max_depth;\n +\tint max_depth_valid;\n @@ t/t4072-diff-max-depth.sh (new)\n +\tcheck_one tree \"$*\" '' 1 file one/file one/two\n +\tcheck_one tree \"$*\" '' 2 file one/file one/two/file one/two/three\n +\tcheck_one tree \"$*\" '' 3 file one/file one/two/file one/two/three/file\n ++\tcheck_one tree \"$*\" '' -1 file one/file one/two/file one/two/three/file\n +\tcheck_one tree \"$*\" one 0 one\n +\tcheck_one tree \"$*\" one 1 one/file one/two\n +\tcheck_one tree \"$*\" one 2 one/file one/two/file one/two/three\n @@ t/t4072-diff-max-depth.sh (new)\n +\tcheck_one tree \"$*\" one/two 0 one/two\n +\tcheck_one tree \"$*\" one/two 1 one/two/file one/two/three\n +\tcheck_one tree \"$*\" one/two 2 one/two/file one/two/three/file\n ++\tcheck_one tree \"$*\" one/two 2 one/two/file one/two/three/file\n +\tcheck_one tree \"$*\" one/two/three 0 one/two/three\n +\tcheck_one tree \"$*\" one/two/three 1 one/two/three/file\n +}\n @@ t/t4072-diff-max-depth.sh (new)\n +\tcheck_one \"$@\" one/two 2 one/two/file one/two/three/file\n +\tcheck_one \"$@\" one/two/three 0\n +\tcheck_one \"$@\" one/two/three 1 one/two/three/file\n ++\n ++\t# Value '-1' for '--max-depth is the same as recursion without limit,\n ++\t# and thus should always succeed.\n ++\tlocal expect=\n ++\tcheck_one \"$@\" '' -1 file one/file one/two/file one/two/three/file\n +}\n +\n +# Check as a modification...\n @@ tree-diff.c\n +\t\t(!path[dirlen] || path[dirlen] == '/');\n +}\n +\n -+static int check_recursion_depth(struct strbuf *name,\n ++static int check_recursion_depth(const struct strbuf *name,\n +\t\t\t\t const struct pathspec *ps,\n +\t\t\t\t int max_depth)\n +{\n @@ tree-diff.c\n +\treturn 0;\n +}\n +\n -+static int should_recurse(struct strbuf *name, struct diff_options *opt)\n ++static int should_recurse(const struct strbuf *name, struct diff_options *opt)\n +{\n +\tif (!opt->flags.recursive)\n +\t\treturn 0;\n @@ tree-diff.c\n +\t * against any internal munging.\n +\t */\n +\tif (opt->pathspec.has_wildcard)\n -+\t\tdie(\"BUG: wildcard pathspecs are incompatible with max-depth\");\n ++\t\tBUG(\"wildcard pathspecs are incompatible with max-depth\");\n +\n +\treturn check_recursion_depth(name, &opt->pathspec, opt->max_depth);\n +}\n\n\n---\n\nbase-commit: 64cbe5e2e8a7b0f92c780b210e602496bd5cad0f\nchange-id: 20250724-toon-max-depth-25d3c19e2607\n\nThanks\n--\nToon\n\n"},{"id":"523774","messageId":"20250807-toon-max-depth-v2-1-50b7e5c81665@iotcl.com","threadId":"63874","inReplyTo":"20250807-toon-max-depth-v2-0-50b7e5c81665@iotcl.com","subject":"[PATCH v2 1/3] combine-diff: zero memory used for callback filepairs","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2025-08-07T20:52:56Z","receivedAt":"2025-08-07T20:53:17Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"From: Jeff King \n\nIn commit 25e5e2bf85 (combine-diff: support format_callback,\n2011-08-19), the combined-diff code learned how to make a multi-sourced\n`diff_filepair` to pass to a diff callback. When we create each\nfilepair, we do not bother to fill in many of the fields, because they\nwould make no sense (e.g. there can be no rename score or broken_pair\nflag because we do not go through the diffcore filters). However, we did\nnot even bother to zero them, leading to random values. Let's make sure\neverything is blank with xcalloc(), just as the regular diff code does.\n\nWe would potentially want to set the `status` flag to\nsomething non-zero, but it is not clear to what. Possibly a\nnew DIFF_STATUS_COMBINED would make sense, as this is not\nstrictly a modification, nor does it fit any other category.\n\nSince it is not yet clear what callers would want, this\npatch simply leaves it as `0`, the same empty flag that is\nseen when `diffcore_std` is not used at all.\n\nSigned-off-by: Jeff King \nSigned-off-by: Toon Claes \n---\n combine-diff.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/combine-diff.c b/combine-diff.c\nindex 4ea2dc93c4..3878faabe7 100644\n--- a/combine-diff.c\n+++ b/combine-diff.c\n@@ -1315,7 +1315,7 @@ static struct diff_filepair *combined_pair(struct combine_diff_path *p,\n \tstruct diff_filepair *pair;\n \tstruct diff_filespec *pool;\n \n-\tpair = xmalloc(sizeof(*pair));\n+\tCALLOC_ARRAY(pair, 1);\n \tCALLOC_ARRAY(pool, st_add(num_parent, 1));\n \tpair->one = pool + 1;\n \tpair->two = pool;\n\n-- \n2.50.1.327.g047016eb4a\n\n"},{"id":"523775","messageId":"20250807-toon-max-depth-v2-2-50b7e5c81665@iotcl.com","threadId":"63874","inReplyTo":"20250807-toon-max-depth-v2-0-50b7e5c81665@iotcl.com","subject":"[PATCH v2 2/3] within_depth: fix return for empty path","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2025-08-07T20:52:57Z","receivedAt":"2025-08-07T20:53:20Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"The within_depth() function is used to check whether pathspecs limited\nby a max-depth parameter are acceptable. It takes a path to check, a\nmaximum depth, and a \"base\" depth. It counts the components in the\npath (by counting slashes), adds them to the base, and compares them to\nthe maximum.\n\nHowever, if the base does not have any slashes at all, we always return\n`true`. If the base depth is 0, then this is correct; no matter what the\nmaximum is, we are always within it. However, if the base depth is\ngreater than 0, then we might return an erroneous result.\n\nThis ends up not causing any user-visible bugs in the current code. The\ncall sites in dir.c always pass a base depth of 0, so are unaffected.\nBut tree_entry_interesting() uses this function differently: it will\npass the prefix of the current entry, along with a `1` if the entry is a\ndirectory, in essence checking whether items inside the entry would be\nof interest. It turns out not to make a difference in behavior, but the\nreasoning is complex.\n\nGiven a tree like:\n\n file\n a/file\n a/b/file\n\nwalking the tree and calling tree_entry_interesting() will yield the\nfollowing results:\n\n (with max_depth=0):\n file: yes\n a: yes\n a/file: no\n a/b: no\n\n (with max_depth=1):\n file: yes\n a: yes\n a/file: yes\n a/b: no\n\nSo we have inconsistent behavior in considering directories interesting.\nIf they are at the edge of our depth but at the root, we will recurse\ninto them, but then find all of their entries uninteresting (e.g., in\nthe first case, we will look at \"a\" but find \"a/*\" uninteresting). But\nif they are at the edge of our depth and not at the root, then we will\nnot recurse (in the second example, we do not even bother entering\n\"a/b\").\n\nThis turns out not to matter because the only caller which uses\nmax-depth pathspecs is cmd_grep(), which only cares about blob entries.\nFrom its perspective, it is exactly the same to not recurse into a\nsubtree, or to recurse and find that it contains no matching entries.\nNot recursing is merely an optimization.\n\nIt is debatable whether tree_entry_interesting() should consider such an\nentry interesting. The only caller does not care if it sees the tree\nitself, and can benefit from the optimization. But if we add a\n\"max-depth\" limiter to regular diffs, then a diff with\nDIFF_OPT_TREE_IN_RECURSIVE would probably want to show the tree itself,\nbut not what it contains.\n\nThis patch just fixes within_depth(), which means we consider such\nentries uninteresting (and makes the current caller happy). If we want\nto change that in the future, then this fix is still the correct first\nstep, as the current behavior is simply inconsistent.\n\nThis has the effect the function tree_entry_interesting() now behaves\nlike following on the first example:\n\n (with max_depth=0):\n file: yes\n a: no\n a/file: no\n a/b: no\n\nMeaning we won't step in \"a/\" no more to realize all \"a/*\" entries are\nuninterested, but we stop at the tree entry itself.\n\nBased-on-patch-by: Jeff King \nSigned-off-by: Toon Claes \n---\n Makefile | 1 +\n dir.c | 2 +-\n t/meson.build | 1 +\n t/unit-tests/u-dir.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++\n 4 files changed, 50 insertions(+), 1 deletion(-)\n\ndiff --git a/Makefile b/Makefile\nindex e11340c1ae..8d403301d9 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1354,6 +1354,7 @@ THIRD_PARTY_SOURCES += $(UNIT_TEST_DIR)/clar/%\n THIRD_PARTY_SOURCES += $(UNIT_TEST_DIR)/clar/clar/%\n \n CLAR_TEST_SUITES += u-ctype\n+CLAR_TEST_SUITES += u-dir\n CLAR_TEST_SUITES += u-example-decorate\n CLAR_TEST_SUITES += u-hash\n CLAR_TEST_SUITES += u-hashmap\ndiff --git a/dir.c b/dir.c\nindex dfb4d40103..71108ac79b 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -277,7 +277,7 @@ int within_depth(const char *name, int namelen,\n \t\tif (depth > max_depth)\n \t\t\treturn 0;\n \t}\n-\treturn 1;\n+\treturn depth <= max_depth;\n }\n \n /*\ndiff --git a/t/meson.build b/t/meson.build\nindex bbeba1a8d5..03245e2160 100644\n--- a/t/meson.build\n+++ b/t/meson.build\n@@ -1,5 +1,6 @@\n clar_test_suites = [\n 'unit-tests/u-ctype.c',\n+ 'unit-tests/u-dir.c',\n 'unit-tests/u-example-decorate.c',\n 'unit-tests/u-hash.c',\n 'unit-tests/u-hashmap.c',\ndiff --git a/t/unit-tests/u-dir.c b/t/unit-tests/u-dir.c\nnew file mode 100644\nindex 0000000000..2d0adaa39e\n--- /dev/null\n+++ b/t/unit-tests/u-dir.c\n@@ -0,0 +1,47 @@\n+#include \"unit-test.h\"\n+#include \"dir.h\"\n+\n+#define TEST_WITHIN_DEPTH(path, depth, max_depth, expect) do { \\\n+\t\tint actual = within_depth(path, strlen(path), \\\n+\t\t\t\t\t depth, max_depth); \\\n+\t\tif (actual != expect) \\\n+\t\t\tcl_failf(\"path '%s' with depth '%d' and max-depth '%d': expected %d, got %d\", \\\n+\t\t\t\t path, depth, max_depth, expect, actual); \\\n+\t} while (0)\n+\n+void test_dir__within_depth(void)\n+{\n+\t/* depth = 0; max_depth = 0 */\n+\tTEST_WITHIN_DEPTH(\"\", 0, 0, 1);\n+\tTEST_WITHIN_DEPTH(\"file\", 0, 0, 1);\n+\tTEST_WITHIN_DEPTH(\"a\", 0, 0, 1);\n+\tTEST_WITHIN_DEPTH(\"a/file\", 0, 0, 0);\n+\tTEST_WITHIN_DEPTH(\"a/b\", 0, 0, 0);\n+\tTEST_WITHIN_DEPTH(\"a/b/file\", 0, 0, 0);\n+\n+\t/* depth = 0; max_depth = 1 */\n+\tTEST_WITHIN_DEPTH(\"\", 0, 1, 1);\n+\tTEST_WITHIN_DEPTH(\"file\", 0, 1, 1);\n+\tTEST_WITHIN_DEPTH(\"a\", 0, 1, 1);\n+\tTEST_WITHIN_DEPTH(\"a/file\", 0, 1, 1);\n+\tTEST_WITHIN_DEPTH(\"a/b\", 0, 1, 1);\n+\tTEST_WITHIN_DEPTH(\"a/b/file\", 0, 1, 0);\n+\n+\t/* depth = 1; max_depth = 1 */\n+\tTEST_WITHIN_DEPTH(\"\", 1, 1, 1);\n+\tTEST_WITHIN_DEPTH(\"file\", 1, 1, 1);\n+\tTEST_WITHIN_DEPTH(\"a\", 1, 1, 1);\n+\tTEST_WITHIN_DEPTH(\"a/file\", 1, 1, 0);\n+\tTEST_WITHIN_DEPTH(\"a/b\", 1, 1, 0);\n+\tTEST_WITHIN_DEPTH(\"a/b/file\", 1, 1, 0);\n+\n+\t/* depth = 1; max_depth = 0 */\n+\tTEST_WITHIN_DEPTH(\"\", 1, 0, 0);\n+\tTEST_WITHIN_DEPTH(\"file\", 1, 0, 0);\n+\tTEST_WITHIN_DEPTH(\"a\", 1, 0, 0);\n+\tTEST_WITHIN_DEPTH(\"a/file\", 1, 0, 0);\n+\tTEST_WITHIN_DEPTH(\"a/b\", 1, 0, 0);\n+\tTEST_WITHIN_DEPTH(\"a/b/file\", 1, 0, 0);\n+\n+\n+}\n\n-- \n2.50.1.327.g047016eb4a\n\n"},{"id":"523776","messageId":"20250807-toon-max-depth-v2-3-50b7e5c81665@iotcl.com","threadId":"63874","inReplyTo":"20250807-toon-max-depth-v2-0-50b7e5c81665@iotcl.com","subject":"[PATCH v2 3/3] diff: teach tree-diff a max-depth parameter","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2025-08-07T20:52:58Z","receivedAt":"2025-08-07T20:53:24Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"From: Jeff King \n\nWhen you are doing a tree-diff, there are basically two options: do not\nrecurse into subtrees at all, or recurse indefinitely. While most\ncallers would want to always recurse and see full pathnames, some may\nwant the efficiency of looking only at a particular level of the tree.\nThis is currently easy to do for the top-level (just turn off\nrecursion), but you cannot say \"show me what changed in subdir/, but do\nnot recurse\".\n\nThis patch adds a max-depth parameter which is measured from the closest\npathspec match, so that you can do:\n\n git log --raw --max-depth=1 -- a/b/c\n\nand see the raw output for a/b/c/, but not those of a/b/c/d/\n(instead of the raw output you would see for a/b/c/d).\n\nCo-authored-by: Toon Claes \nSigned-off-by: Toon Claes \n---\n Documentation/diff-options.adoc | 28 ++++++++++\n diff-lib.c | 5 ++\n diff.c | 24 +++++++++\n diff.h | 8 +++\n t/meson.build | 1 +\n t/t4072-diff-max-depth.sh | 116 ++++++++++++++++++++++++++++++++++++++++\n tree-diff.c | 78 +++++++++++++++++++++++++--\n 7 files changed, 257 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/diff-options.adoc b/Documentation/diff-options.adoc\nindex f3a35d8141..b4cbbd4290 100644\n--- a/Documentation/diff-options.adoc\n+++ b/Documentation/diff-options.adoc\n@@ -893,5 +893,33 @@ endif::git-format-patch[]\n \treverted with `--ita-visible-in-index`. Both options are\n \texperimental and could be removed in future.\n \n+--max-depth=::\n+\tFor each pathspec given on command line, descend at most ``\n+\tlevels of directories. A value of `-1` means no limit.\n+\tCannot be combined with wildcards in the pathspec.\n+\tGiven a tree containing `foo/bar/baz`, the following list shows the\n+\tmatches generated by each set of options:\n++\n+--\n+ - `--max-depth=0 -- foo`: `foo`\n+\n+ - `--max-depth=1 -- foo`: `foo/bar`\n+\n+ - `--max-depth=1 -- foo/bar`: `foo/bar/baz`\n+\n+ - `--max-depth=1 -- foo foo/bar`: `foo/bar/baz`\n+\n+ - `--max-depth=2 -- foo`: `foo/bar/baz`\n+--\n++\n+If no pathspec is given, the depth is measured as if all\n+top-level entries were specified. Note that this is different\n+than measuring from the root, in that `--max-depth=0` would\n+still return `foo`. This allows you to still limit depth while\n+asking for a subset of the top-level entries.\n++\n+Note that this option is only supported for diffs between tree objects,\n+not against the index or working tree.\n+\n For more detailed explanation on these common options, see also\n linkgit:gitdiffcore[7].\ndiff --git a/diff-lib.c b/diff-lib.c\nindex 244468dd1a..b8f8f3bc31 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -115,6 +115,9 @@ void run_diff_files(struct rev_info *revs, unsigned int option)\n \tuint64_t start = getnanotime();\n \tstruct index_state *istate = revs->diffopt.repo->index;\n \n+\tif (revs->diffopt.max_depth_valid)\n+\t\tdie(_(\"max-depth is not supported for worktree diffs\"));\n+\n \tdiff_set_mnemonic_prefix(&revs->diffopt, \"i/\", \"w/\");\n \n \trefresh_fsmonitor(istate);\n@@ -560,6 +563,8 @@ static int diff_cache(struct rev_info *revs,\n \topts.dst_index = NULL;\n \topts.pathspec = &revs->diffopt.pathspec;\n \topts.pathspec->recursive = 1;\n+\tif (revs->diffopt.max_depth_valid)\n+\t\tdie(_(\"max-depth is not supported for index diffs\"));\n \n \tinit_tree_desc(&t, &tree->object.oid, tree->buffer, tree->size);\n \treturn unpack_trees(1, &t, &opts);\ndiff --git a/diff.c b/diff.c\nindex dca87e164f..b548e952fc 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -4988,6 +4988,9 @@ void diff_setup_done(struct diff_options *options)\n \t\t\toptions->filter = ~filter_bit[DIFF_STATUS_FILTER_AON];\n \t\toptions->filter &= ~options->filter_not;\n \t}\n+\n+\tif (options->pathspec.has_wildcard && options->max_depth_valid)\n+\t\tdie(\"max-depth cannot be used with wildcard pathspecs\");\n }\n \n int parse_long_opt(const char *opt, const char **argv,\n@@ -5622,6 +5625,23 @@ static int diff_opt_rotate_to(const struct option *opt, const char *arg, int uns\n \treturn 0;\n }\n \n+static int diff_opt_max_depth(const struct option *opt,\n+\t\t\t const char *arg, int unset)\n+{\n+\tstruct diff_options *options = opt->value;\n+\n+\tBUG_ON_OPT_NEG(unset);\n+\n+\tif (!git_parse_int(arg, &options->max_depth))\n+\t\treturn error(_(\"invalid value for '%s': '%s'\"),\n+\t\t\t \"--max-depth\", arg);\n+\n+\toptions->flags.recursive = 1;\n+\toptions->max_depth_valid = options->max_depth >= 0;\n+\n+\treturn 0;\n+}\n+\n /*\n * Consider adding new flags to __git_diff_common_options\n * in contrib/completion/git-completion.bash\n@@ -5894,6 +5914,10 @@ struct option *add_diff_options(const struct option *opts,\n \t\tOPT_CALLBACK_F(0, \"diff-filter\", options, N_(\"[(A|C|D|M|R|T|U|X|B)...[*]]\"),\n \t\t\t N_(\"select files by diff type\"),\n \t\t\t PARSE_OPT_NONEG, diff_opt_diff_filter),\n+\t\tOPT_CALLBACK_F(0, \"max-depth\", options, N_(\"\"),\n+\t\t\t N_(\"maximum tree depth to recurse\"),\n+\t\t\t PARSE_OPT_NONEG, diff_opt_max_depth),\n+\n \t\t{\n \t\t\t.type = OPTION_CALLBACK,\n \t\t\t.long_name = \"output\",\ndiff --git a/diff.h b/diff.h\nindex 62e5768a9a..bbced5f745 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -404,6 +404,14 @@ struct diff_options {\n \tstruct strmap *additional_path_headers;\n \n \tint no_free;\n+\n+\t/*\n+\t * The value '0' is a valid max-depth (for no recursion), and value '-1'\n+\t * also (for unlimited recursion), so the extra \"valid\" flag is used to\n+\t * determined whether the user specified option --max-depth.\n+\t */\n+\tint max_depth;\n+\tint max_depth_valid;\n };\n \n unsigned diff_filter_bit(char status);\ndiff --git a/t/meson.build b/t/meson.build\nindex 03245e2160..adae038f4a 100644\n--- a/t/meson.build\n+++ b/t/meson.build\n@@ -487,6 +487,7 @@ integration_tests = [\n 't4069-remerge-diff.sh',\n 't4070-diff-pairs.sh',\n 't4071-diff-minimal.sh',\n+ 't4072-diff-max-depth.sh',\n 't4100-apply-stat.sh',\n 't4101-apply-nonl.sh',\n 't4102-apply-rename.sh',\ndiff --git a/t/t4072-diff-max-depth.sh b/t/t4072-diff-max-depth.sh\nnew file mode 100755\nindex 0000000000..0fbf1321f7\n--- /dev/null\n+++ b/t/t4072-diff-max-depth.sh\n@@ -0,0 +1,116 @@\n+#!/bin/sh\n+\n+test_description='check that diff --max-depth will limit recursion'\n+. ./test-lib.sh\n+\n+make_dir() {\n+\tmkdir -p \"$1\" &&\n+\techo \"$2\" >\"$1/file\"\n+}\n+\n+make_files() {\n+\techo \"$1\" >file &&\n+\tmake_dir one \"$1\" &&\n+\tmake_dir one/two \"$1\" &&\n+\tmake_dir one/two/three \"$1\"\n+}\n+\n+test_expect_success 'setup' '\n+\tgit commit --allow-empty -m empty &&\n+\tgit tag empty &&\n+\tmake_files added &&\n+\tgit add . &&\n+\tgit commit -m added &&\n+\tmake_files modified &&\n+\tgit add . &&\n+\tgit commit -m modified &&\n+\tmake_files index &&\n+\tgit add . &&\n+\tmake_files worktree\n+'\n+\n+test_expect_success '--max-depth is disallowed with wildcard pathspecs' '\n+\ttest_must_fail git diff-tree --max-depth=0 HEAD^ HEAD -- \"f*\"\n+'\n+\n+check_one() {\n+\ttype=$1; shift\n+\targs=$1; shift\n+\tpath=$1; shift\n+\tdepth=$1; shift\n+\ttest_expect_${expect:-success} \"diff-$type $args, path=$path, depth=$depth\" \"\n+\t\tfor i in $*; do echo \\$i; done >expect &&\n+\t\tgit diff-$type --max-depth=$depth --name-only $args -- $path >actual &&\n+\t\ttest_cmp expect actual\n+\t\"\n+}\n+\n+# For tree comparisons, we expect to see subtrees at the boundary\n+# get their own entry.\n+check_trees() {\n+\tcheck_one tree \"$*\" '' 0 file one\n+\tcheck_one tree \"$*\" '' 1 file one/file one/two\n+\tcheck_one tree \"$*\" '' 2 file one/file one/two/file one/two/three\n+\tcheck_one tree \"$*\" '' 3 file one/file one/two/file one/two/three/file\n+\tcheck_one tree \"$*\" '' -1 file one/file one/two/file one/two/three/file\n+\tcheck_one tree \"$*\" one 0 one\n+\tcheck_one tree \"$*\" one 1 one/file one/two\n+\tcheck_one tree \"$*\" one 2 one/file one/two/file one/two/three\n+\tcheck_one tree \"$*\" one 3 one/file one/two/file one/two/three/file\n+\tcheck_one tree \"$*\" one/two 0 one/two\n+\tcheck_one tree \"$*\" one/two 1 one/two/file one/two/three\n+\tcheck_one tree \"$*\" one/two 2 one/two/file one/two/three/file\n+\tcheck_one tree \"$*\" one/two 2 one/two/file one/two/three/file\n+\tcheck_one tree \"$*\" one/two/three 0 one/two/three\n+\tcheck_one tree \"$*\" one/two/three 1 one/two/three/file\n+}\n+\n+# But for index comparisons, we do not store subtrees at all, so we do not\n+# expect them.\n+check_index() {\n+\tcheck_one \"$@\" '' 0 file\n+\tcheck_one \"$@\" '' 1 file one/file\n+\tcheck_one \"$@\" '' 2 file one/file one/two/file\n+\tcheck_one \"$@\" '' 3 file one/file one/two/file one/two/three/file\n+\tcheck_one \"$@\" one 0\n+\tcheck_one \"$@\" one 1 one/file\n+\tcheck_one \"$@\" one 2 one/file one/two/file\n+\tcheck_one \"$@\" one 3 one/file one/two/file one/two/three/file\n+\tcheck_one \"$@\" one/two 0\n+\tcheck_one \"$@\" one/two 1 one/two/file\n+\tcheck_one \"$@\" one/two 2 one/two/file one/two/three/file\n+\tcheck_one \"$@\" one/two/three 0\n+\tcheck_one \"$@\" one/two/three 1 one/two/three/file\n+\n+\t# Value '-1' for '--max-depth is the same as recursion without limit,\n+\t# and thus should always succeed.\n+\tlocal expect=\n+\tcheck_one \"$@\" '' -1 file one/file one/two/file one/two/three/file\n+}\n+\n+# Check as a modification...\n+check_trees HEAD^ HEAD\n+# ...and as an addition...\n+check_trees empty HEAD\n+# ...and as a deletion.\n+check_trees HEAD empty\n+\n+# We currently only implement max-depth for trees.\n+expect=failure\n+# Check index against a tree\n+check_index index \"--cached HEAD\"\n+# and index against the worktree\n+check_index files \"\"\n+expect=\n+\n+test_expect_success 'find shortest path within embedded pathspecs' '\n+\tcat >expect <<-\\EOF &&\n+\tone/file\n+\tone/two/file\n+\tone/two/three/file\n+\tEOF\n+\tgit diff-tree --max-depth=2 --name-only HEAD^ HEAD -- one one/two >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_done\ndiff --git a/tree-diff.c b/tree-diff.c\nindex e00fc2f450..5988148b60 100644\n--- a/tree-diff.c\n+++ b/tree-diff.c\n@@ -13,6 +13,7 @@\n #include \"tree-walk.h\"\n #include \"environment.h\"\n #include \"repository.h\"\n+#include \"dir.h\"\n \n /*\n * Some mode bits are also used internally for computations.\n@@ -48,6 +49,73 @@\n \t\tfree((x)); \\\n } while(0)\n \n+/* Returns true if and only if \"dir\" is a leading directory of \"path\" */\n+static int is_dir_prefix(const char *path, const char *dir, int dirlen)\n+{\n+\treturn !strncmp(path, dir, dirlen) &&\n+\t\t(!path[dirlen] || path[dirlen] == '/');\n+}\n+\n+static int check_recursion_depth(const struct strbuf *name,\n+\t\t\t\t const struct pathspec *ps,\n+\t\t\t\t int max_depth)\n+{\n+\tint i;\n+\n+\tif (!ps->nr)\n+\t\treturn within_depth(name->buf, name->len, 1, max_depth);\n+\n+\t/*\n+\t * We look through the pathspecs in reverse-sorted order, because we\n+\t * want to find the longest match first (e.g., \"a/b\" is better for\n+\t * checking depth than \"a/b/c\").\n+\t */\n+\tfor (i = ps->nr - 1; i >= 0; i--) {\n+\t\tconst struct pathspec_item *item = ps->items+i;\n+\n+\t\t/*\n+\t\t * If the name to match is longer than the pathspec, then we\n+\t\t * are only interested if the pathspec matches and we are\n+\t\t * within the allowed depth.\n+\t\t */\n+\t\tif (name->len >= item->len) {\n+\t\t\tif (!is_dir_prefix(name->buf, item->match, item->len))\n+\t\t\t\tcontinue;\n+\t\t\treturn within_depth(name->buf + item->len,\n+\t\t\t\t\t name->len - item->len,\n+\t\t\t\t\t 1, max_depth);\n+\t\t}\n+\n+\t\t/*\n+\t\t * Otherwise, our name is shorter than the pathspec. We need to\n+\t\t * check if it is a prefix of the pathspec; if so, we must\n+\t\t * always recurse in order to process further (the resulting\n+\t\t * paths we find might or might not match our pathspec, but we\n+\t\t * cannot know until we recurse).\n+\t\t */\n+\t\tif (is_dir_prefix(item->match, name->buf, name->len))\n+\t\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n+static int should_recurse(const struct strbuf *name, struct diff_options *opt)\n+{\n+\tif (!opt->flags.recursive)\n+\t\treturn 0;\n+\tif (!opt->max_depth_valid)\n+\t\treturn 1;\n+\n+\t/*\n+\t * We catch this during diff_setup_done, but let's double-check\n+\t * against any internal munging.\n+\t */\n+\tif (opt->pathspec.has_wildcard)\n+\t\tBUG(\"wildcard pathspecs are incompatible with max-depth\");\n+\n+\treturn check_recursion_depth(name, &opt->pathspec, opt->max_depth);\n+}\n+\n static void ll_diff_tree_paths(\n \tstruct combine_diff_path ***tail, const struct object_id *oid,\n \tconst struct object_id **parents_oid, int nparent,\n@@ -170,9 +238,13 @@ static void emit_path(struct combine_diff_path ***tail,\n \t\tmode = 0;\n \t}\n \n-\tif (opt->flags.recursive && isdir) {\n-\t\trecurse = 1;\n-\t\temitthis = opt->flags.tree_in_recursive;\n+\tif (isdir) {\n+\t\tstrbuf_add(base, path, pathlen);\n+\t\tif (should_recurse(base, opt)) {\n+\t\t\trecurse = 1;\n+\t\t\temitthis = opt->flags.tree_in_recursive;\n+\t\t}\n+\t\tstrbuf_setlen(base, old_baselen);\n \t}\n \n \tif (emitthis) {\n\n-- \n2.50.1.327.g047016eb4a\n\n"},{"id":"524179","messageId":"xmqqbjoi6ijj.fsf@gitster.g","threadId":"63874","inReplyTo":"20250807-toon-max-depth-v2-0-50b7e5c81665@iotcl.com","subject":"Re: [PATCH v2 0/3] Teach git-diff-tree(1) option --max-depth","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-08-14T15:15:44Z","receivedAt":"2025-08-14T15:15:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Toon Claes writes:\n\n> Changes in v2:\n> - Added unit-tests for within_depth() in dir.c. Originally the patch\n> was a oneline change by Peff, but I've added a bunch of code and\n> extended the commit message, so I've set myself as the author and set\n> a Based-on-patch-by trailer for Peff. I hope that's okay?\n> - Added support for --max-depth=-1 and extended code comments why we\n> need the max_depth_valid flag. With these modification it did no\n> longer feel appropriate to keep Peff's Signed-off-by trailer.\n> - Made die() messages translatable.\n> - Small tweaks to the docs.\n> - Added some const-correctness.\n> - Switched from `die(\"BUG: ...\")` to `BUG(...)`.\n> - Link to v1: https://lore.kernel.org/r/20250729-toon-max-depth-v1-0-c177e39c40fb@iotcl.com\n\nThis round seems to have attracted no comments, after seeing and\nreacting to a few comments in the previous rounds. Should this be\na part of the first batch after 2.51 final gets tagged?\n\nThanks.\n\n"},{"id":"524226","messageId":"aJ7C3pK5WmDv7zIo@pks.im","threadId":"63874","inReplyTo":"xmqqbjoi6ijj.fsf@gitster.g","subject":"Re: [PATCH v2 0/3] Teach git-diff-tree(1) option --max-depth","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-08-15T05:17:18Z","receivedAt":"2025-08-15T05:17:26Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Aug 14, 2025 at 08:15:44AM -0700, Junio C Hamano wrote:\n> Toon Claes writes:\n> \n> > Changes in v2:\n> > - Added unit-tests for within_depth() in dir.c. Originally the patch\n> > was a oneline change by Peff, but I've added a bunch of code and\n> > extended the commit message, so I've set myself as the author and set\n> > a Based-on-patch-by trailer for Peff. I hope that's okay?\n> > - Added support for --max-depth=-1 and extended code comments why we\n> > need the max_depth_valid flag. With these modification it did no\n> > longer feel appropriate to keep Peff's Signed-off-by trailer.\n> > - Made die() messages translatable.\n> > - Small tweaks to the docs.\n> > - Added some const-correctness.\n> > - Switched from `die(\"BUG: ...\")` to `BUG(...)`.\n> > - Link to v1: https://lore.kernel.org/r/20250729-toon-max-depth-v1-0-c177e39c40fb@iotcl.com\n> \n> This round seems to have attracted no comments, after seeing and\n> reacting to a few comments in the previous rounds. Should this be\n> a part of the first batch after 2.51 final gets tagged?\n\nMy nits all got addressed judging by the range diff, so I'm happy with\nthis version.\n\nThanks!\n\nPatrick\n"}]}