Skip to content

Commit 8107d36

Browse files
peffgitster
authored andcommitted
diff: teach tree-diff a max-depth parameter
When you are doing a tree-diff, there are basically two options: do not recurse into subtrees at all, or recurse indefinitely. While most callers would want to always recurse and see full pathnames, some may want the efficiency of looking only at a particular level of the tree. This is currently easy to do for the top-level (just turn off recursion), but you cannot say "show me what changed in subdir/, but do not recurse". This patch adds a max-depth parameter which is measured from the closest pathspec match, so that you can do: git log --raw --max-depth=1 -- a/b/c and see the raw output for a/b/c/, but not those of a/b/c/d/ (instead of the raw output you would see for a/b/c/d). Signed-off-by: Jeff King <[email protected]> Signed-off-by: Toon Claes <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent e5add69 commit 8107d36

File tree

7 files changed

+246
-3
lines changed

7 files changed

+246
-3
lines changed

Documentation/diff-options.adoc

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -887,5 +887,33 @@ endif::git-format-patch[]
887887
reverted with `--ita-visible-in-index`. Both options are
888888
experimental and could be removed in future.
889889

890+
--max-depth=<n>::
891+
892+
Limit diff recursion to `<n>` levels (implies `-r`). The depth
893+
is measured from the closest pathspec. Given a tree containing
894+
`foo/bar/baz`, the following list shows the matches generated by
895+
each set of options:
896+
+
897+
--
898+
- `--max-depth=0 -- foo`: `foo`
899+
900+
- `--max-depth=1 -- foo`: `foo/bar`
901+
902+
- `--max-depth=1 -- foo/bar`: `foo/bar/baz`
903+
904+
- `--max-depth=1 -- foo foo/bar`: `foo/bar/baz`
905+
906+
- `--max-depth=2 -- foo`: `foo/bar/baz`
907+
--
908+
+
909+
If no pathspec is given, the depth is measured as if all
910+
top-level entries were specified. Note that this is different
911+
than measuring from the root, in that `--max-depth=0` would
912+
still return `foo`. This allows you to still limit depth while
913+
asking for a subset of the top-level entries.
914+
+
915+
Note that this option is only supported for diffs between tree objects,
916+
not against the index or working tree.
917+
890918
For more detailed explanation on these common options, see also
891919
linkgit:gitdiffcore[7].

diff-lib.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,9 @@ void run_diff_files(struct rev_info *revs, unsigned int option)
115115
uint64_t start = getnanotime();
116116
struct index_state *istate = revs->diffopt.repo->index;
117117

118+
if (revs->diffopt.max_depth_valid)
119+
die("max-depth is not supported for worktree diffs");
120+
118121
diff_set_mnemonic_prefix(&revs->diffopt, "i/", "w/");
119122

120123
refresh_fsmonitor(istate);
@@ -560,6 +563,8 @@ static int diff_cache(struct rev_info *revs,
560563
opts.dst_index = NULL;
561564
opts.pathspec = &revs->diffopt.pathspec;
562565
opts.pathspec->recursive = 1;
566+
if (revs->diffopt.max_depth_valid)
567+
die("max-depth is not supported for index diffs");
563568

564569
init_tree_desc(&t, &tree->object.oid, tree->buffer, tree->size);
565570
return unpack_trees(1, &t, &opts);

diff.c

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4988,6 +4988,9 @@ void diff_setup_done(struct diff_options *options)
49884988
options->filter = ~filter_bit[DIFF_STATUS_FILTER_AON];
49894989
options->filter &= ~options->filter_not;
49904990
}
4991+
4992+
if (options->pathspec.has_wildcard && options->max_depth_valid)
4993+
die("max-depth cannot be used with wildcard pathspecs");
49914994
}
49924995

