Skip to content

Commit 304812e

Browse files
vmiklosgitster
vmiklos
authored and
gitster
committed
log: improve --follow following renames for non-linear history
Have a repo with a subtree merge, do a 'git log --follow prefix/test.c', the output only contains history in the outer repo, not commits that were merged via a subtree merge. What happens is that 'git log --follow' stores the followed path only in opt->diffopt.pathspec, so in case the commit history is non-linear, and multiple parents have renames to the followed path, then the end result isn't really defined: the first commit that happens to be visited in one of the parents update opt->diffopt.pathspec, and from that point, only that updated path is visited. Fix the problem by introducing a commit -> path map (follow_pathspec_slab) that stores what will be a path to follow when visiting that parent. At the top of log_tree_commit(), if the slab has an entry for this commit, we replace opt->diffopt.pathspec with a path from this entry, so the correct path is followed, even if an unrelated sub-tree changed the path to be followed to something else. After log_tree_diff() runs, we record each parent's path in the slab. As a result, the walk order doesn't matter, which was exactly the source of problems previously. This helps with subtree merges (rename happens inside the merge commit), but also fixes the general case when the rename happens in the history of parents, not in the merge commit itself. Signed-off-by: Miklos Vajna <vmiklos@collabora.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent 26d8d94 commit 304812e

7 files changed

Lines changed: 254 additions & 2 deletions

File tree

‎Documentation/config/log.adoc‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,7 @@ This is the same as the `--decorate` option of the `git log`.
5353
`log.follow`::
5454
If `true`, `git log` will act as if the `--follow` option was used when
5555
a single <path> is given. This has the same limitations as `--follow`,
56-
i.e. it cannot be used to follow multiple files and does not work well
57-
on non-linear history.
56+
i.e. it cannot be used to follow multiple files.
5857

5958
`log.graphColors`::
6059
A list of colors, separated by commas, that can be used to draw

‎log-tree.c‎

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
#include "git-compat-util.h"
55
#include "commit-reach.h"
6+
#include "commit-slab.h"
67
#include "config.h"
78
#include "diff.h"
89
#include "diffcore.h"
@@ -1089,6 +1090,96 @@ static int do_remerge_diff(struct rev_info *opt,
10891090
return !opt->loginfo;
10901091
}
10911092

