Skip to content

Commit

Permalink
Merge branch 'jc/ignore-epipe-in-filter'
Browse files Browse the repository at this point in the history
Filter scripts were run with SIGPIPE disabled on the Git side,
expecting that they may not read what Git feeds them to filter.
We however treated a filter that does not read its input fully
before exiting as an error.

This changes semantics, but arguably in a good way.  If a filter
can produce its output without consuming its input using whatever
magic, we now let it do so, instead of diagnosing it as a
programming error.

* jc/ignore-epipe-in-filter:
  filter_buffer_or_fd(): ignore EPIPE
  copy.c: make copy_fd() report its status silently
  • Loading branch information
Junio C Hamano committed May 22, 2015
2 parents 5bf6668 + 0c4dd67 commit ddaf4e2
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 8 deletions.
4 changes: 4 additions & 0 deletions cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -1541,9 +1541,13 @@ extern const char *git_mailmap_blob;
extern void maybe_flush_or_die(FILE *, const char *);
__attribute__((format (printf, 2, 3)))
extern void fprintf_or_die(FILE *, const char *fmt, ...);

#define COPY_READ_ERROR (-2)
#define COPY_WRITE_ERROR (-3)
extern int copy_fd(int ifd, int ofd);
extern int copy_file(const char *dst, const char *src, int mode);
extern int copy_file_with_time(const char *dst, const char *src, int mode);

extern void write_or_die(int fd, const void *buf, size_t count);
extern int write_or_whine(int fd, const void *buf, size_t count, const char *msg);
extern int write_or_whine_pipe(int fd, const void *buf, size_t count, const char *msg);
Expand Down
7 changes: 6 additions & 1 deletion convert.c
Original file line number Diff line number Diff line change
Expand Up @@ -356,9 +356,14 @@ static int filter_buffer_or_fd(int in, int out, void *data)
sigchain_push(SIGPIPE, SIG_IGN);

if (params->src) {
write_err = (write_in_full(child_process.in, params->src, params->size) < 0);
write_err = (write_in_full(child_process.in,
params->src, params->size) < 0);
if (errno == EPIPE)
write_err = 0;
} else {
write_err = copy_fd(params->fd, child_process.in);
if (write_err == COPY_WRITE_ERROR && errno == EPIPE)
write_err = 0;
}

if (close(child_process.in))
Expand Down
17 changes: 11 additions & 6 deletions copy.c
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,10 @@ int copy_fd(int ifd, int ofd)
ssize_t len = xread(ifd, buffer, sizeof(buffer));
if (!len)
break;
if (len < 0) {
return error("copy-fd: read returned %s",
strerror(errno));
}
if (len < 0)
return COPY_READ_ERROR;
if (write_in_full(ofd, buffer, len) < 0)
return error("copy-fd: write returned %s",
strerror(errno));
return COPY_WRITE_ERROR;
}
return 0;
}
Expand Down Expand Up @@ -43,6 +40,14 @@ int copy_file(const char *dst, const char *src, int mode)
return fdo;
}
status = copy_fd(fdi, fdo);
switch (status) {
case COPY_READ_ERROR:
error("copy-fd: read returned %s", strerror(errno));
break;
case COPY_WRITE_ERROR:
error("copy-fd: write returned %s", strerror(errno));
break;
}
close(fdi);
if (close(fdo) != 0)
return error("%s: close error: %s", dst, strerror(errno));
Expand Down
2 changes: 1 addition & 1 deletion lockfile.c
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ int hold_lock_file_for_append(struct lock_file *lk, const char *path, int flags)
int save_errno = errno;

if (flags & LOCK_DIE_ON_ERROR)
exit(128);
die("failed to prepare '%s' for appending", path);
close(orig_fd);
rollback_lock_file(lk);
errno = save_errno;
Expand Down
10 changes: 10 additions & 0 deletions t/t0021-conversion.sh
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,16 @@ test_expect_success 'filtering large input to small output should use little mem
GIT_MMAP_LIMIT=1m GIT_ALLOC_LIMIT=1m git add 30MB
'

test_expect_success 'filter that does not read is fine' '
test-genrandom foo $((128 * 1024 + 1)) >big &&
echo "big filter=epipe" >.gitattributes &&
git config filter.epipe.clean "echo xyzzy" &&
git add big &&
git cat-file blob :big >actual &&
echo xyzzy >expect &&
test_cmp expect actual
'

test_expect_success EXPENSIVE 'filter large file' '
git config filter.largefile.smudge cat &&
git config filter.largefile.clean cat &&
Expand Down

0 comments on commit ddaf4e2

Please sign in to comment.