49934996
int parse_long_opt(const char *opt, const char **argv,
@@ -5622,6 +5625,18 @@ static int diff_opt_rotate_to(const struct option *opt, const char *arg, int uns
56225625
return 0;
56235626
}
56245627

5628+
static int diff_opt_max_depth(const struct option *opt,
5629+
const char *arg, int unset)
5630+
{
5631+
struct diff_options *options = opt->value;
5632+
5633+
BUG_ON_OPT_NEG(unset);
5634+
options->flags.recursive = 1;
5635+
options->max_depth = strtol(arg, NULL, 10);
5636+
options->max_depth_valid = 1;
5637+
return 0;
5638+
}
5639+
56255640
/*
56265641
* Consider adding new flags to __git_diff_common_options
56275642
* in contrib/completion/git-completion.bash
@@ -5894,6 +5909,10 @@ struct option *add_diff_options(const struct option *opts,
58945909
OPT_CALLBACK_F(0, "diff-filter", options, N_("[(A|C|D|M|R|T|U|X|B)...[*]]"),
58955910
N_("select files by diff type"),
58965911
PARSE_OPT_NONEG, diff_opt_diff_filter),
5912+
OPT_CALLBACK_F(0, "max-depth", options, N_("<depth>"),
5913+
N_("maximum tree depth to recurse"),
5914+
PARSE_OPT_NONEG, diff_opt_max_depth),
5915+
58975916
{
58985917
.type = OPTION_CALLBACK,
58995918
.long_name = "output",

diff.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,15 @@ struct diff_options {
404404
struct strmap *additional_path_headers;
405405

406406
int no_free;
407+
408+
/*
409+
* The extra "valid" flag is a slight hack. The value "0" is perfectly
410+
* valid for max-depth. We would normally use -1 to indicate "not set",
411+
* but there are many code paths which assume that assume that just
412+
* zero-ing out a diff_options is enough to initialize it.
413+
*/
414+
int max_depth;
415+
int max_depth_valid;
407416
};
408417

409418
unsigned diff_filter_bit(char status);

