Skip to content

Commit 4ab4a6d

Browse files
gitstergitster
committed
Merge branch 'tr/log-full-diff-keep-true-parents'
Output from "git log --full-diff -- <pathspec>" looked strange, because comparison was done with the previous ancestor that touched the specified <pathspec>, causing the patches for paths outside the pathspec to show more than the single commit has changed. Tweak "git reflog -p" for the same reason using the same mechanism. * tr/log-full-diff-keep-true-parents: log: use true parents for diff when walking reflogs log: use true parents for diff even when rewriting
2 parents 24703ea + 838f9a1 commit 4ab4a6d

File tree

8 files changed

+134
-3
lines changed

8 files changed

+134
-3
lines changed

‎combine-diff.c‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "refs.h"
1111
#include "userdiff.h"
1212
#include "sha1-array.h"
13+
#include "revision.h"
1314

1415
static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr, int n, int num_parent)
1516
{
@@ -1383,7 +1384,7 @@ void diff_tree_combined(const unsigned char *sha1,
13831384
void diff_tree_combined_merge(const struct commit *commit, int dense,
13841385
struct rev_info *rev)
13851386
{
1386-
struct commit_list *parent = commit->parents;
1387+
struct commit_list *parent = get_saved_parents(rev, commit);
13871388
struct sha1_array parents = SHA1_ARRAY_INIT;
13881389

13891390
while (parent) {

‎commit.c‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,22 @@ unsigned commit_list_count(const struct commit_list *l)
377377
return c;
378378
}
379379

380+
struct commit_list *copy_commit_list(struct commit_list *list)
381+
{
382+
struct commit_list *head = NULL;
383+
struct commit_list **pp = &head;
384+
while (list) {
385+
struct commit_list *new;
386+
new = xmalloc(sizeof(struct commit_list));
387+
new->item = list->item;
388+
new->next = NULL;
389+
*pp = new;
390+
pp = &new->next;
391+
list = list->next;
392+
}
393+
return head;
394+
}
395+
380396
void free_commit_list(struct commit_list *list)
381397
{
382398
while (list) {

‎commit.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,9 @@ struct commit_list *commit_list_insert_by_date(struct commit *item,
6262
struct commit_list **list);
6363
void commit_list_sort_by_date(struct commit_list **list);
6464

65+
/* Shallow copy of the input list */
66+
struct commit_list *copy_commit_list(struct commit_list *list);
67+
6568
void free_commit_list(struct commit_list *list);
6669

6770
/* Commit formats */

‎log-tree.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -738,7 +738,7 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
738738
sha1 = commit->tree->object.sha1;
739739

740740
/* Root commit? */
741-
parents = commit->parents;
741+
parents = get_saved_parents(opt, commit);
742742
if (!parents) {
743743
if (opt->show_root_diff) {
744744
diff_root_tree_sha1(sha1, "", &opt->diffopt);

‎revision.c‎

Lines changed: 64 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include "string-list.h"
1616
#include "line-log.h"
1717
#include "mailmap.h"
18+
#include "commit-slab.h"
1819

1920
volatile show_early_output_fn_t show_early_output;
2021

@@ -2763,7 +2764,7 @@ static int commit_match(struct commit *commit, struct rev_info *opt)
27632764
return retval;
27642765
}
27652766

2766-
static inline int want_ancestry(struct rev_info *revs)
2767+
static inline int want_ancestry(const struct rev_info *revs)
27672768
{
27682769
return (revs->rewrite_parents || revs->children.name);
27692770
}
@@ -2820,6 +2821,14 @@ enum commit_action simplify_commit(struct rev_info *revs, struct commit *commit)
28202821
if (action == commit_show &&
28212822
!revs->show_all &&
28222823
revs->prune && revs->dense && want_ancestry(revs)) {
2824+
/*
2825+
* --full-diff on simplified parents is no good: it
2826+
* will show spurious changes from the commits that
2827+
* were elided. So we save the parents on the side
2828+
* when --full-diff is in effect.
2829+
*/
2830+
if (revs->full_diff)
2831+
save_parents(revs, commit);
28232832
if (rewrite_parents(revs, commit, rewrite_one) < 0)
28242833
return commit_error;
28252834
}
@@ -2839,6 +2848,7 @@ static struct commit *get_revision_1(struct rev_info *revs)
28392848
free(entry);
28402849

28412850
if (revs->reflog_info) {
2851+
save_parents(revs, commit);
28422852
fake_reflog_parent(revs->reflog_info, commit);
28432853
commit->object.flags &= ~(ADDED | SEEN | SHOWN);
28442854
}
@@ -3038,6 +3048,8 @@ struct commit *get_revision(struct rev_info *revs)
30383048
c = get_revision_internal(revs);
30393049
if (c && revs->graph)
30403050
graph_update(revs->graph, c);
3051+
if (!c)
3052+
free_saved_parents(revs);
30413053
return c;
30423054
}
30433055

@@ -3069,3 +3081,54 @@ void put_revision_mark(const struct rev_info *revs, const struct commit *commit)
30693081
fputs(mark, stdout);
30703082
putchar(' ');
30713083
}
3084+
3085+
define_commit_slab(saved_parents, struct commit_list *);
3086+
3087+
#define EMPTY_PARENT_LIST ((struct commit_list *)-1)
3088+
3089+
void save_parents(struct rev_info *revs, struct commit *commit)
3090+
{
3091+
struct commit_list **pp;
3092+
3093+
if (!revs->saved_parents_slab) {
3094+
revs->saved_parents_slab = xmalloc(sizeof(struct saved_parents));
3095+
init_saved_parents(revs->saved_parents_slab);
3096+
}
3097+
3098+
pp = saved_parents_at(revs->saved_parents_slab, commit);
3099+
3100+
/*
3101+
* When walking with reflogs, we may visit the same commit
3102+
* several times: once for each appearance in the reflog.
3103+
*
3104+
* In this case, save_parents() will be called multiple times.
3105+
* We want to keep only the first set of parents. We need to
3106+
* store a sentinel value for an empty (i.e., NULL) parent
3107+
* list to distinguish it from a not-yet-saved list, however.
3108+
*/
3109+
if (*pp)
3110+
return;
3111+
if (commit->parents)
3112+
*pp = copy_commit_list(commit->parents);
3113+
else
3114+
*pp = EMPTY_PARENT_LIST;
3115+
}
3116+
3117+
struct commit_list *get_saved_parents(struct rev_info *revs, const struct commit *commit)
3118+
{
3119+
struct commit_list *parents;
3120+
3121+
if (!revs->saved_parents_slab)
3122+
return commit->parents;
3123+
3124+
parents = *saved_parents_at(revs->saved_parents_slab, commit);
3125+
if (parents == EMPTY_PARENT_LIST)
3126+
return NULL;
3127+
return parents;
3128+
}
3129+
3130+
void free_saved_parents(struct rev_info *revs)
3131+
{
3132+
if (revs->saved_parents_slab)
3133+
clear_saved_parents(revs->saved_parents_slab);
3134+
}

‎revision.h‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
struct rev_info;
2626
struct log_info;
2727
struct string_list;
28+
struct saved_parents;
2829

2930
struct rev_cmdline_info {
3031
unsigned int nr;
@@ -187,6 +188,9 @@ struct rev_info {
187188

188189
/* line level range that we are chasing */
189190
struct decoration line_log_data;
191+
192+
/* copies of the parent lists, for --full-diff display */
193+
struct saved_parents *saved_parents_slab;
190194
};
191195

192196
#define REV_TREE_SAME 0
@@ -273,4 +277,20 @@ typedef enum rewrite_result (*rewrite_parent_fn_t)(struct rev_info *revs, struct
273277

274278
extern int rewrite_parents(struct rev_info *revs, struct commit *commit,
275279
rewrite_parent_fn_t rewrite_parent);
280+
281+
/*
282+
* Save a copy of the parent list, and return the saved copy. This is
283+
* used by the log machinery to retrieve the original parents when
284+
* commit->parents has been modified by history simpification.
285+
*
286+
* You may only call save_parents() once per commit (this is checked
287+
* for non-root commits).
288+
*
289+
* get_saved_parents() will transparently return commit->parents if
290+
* history simplification is off.
291+
*/
292+
extern void save_parents(struct rev_info *revs, struct commit *commit);
293+
extern struct commit_list *get_saved_parents(struct rev_info *revs, const struct commit *commit);
294+
extern void free_saved_parents(struct rev_info *revs);
295+
276296
#endif

‎t/t1411-reflog-show.sh‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,4 +144,26 @@ test_expect_success 'empty reflog file' '
144144
test_cmp expect actual
145145
'
146146

147+
# This guards against the alternative of showing the diffs vs. the
148+
# reflog ancestor. The reflog used is designed to list the commits
149+
# more than once, so as to exercise the corresponding logic.
150+
test_expect_success 'git log -g -p shows diffs vs. parents' '
151+
test_commit two &&
152+
git branch flipflop &&
153+
git update-ref refs/heads/flipflop -m flip1 HEAD^ &&
154+
git update-ref refs/heads/flipflop -m flop1 HEAD &&
155+
git update-ref refs/heads/flipflop -m flip2 HEAD^ &&
156+
git log -g -p flipflop >reflog &&
157+
grep -v ^Reflog reflog >actual &&
158+
git log -1 -p HEAD^ >log.one &&
159+
git log -1 -p HEAD >log.two &&
160+
(
161+
cat log.one; echo
162+
cat log.two; echo
163+
cat log.one; echo
164+
cat log.two
165+
) >expect &&
166+
test_cmp expect actual
167+
'
168+
147169
test_done

‎t/t6012-rev-list-simplify.sh‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,4 +127,10 @@ test_expect_success 'full history simplification without parent' '
127127
}
128128
'
129129

130+
test_expect_success '--full-diff is not affected by --parents' '
131+
git log -p --pretty="%H" --full-diff -- file >expected &&
132+
git log -p --pretty="%H" --full-diff --parents -- file >actual &&
133+
test_cmp expected actual
134+
'
135+
130136
test_done

0 commit comments

Comments
 (0)