From 1bf1a859ae980a73552c76fe20a5952a763ec81e Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Tue, 23 May 2006 19:08:01 -0700 Subject: [PATCH 1/5] apply: treat EOF as proper context. Catalin noticed that we do not treat end-of-file condition shown in the patch text as the patch context. This causes a patch that appends at the end of the file to cleanly apply even if something else has been appended to the file. If this happened in the middle, we would refuse by saying that the file has conflicting modifications. Signed-off-by: Linus Torvalds Signed-off-by: Junio C Hamano --- apply.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/apply.c b/apply.c index 0ed9d132e8..768b572a81 100644 --- a/apply.c +++ b/apply.c @@ -1333,6 +1333,7 @@ static int apply_line(char *output, const char *patch, int plen) static int apply_one_fragment(struct buffer_desc *desc, struct fragment *frag) { + int match_end; char *buf = desc->buffer; const char *patch = frag->patch; int offset, size = frag->size; @@ -1395,10 +1396,19 @@ static int apply_one_fragment(struct buffer_desc *desc, struct fragment *frag) newlines = new; leading = frag->leading; trailing = frag->trailing; + + /* + * If we don't have any trailing data in the patch, + * we want it to match at the end of the file. + */ + match_end = !trailing; + lines = 0; pos = frag->newpos; for (;;) { offset = find_offset(buf, desc->size, oldlines, oldsize, pos, &lines); + if (match_end && offset + oldsize != desc->size) + offset = -1; if (offset >= 0) { int diff = newsize - oldsize; unsigned long size = desc->size + diff; @@ -1428,6 +1438,10 @@ static int apply_one_fragment(struct buffer_desc *desc, struct fragment *frag) /* Am I at my context limits? */ if ((leading <= p_context) && (trailing <= p_context)) break; + if (match_end) { + match_end = 0; + continue; + } /* Reduce the number of context lines * Reduce both leading and trailing if they are equal * otherwise just reduce the larger context. From cc189c2ca2c725c430f100f61e7c4a6849f93163 Mon Sep 17 00:00:00 2001 From: Catalin Marinas Date: Tue, 23 May 2006 22:48:36 +0100 Subject: [PATCH 2/5] Add a test-case for git-apply trying to add an ending line git-apply adding an ending line doesn't seem to fail if the ending line is already present in the patched file. Signed-off-by: Catalin Marinas Signed-off-by: Junio C Hamano --- t/t4113-apply-ending.sh | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100755 t/t4113-apply-ending.sh diff --git a/t/t4113-apply-ending.sh b/t/t4113-apply-ending.sh new file mode 100755 index 0000000000..d021ae84c3 --- /dev/null +++ b/t/t4113-apply-ending.sh @@ -0,0 +1,35 @@ +#!/bin/sh +# +# Copyright (c) 2006 Catalin Marinas +# + +test_description='git-apply trying to add an ending line. + +' +. ./test-lib.sh + +# setup + +cat >test-patch <<\EOF +diff --git a/file b/file +--- a/file ++++ b/file +@@ -1,2 +1,3 @@ + a + b ++c +EOF + +echo 'a' >file +echo 'b' >>file +echo 'c' >>file + +test_expect_success setup \ + 'git-update-index --add file' + +# test + +test_expect_failure apply \ + 'git-apply --index test-patch' + +test_done From 65aadb92a1ce9605fa2f412b51de91781a3ef3d6 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 24 May 2006 13:19:50 -0700 Subject: [PATCH 3/5] apply: force matching at the beginning. When there is no leading context, the patch must match at the beginning of preimage; otherwise there is a "patch adds these lines while the other lines were added to the original file" conflict. This is the opposite of match_end fix earlier in this series. Unlike matching at the end case, we can additionally check the preimage line number recorded in the patch, so the change is not symmetrical with the earlier one. Signed-off-by: Junio C Hamano --- apply.c | 13 ++++++++----- t/t4113-apply-ending.sh | 20 +++++++++++++++++++- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/apply.c b/apply.c index 768b572a81..f11fb18b69 100644 --- a/apply.c +++ b/apply.c @@ -1333,7 +1333,7 @@ static int apply_line(char *output, const char *patch, int plen) static int apply_one_fragment(struct buffer_desc *desc, struct fragment *frag) { - int match_end; + int match_beginning, match_end; char *buf = desc->buffer; const char *patch = frag->patch; int offset, size = frag->size; @@ -1398,9 +1398,10 @@ static int apply_one_fragment(struct buffer_desc *desc, struct fragment *frag) trailing = frag->trailing; /* - * If we don't have any trailing data in the patch, - * we want it to match at the end of the file. + * If we don't have any leading/trailing data in the patch, + * we want it to match at the beginning/end of the file. */ + match_beginning = !leading && (frag->oldpos == 1); match_end = !trailing; lines = 0; @@ -1409,6 +1410,8 @@ static int apply_one_fragment(struct buffer_desc *desc, struct fragment *frag) offset = find_offset(buf, desc->size, oldlines, oldsize, pos, &lines); if (match_end && offset + oldsize != desc->size) offset = -1; + if (match_beginning && offset) + offset = -1; if (offset >= 0) { int diff = newsize - oldsize; unsigned long size = desc->size + diff; @@ -1438,8 +1441,8 @@ static int apply_one_fragment(struct buffer_desc *desc, struct fragment *frag) /* Am I at my context limits? */ if ((leading <= p_context) && (trailing <= p_context)) break; - if (match_end) { - match_end = 0; + if (match_beginning || match_end) { + match_beginning = match_end = 0; continue; } /* Reduce the number of context lines diff --git a/t/t4113-apply-ending.sh b/t/t4113-apply-ending.sh index d021ae84c3..7fd0cf62ec 100755 --- a/t/t4113-apply-ending.sh +++ b/t/t4113-apply-ending.sh @@ -29,7 +29,25 @@ test_expect_success setup \ # test -test_expect_failure apply \ +test_expect_failure 'apply at the end' \ 'git-apply --index test-patch' +cat >test-patch <<\EOF +diff a/file b/file +--- a/file ++++ b/file +@@ -1,2 +1,3 @@ ++a + b + c +EOF + +echo >file 'a +b +c' +git-update-index file + +test_expect_failure 'apply at the beginning' \ + 'git-apply --index test-patch' + test_done From f81daefe56b3c97b93a851e1ada14eeca0dea47a Mon Sep 17 00:00:00 2001 From: Timo Hirvonen Date: Wed, 24 May 2006 14:08:46 +0300 Subject: [PATCH 4/5] Builtin git-cat-file Signed-off-by: Timo Hirvonen Signed-off-by: Junio C Hamano --- Makefile | 6 +++--- cat-file.c => builtin-cat-file.c | 3 ++- builtin.h | 1 + git.c | 3 ++- 4 files changed, 8 insertions(+), 5 deletions(-) rename cat-file.c => builtin-cat-file.c (97%) diff --git a/Makefile b/Makefile index 7e6517f62b..dbf19c6277 100644 --- a/Makefile +++ b/Makefile @@ -149,7 +149,6 @@ SIMPLE_PROGRAMS = \ # ... and all the rest that could be moved out of bindir to gitexecdir PROGRAMS = \ - git-cat-file$X \ git-checkout-index$X git-clone-pack$X \ git-convert-objects$X git-fetch-pack$X git-fsck-objects$X \ git-hash-object$X git-index-pack$X git-local-fetch$X \ @@ -174,7 +173,7 @@ BUILT_INS = git-log$X git-whatchanged$X git-show$X \ git-ls-files$X git-ls-tree$X \ git-read-tree$X git-commit-tree$X \ git-apply$X git-show-branch$X git-diff-files$X \ - git-diff-index$X git-diff-stages$X git-diff-tree$X + git-diff-index$X git-diff-stages$X git-diff-tree$X git-cat-file$X # what 'all' will build and 'install' will install, in gitexecdir ALL_PROGRAMS = $(PROGRAMS) $(SIMPLE_PROGRAMS) $(SCRIPTS) @@ -228,7 +227,8 @@ BUILTIN_OBJS = \ builtin-ls-files.o builtin-ls-tree.o \ builtin-read-tree.o builtin-commit-tree.o \ builtin-apply.o builtin-show-branch.o builtin-diff-files.o \ - builtin-diff-index.o builtin-diff-stages.o builtin-diff-tree.o + builtin-diff-index.o builtin-diff-stages.o builtin-diff-tree.o \ + builtin-cat-file.o GITLIBS = $(LIB_FILE) $(XDIFF_LIB) LIBS = $(GITLIBS) -lz diff --git a/cat-file.c b/builtin-cat-file.c similarity index 97% rename from cat-file.c rename to builtin-cat-file.c index 7413feed78..8ab136e981 100644 --- a/cat-file.c +++ b/builtin-cat-file.c @@ -7,6 +7,7 @@ #include "exec_cmd.h" #include "tag.h" #include "tree.h" +#include "builtin.h" static void flush_buffer(const char *buf, unsigned long size) { @@ -93,7 +94,7 @@ static int pprint_tag(const unsigned char *sha1, const char *buf, unsigned long return 0; } -int main(int argc, char **argv) +int cmd_cat_file(int argc, const char **argv, char **envp) { unsigned char sha1[20]; char type[20]; diff --git a/builtin.h b/builtin.h index 714b97578c..738ec3d945 100644 --- a/builtin.h +++ b/builtin.h @@ -42,5 +42,6 @@ extern int cmd_diff_files(int argc, const char **argv, char **envp); extern int cmd_diff_index(int argc, const char **argv, char **envp); extern int cmd_diff_stages(int argc, const char **argv, char **envp); extern int cmd_diff_tree(int argc, const char **argv, char **envp); +extern int cmd_cat_file(int argc, const char **argv, char **envp); #endif diff --git a/git.c b/git.c index 5a884bb07a..10ea934bcf 100644 --- a/git.c +++ b/git.c @@ -68,7 +68,8 @@ static void handle_internal_command(int argc, const char **argv, char **envp) { "diff-files", cmd_diff_files }, { "diff-index", cmd_diff_index }, { "diff-stages", cmd_diff_stages }, - { "diff-tree", cmd_diff_tree } + { "diff-tree", cmd_diff_tree }, + { "cat-file", cmd_cat_file } }; int i; From 4d548150ace0816dd5fe678cdbde75b13d5e5249 Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Wed, 24 May 2006 08:30:54 -0700 Subject: [PATCH 5/5] Clean up sha1 file writing This cleans up and future-proofs the sha1 file writing in sha1_file.c. In particular, instead of doing a simple "write()" call and just verifying that it succeeds (or - as in one place - just assuming it does), it uses "write_buffer()" to write data to the file descriptor while correctly checking for partial writes, EINTR etc. It also splits up write_sha1_to_fd() to be a lot more readable: if we need to re-create the compressed object, we do so in a separate helper function, making the logic a whole lot more modular and obvious. Signed-off-by: Linus Torvalds Signed-off-by: Junio C Hamano --- sha1_file.c | 145 +++++++++++++++++++++++++++++----------------------- 1 file changed, 81 insertions(+), 64 deletions(-) diff --git a/sha1_file.c b/sha1_file.c index e444d9df1b..f77c18934a 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -1399,6 +1399,25 @@ int move_temp_to_file(const char *tmpfile, char *filename) return 0; } +static int write_buffer(int fd, const void *buf, size_t len) +{ + while (len) { + ssize_t size; + + size = write(fd, buf, len); + if (!size) + return error("file write: disk full"); + if (size < 0) { + if (errno == EINTR || errno == EAGAIN) + continue; + return error("file write error (%s)", strerror(errno)); + } + len -= size; + buf += size; + } + return 0; +} + int write_sha1_file(void *buf, unsigned long len, const char *type, unsigned char *returnsha1) { int size; @@ -1465,8 +1484,8 @@ int write_sha1_file(void *buf, unsigned long len, const char *type, unsigned cha deflateEnd(&stream); size = stream.total_out; - if (write(fd, compressed, size) != size) - die("unable to write file"); + if (write_buffer(fd, compressed, size) < 0) + die("unable to write sha1 file"); fchmod(fd, 0444); close(fd); free(compressed); @@ -1474,73 +1493,70 @@ int write_sha1_file(void *buf, unsigned long len, const char *type, unsigned cha return move_temp_to_file(tmpfile, filename); } +/* + * We need to unpack and recompress the object for writing + * it out to a different file. + */ +static void *repack_object(const unsigned char *sha1, unsigned long *objsize) +{ + size_t size; + z_stream stream; + unsigned char *unpacked; + unsigned long len; + char type[20]; + char hdr[50]; + int hdrlen; + void *buf; + + // need to unpack and recompress it by itself + unpacked = read_packed_sha1(sha1, type, &len); + + hdrlen = sprintf(hdr, "%s %lu", type, len) + 1; + + /* Set it up */ + memset(&stream, 0, sizeof(stream)); + deflateInit(&stream, Z_BEST_COMPRESSION); + size = deflateBound(&stream, len + hdrlen); + buf = xmalloc(size); + + /* Compress it */ + stream.next_out = buf; + stream.avail_out = size; + + /* First header.. */ + stream.next_in = (void *)hdr; + stream.avail_in = hdrlen; + while (deflate(&stream, 0) == Z_OK) + /* nothing */; + + /* Then the data itself.. */ + stream.next_in = unpacked; + stream.avail_in = len; + while (deflate(&stream, Z_FINISH) == Z_OK) + /* nothing */; + deflateEnd(&stream); + free(unpacked); + + *objsize = stream.total_out; + return buf; +} + int write_sha1_to_fd(int fd, const unsigned char *sha1) { - ssize_t size; + int retval; unsigned long objsize; - int posn = 0; - void *map = map_sha1_file_internal(sha1, &objsize); - void *buf = map; - void *temp_obj = NULL; - z_stream stream; + void *buf = map_sha1_file_internal(sha1, &objsize); - if (!buf) { - unsigned char *unpacked; - unsigned long len; - char type[20]; - char hdr[50]; - int hdrlen; - // need to unpack and recompress it by itself - unpacked = read_packed_sha1(sha1, type, &len); - - hdrlen = sprintf(hdr, "%s %lu", type, len) + 1; - - /* Set it up */ - memset(&stream, 0, sizeof(stream)); - deflateInit(&stream, Z_BEST_COMPRESSION); - size = deflateBound(&stream, len + hdrlen); - temp_obj = buf = xmalloc(size); - - /* Compress it */ - stream.next_out = buf; - stream.avail_out = size; - - /* First header.. */ - stream.next_in = (void *)hdr; - stream.avail_in = hdrlen; - while (deflate(&stream, 0) == Z_OK) - /* nothing */; - - /* Then the data itself.. */ - stream.next_in = unpacked; - stream.avail_in = len; - while (deflate(&stream, Z_FINISH) == Z_OK) - /* nothing */; - deflateEnd(&stream); - free(unpacked); - - objsize = stream.total_out; + if (buf) { + retval = write_buffer(fd, buf, objsize); + munmap(buf, objsize); + return retval; } - do { - size = write(fd, buf + posn, objsize - posn); - if (size <= 0) { - if (!size) { - fprintf(stderr, "write closed\n"); - } else { - perror("write "); - } - return -1; - } - posn += size; - } while (posn < objsize); - - if (map) - munmap(map, objsize); - if (temp_obj) - free(temp_obj); - - return 0; + buf = repack_object(sha1, &objsize); + retval = write_buffer(fd, buf, objsize); + free(buf); + return retval; } int write_sha1_from_fd(const unsigned char *sha1, int fd, char *buffer, @@ -1579,7 +1595,8 @@ int write_sha1_from_fd(const unsigned char *sha1, int fd, char *buffer, SHA1_Update(&c, discard, sizeof(discard) - stream.avail_out); } while (stream.avail_in && ret == Z_OK); - write(local, buffer, *bufposn - stream.avail_in); + if (write_buffer(local, buffer, *bufposn - stream.avail_in) < 0) + die("unable to write sha1 file"); memmove(buffer, buffer + *bufposn - stream.avail_in, stream.avail_in); *bufposn = stream.avail_in;