t/meson.build

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -502,6 +502,7 @@ integration_tests = [
502502
't4069-remerge-diff.sh',
503503
't4070-diff-pairs.sh',
504504
't4071-diff-minimal.sh',
505+
't4072-diff-max-depth.sh',
505506
't4100-apply-stat.sh',
506507
't4101-apply-nonl.sh',
507508
't4102-apply-rename.sh',

t/t4072-diff-max-depth.sh

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
#!/bin/sh
2+
3+
test_description='check that diff --max-depth will limit recursion'
4+
. ./test-lib.sh
5+
6+
make_dir() {
7+
mkdir -p "$1" &&
8+
echo "$2" >"$1/file"
9+
}
10+
11+
make_files() {
12+
echo "$1" >file &&
13+
make_dir one "$1" &&
14+
make_dir one/two "$1" &&
15+
make_dir one/two/three "$1"
16+
}
17+
18+
test_expect_success 'setup' '
19+
git commit --allow-empty -m empty &&
20+
git tag empty &&
21+
make_files added &&
22+
git add . &&
23+
git commit -m added &&
24+
make_files modified &&
25+
git add . &&
26+
git commit -m modified &&
27+
make_files index &&
28+
git add . &&
29+
make_files worktree
30+
'
31+
32+
test_expect_success '--max-depth is disallowed with wildcard pathspecs' '
33+
test_must_fail git diff-tree --max-depth=0 HEAD^ HEAD -- "f*"
34+
'
35+
36+
check_one() {
37+
type=$1; shift
38+
args=$1; shift
39+
path=$1; shift
40+
depth=$1; shift
41+
test_expect_${expect:-success} "diff-$type $args, path=$path, depth=$depth" "
42+
for i in $*; do echo \$i; done >expect &&
43+
git diff-$type --max-depth=$depth --name-only $args -- $path >actual &&
44+
test_cmp expect actual
45+
"
46+
}
47+
48+
# For tree comparisons, we expect to see subtrees at the boundary
49+
# get their own entry.
50+
check_trees() {
51+
check_one tree "$*" '' 0 file one
52+
check_one tree "$*" '' 1 file one/file one/two
53+
check_one tree "$*" '' 2 file one/file one/two/file one/two/three
54+
check_one tree "$*" '' 3 file one/file one/two/file one/two/three/file
55+
check_one tree "$*" one 0 one
56+
check_one tree "$*" one 1 one/file one/two
57+
check_one tree "$*" one 2 one/file one/two/file one/two/three
58+
check_one tree "$*" one 3 one/file one/two/file one/two/three/file
59+
check_one tree "$*" one/two 0 one/two
60+
check_one tree "$*" one/two 1 one/two/file one/two/three
61+
check_one tree "$*" one/two 2 one/two/file one/two/three/file
62+
check_one tree "$*" one/two/three 0 one/two/three
63+
check_one tree "$*" one/two/three 1 one/two/three/file
64+
}
65+
66+
# But for index comparisons, we do not store subtrees at all, so we do not
67+
# expect them.
68+
check_index() {
69+
check_one "$@" '' 0 file
70+
check_one "$@" '' 1 file one/file
71+
check_one "$@" '' 2 file one/file one/two/file
72+
check_one "$@" '' 3 file one/file one/two/file one/two/three/file
73+
check_one "$@" one 0
74+
check_one "$@" one 1 one/file
75+
check_one "$@" one 2 one/file one/two/file
76+
check_one "$@" one 3 one/file one/two/file one/two/three/file
77+
check_one "$@" one/two 0
78+
check_one "$@" one/two 1 one/two/file
79+
check_one "$@" one/two 2 one/two/file one/two/three/file
80+
check_one "$@" one/two/three 0
81+
check_one "$@" one/two/three 1 one/two/three/file
82+
}
83+
84+
# Check as a modification...
85+
check_trees HEAD^ HEAD
86+
# ...and as an addition...
87+
check_trees empty HEAD
88+
# ...and as a deletion.
89+
check_trees HEAD empty
90+
91+
# We currently only implement max-depth for trees.
92+
expect=failure
93+
# Check index against a tree
94+
check_index index "--cached HEAD"
95+
# and index against the worktree
96+
check_index files ""
97+
expect=
98+
99+
test_expect_success 'find shortest path within embedded pathspecs' '
100+
cat >expect <<-\EOF &&
101+
one/file
102+
one/two/file
103+
one/two/three/file
104+
EOF
105+
git diff-tree --max-depth=2 --name-only HEAD^ HEAD -- one one/two >actual &&
106+
test_cmp expect actual
107+
'
108+
109+
test_done

tree-diff.c

Lines changed: 75 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include "tree-walk.h"
1414
#include "environment.h"
1515
#include "repository.h"
16+
#include "dir.h"
1617

1718
/*
1819
* Some mode bits are also used internally for computations.
@@ -48,6 +49,73 @@
4849
free((x)); \
4950
} while(0)
5051

52+
/* Returns true if and only if "dir" is a leading directory of "path" */
53+
static int is_dir_prefix(const char *path, const char *dir, int dirlen)
54+
{
55+
return !strncmp(path, dir, dirlen) &&
56+
(!path[dirlen] || path[dirlen] == '/');
57+
}
58+
59+
static int check_recursion_depth(struct strbuf *name,
60+
const struct pathspec *ps,
61+
int max_depth)
62+
{
63+
int i;
64+
65+
if (!ps->nr)
66+
return within_depth(name->buf, name->len, 1, max_depth);
67+
68+
/*
69+
* We look through the pathspecs in reverse-sorted order, because we
70+
* want to find the longest match first (e.g., "a/b" is better for
71+
* checking depth than "a/b/c").
72+
*/
73+
for (i = ps->nr - 1; i >= 0; i--) {
74+
const struct pathspec_item *item = ps->items+i;
75+
76+
/*
77+
* If the name to match is longer than the pathspec, then we
78+
* are only interested if the pathspec matches and we are
79+
* within the allowed depth.
80+
*/
81+
if (name->len >= item->len) {
82+
if (!is_dir_prefix(name->buf, item->match, item->len))
83+
continue;
84+
return within_depth(name->buf + item->len,
85+
name->len - item->len,
86+
1, max_depth);
87+
}
88+
89+
/*
90+
* Otherwise, our name is shorter than the pathspec. We need to
91+
* check if it is a prefix of the pathspec; if so, we must
92+
* always recurse in order to process further (the resulting
93+
* paths we find might or might not match our pathspec, but we
94+
* cannot know until we recurse).
95+
*/
96+
if (is_dir_prefix(item->match, name->buf, name->len))
97+
return 1;
98+
}
99+
return 0;
100+
}
101+
102+
static int should_recurse(struct strbuf *name, struct diff_options *opt)
103+
{
104+
if (!opt->flags.recursive)
105+
return 0;
106+
if (!opt->max_depth_valid)
107+
return 1;
108+
109+
/*
110+
* We catch this during diff_setup_done, but let's double-check
111+
* against any internal munging.
112+
*/
113+
if (opt->pathspec.has_wildcard)
114+
die("BUG: wildcard pathspecs are incompatible with max-depth");
115+
116+
return check_recursion_depth(name, &opt->pathspec, opt->max_depth);
117+
}
118+
51119
static void ll_diff_tree_paths(
52120
struct combine_diff_path ***tail, const struct object_id *oid,
53121
const struct object_id **parents_oid, int nparent,
@@ -170,9 +238,13 @@ static void emit_path(struct combine_diff_path ***tail,
170238
mode = 0;
171239
}
172240

173-
if (opt->flags.recursive && isdir) {
174-
recurse = 1;
175-
emitthis = opt->flags.tree_in_recursive;
241+
if (isdir) {
242+
strbuf_add(base, path, pathlen);
243+
if (should_recurse(base, opt)) {
244+
recurse = 1;
245+
emitthis = opt->flags.tree_in_recursive;
246+
}
247+
strbuf_setlen(base, old_baselen);
176248
}
177249

178250
if (emitthis) {

0 commit comments

Comments
 (0)