diff --no-index: support reading from named pipes
In some shells, such as bash and zsh, it's possible to use a command substitution to provide the output of a command as a file argument to another process, like so: diff -u <(printf "a\nb\n") <(printf "a\nc\n") However, this syntax does not produce useful results with "git diff --no-index". On macOS, the arguments to the command are named pipes under /dev/fd, and git diff doesn't know how to handle a named pipe. On Linux, the arguments are symlinks to pipes, so git diff "helpfully" diffs these symlinks, comparing their targets like "pipe:[1234]" and "pipe:[5678]". To address this "diff --no-index" is changed so that if a path given on the commandline is a named pipe or a symbolic link that resolves to a named pipe then we read the data to diff from that pipe. This is implemented by generalizing the code that already exists to handle reading from stdin when the user passes the path "-". If the user tries to compare a named pipe to a directory then we die as we do when trying to compare stdin to a directory. As process substitution is not support by POSIX this change is tested by using a pipe and a symbolic link to a pipe. Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Phillip Wood committed
Jul 5, 2023 at 20:49 UTC
1e3f26542a6ecd3006c2c0d5ccc0bae4a700f7e5
2 files changed
+124
-27
diff-no-index.c
+84
-27
@@ -41,41 +41,81 @@ static int read_directory_contents(const char *path, struct string_list *list)
41
*/
42
static const char file_from_standard_input[] = "-";
43
44
-static int get_mode(const char *path, int *mode)
44
+/*
45
+ * For paths given on the command-line we treat "-" as stdin and named
46
+ * pipes and symbolic links to named pipes specially.
47
+ */
48
+enum special {
49
+ SPECIAL_NONE,
50
+ SPECIAL_STDIN,
51
+ SPECIAL_PIPE,
52
+};
53
+
54
+static int get_mode(const char *path, int *mode, enum special *special)
55
{
56
struct stat st;
57
48
- if (!path || !strcmp(path, "/dev/null"))
58
+ if (!path || !strcmp(path, "/dev/null")) {
59
*mode = 0;
60
#ifdef GIT_WINDOWS_NATIVE
51
- else if (!strcasecmp(path, "nul"))
61
+ } else if (!strcasecmp(path, "nul")) {
62
*mode = 0;
63
#endif
54
- else if (path == file_from_standard_input)
64
+ } else if (path == file_from_standard_input) {
65
*mode = create_ce_mode(0666);
56
- else if (lstat(path, &st))
66
+ *special = SPECIAL_STDIN;
67
+ } else if (lstat(path, &st)) {
68
return error("Could not access '%s'", path);
58
- else
69
+ } else {
70
*mode = st.st_mode;
71
+ }
72
+ /*
73
+ * For paths on the command-line treat named pipes and symbolic
74
+ * links that resolve to a named pipe specially.
75
+ */
76
+ if (special &&
77
+ (S_ISFIFO(*mode) ||
78
+ (S_ISLNK(*mode) && !stat(path, &st) && S_ISFIFO(st.st_mode)))) {
79
+ *mode = create_ce_mode(0666);
80
+ *special = SPECIAL_PIPE;
81
+ }
82
+
83
return 0;
84
}
85
63
-static void populate_from_stdin(struct diff_filespec *s)
86
+static void populate_common(struct diff_filespec *s, struct strbuf *buf)
87
{
65
- struct strbuf buf = STRBUF_INIT;
88
size_t size = 0;
89
68
- if (strbuf_read(&buf, 0, 0) < 0)
69
- die_errno("error while reading from stdin");
70
-
90
s->should_munmap = 0;
72
- s->data = strbuf_detach(&buf, &size);
91
+ s->data = strbuf_detach(buf, &size);
92
s->size = size;
93
s->should_free = 1;
94
s->is_stdin = 1;
95
}
96
78
-static struct diff_filespec *noindex_filespec(const char *name, int mode)
97
+static void populate_from_pipe(struct diff_filespec *s)
98
+{
99
+ struct strbuf buf = STRBUF_INIT;
100
+ int fd = xopen(s->path, O_RDONLY);
101
+
102
+ if (strbuf_read(&buf, fd, 0) < 0)
103
+ die_errno("error while reading from '%s'", s->path);
104
+ close(fd);
105
+ populate_common(s, &buf);
106
+}
107
+
108
+static void populate_from_stdin(struct diff_filespec *s)
109
+{
110
+ struct strbuf buf = STRBUF_INIT;
111
+
112
+ if (strbuf_read(&buf, 0, 0) < 0)
113
+ die_errno("error while reading from stdin");
114
+ populate_common(s, &buf);
115
+}
116
+
117
+static struct diff_filespec *noindex_filespec(const char *name, int mode,
118
+ enum special special)
119
{
120
struct diff_filespec *s;
121
@@ -83,17 +123,22 @@ static struct diff_filespec *noindex_filespec(const char *name, int mode)
123
name = "/dev/null";
124
s = alloc_filespec(name);
125
fill_filespec(s, null_oid(), 0, mode);
86
- if (name == file_from_standard_input)
126
+ if (special == SPECIAL_STDIN)
127
populate_from_stdin(s);
128
+ else if (special == SPECIAL_PIPE)
129
+ populate_from_pipe(s);
130
return s;
131
}
132
133
static int queue_diff(struct diff_options *o,
92
- const char *name1, const char *name2)
134
+ const char *name1, const char *name2, int recursing)
135
{
136
int mode1 = 0, mode2 = 0;
137
+ enum special special1 = SPECIAL_NONE, special2 = SPECIAL_NONE;
138
96
- if (get_mode(name1, &mode1) || get_mode(name2, &mode2))
139
+ /* Paths can only be special if we're not recursing. */
140
+ if (get_mode(name1, &mode1, recursing ? NULL : &special1) ||
141
+ get_mode(name2, &mode2, recursing ? NULL : &special2))
142
return -1;
143
144
if (mode1 && mode2 && S_ISDIR(mode1) != S_ISDIR(mode2)) {
@@ -101,14 +146,14 @@ static int queue_diff(struct diff_options *o,
146
147
if (S_ISDIR(mode1)) {
148
/* 2 is file that is created */
104
- d1 = noindex_filespec(NULL, 0);
105
- d2 = noindex_filespec(name2, mode2);
149
+ d1 = noindex_filespec(NULL, 0, SPECIAL_NONE);
150
+ d2 = noindex_filespec(name2, mode2, special2);
151
name2 = NULL;
152
mode2 = 0;
153
} else {
154
/* 1 is file that is deleted */
110
- d1 = noindex_filespec(name1, mode1);
111
- d2 = noindex_filespec(NULL, 0);
155
+ d1 = noindex_filespec(name1, mode1, special1);
156
+ d2 = noindex_filespec(NULL, 0, SPECIAL_NONE);
157
name1 = NULL;
158
mode1 = 0;
159
}
@@ -173,7 +218,7 @@ static int queue_diff(struct diff_options *o,
218
n2 = buffer2.buf;
219
}
220
176
- ret = queue_diff(o, n1, n2);
221
+ ret = queue_diff(o, n1, n2, 1);
222
}
223
string_list_clear(&p1, 0);
224
string_list_clear(&p2, 0);
@@ -189,8 +234,8 @@ static int queue_diff(struct diff_options *o,
234
SWAP(name1, name2);
235
}
236
192
- d1 = noindex_filespec(name1, mode1);
193
- d2 = noindex_filespec(name2, mode2);
237
+ d1 = noindex_filespec(name1, mode1, special1);
238
+ d2 = noindex_filespec(name2, mode2, special2);
239
diff_queue(&diff_queued_diff, d1, d2);
240
return 0;
241
}
@@ -215,15 +260,27 @@ static void append_basename(struct strbuf *path, const char *dir, const char *fi
260
*/
261
static void fixup_paths(const char **path, struct strbuf *replacement)
262
{
218
- unsigned int isdir0, isdir1;
263
+ struct stat st;
264
+ unsigned int isdir0 = 0, isdir1 = 0;
265
+ unsigned int ispipe0 = 0, ispipe1 = 0;
266
220
- isdir0 = path[0] != file_from_standard_input && is_directory(path[0]);
221
- isdir1 = path[1] != file_from_standard_input && is_directory(path[1]);
267
+ if (path[0] != file_from_standard_input && !stat(path[0], &st)) {
268
+ isdir0 = S_ISDIR(st.st_mode);
269
+ ispipe0 = S_ISFIFO(st.st_mode);
270
+ }
271
+
272
+ if (path[1] != file_from_standard_input && !stat(path[1], &st)) {
273
+ isdir1 = S_ISDIR(st.st_mode);
274
+ ispipe1 = S_ISFIFO(st.st_mode);
275
+ }
276
277
if ((path[0] == file_from_standard_input && isdir1) ||
278
(isdir0 && path[1] == file_from_standard_input))
279
die(_("cannot compare stdin to a directory"));
280
281
+ if ((isdir0 && ispipe1) || (ispipe0 && isdir1))
282
+ die(_("cannot compare a named pipe to a directory"));
283
+
284
if (isdir0 == isdir1)
285
return;
286
if (isdir0) {
@@ -297,7 +354,7 @@ int diff_no_index(struct rev_info *revs,
354
setup_diff_pager(&revs->diffopt);
355
revs->diffopt.flags.exit_with_status = 1;
356
300
- if (queue_diff(&revs->diffopt, paths[0], paths[1]))
357
+ if (queue_diff(&revs->diffopt, paths[0], paths[1], 0))
358
goto out;
359
diff_set_mnemonic_prefix(&revs->diffopt, "1/", "2/");
360
diffcore_std(&revs->diffopt);
t/t4053-diff-no-index.sh
+40
@@ -229,4 +229,44 @@ test_expect_success 'diff --no-index refuses to diff stdin and a directory' '
229
grep "fatal: cannot compare stdin to a directory" err
230
'
231
232
+test_expect_success PIPE 'diff --no-index refuses to diff a named pipe and a directory' '
233
+ test_when_finished "rm -f pipe" &&
234
+ mkfifo pipe &&
235
+ {
236
+ (>pipe) &
237
+ } &&
238
+ test_when_finished "kill $!" &&
239
+ test_must_fail git diff --no-index -- pipe a 2>err &&
240
+ grep "fatal: cannot compare a named pipe to a directory" err
241
+'
242
+
243
+test_expect_success PIPE,SYMLINKS 'diff --no-index reads from pipes' '
244
+ test_when_finished "rm -f old new new-link" &&
245
+ mkfifo old &&
246
+ mkfifo new &&
247
+ ln -s new new-link &&
248
+ {
249
+ (test_write_lines a b c >old) &
250
+ } &&
251
+ test_when_finished "! kill $!" &&
252
+ {
253
+ (test_write_lines a x c >new) &
254
+ } &&
255
+ test_when_finished "! kill $!" &&
256
+
257
+ cat >expect <<-EOF &&
258
+ diff --git a/old b/new-link
259
+ --- a/old
260
+ +++ b/new-link
261
+ @@ -1,3 +1,3 @@
262
+ a
263
+ -b
264
+ +x
265
+ c
266
+ EOF
267
+
268
+ test_expect_code 1 git diff --no-index old new-link >actual &&
269
+ test_cmp expect actual
270
+'
271
+
272
test_done