1093+
/* Per-commit path storage for --follow across merges */
1094+
define_commit_slab(follow_pathspec_slab, char *);
1095+
1096+
static const char *pathspec_single_path(const struct pathspec *ps)
1097+
{
1098+
if (ps->nr != 1)
1099+
return NULL;
1100+
return ps->items[0].match;
1101+
}
1102+
1103+
static void set_pathspec_to_single_path(struct pathspec *ps, const char *path)
1104+
{
1105+
const char *paths[2] = { path, NULL };
1106+
1107+
clear_pathspec(ps);
1108+
parse_pathspec(ps,
1109+
PATHSPEC_ALL_MAGIC & ~PATHSPEC_LITERAL,
1110+
PATHSPEC_LITERAL_PATH, "", paths);
1111+
}
1112+
1113+
static void remember_follow_pathspec(struct rev_info *opt,
1114+
struct commit *c, const char *path)
1115+
{
1116+
char **slot;
1117+
1118+
if (!path)
1119+
return;
1120+
if (!opt->follow_pathspec_slab) {
1121+
opt->follow_pathspec_slab = xmalloc(sizeof(*opt->follow_pathspec_slab));
1122+
init_follow_pathspec_slab(opt->follow_pathspec_slab);
1123+
}
1124+
slot = follow_pathspec_slab_at(opt->follow_pathspec_slab, c);
1125+
if (*slot && !strcmp(*slot, path))
1126+
return;
1127+
free(*slot);
1128+
*slot = xstrdup(path);
1129+
}
1130+
1131+
static const char *recall_follow_pathspec(struct rev_info *opt,
1132+
struct commit *c)
1133+
{
1134+
char **slot;
1135+
1136+
if (!opt->follow_pathspec_slab)
1137+
return NULL;
1138+
slot = follow_pathspec_slab_peek(opt->follow_pathspec_slab, c);
1139+
return slot ? *slot : NULL;
1140+
}
1141+
1142+
static void free_follow_pathspec_slot(char **slot)
1143+
{
1144+
FREE_AND_NULL(*slot);
1145+
}
1146+
1147+
void release_follow_pathspec_slab(struct rev_info *opt)
1148+
{
1149+
if (!opt->follow_pathspec_slab)
1150+
return;
1151+
deep_clear_follow_pathspec_slab(opt->follow_pathspec_slab,
1152+
free_follow_pathspec_slot);
1153+
FREE_AND_NULL(opt->follow_pathspec_slab);
1154+
}
1155+
1156+
/* Compute a path to follow in parent, if there is one */
1157+
static void propagate_follow_pathspec_to_parent(struct rev_info *opt,
1158+
struct commit *commit,
1159+
struct commit *parent)
1160+
{
1161+
struct diff_options diff_opts;
1162+
const char *path;
1163+
1164+
parse_commit_or_die(parent);
1165+
repo_diff_setup(opt->diffopt.repo, &diff_opts);
1166+
copy_pathspec(&diff_opts.pathspec, &opt->diffopt.pathspec);
1167+
diff_opts.flags.recursive = 1;
1168+
diff_opts.flags.follow_renames = 1;
1169+
diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
1170+
diff_setup_done(&diff_opts);
1171+
diff_tree_oid(get_commit_tree_oid(parent),
1172+
get_commit_tree_oid(commit),
1173+
"", &diff_opts);
1174+
1175+
path = pathspec_single_path(&diff_opts.pathspec);
1176+
if (path)
1177+
remember_follow_pathspec(opt, parent, path);
1178+
1179+
diff_queue_clear(&diff_queued_diff);
1180+
diff_free(&diff_opts);
1181+
}
1182+
10921183
/*
10931184
* Show the diff of a commit.
10941185
*
@@ -1185,6 +1276,16 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)
11851276
opt->loginfo = &log;
11861277
opt->diffopt.no_free = 1;
11871278

1279+
/* Any recorded path for this commit? If so, restore it */
1280+
if (opt->diffopt.flags.follow_renames) {
1281+
const char *stored = recall_follow_pathspec(opt, commit);
1282+
if (stored) {
1283+
const char *current = pathspec_single_path(&opt->diffopt.pathspec);
1284+
if (!current || strcmp(current, stored))
1285+
set_pathspec_to_single_path(&opt->diffopt.pathspec, stored);
1286+
}
1287+
}
1288+
11881289
if (opt->track_linear && !opt->linear && !opt->reverse_output_stage)
11891290
fprintf(opt->diffopt.file, "\n%s\n", opt->break_bar);
11901291
shown = log_tree_diff(opt, commit, &log);
@@ -1197,6 +1298,21 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)
11971298
fprintf(opt->diffopt.file, "\n%s\n", opt->break_bar);
11981299
if (shown)
11991300
show_diff_of_diff(opt);
1301+
1302+
/* Record what path each parent of this commit should use */
1303+
if (opt->diffopt.flags.follow_renames) {
1304+
struct commit_list *parents = get_saved_parents(opt, commit);
1305+
if (parents && parents->next) {
1306+
struct commit_list *p;
1307+
for (p = parents; p; p = p->next)
1308+
propagate_follow_pathspec_to_parent(opt, commit,
1309+
p->item);
1310+
} else if (parents) {
1311+
remember_follow_pathspec(opt, parents->item,
1312+
pathspec_single_path(&opt->diffopt.pathspec));
1313+
}
1314+
}
1315+
12001316
opt->loginfo = NULL;
12011317
maybe_flush_or_die(opt->diffopt.file, "stdout");
12021318
opt->diffopt.no_free = no_free;

