Skip to content

Commit 7dae8b2

Browse files
committed
diff -c -p: do not die on submodules
The combine diff logic knew only about blobs (and their checked-out form in the work tree, either regular files or symlinks), and barfed when fed submodules. This "externalizes" gitlinks in the same way as the normal patch generation codepath does (i.e. "Subproject commit Xxx\n") to fix the issue. Signed-off-by: Junio C Hamano <[email protected]>
1 parent c922b01 commit 7dae8b2

File tree

2 files changed

+65
-12
lines changed

2 files changed

+65
-12
lines changed

combine-diff.c

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include "quote.h"
77
#include "xdiff-interface.h"
88
#include "log-tree.h"
9+
#include "refs.h"
910

1011
static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr, int n, int num_parent)
1112
{
@@ -90,18 +91,24 @@ struct sline {
9091
unsigned long *p_lno;
9192
};
9293

93-
static char *grab_blob(const unsigned char *sha1, unsigned long *size)
94+
static char *grab_blob(const unsigned char *sha1, unsigned int mode, unsigned long *size)
9495
{
9596
char *blob;
9697
enum object_type type;
97-
if (is_null_sha1(sha1)) {
98+
99+
if (S_ISGITLINK(mode)) {
100+
blob = xmalloc(100);
101+
*size = snprintf(blob, 100,
102+
"Subproject commit %s\n", sha1_to_hex(sha1));
103+
} else if (is_null_sha1(sha1)) {
98104
/* deleted blob */
99105
*size = 0;
100106
return xcalloc(1, 1);
107+
} else {
108+
blob = read_sha1_file(sha1, &type, size);
109+
if (type != OBJ_BLOB)
110+
die("object '%s' is not a blob!", sha1_to_hex(sha1));
101111
}
102-
blob = read_sha1_file(sha1, &type, size);
103-
if (type != OBJ_BLOB)
104-
die("object '%s' is not a blob!", sha1_to_hex(sha1));
105112
return blob;
106113
}
107114

@@ -197,7 +204,8 @@ static void consume_line(void *state_, char *line, unsigned long len)
197204
}
198205
}
199206

200-
static void combine_diff(const unsigned char *parent, mmfile_t *result_file,
207+
static void combine_diff(const unsigned char *parent, unsigned int mode,
208+
mmfile_t *result_file,
201209
struct sline *sline, unsigned int cnt, int n,
202210
int num_parent)
203211
{
@@ -213,7 +221,7 @@ static void combine_diff(const unsigned char *parent, mmfile_t *result_file,
213221
if (!cnt)
214222
return; /* result deleted */
215223

216-
parent_file.ptr = grab_blob(parent, &sz);
224+
parent_file.ptr = grab_blob(parent, mode, &sz);
217225
parent_file.size = sz;
218226
xpp.flags = XDF_NEED_MINIMAL;
219227
memset(&xecfg, 0, sizeof(xecfg));
@@ -692,7 +700,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
692700
context = opt->context;
693701
/* Read the result of merge first */
694702
if (!working_tree_file)
695-
result = grab_blob(elem->sha1, &result_size);
703+
result = grab_blob(elem->sha1, elem->mode, &result_size);
696704
else {
697705
/* Used by diff-tree to read from the working tree */
698706
struct stat st;
@@ -712,9 +720,13 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
712720
}
713721
result[len] = 0;
714722
elem->mode = canon_mode(st.st_mode);
715-
}
716-
else if (0 <= (fd = open(elem->path, O_RDONLY)) &&
717-
!fstat(fd, &st)) {
723+
} else if (S_ISDIR(st.st_mode)) {
724+
unsigned char sha1[20];
725+
if (resolve_gitlink_ref(elem->path, "HEAD", sha1) < 0)
726+
result = grab_blob(elem->sha1, elem->mode, &result_size);
727+
else
728+
result = grab_blob(sha1, elem->mode, &result_size);
729+
} else if (0 <= (fd = open(elem->path, O_RDONLY))) {
718730
size_t len = xsize_t(st.st_size);
719731
ssize_t done;
720732
int is_file, i;
@@ -807,7 +819,9 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
807819
}
808820
}
809821
if (i <= j)
810-
combine_diff(elem->parent[i].sha1, &result_file, sline,
822+
combine_diff(elem->parent[i].sha1,
823+
elem->parent[i].mode,
824+
&result_file, sline,
811825
cnt, i, num_parent);
812826
if (elem->parent[i].mode != elem->mode)
813827
mode_differs = 1;

t/t4027-diff-submodule.sh

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,4 +57,43 @@ test_expect_success 'git diff (empty submodule dir)' '
5757
test_cmp empty actual.empty
5858
'
5959

60+
test_expect_success 'conflicted submodule setup' '
61+
62+
# 39 efs
63+
c=fffffffffffffffffffffffffffffffffffffff
64+
(
65+
echo "000000 $_z40 0 sub"
66+
echo "160000 1$c 1 sub"
67+
echo "160000 2$c 2 sub"
68+
echo "160000 3$c 3 sub"
69+
) | git update-index --index-info &&
70+
echo >expect.nosub '\''diff --cc sub
71+
index 2ffffff,3ffffff..0000000
72+
--- a/sub
73+
+++ b/sub
74+
@@@ -1,1 -1,1 +1,1 @@@
75+
- Subproject commit 2fffffffffffffffffffffffffffffffffffffff
76+
-Subproject commit 3fffffffffffffffffffffffffffffffffffffff
77+
++Subproject commit 0000000000000000000000000000000000000000'\'' &&
78+
79+
hh=$(git rev-parse HEAD) &&
80+
sed -e "s/$_z40/$hh/" expect.nosub >expect.withsub
81+
82+
'
83+
84+
test_expect_success 'combined (empty submodule)' '
85+
rm -fr sub && mkdir sub &&
86+
git diff >actual &&
87+
test_cmp expect.nosub actual
88+
'
89+
90+
test_expect_success 'combined (with submodule)' '
91+
rm -fr sub &&
92+
git clone --no-checkout . sub &&
93+
git diff >actual &&
94+
test_cmp expect.withsub actual
95+
'
96+
97+
98+
6099
test_done

0 commit comments

Comments
 (0)