‎log-tree.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ struct decoration_options {
2626
int parse_decorate_color_config(const char *var, const char *slot_name, const char *value);
2727
int log_tree_diff_flush(struct rev_info *);
2828
int log_tree_commit(struct rev_info *, struct commit *);
29+
void release_follow_pathspec_slab(struct rev_info *);
2930
void show_log(struct rev_info *opt);
3031
void format_decorations(struct strbuf *sb, const struct commit *commit,
3132
enum git_colorbool use_color, const struct decoration_options *opts);

‎revision.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include "decorate.h"
2727
#include "string-list.h"
2828
#include "line-log.h"
29+
#include "log-tree.h"
2930
#include "mailmap.h"
3031
#include "commit-slab.h"
3132
#include "cache-tree.h"
@@ -3304,6 +3305,7 @@ void release_revisions(struct rev_info *revs)
33043305
line_log_free(revs);
33053306
oidset_clear(&revs->missing_commits);
33063307
release_revisions_bloom_keyvecs(revs);
3308+
release_follow_pathspec_slab(revs);
33073309
}
33083310

33093311
static void add_child(struct rev_info *revs, struct commit *parent, struct commit *child)

‎revision.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ struct repository;
6666
struct rev_info;
6767
struct string_list;
6868
struct saved_parents;
69+
struct follow_pathspec_slab;
6970
struct bloom_keyvec;
7071
struct bloom_filter_settings;
7172
struct option;
@@ -363,6 +364,9 @@ struct rev_info {
363364
/* copies of the parent lists, for --full-diff display */
364365
struct saved_parents *saved_parents_slab;
365366

367+
/* per-commit pathspec for --follow across merges */
368+
struct follow_pathspec_slab *follow_pathspec_slab;
369+
366370
struct commit_list *previous_parents;
367371
struct commit_list *ancestry_path_bottoms;
368372
const char *break_bar;

‎t/meson.build‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -576,6 +576,7 @@ integration_tests = [
576576
't4215-log-skewed-merges.sh',
577577
't4216-log-bloom.sh',
578578
't4217-log-limit.sh',
579+
't4219-log-follow-merge.sh',
579580
't4252-am-options.sh',
580581
't4253-am-keep-cr-dos.sh',
581582
't4254-am-corrupt.sh',

‎t/t4219-log-follow-merge.sh‎

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
#!/bin/sh
2+
3+
test_description='Test --follow follows renames across merges'
4+
5+
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master
6+
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
7+
8+
. ./test-lib.sh
9+
10+
test_expect_success 'setup subtree-merged repository' '
11+
git init inner &&
12+
echo inner >inner/inner.txt &&
13+
git -C inner add inner.txt &&
14+
git -C inner commit -m "inner init" &&
15+
16+
git init outer &&
17+
echo outer >outer/outer.txt &&
18+
git -C outer add outer.txt &&
19+
git -C outer commit -m "outer init" &&
20+
21+
git -C outer fetch ../inner master &&
22+
git -C outer merge -s ours --no-commit --allow-unrelated-histories \
23+
FETCH_HEAD &&
24+
git -C outer read-tree --prefix=inner/ -u FETCH_HEAD &&
25+
git -C outer commit -m "Merge inner repo into inner/ subdirectory"
26+
'
27+
28+
test_expect_success '--follow finds the pre-merge commit through a subtree merge' '
29+
git -C outer log --follow --pretty=tformat:%s inner/inner.txt >actual &&
30+
echo "inner init" >expect &&
31+
test_cmp expect actual
32+
'
33+
34+
test_expect_success 'setup merge of two branches that both renamed a file to README' '
35+
git init foo &&
36+
mkdir foo/foo &&
37+
echo "foo readme" >foo/foo/README &&
38+
git -C foo add foo/README &&
39+
git -C foo commit -m "add foo README" &&
40+
41+
git -C foo mv foo/README README &&
42+
git -C foo commit -m "promote foo README to toplevel" &&
43+
44+
echo "foo c" >foo/foo.c &&
45+
git -C foo add foo.c &&
46+
git -C foo commit -m "add foo C impl" &&
47+
48+
git init bar &&
49+
mkdir bar/bar &&
50+
echo "bar readme" >bar/bar/README &&
51+
git -C bar add bar/README &&
52+
git -C bar commit -m "add bar README" &&
53+
54+
git -C bar mv bar/README README &&
55+
git -C bar commit -m "promote bar README to toplevel" &&
56+
57+
echo "bar c" >bar/bar.c &&
58+
git -C bar add bar.c &&
59+
git -C bar commit -m "add bar C impl" &&
60+
61+
git -C foo fetch ../bar master &&
62+
git -C foo merge -s ours --no-commit --allow-unrelated-histories \
63+
FETCH_HEAD &&
64+
git -C foo checkout FETCH_HEAD -- bar.c &&
65+
git -C foo commit -m "merge bar into foo"
66+
'
67+
68+
test_expect_success '--follow follows renames across both sides of a merge' '
69+
git -C foo log --follow --pretty=tformat:%s README >actual &&
70+
sort actual >actual.sorted &&
71+
cat >expect <<-\EOF &&
72+
add bar README
73+
add foo README
74+
promote bar README to toplevel
75+
promote foo README to toplevel
76+
EOF
77+
test_cmp expect actual.sorted
78+
'
79+
80+
test_expect_success 'setup diamond with renames on both sides of a fork' '
81+
git init diamond &&
82+
test_lines="line 1\nline 2\nline 3\nline 4\nline 5\n" &&
83+
84+
printf "$test_lines" >diamond/path0 &&
85+
git -C diamond add path0 &&
86+
git -C diamond commit -m "A: add path0" &&
87+
88+
git -C diamond checkout -b upper &&
89+
printf "line 1\nline 2\nline 3 modified by B\nline 4\nline 5\n" \
90+
>diamond/path0 &&
91+
git -C diamond commit -am "B: modify path0 on upper" &&
92+
git -C diamond mv path0 path1 &&
93+
git -C diamond commit -m "X: rename path0 to path1" &&
94+
95+
git -C diamond checkout -b lower master &&
96+
printf "line 1\nline 2\nline 3 modified by C\nline 4\nline 5\n" \
97+
>diamond/path0 &&
98+
git -C diamond commit -am "C: modify path0 on lower" &&
99+
git -C diamond mv path0 path2 &&
100+
git -C diamond commit -m "Y: rename path0 to path2" &&
101+
102+
git -C diamond checkout upper &&
103+
git -C diamond merge -s ours --no-commit lower &&
104+
git -C diamond rm path1 &&
105+
printf "line 1\nline 2\nline 3 merged\nline 4\nline 5\n" \
106+
>diamond/path &&
107+
git -C diamond add path &&
108+
git -C diamond commit -m "M: merge with rename to path" &&
109+
110+
printf "line 1\nline 2\nline 3 merged again\nline 4\nline 5\n" \
111+
>diamond/path &&
112+
git -C diamond commit -am "Z: modify path"
113+
'
114+
115+
test_expect_success '--follow follows renames through a fork in a single history' '
116+
git -C diamond log --follow --pretty=tformat:%s path >actual &&
117+
sort actual >actual.sorted &&
118+
cat >expect <<-\EOF &&
119+
A: add path0
120+
B: modify path0 on upper
121+
C: modify path0 on lower
122+
X: rename path0 to path1
123+
Y: rename path0 to path2
124+
Z: modify path
125+
EOF
126+
test_cmp expect actual.sorted
127+
'
128+
129+
test_done

0 commit comments

Comments
 (0)