{"thread":{"id":"42726","subject":"[PATCH v8 00/41] libify apply and use lib in am, part 2","startedAt":"2016-06-27T18:24:57Z","lastAt":"2016-07-30T17:39:13Z","messageCount":59,"participants":["Christian Couder","Junio C Hamano","Duy Nguyen"],"isPatch":true,"patchVersion":8,"patchTotal":41},"messages":[{"id":"290268","messageId":"20160627182429.31550-1-chriscool@tuxfamily.org","threadId":"42726","inReplyTo":null,"subject":"[PATCH v8 00/41] libify apply and use lib in am, part 2","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-27T18:23:48Z","receivedAt":"2016-06-27T18:24:57Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Goal\n~~~~\n\nThis is a patch series about libifying `git apply` functionality, and\nusing this libified functionality in `git am`, so that no 'git apply'\nprocess is spawn anymore. This makes `git am` significantly faster, so\n`git rebase`, when it uses the am backend, is also significantly\nfaster.\n\nPrevious discussions and patches series\n~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n\nThis has initially been discussed in the following thread:\n\n  http://thread.gmane.org/gmane.comp.version-control.git/287236/\n\nThen the following patch series were sent:\n\nRFC: http://thread.gmane.org/gmane.comp.version-control.git/288489/\nv1: http://thread.gmane.org/gmane.comp.version-control.git/292324/\nv2: http://thread.gmane.org/gmane.comp.version-control.git/294248/\nv3: http://thread.gmane.org/gmane.comp.version-control.git/295429/\nv4: http://thread.gmane.org/gmane.comp.version-control.git/296350/\nv5: http://thread.gmane.org/gmane.comp.version-control.git/296490/\nv6: http://thread.gmane.org/gmane.comp.version-control.git/297024/\nv7: http://thread.gmane.org/gmane.comp.version-control.git/297193/\n\nHighlevel view of the patches in the series\n~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n\nThis new patch series is built on top of the above previous work.\n\nMore precisely, this is \"part 2\" of the full patch series which is\nbuilt on top of the \"part 1\" of the full patch series. And as the\n\"part 1\" is now in \"next\", this \"part 2\" is built on top of \"next\".\n\n  - Patches 01/41 is new in v8.\n\nIt renames some structs and constants that will be moved into apply.h\nto give them a more specific name as suggested by Junio.\n\n  - Patches 02/41 to 32/41 were in v1, v2, v6 and v7.\n\nThey finish libifying the apply functionality that was in\nbuiltin/apply.c and move it into apply.{c,h}, but the libified\nfunctionality is not yet used in `git am`.\n\nThere are a number of changes in these patches compared to v7, so that\napply_all_patches() will return 1 in case some patches don't apply and\n128 in case a previously fatal error happened. This makes it possible\nfor `git apply` to return the same exit code as it used to return\nbefore libification.\n\nTo do that a number of functions like find_header(), parse_chunk(),\napply_patch(), etc have been made to return -128 instead of -1 when it\nwas necessary.\n\n  - Patches 33/41 to 39/41 were in v2, v6 and v7.\n\nThey implement a way to make the libified apply silent by adding a new\n'be_silent' flag into 'struct apply_state'. It is a new feature in the\nlibified apply functionality.\n\nThis could be in a separate series, but unfortunately using the\nlibified apply in \"git am\" unmasks the fact that \"git am\", since it\nwas a shell script, has been silencing the apply functionality by\nredirecting file descriptors to /dev/null and it looks like this is\nnot acceptable in C.\n\nThere are no changes in these patches compared to v7.\n\n  - Patch 40/41 was in v1, v2, v6 and v7.\n\nThis patch makes `git am` use the libified functionality. It is the\nsame as in v7.\n\n  - Patch 41/41 was in v6 and v7.\n\nIt replaces some calls to error() with calls to error_errno(). It is\nthe nearly the same as in v7. The only change is that one call to\nerror() is now replaced by error_errno() earlier in the series.\n\nGeneral comments\n~~~~~~~~~~~~~~~~\n\nSorry if this patch series is still long. I can split it into two or\nmore series if it is prefered.\n\nI will send a diff between this version and the previous one, as a\nreply to this email.\n\nThe benefits are not just related to not creating new processes. When\n`git am` launched a `git apply` process, this new process had to read\nthe index from disk. Then after the `git apply`process had terminated,\n`git am` dropped its index and read the index from disk to get the\nindex that had been modified by the `git apply`process. This was\ninefficient and also prevented the split-index mechanism to provide\nmany performance benefits.\n\nUsing this series as rebase material, Duy explains it like this:\n\n > Without the series, the picture is not so surprising. We run git-apply\n > 80+ times, each consists of this sequence\n >\n > read index\n > write index (cache tree updates only)\n > read index again\n > optionally initialize name hash (when new entries are added, I guess)\n > read packed-refs\n > write index\n >\n > With this series, we run a single git-apply which does\n >\n > read index (and sharedindex too if in split-index mode)\n > initialize name hash\n > write index 80+ times\n\n(See: http://thread.gmane.org/gmane.comp.version-control.git/292324/focus=292460)\n\nLinks\n~~~~~\n\nThis patch series is available here:\n\nhttps://github.com/chriscool/git/commits/libify-apply-use-in-am\n\nThe previous versions are available there:\n\nv1: https://github.com/chriscool/git/commits/libify-apply-use-in-am25 \nv2: https://github.com/chriscool/git/commits/libify-apply-use-in-am54\nv6: https://github.com/chriscool/git/commits/libify-apply-use-in-am65\nv7: https://github.com/chriscool/git/commits/libify-apply-use-in-am75\n\nPerformance numbers\n~~~~~~~~~~~~~~~~~~~\n\nOnly tests on Linux have been performed using a very early version of\nthis series. It could be interesting to test on other platforms\nespecially Windows and perhaps OSX too.\n\n  - Around mid April Ævar did a huge many-hundred commit rebase on the\n    kernel with untracked cache.\n\ncommand: git rebase --onto 1993b17 52bef0c 29dde7c\n\nVanilla \"next\" without split index:                1m54.953s\nVanilla \"next\" with split index:                   1m22.476s\nThis series on top of \"next\" without split index:  1m12.034s\nThis series on top of \"next\" with split index:     0m15.678s\n\nÆvar used his Debian laptop with SSD.\n\n  - Around mid April I tested rebasing 13 commits in Booking.com's\n    monorepo on a Red Hat 6.5 server with split-index and\n    GIT_TRACE_PERFORMANCE=1.\n\nWith Git v2.8.0, the rebase took 6.375888383 s, with the git am\ncommand launched by the rebase command taking 3.705677431 s.\n\nWith this series on top of next, the rebase took 3.044529494 s, with\nthe git am command launched by the rebase command taking 0.583521168\ns.\n\nChristian Couder (41):\n  apply: make some names more specific\n  apply: move 'struct apply_state' to apply.h\n  builtin/apply: make apply_patch() return -1 or -128 instead of\n    die()ing\n  builtin/apply: read_patch_file() return -1 instead of die()ing\n  builtin/apply: make find_header() return -128 instead of die()ing\n  builtin/apply: make parse_chunk() return a negative integer on error\n  builtin/apply: make parse_single_patch() return -1 on error\n  builtin/apply: make parse_whitespace_option() return -1 instead of\n    die()ing\n  builtin/apply: make parse_ignorewhitespace_option() return -1 instead\n    of die()ing\n  builtin/apply: move init_apply_state() to apply.c\n  apply: make init_apply_state() return -1 instead of exit()ing\n  builtin/apply: make check_apply_state() return -1 instead of die()ing\n  builtin/apply: move check_apply_state() to apply.c\n  builtin/apply: make apply_all_patches() return 128 or 1 on error\n  builtin/apply: make parse_traditional_patch() return -1 on error\n  builtin/apply: make gitdiff_*() return 1 at end of header\n  builtin/apply: make gitdiff_*() return -1 on error\n  builtin/apply: change die_on_unsafe_path() to check_unsafe_path()\n  builtin/apply: make build_fake_ancestor() return -1 on error\n  builtin/apply: make remove_file() return -1 on error\n  builtin/apply: make add_conflicted_stages_file() return -1 on error\n  builtin/apply: make add_index_file() return -1 on error\n  builtin/apply: make create_file() return -1 on error\n  builtin/apply: make write_out_one_result() return -1 on error\n  builtin/apply: make write_out_results() return -1 on error\n  builtin/apply: make try_create_file() return -1 on error\n  builtin/apply: make create_one_file() return -1 on error\n  builtin/apply: rename option parsing functions\n  apply: rename and move opt constants to apply.h\n  Move libified code from builtin/apply.c to apply.{c,h}\n  apply: make some parsing functions static again\n  environment: add set_index_file()\n  write_or_die: use warning() instead of fprintf(stderr, ...)\n  apply: add 'be_silent' variable to 'struct apply_state'\n  apply: make 'be_silent' incompatible with 'apply_verbosely'\n  apply: don't print on stdout when be_silent is set\n  usage: add set_warn_routine()\n  usage: add get_error_routine() and get_warn_routine()\n  apply: change error_routine when be_silent is set\n  builtin/am: use apply api in run_apply()\n  apply: use error_errno() where possible\n\n Makefile               |    1 +\n apply.c                | 4891 ++++++++++++++++++++++++++++++++++++++++++++++++\n apply.h                |  133 ++\n builtin/am.c           |   91 +-\n builtin/apply.c        | 4803 +----------------------------------------------\n cache.h                |    1 +\n environment.c          |   10 +\n git-compat-util.h      |    3 +\n t/t4012-diff-binary.sh |    4 +-\n t/t4254-am-corrupt.sh  |    2 +-\n usage.c                |   15 +\n write_or_die.c         |    6 +-\n 12 files changed, 5147 insertions(+), 4813 deletions(-)\n create mode 100644 apply.c\n create mode 100644 apply.h\n\n-- \n2.9.0.172.gfb57a78\n\n"},{"id":"290269","messageId":"20160627182429.31550-4-chriscool@tuxfamily.org","threadId":"42726","inReplyTo":"20160627182429.31550-1-chriscool@tuxfamily.org","subject":"[PATCH v8 03/41] builtin/apply: make apply_patch() return -1 or -128 instead of die()ing","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-27T18:23:51Z","receivedAt":"2016-06-27T18:25:01Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"To libify `git apply` functionality we have to signal errors\nto the caller instead of die()ing.\n\nAs a first step in this direction, let's make apply_patch() return\n-1 or -128 in case of errors instead of dying. For now its only\ncaller apply_all_patches() will exit(128) when apply_patch()\nreturn -128 and it will exit(1) when it returns -1.\n\nWe exit() with code 128 because that was what die() was doing\nand we want to keep the distinction between exiting with code 1\nand exiting with code 128.\n\nHelped-by: Eric Sunshine <sunshine@sunshineco.com>\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin/apply.c | 60 ++++++++++++++++++++++++++++++++++++++++++---------------\n 1 file changed, 45 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex ad216d3..2aea203 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -4404,6 +4404,15 @@ static struct lock_file lock_file;\n #define INACCURATE_EOF\t(1<<0)\n #define RECOUNT\t\t(1<<1)\n \n+/*\n+ * Try to apply a patch.\n+ *\n+ * Returns:\n+ *  -128 if a bad error happened (like patch unreadable)\n+ *  -1 if patch did not apply and user cannot deal with it\n+ *   0 if the patch applied\n+ *   1 if the patch did not apply but user might fix it\n+ */\n static int apply_patch(struct apply_state *state,\n \t\t       int fd,\n \t\t       const char *filename,\n@@ -4413,6 +4422,7 @@ static int apply_patch(struct apply_state *state,\n \tstruct strbuf buf = STRBUF_INIT; /* owns the patch text */\n \tstruct patch *list = NULL, **listp = &list;\n \tint skipped_patch = 0;\n+\tint res = 0;\n \n \tstate->patch_input_file = filename;\n \tread_patch_file(&buf, fd);\n@@ -4445,8 +4455,11 @@ static int apply_patch(struct apply_state *state,\n \t\toffset += nr;\n \t}\n \n-\tif (!list && !skipped_patch)\n-\t\tdie(_(\"unrecognized input\"));\n+\tif (!list && !skipped_patch) {\n+\t\terror(_(\"unrecognized input\"));\n+\t\tres = -128;\n+\t\tgoto end;\n+\t}\n \n \tif (state->whitespace_error && (state->ws_error_action == die_on_ws_error))\n \t\tstate->apply = 0;\n@@ -4455,21 +4468,23 @@ static int apply_patch(struct apply_state *state,\n \tif (state->update_index && state->newfd < 0)\n \t\tstate->newfd = hold_locked_index(state->lock_file, 1);\n \n-\tif (state->check_index) {\n-\t\tif (read_cache() < 0)\n-\t\t\tdie(_(\"unable to read index file\"));\n+\tif (state->check_index && read_cache() < 0) {\n+\t\terror(_(\"unable to read index file\"));\n+\t\tres = -128;\n+\t\tgoto end;\n \t}\n \n \tif ((state->check || state->apply) &&\n \t    check_patch_list(state, list) < 0 &&\n-\t    !state->apply_with_reject)\n-\t\texit(1);\n+\t    !state->apply_with_reject) {\n+\t\tres = -1;\n+\t\tgoto end;\n+\t}\n \n \tif (state->apply && write_out_results(state, list)) {\n-\t\tif (state->apply_with_reject)\n-\t\t\texit(1);\n \t\t/* with --3way, we still need to write the index out */\n-\t\treturn 1;\n+\t\tres = state->apply_with_reject ? -1 : 1;\n+\t\tgoto end;\n \t}\n \n \tif (state->fake_ancestor)\n@@ -4484,10 +4499,11 @@ static int apply_patch(struct apply_state *state,\n \tif (state->summary)\n \t\tsummary_patch_list(list);\n \n+end:\n \tfree_patch_list(list);\n \tstrbuf_release(&buf);\n \tstring_list_clear(&state->fn_table, 0);\n-\treturn 0;\n+\treturn res;\n }\n \n static void git_apply_config(void)\n@@ -4625,6 +4641,7 @@ static int apply_all_patches(struct apply_state *state,\n \t\t\t     int options)\n {\n \tint i;\n+\tint res;\n \tint errs = 0;\n \tint read_stdin = 1;\n \n@@ -4633,7 +4650,10 @@ static int apply_all_patches(struct apply_state *state,\n \t\tint fd;\n \n \t\tif (!strcmp(arg, \"-\")) {\n-\t\t\terrs |= apply_patch(state, 0, \"<stdin>\", options);\n+\t\t\tres = apply_patch(state, 0, \"<stdin>\", options);\n+\t\t\tif (res < 0)\n+\t\t\t\tgoto end;\n+\t\t\terrs |= res;\n \t\t\tread_stdin = 0;\n \t\t\tcontinue;\n \t\t} else if (0 < state->prefix_length)\n@@ -4646,12 +4666,19 @@ static int apply_all_patches(struct apply_state *state,\n \t\t\tdie_errno(_(\"can't open patch '%s'\"), arg);\n \t\tread_stdin = 0;\n \t\tset_default_whitespace_mode(state);\n-\t\terrs |= apply_patch(state, fd, arg, options);\n+\t\tres = apply_patch(state, fd, arg, options);\n+\t\tif (res < 0)\n+\t\t\tgoto end;\n+\t\terrs |= res;\n \t\tclose(fd);\n \t}\n \tset_default_whitespace_mode(state);\n-\tif (read_stdin)\n-\t\terrs |= apply_patch(state, 0, \"<stdin>\", options);\n+\tif (read_stdin) {\n+\t\tres = apply_patch(state, 0, \"<stdin>\", options);\n+\t\tif (res < 0)\n+\t\t\tgoto end;\n+\t\terrs |= res;\n+\t}\n \n \tif (state->whitespace_error) {\n \t\tif (state->squelch_whitespace_errors &&\n@@ -4687,6 +4714,9 @@ static int apply_all_patches(struct apply_state *state,\n \t}\n \n \treturn !!errs;\n+\n+end:\n+\texit(res == -1 ? 1 : 128);\n }\n \n int cmd_apply(int argc, const char **argv, const char *prefix)\n-- \n2.9.0.172.gfb57a78\n\n"},{"id":"290270","messageId":"20160627182429.31550-7-chriscool@tuxfamily.org","threadId":"42726","inReplyTo":"20160627182429.31550-1-chriscool@tuxfamily.org","subject":"[PATCH v8 06/41] builtin/apply: make parse_chunk() return a negative integer on error","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-27T18:23:54Z","receivedAt":"2016-06-27T18:25:08Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"To libify `git apply` functionality we have to signal errors to the\ncaller instead of die()ing or exit()ing.\n\nTo do that in a compatible manner with the rest of the error handling\nin builtin/apply.c, parse_chunk() should return a negative integer\ninstead of calling die() or exit().\n\nAs parse_chunk() is called only by apply_patch() which already\nreturns either -1 or -128 when an error happened, let's make it also\nreturn -1 or -128.\n\nThis makes it compatible with what find_header() and parse_binary()\nalready return.\n\nHelped-by: Eric Sunshine <sunshine@sunshineco.com>\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin/apply.c | 22 ++++++++++++++--------\n 1 file changed, 14 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex aff0d66..b10a7e3 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -1996,22 +1996,22 @@ static int use_patch(struct apply_state *state, struct patch *p)\n \treturn !state->has_include;\n }\n \n-\n /*\n  * Read the patch text in \"buffer\" that extends for \"size\" bytes; stop\n  * reading after seeing a single patch (i.e. changes to a single file).\n  * Create fragments (i.e. patch hunks) and hang them to the given patch.\n- * Return the number of bytes consumed, so that the caller can call us\n- * again for the next patch.\n+ *\n+ * Returns:\n+ *   -1 if no header was found or parse_binary() failed,\n+ *   -128 on another error,\n+ *   the number of bytes consumed otherwise,\n+ *     so that the caller can call us again for the next patch.\n  */\n static int parse_chunk(struct apply_state *state, char *buffer, unsigned long size, struct patch *patch)\n {\n \tint hdrsize, patchsize;\n \tint offset = find_header(state, buffer, size, &hdrsize, patch);\n \n-\tif (offset == -128)\n-\t\texit(128);\n-\n \tif (offset < 0)\n \t\treturn offset;\n \n@@ -2071,8 +2071,10 @@ static int parse_chunk(struct apply_state *state, char *buffer, unsigned long si\n \t\t * empty to us here.\n \t\t */\n \t\tif ((state->apply || state->check) &&\n-\t\t    (!patch->is_binary && !metadata_changes(patch)))\n-\t\t\tdie(_(\"patch with only garbage at line %d\"), state->linenr);\n+\t\t    (!patch->is_binary && !metadata_changes(patch))) {\n+\t\t\terror(_(\"patch with only garbage at line %d\"), state->linenr);\n+\t\t\treturn -128;\n+\t\t}\n \t}\n \n \treturn offset + hdrsize + patchsize;\n@@ -4455,6 +4457,10 @@ static int apply_patch(struct apply_state *state,\n \t\tnr = parse_chunk(state, buf.buf + offset, buf.len - offset, patch);\n \t\tif (nr < 0) {\n \t\t\tfree_patch(patch);\n+\t\t\tif (nr == -128) {\n+\t\t\t\tres = -128;\n+\t\t\t\tgoto end;\n+\t\t\t}\n \t\t\tbreak;\n \t\t}\n \t\tif (state->apply_in_reverse)\n-- \n2.9.0.172.gfb57a78\n\n"},{"id":"290271","messageId":"20160627182429.31550-8-chriscool@tuxfamily.org","threadId":"42726","inReplyTo":"20160627182429.31550-1-chriscool@tuxfamily.org","subject":"[PATCH v8 07/41] builtin/apply: make parse_single_patch() return -1 on error","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-27T18:23:55Z","receivedAt":"2016-06-27T18:25:12Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"To libify `git apply` functionality we have to signal errors to the\ncaller instead of die()ing.\n\nTo do that in a compatible manner with the rest of the error handling\nin builtin/apply.c, parse_single_patch() should return a negative\ninteger instead of calling die().\n\nLet's do that by using error() and let's adjust the related test\ncases accordingly.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin/apply.c        | 17 +++++++++++++----\n t/t4012-diff-binary.sh |  4 ++--\n 2 files changed, 15 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex b10a7e3..65927f1 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -1671,6 +1671,10 @@ static int parse_fragment(struct apply_state *state,\n  *\n  * The (fragment->patch, fragment->size) pair points into the memory given\n  * by the caller, not a copy, when we return.\n+ *\n+ * Returns:\n+ *   -1 in case of error,\n+ *   the number of bytes in the patch otherwise.\n  */\n static int parse_single_patch(struct apply_state *state,\n \t\t\t      const char *line,\n@@ -1688,8 +1692,10 @@ static int parse_single_patch(struct apply_state *state,\n \t\tfragment = xcalloc(1, sizeof(*fragment));\n \t\tfragment->linenr = state->linenr;\n \t\tlen = parse_fragment(state, line, size, patch, fragment);\n-\t\tif (len <= 0)\n-\t\t\tdie(_(\"corrupt patch at line %d\"), state->linenr);\n+\t\tif (len <= 0) {\n+\t\t\tfree(fragment);\n+\t\t\treturn error(_(\"corrupt patch at line %d\"), state->linenr);\n+\t\t}\n \t\tfragment->patch = line;\n \t\tfragment->size = len;\n \t\toldlines += fragment->oldlines;\n@@ -1725,9 +1731,9 @@ static int parse_single_patch(struct apply_state *state,\n \t\tpatch->is_delete = 0;\n \n \tif (0 < patch->is_new && oldlines)\n-\t\tdie(_(\"new file %s depends on old contents\"), patch->new_name);\n+\t\treturn error(_(\"new file %s depends on old contents\"), patch->new_name);\n \tif (0 < patch->is_delete && newlines)\n-\t\tdie(_(\"deleted file %s still has contents\"), patch->old_name);\n+\t\treturn error(_(\"deleted file %s still has contents\"), patch->old_name);\n \tif (!patch->is_delete && !newlines && context)\n \t\tfprintf_ln(stderr,\n \t\t\t   _(\"** warning: \"\n@@ -2029,6 +2035,9 @@ static int parse_chunk(struct apply_state *state, char *buffer, unsigned long si\n \t\t\t\t       size - offset - hdrsize,\n \t\t\t\t       patch);\n \n+\tif (patchsize < 0)\n+\t\treturn -128;\n+\n \tif (!patchsize) {\n \t\tstatic const char git_binary[] = \"GIT binary patch\\n\";\n \t\tint hd = hdrsize + offset;\ndiff --git a/t/t4012-diff-binary.sh b/t/t4012-diff-binary.sh\nindex 643d729..0a8af76 100755\n--- a/t/t4012-diff-binary.sh\n+++ b/t/t4012-diff-binary.sh\n@@ -68,7 +68,7 @@ test_expect_success C_LOCALE_OUTPUT 'apply detecting corrupt patch correctly' '\n \tsed -e \"s/-CIT/xCIT/\" <output >broken &&\n \ttest_must_fail git apply --stat --summary broken 2>detected &&\n \tdetected=$(cat detected) &&\n-\tdetected=$(expr \"$detected\" : \"fatal.*at line \\\\([0-9]*\\\\)\\$\") &&\n+\tdetected=$(expr \"$detected\" : \"error.*at line \\\\([0-9]*\\\\)\\$\") &&\n \tdetected=$(sed -ne \"${detected}p\" broken) &&\n \ttest \"$detected\" = xCIT\n '\n@@ -77,7 +77,7 @@ test_expect_success C_LOCALE_OUTPUT 'apply detecting corrupt patch correctly' '\n \tgit diff --binary | sed -e \"s/-CIT/xCIT/\" >broken &&\n \ttest_must_fail git apply --stat --summary broken 2>detected &&\n \tdetected=$(cat detected) &&\n-\tdetected=$(expr \"$detected\" : \"fatal.*at line \\\\([0-9]*\\\\)\\$\") &&\n+\tdetected=$(expr \"$detected\" : \"error.*at line \\\\([0-9]*\\\\)\\$\") &&\n \tdetected=$(sed -ne \"${detected}p\" broken) &&\n \ttest \"$detected\" = xCIT\n '\n-- \n2.9.0.172.gfb57a78\n\n"},{"id":"290272","messageId":"20160627182429.31550-2-chriscool@tuxfamily.org","threadId":"42726","inReplyTo":"20160627182429.31550-1-chriscool@tuxfamily.org","subject":"[PATCH v8 01/41] apply: make some names more specific","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-27T18:23:49Z","receivedAt":"2016-06-27T18:25:16Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"To prepare for some structs and constants being moved from\nbuiltin/apply.c to apply.h, we should give them some more\nspecific names to avoid possible name collisions in th global\nnamespace.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin/apply.c | 20 ++++++++++----------\n 1 file changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex ecb7f1b..c4d9396 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -21,7 +21,7 @@\n #include \"ll-merge.h\"\n #include \"rerere.h\"\n \n-enum ws_error_action {\n+enum apply_ws_error_action {\n \tnowarn_ws_error,\n \twarn_on_ws_error,\n \tdie_on_ws_error,\n@@ -29,7 +29,7 @@ enum ws_error_action {\n };\n \n \n-enum ws_ignore {\n+enum apply_ws_ignore {\n \tignore_ws_none,\n \tignore_ws_change\n };\n@@ -45,8 +45,8 @@ enum ws_ignore {\n  * See also \"struct string_list symlink_changes\" in \"struct\n  * apply_state\".\n  */\n-#define SYMLINK_GOES_AWAY 01\n-#define SYMLINK_IN_RESULT 02\n+#define APPLY_SYMLINK_GOES_AWAY 01\n+#define APPLY_SYMLINK_IN_RESULT 02\n \n struct apply_state {\n \tconst char *prefix;\n@@ -110,8 +110,8 @@ struct apply_state {\n \tstruct string_list fn_table;\n \n \t/* These control whitespace errors */\n-\tenum ws_error_action ws_error_action;\n-\tenum ws_ignore ws_ignore_action;\n+\tenum apply_ws_error_action ws_error_action;\n+\tenum apply_ws_ignore ws_ignore_action;\n \tconst char *whitespace_option;\n \tint whitespace_error;\n \tint squelch_whitespace_errors;\n@@ -3750,11 +3750,11 @@ static void prepare_symlink_changes(struct apply_state *state, struct patch *pat\n \t\tif ((patch->old_name && S_ISLNK(patch->old_mode)) &&\n \t\t    (patch->is_rename || patch->is_delete))\n \t\t\t/* the symlink at patch->old_name is removed */\n-\t\t\tregister_symlink_changes(state, patch->old_name, SYMLINK_GOES_AWAY);\n+\t\t\tregister_symlink_changes(state, patch->old_name, APPLY_SYMLINK_GOES_AWAY);\n \n \t\tif (patch->new_name && S_ISLNK(patch->new_mode))\n \t\t\t/* the symlink at patch->new_name is created or remains */\n-\t\t\tregister_symlink_changes(state, patch->new_name, SYMLINK_IN_RESULT);\n+\t\t\tregister_symlink_changes(state, patch->new_name, APPLY_SYMLINK_IN_RESULT);\n \t}\n }\n \n@@ -3769,9 +3769,9 @@ static int path_is_beyond_symlink_1(struct apply_state *state, struct strbuf *na\n \t\t\tbreak;\n \t\tname->buf[name->len] = '\\0';\n \t\tchange = check_symlink_changes(state, name->buf);\n-\t\tif (change & SYMLINK_IN_RESULT)\n+\t\tif (change & APPLY_SYMLINK_IN_RESULT)\n \t\t\treturn 1;\n-\t\tif (change & SYMLINK_GOES_AWAY)\n+\t\tif (change & APPLY_SYMLINK_GOES_AWAY)\n \t\t\t/*\n \t\t\t * This cannot be \"return 0\", because we may\n \t\t\t * see a new one created at a higher level.\n-- \n2.9.0.172.gfb57a78\n\n"},{"id":"290273","messageId":"20160627182429.31550-13-chriscool@tuxfamily.org","threadId":"42726","inReplyTo":"20160627182429.31550-1-chriscool@tuxfamily.org","subject":"[PATCH v8 12/41] builtin/apply: make check_apply_state() return -1 instead of die()ing","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-27T18:24:00Z","receivedAt":"2016-06-27T18:25:19Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"To libify `git apply` functionality we have to signal errors to the\ncaller instead of die()ing.\n\nTo do that in a compatible manner with the rest of the error handling\nin \"builtin/apply.c\", check_apply_state() should return -1 instead of\ncalling die().\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin/apply.c | 19 +++++++++++--------\n 1 file changed, 11 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex 2a4a9d1..955e94a 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -4551,17 +4551,17 @@ static int option_parse_directory(const struct option *opt,\n \treturn 0;\n }\n \n-static void check_apply_state(struct apply_state *state, int force_apply)\n+static int check_apply_state(struct apply_state *state, int force_apply)\n {\n \tint is_not_gitdir = !startup_info->have_repository;\n \n \tif (state->apply_with_reject && state->threeway)\n-\t\tdie(\"--reject and --3way cannot be used together.\");\n+\t\treturn error(\"--reject and --3way cannot be used together.\");\n \tif (state->cached && state->threeway)\n-\t\tdie(\"--cached and --3way cannot be used together.\");\n+\t\treturn error(\"--cached and --3way cannot be used together.\");\n \tif (state->threeway) {\n \t\tif (is_not_gitdir)\n-\t\t\tdie(_(\"--3way outside a repository\"));\n+\t\t\treturn error(_(\"--3way outside a repository\"));\n \t\tstate->check_index = 1;\n \t}\n \tif (state->apply_with_reject)\n@@ -4569,16 +4569,18 @@ static void check_apply_state(struct apply_state *state, int force_apply)\n \tif (!force_apply && (state->diffstat || state->numstat || state->summary || state->check || state->fake_ancestor))\n \t\tstate->apply = 0;\n \tif (state->check_index && is_not_gitdir)\n-\t\tdie(_(\"--index outside a repository\"));\n+\t\treturn error(_(\"--index outside a repository\"));\n \tif (state->cached) {\n \t\tif (is_not_gitdir)\n-\t\t\tdie(_(\"--cached outside a repository\"));\n+\t\t\treturn error(_(\"--cached outside a repository\"));\n \t\tstate->check_index = 1;\n \t}\n \tif (state->check_index)\n \t\tstate->unsafe_paths = 0;\n \tif (!state->lock_file)\n-\t\tdie(\"BUG: state->lock_file should not be NULL\");\n+\t\treturn error(\"BUG: state->lock_file should not be NULL\");\n+\n+\treturn 0;\n }\n \n static int apply_all_patches(struct apply_state *state,\n@@ -4747,7 +4749,8 @@ int cmd_apply(int argc, const char **argv, const char *prefix)\n \targc = parse_options(argc, argv, state.prefix, builtin_apply_options,\n \t\t\tapply_usage, 0);\n \n-\tcheck_apply_state(&state, force_apply);\n+\tif (check_apply_state(&state, force_apply))\n+\t\texit(128);\n \n \tret = apply_all_patches(&state, argc, argv, options);\n \n-- \n2.9.0.172.gfb57a78\n\n"},{"id":"290274","messageId":"20160627182429.31550-20-chriscool@tuxfamily.org","threadId":"42726","inReplyTo":"20160627182429.31550-1-chriscool@tuxfamily.org","subject":"[PATCH v8 19/41] builtin/apply: make build_fake_ancestor() return -1 on error","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-27T18:24:07Z","receivedAt":"2016-06-27T18:25:26Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"To libify `git apply` functionality we have to signal errors to the\ncaller instead of die()ing.\n\nTo do that in a compatible manner with the rest of the error handling\nin \"builtin/apply.c\", build_fake_ancestor() should return -1 instead\nof calling die().\n\nHelped-by: Eric Sunshine <sunshine@sunshineco.com>\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin/apply.c | 41 ++++++++++++++++++++++++++---------------\n 1 file changed, 26 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex d3a9da2..bb1dffa 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -3900,11 +3900,12 @@ static int preimage_sha1_in_gitlink_patch(struct patch *p, unsigned char sha1[20\n }\n \n /* Build an index that contains the just the files needed for a 3way merge */\n-static void build_fake_ancestor(struct patch *list, const char *filename)\n+static int build_fake_ancestor(struct patch *list, const char *filename)\n {\n \tstruct patch *patch;\n \tstruct index_state result = { NULL };\n \tstatic struct lock_file lock;\n+\tint res;\n \n \t/* Once we start supporting the reverse patch, it may be\n \t * worth showing the new sha1 prefix, but until then...\n@@ -3922,31 +3923,38 @@ static void build_fake_ancestor(struct patch *list, const char *filename)\n \t\t\tif (!preimage_sha1_in_gitlink_patch(patch, sha1))\n \t\t\t\t; /* ok, the textual part looks sane */\n \t\t\telse\n-\t\t\t\tdie(\"sha1 information is lacking or useless for submodule %s\",\n-\t\t\t\t    name);\n+\t\t\t\treturn error(\"sha1 information is lacking or \"\n+\t\t\t\t\t     \"useless for submodule %s\", name);\n \t\t} else if (!get_sha1_blob(patch->old_sha1_prefix, sha1)) {\n \t\t\t; /* ok */\n \t\t} else if (!patch->lines_added && !patch->lines_deleted) {\n \t\t\t/* mode-only change: update the current */\n \t\t\tif (get_current_sha1(patch->old_name, sha1))\n-\t\t\t\tdie(\"mode change for %s, which is not \"\n-\t\t\t\t    \"in current HEAD\", name);\n+\t\t\t\treturn error(\"mode change for %s, which is not \"\n+\t\t\t\t\t     \"in current HEAD\", name);\n \t\t} else\n-\t\t\tdie(\"sha1 information is lacking or useless \"\n-\t\t\t    \"(%s).\", name);\n+\t\t\treturn error(\"sha1 information is lacking or useless \"\n+\t\t\t\t     \"(%s).\", name);\n \n \t\tce = make_cache_entry(patch->old_mode, sha1, name, 0, 0);\n \t\tif (!ce)\n-\t\t\tdie(_(\"make_cache_entry failed for path '%s'\"), name);\n-\t\tif (add_index_entry(&result, ce, ADD_CACHE_OK_TO_ADD))\n-\t\t\tdie (\"Could not add %s to temporary index\", name);\n+\t\t\treturn error(_(\"make_cache_entry failed for path '%s'\"),\n+\t\t\t\t     name);\n+\t\tif (add_index_entry(&result, ce, ADD_CACHE_OK_TO_ADD)) {\n+\t\t\tfree(ce);\n+\t\t\treturn error(\"Could not add %s to temporary index\",\n+\t\t\t\t     name);\n+\t\t}\n \t}\n \n \thold_lock_file_for_update(&lock, filename, LOCK_DIE_ON_ERROR);\n-\tif (write_locked_index(&result, &lock, COMMIT_LOCK))\n-\t\tdie (\"Could not write temporary index to %s\", filename);\n-\n+\tres = write_locked_index(&result, &lock, COMMIT_LOCK);\n \tdiscard_index(&result);\n+\n+\tif (res)\n+\t\treturn error(\"Could not write temporary index to %s\", filename);\n+\n+\treturn 0;\n }\n \n static void stat_patch_list(struct apply_state *state, struct patch *patch)\n@@ -4495,8 +4503,11 @@ static int apply_patch(struct apply_state *state,\n \t\tgoto end;\n \t}\n \n-\tif (state->fake_ancestor)\n-\t\tbuild_fake_ancestor(list, state->fake_ancestor);\n+\tif (state->fake_ancestor &&\n+\t    build_fake_ancestor(list, state->fake_ancestor)) {\n+\t\tres = -128;\n+\t\tgoto end;\n+\t}\n \n \tif (state->diffstat)\n \t\tstat_patch_list(state, list);\n-- \n2.9.0.172.gfb57a78\n\n"},{"id":"290275","messageId":"20160627182429.31550-15-chriscool@tuxfamily.org","threadId":"42726","inReplyTo":"20160627182429.31550-1-chriscool@tuxfamily.org","subject":"[PATCH v8 14/41] builtin/apply: make apply_all_patches() return 128 or 1 on error","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-27T18:24:02Z","receivedAt":"2016-06-27T18:25:32Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"To finish libifying the apply functionality, apply_all_patches() should not\ndie() or exit() in case of error, but return either 128 or 1, so that it\ngives the same exit code as when die() or exit(1) is called. This way\nscripts relying on the exit code don't need to be changed.\n\nWhile doing that we must take care that file descriptors are properly closed\nand, if needed, reset to a sensible value.\n\nAlso, according to the lockfile API, when finished with a lockfile, one\nshould either commit it or roll it back.\n\nThis is even more important now that the same lockfile can be passed\nto init_apply_state() many times to be reused by series of calls to\nthe apply lib functions.\n\nHelped-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\nHelped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>\nHelped-by: Eric Sunshine <sunshine@sunshineco.com>\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin/apply.c | 37 ++++++++++++++++++++++++++-----------\n 1 file changed, 26 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex 18c5419..29ca524 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -4578,15 +4578,18 @@ static int apply_all_patches(struct apply_state *state,\n \t\t\t\t\t      arg);\n \n \t\tfd = open(arg, O_RDONLY);\n-\t\tif (fd < 0)\n-\t\t\tdie_errno(_(\"can't open patch '%s'\"), arg);\n+\t\tif (fd < 0) {\n+\t\t\terror(_(\"can't open patch '%s': %s\"), arg, strerror(errno));\n+\t\t\tres = -128;\n+\t\t\tgoto end;\n+\t\t}\n \t\tread_stdin = 0;\n \t\tset_default_whitespace_mode(state);\n \t\tres = apply_patch(state, fd, arg, options);\n+\t\tclose(fd);\n \t\tif (res < 0)\n \t\t\tgoto end;\n \t\terrs |= res;\n-\t\tclose(fd);\n \t}\n \tset_default_whitespace_mode(state);\n \tif (read_stdin) {\n@@ -4606,11 +4609,14 @@ static int apply_all_patches(struct apply_state *state,\n \t\t\t\t   squelched),\n \t\t\t\tsquelched);\n \t\t}\n-\t\tif (state->ws_error_action == die_on_ws_error)\n-\t\t\tdie(Q_(\"%d line adds whitespace errors.\",\n-\t\t\t       \"%d lines add whitespace errors.\",\n-\t\t\t       state->whitespace_error),\n-\t\t\t    state->whitespace_error);\n+\t\tif (state->ws_error_action == die_on_ws_error) {\n+\t\t\terror(Q_(\"%d line adds whitespace errors.\",\n+\t\t\t\t \"%d lines add whitespace errors.\",\n+\t\t\t\t state->whitespace_error),\n+\t\t\t      state->whitespace_error);\n+\t\t\tres = -128;\n+\t\t\tgoto end;\n+\t\t}\n \t\tif (state->applied_after_fixing_ws && state->apply)\n \t\t\twarning(\"%d line%s applied after\"\n \t\t\t\t\" fixing whitespace errors.\",\n@@ -4624,15 +4630,24 @@ static int apply_all_patches(struct apply_state *state,\n \t}\n \n \tif (state->update_index) {\n-\t\tif (write_locked_index(&the_index, state->lock_file, COMMIT_LOCK))\n-\t\t\tdie(_(\"Unable to write new index file\"));\n+\t\tres = write_locked_index(&the_index, state->lock_file, COMMIT_LOCK);\n+\t\tif (res) {\n+\t\t\terror(_(\"Unable to write new index file\"));\n+\t\t\tres = -128;\n+\t\t\tgoto end;\n+\t\t}\n \t\tstate->newfd = -1;\n \t}\n \n \treturn !!errs;\n \n end:\n-\texit(res == -1 ? 1 : 128);\n+\tif (state->newfd >= 0) {\n+\t\trollback_lock_file(state->lock_file);\n+\t\tstate->newfd = -1;\n+\t}\n+\n+\treturn (res == -1 ? 1 : 128);\n }\n \n int cmd_apply(int argc, const char **argv, const char *prefix)\n-- \n2.9.0.172.gfb57a78\n\n"},{"id":"290276","messageId":"20160627182429.31550-17-chriscool@tuxfamily.org","threadId":"42726","inReplyTo":"20160627182429.31550-1-chriscool@tuxfamily.org","subject":"[PATCH v8 16/41] builtin/apply: make gitdiff_*() return 1 at end of header","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-27T18:24:04Z","receivedAt":"2016-06-27T18:25:36Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"The gitdiff_*() functions that are called as p->fn() in parse_git_header()\nshould return 1 instead of -1 in case of end of header or unrecognized\ninput, as these are not real errors. It just instructs the parser to break\nout.\n\nThis makes it possible for gitdiff_*() functions to return -1 in case of a\nreal error. This will be done in a following patch.\n\nHelped-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin/apply.c | 12 +++++++++---\n 1 file changed, 9 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex 174dbf3..0bed352 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -812,7 +812,7 @@ static int gitdiff_hdrend(struct apply_state *state,\n \t\t\t  const char *line,\n \t\t\t  struct patch *patch)\n {\n-\treturn -1;\n+\treturn 1;\n }\n \n /*\n@@ -1016,7 +1016,7 @@ static int gitdiff_unrecognized(struct apply_state *state,\n \t\t\t\tconst char *line,\n \t\t\t\tstruct patch *patch)\n {\n-\treturn -1;\n+\treturn 1;\n }\n \n /*\n@@ -1248,9 +1248,13 @@ static int parse_git_header(struct apply_state *state,\n \t\tfor (i = 0; i < ARRAY_SIZE(optable); i++) {\n \t\t\tconst struct opentry *p = optable + i;\n \t\t\tint oplen = strlen(p->str);\n+\t\t\tint res;\n \t\t\tif (len < oplen || memcmp(p->str, line, oplen))\n \t\t\t\tcontinue;\n-\t\t\tif (p->fn(state, line + oplen, patch) < 0)\n+\t\t\tres = p->fn(state, line + oplen, patch);\n+\t\t\tif (res < 0)\n+\t\t\t\treturn -1;\n+\t\t\tif (res > 0)\n \t\t\t\treturn offset;\n \t\t\tbreak;\n \t\t}\n@@ -1430,6 +1434,8 @@ static int find_header(struct apply_state *state,\n \t\t */\n \t\tif (!memcmp(\"diff --git \", line, 11)) {\n \t\t\tint git_hdr_len = parse_git_header(state, line, len, size, patch);\n+\t\t\tif (git_hdr_len < 0)\n+\t\t\t\treturn -128;\n \t\t\tif (git_hdr_len <= len)\n \t\t\t\tcontinue;\n \t\t\tif (!patch->old_name && !patch->new_name) {\n-- \n2.9.0.172.gfb57a78\n\n"},{"id":"290277","messageId":"20160627182429.31550-29-chriscool@tuxfamily.org","threadId":"42726","inReplyTo":"20160627182429.31550-1-chriscool@tuxfamily.org","subject":"[PATCH v8 28/41] builtin/apply: rename option parsing functions","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-27T18:24:16Z","receivedAt":"2016-06-27T18:25:39Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"As these functions are going to be part of the libified\napply api, let's give them a name that is more specific\nto the apply api.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin/apply.c | 40 ++++++++++++++++++++--------------------\n 1 file changed, 20 insertions(+), 20 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex 5c67713..c08ecde 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -4585,16 +4585,16 @@ end:\n \treturn res;\n }\n \n-static int option_parse_exclude(const struct option *opt,\n-\t\t\t\tconst char *arg, int unset)\n+static int apply_option_parse_exclude(const struct option *opt,\n+\t\t\t\t      const char *arg, int unset)\n {\n \tstruct apply_state *state = opt->value;\n \tadd_name_limit(state, arg, 1);\n \treturn 0;\n }\n \n-static int option_parse_include(const struct option *opt,\n-\t\t\t\tconst char *arg, int unset)\n+static int apply_option_parse_include(const struct option *opt,\n+\t\t\t\t      const char *arg, int unset)\n {\n \tstruct apply_state *state = opt->value;\n \tadd_name_limit(state, arg, 0);\n@@ -4602,9 +4602,9 @@ static int option_parse_include(const struct option *opt,\n \treturn 0;\n }\n \n-static int option_parse_p(const struct option *opt,\n-\t\t\t  const char *arg,\n-\t\t\t  int unset)\n+static int apply_option_parse_p(const struct option *opt,\n+\t\t\t\tconst char *arg,\n+\t\t\t\tint unset)\n {\n \tstruct apply_state *state = opt->value;\n \tstate->p_value = atoi(arg);\n@@ -4612,8 +4612,8 @@ static int option_parse_p(const struct option *opt,\n \treturn 0;\n }\n \n-static int option_parse_space_change(const struct option *opt,\n-\t\t\t\t     const char *arg, int unset)\n+static int apply_option_parse_space_change(const struct option *opt,\n+\t\t\t\t\t   const char *arg, int unset)\n {\n \tstruct apply_state *state = opt->value;\n \tif (unset)\n@@ -4623,8 +4623,8 @@ static int option_parse_space_change(const struct option *opt,\n \treturn 0;\n }\n \n-static int option_parse_whitespace(const struct option *opt,\n-\t\t\t\t   const char *arg, int unset)\n+static int apply_option_parse_whitespace(const struct option *opt,\n+\t\t\t\t\t const char *arg, int unset)\n {\n \tstruct apply_state *state = opt->value;\n \tstate->whitespace_option = arg;\n@@ -4633,8 +4633,8 @@ static int option_parse_whitespace(const struct option *opt,\n \treturn 0;\n }\n \n-static int option_parse_directory(const struct option *opt,\n-\t\t\t\t  const char *arg, int unset)\n+static int apply_option_parse_directory(const struct option *opt,\n+\t\t\t\t\tconst char *arg, int unset)\n {\n \tstruct apply_state *state = opt->value;\n \tstrbuf_reset(&state->root);\n@@ -4752,13 +4752,13 @@ int cmd_apply(int argc, const char **argv, const char *prefix)\n \tstruct option builtin_apply_options[] = {\n \t\t{ OPTION_CALLBACK, 0, \"exclude\", &state, N_(\"path\"),\n \t\t\tN_(\"don't apply changes matching the given path\"),\n-\t\t\t0, option_parse_exclude },\n+\t\t\t0, apply_option_parse_exclude },\n \t\t{ OPTION_CALLBACK, 0, \"include\", &state, N_(\"path\"),\n \t\t\tN_(\"apply changes matching the given path\"),\n-\t\t\t0, option_parse_include },\n+\t\t\t0, apply_option_parse_include },\n \t\t{ OPTION_CALLBACK, 'p', NULL, &state, N_(\"num\"),\n \t\t\tN_(\"remove <num> leading slashes from traditional diff paths\"),\n-\t\t\t0, option_parse_p },\n+\t\t\t0, apply_option_parse_p },\n \t\tOPT_BOOL(0, \"no-add\", &state.no_add,\n \t\t\tN_(\"ignore additions made by the patch\")),\n \t\tOPT_BOOL(0, \"stat\", &state.diffstat,\n@@ -4790,13 +4790,13 @@ int cmd_apply(int argc, const char **argv, const char *prefix)\n \t\t\t\tN_(\"ensure at least <n> lines of context match\")),\n \t\t{ OPTION_CALLBACK, 0, \"whitespace\", &state, N_(\"action\"),\n \t\t\tN_(\"detect new or modified lines that have whitespace errors\"),\n-\t\t\t0, option_parse_whitespace },\n+\t\t\t0, apply_option_parse_whitespace },\n \t\t{ OPTION_CALLBACK, 0, \"ignore-space-change\", &state, NULL,\n \t\t\tN_(\"ignore changes in whitespace when finding context\"),\n-\t\t\tPARSE_OPT_NOARG, option_parse_space_change },\n+\t\t\tPARSE_OPT_NOARG, apply_option_parse_space_change },\n \t\t{ OPTION_CALLBACK, 0, \"ignore-whitespace\", &state, NULL,\n \t\t\tN_(\"ignore changes in whitespace when finding context\"),\n-\t\t\tPARSE_OPT_NOARG, option_parse_space_change },\n+\t\t\tPARSE_OPT_NOARG, apply_option_parse_space_change },\n \t\tOPT_BOOL('R', \"reverse\", &state.apply_in_reverse,\n \t\t\tN_(\"apply the patch in reverse\")),\n \t\tOPT_BOOL(0, \"unidiff-zero\", &state.unidiff_zero,\n@@ -4814,7 +4814,7 @@ int cmd_apply(int argc, const char **argv, const char *prefix)\n \t\t\tRECOUNT),\n \t\t{ OPTION_CALLBACK, 0, \"directory\", &state, N_(\"root\"),\n \t\t\tN_(\"prepend <root> to all filenames\"),\n-\t\t\t0, option_parse_directory },\n+\t\t\t0, apply_option_parse_directory },\n \t\tOPT_END()\n \t};\n \n-- \n2.9.0.172.gfb57a78\n\n"},{"id":"290278","messageId":"20160627182429.31550-30-chriscool@tuxfamily.org","threadId":"42726","inReplyTo":"20160627182429.31550-1-chriscool@tuxfamily.org","subject":"[PATCH v8 29/41] apply: rename and move opt constants to apply.h","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-27T18:24:17Z","receivedAt":"2016-06-27T18:25:45Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"The constants for the \"inaccurate-eof\" and the \"recount\" options will\nbe used in both \"apply.c\" and \"builtin/apply.c\", so they need to go\ninto \"apply.h\", and therefore they need a name that is more specific\nto the API they belong to.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n apply.h         |  3 +++\n builtin/apply.c | 11 ++++-------\n 2 files changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/apply.h b/apply.h\nindex 53f09b5..ca1dcee 100644\n--- a/apply.h\n+++ b/apply.h\n@@ -108,4 +108,7 @@ extern int init_apply_state(struct apply_state *state,\n extern void clear_apply_state(struct apply_state *state);\n extern int check_apply_state(struct apply_state *state, int force_apply);\n \n+#define APPLY_OPT_INACCURATE_EOF\t(1<<0)\n+#define APPLY_OPT_RECOUNT\t\t(1<<1)\n+\n #endif\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex c08ecde..467b31f 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -4460,9 +4460,6 @@ static int write_out_results(struct apply_state *state, struct patch *list)\n \n static struct lock_file lock_file;\n \n-#define INACCURATE_EOF\t(1<<0)\n-#define RECOUNT\t\t(1<<1)\n-\n /*\n  * Try to apply a patch.\n  *\n@@ -4492,8 +4489,8 @@ static int apply_patch(struct apply_state *state,\n \t\tint nr;\n \n \t\tpatch = xcalloc(1, sizeof(*patch));\n-\t\tpatch->inaccurate_eof = !!(options & INACCURATE_EOF);\n-\t\tpatch->recount =  !!(options & RECOUNT);\n+\t\tpatch->inaccurate_eof = !!(options & APPLY_OPT_INACCURATE_EOF);\n+\t\tpatch->recount =  !!(options & APPLY_OPT_RECOUNT);\n \t\tnr = parse_chunk(state, buf.buf + offset, buf.len - offset, patch);\n \t\tif (nr < 0) {\n \t\t\tfree_patch(patch);\n@@ -4808,10 +4805,10 @@ int cmd_apply(int argc, const char **argv, const char *prefix)\n \t\tOPT__VERBOSE(&state.apply_verbosely, N_(\"be verbose\")),\n \t\tOPT_BIT(0, \"inaccurate-eof\", &options,\n \t\t\tN_(\"tolerate incorrectly detected missing new-line at the end of file\"),\n-\t\t\tINACCURATE_EOF),\n+\t\t\tAPPLY_OPT_INACCURATE_EOF),\n \t\tOPT_BIT(0, \"recount\", &options,\n \t\t\tN_(\"do not trust the line counts in the hunk headers\"),\n-\t\t\tRECOUNT),\n+\t\t\tAPPLY_OPT_RECOUNT),\n \t\t{ OPTION_CALLBACK, 0, \"directory\", &state, N_(\"root\"),\n \t\t\tN_(\"prepend <root> to all filenames\"),\n \t\t\t0, apply_option_parse_directory },\n-- \n2.9.0.172.gfb57a78\n\n"},{"id":"290279","messageId":"20160627182429.31550-24-chriscool@tuxfamily.org","threadId":"42726","inReplyTo":"20160627182429.31550-1-chriscool@tuxfamily.org","subject":"[PATCH v8 23/41] builtin/apply: make create_file() return -1 on error","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-27T18:24:11Z","receivedAt":"2016-06-27T18:25:48Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"To libify `git apply` functionality we have to signal errors to the\ncaller instead of exit()ing.\n\nTo do that in a compatible manner with the rest of the error handling\nin \"builtin/apply.c\", create_file() should just return what\nadd_conflicted_stages_file() and add_index_file() are returning\ninstead of calling exit().\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin/apply.c | 25 +++++++++++++------------\n 1 file changed, 13 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex eadff4d..bce3988 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -4269,7 +4269,7 @@ static int add_conflicted_stages_file(struct apply_state *state,\n \treturn 0;\n }\n \n-static void create_file(struct apply_state *state, struct patch *patch)\n+static int create_file(struct apply_state *state, struct patch *patch)\n {\n \tchar *path = patch->new_name;\n \tunsigned mode = patch->new_mode;\n@@ -4280,13 +4280,10 @@ static void create_file(struct apply_state *state, struct patch *patch)\n \t\tmode = S_IFREG | 0644;\n \tcreate_one_file(state, path, mode, buf, size);\n \n-\tif (patch->conflicted_threeway) {\n-\t\tif (add_conflicted_stages_file(state, patch))\n-\t\t\texit(128);\n-\t} else {\n-\t\tif (add_index_file(state, path, mode, buf, size))\n-\t\t\texit(128);\n-\t}\n+\tif (patch->conflicted_threeway)\n+\t\treturn add_conflicted_stages_file(state, patch);\n+\telse\n+\t\treturn add_index_file(state, path, mode, buf, size);\n }\n \n /* phase zero is to remove, phase one is to create */\n@@ -4302,8 +4299,10 @@ static void write_out_one_result(struct apply_state *state,\n \t\treturn;\n \t}\n \tif (patch->is_new > 0 || patch->is_copy) {\n-\t\tif (phase == 1)\n-\t\t\tcreate_file(state, patch);\n+\t\tif (phase == 1) {\n+\t\t\tif (create_file(state, patch))\n+\t\t\t\texit(128);\n+\t\t}\n \t\treturn;\n \t}\n \t/*\n@@ -4314,8 +4313,10 @@ static void write_out_one_result(struct apply_state *state,\n \t\tif (remove_file(state, patch, patch->is_rename))\n \t\t\texit(128);\n \t}\n-\tif (phase == 1)\n-\t\tcreate_file(state, patch);\n+\tif (phase == 1) {\n+\t\tif (create_file(state, patch))\n+\t\t\texit(128);\n+\t}\n }\n \n static int write_out_one_reject(struct apply_state *state, struct patch *patch)\n-- \n2.9.0.172.gfb57a78\n\n"},{"id":"290280","messageId":"20160627182429.31550-25-chriscool@tuxfamily.org","threadId":"42726","inReplyTo":"20160627182429.31550-1-chriscool@tuxfamily.org","subject":"[PATCH v8 24/41] builtin/apply: make write_out_one_result() return -1 on error","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-27T18:24:12Z","receivedAt":"2016-06-27T18:25:51Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"To libify `git apply` functionality we have to signal errors to the\ncaller instead of exit()ing.\n\nTo do that in a compatible manner with the rest of the error handling\nin \"builtin/apply.c\", write_out_one_result() should just return what\nremove_file() and create_file() are returning instead of calling\nexit().\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin/apply.c | 38 ++++++++++++++++----------------------\n 1 file changed, 16 insertions(+), 22 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex bce3988..6ec87e6 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -4287,36 +4287,29 @@ static int create_file(struct apply_state *state, struct patch *patch)\n }\n \n /* phase zero is to remove, phase one is to create */\n-static void write_out_one_result(struct apply_state *state,\n-\t\t\t\t struct patch *patch,\n-\t\t\t\t int phase)\n+static int write_out_one_result(struct apply_state *state,\n+\t\t\t\tstruct patch *patch,\n+\t\t\t\tint phase)\n {\n \tif (patch->is_delete > 0) {\n-\t\tif (phase == 0) {\n-\t\t\tif (remove_file(state, patch, 1))\n-\t\t\t\texit(128);\n-\t\t}\n-\t\treturn;\n+\t\tif (phase == 0)\n+\t\t\treturn remove_file(state, patch, 1);\n+\t\treturn 0;\n \t}\n \tif (patch->is_new > 0 || patch->is_copy) {\n-\t\tif (phase == 1) {\n-\t\t\tif (create_file(state, patch))\n-\t\t\t\texit(128);\n-\t\t}\n-\t\treturn;\n+\t\tif (phase == 1)\n+\t\t\treturn create_file(state, patch);\n+\t\treturn 0;\n \t}\n \t/*\n \t * Rename or modification boils down to the same\n \t * thing: remove the old, write the new\n \t */\n-\tif (phase == 0) {\n-\t\tif (remove_file(state, patch, patch->is_rename))\n-\t\t\texit(128);\n-\t}\n-\tif (phase == 1) {\n-\t\tif (create_file(state, patch))\n-\t\t\texit(128);\n-\t}\n+\tif (phase == 0)\n+\t\treturn remove_file(state, patch, patch->is_rename);\n+\tif (phase == 1)\n+\t\treturn create_file(state, patch);\n+\treturn 0;\n }\n \n static int write_out_one_reject(struct apply_state *state, struct patch *patch)\n@@ -4403,7 +4396,8 @@ static int write_out_results(struct apply_state *state, struct patch *list)\n \t\t\tif (l->rejected)\n \t\t\t\terrs = 1;\n \t\t\telse {\n-\t\t\t\twrite_out_one_result(state, l, phase);\n+\t\t\t\tif (write_out_one_result(state, l, phase))\n+\t\t\t\t\texit(128);\n \t\t\t\tif (phase == 1) {\n \t\t\t\t\tif (write_out_one_reject(state, l))\n \t\t\t\t\t\terrs = 1;\n-- \n2.9.0.172.gfb57a78\n\n"},{"id":"290281","messageId":"20160627182429.31550-22-chriscool@tuxfamily.org","threadId":"42726","inReplyTo":"20160627182429.31550-1-chriscool@tuxfamily.org","subject":"[PATCH v8 21/41] builtin/apply: make add_conflicted_stages_file() return -1 on error","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-27T18:24:09Z","receivedAt":"2016-06-27T18:25:55Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"To libify `git apply` functionality we have to signal errors to the\ncaller instead of die()ing.\n\nTo do that in a compatible manner with the rest of the error handling\nin \"builtin/apply.c\", add_conflicted_stages_file() should return -1\ninstead of calling die().\n\nHelped-by: Eric Sunshine <sunshine@sunshineco.com>\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin/apply.c | 20 +++++++++++++-------\n 1 file changed, 13 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex cb3ef1c..b0fd5f7 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -4224,7 +4224,7 @@ static void create_one_file(struct apply_state *state,\n \tdie_errno(_(\"unable to write file '%s' mode %o\"), path, mode);\n }\n \n-static void add_conflicted_stages_file(struct apply_state *state,\n+static int add_conflicted_stages_file(struct apply_state *state,\n \t\t\t\t       struct patch *patch)\n {\n \tint stage, namelen;\n@@ -4232,7 +4232,7 @@ static void add_conflicted_stages_file(struct apply_state *state,\n \tstruct cache_entry *ce;\n \n \tif (!state->update_index)\n-\t\treturn;\n+\t\treturn 0;\n \tnamelen = strlen(patch->new_name);\n \tce_size = cache_entry_size(namelen);\n \tmode = patch->new_mode ? patch->new_mode : (S_IFREG | 0644);\n@@ -4247,9 +4247,14 @@ static void add_conflicted_stages_file(struct apply_state *state,\n \t\tce->ce_flags = create_ce_flags(stage);\n \t\tce->ce_namelen = namelen;\n \t\thashcpy(ce->sha1, patch->threeway_stage[stage - 1].hash);\n-\t\tif (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD) < 0)\n-\t\t\tdie(_(\"unable to add cache entry for %s\"), patch->new_name);\n+\t\tif (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD) < 0) {\n+\t\t\tfree(ce);\n+\t\t\treturn error(_(\"unable to add cache entry for %s\"),\n+\t\t\t\t     patch->new_name);\n+\t\t}\n \t}\n+\n+\treturn 0;\n }\n \n static void create_file(struct apply_state *state, struct patch *patch)\n@@ -4263,9 +4268,10 @@ static void create_file(struct apply_state *state, struct patch *patch)\n \t\tmode = S_IFREG | 0644;\n \tcreate_one_file(state, path, mode, buf, size);\n \n-\tif (patch->conflicted_threeway)\n-\t\tadd_conflicted_stages_file(state, patch);\n-\telse\n+\tif (patch->conflicted_threeway) {\n+\t\tif (add_conflicted_stages_file(state, patch))\n+\t\t\texit(128);\n+\t} else\n \t\tadd_index_file(state, path, mode, buf, size);\n }\n \n-- \n2.9.0.172.gfb57a78\n\n"},{"id":"290282","messageId":"20160627182429.31550-21-chriscool@tuxfamily.org","threadId":"42726","inReplyTo":"20160627182429.31550-1-chriscool@tuxfamily.org","subject":"[PATCH v8 20/41] builtin/apply: make remove_file() return -1 on error","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-27T18:24:08Z","receivedAt":"2016-06-27T18:25:56Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"To libify `git apply` functionality we have to signal errors to the\ncaller instead of die()ing.\n\nTo do that in a compatible manner with the rest of the error handling\nin \"builtin/apply.c\", remove_file() should return -1 instead of\ncalling die().\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin/apply.c | 17 +++++++++++------\n 1 file changed, 11 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex bb1dffa..cb3ef1c 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -4085,17 +4085,18 @@ static void patch_stats(struct apply_state *state, struct patch *patch)\n \t}\n }\n \n-static void remove_file(struct apply_state *state, struct patch *patch, int rmdir_empty)\n+static int remove_file(struct apply_state *state, struct patch *patch, int rmdir_empty)\n {\n \tif (state->update_index) {\n \t\tif (remove_file_from_cache(patch->old_name) < 0)\n-\t\t\tdie(_(\"unable to remove %s from index\"), patch->old_name);\n+\t\t\treturn error(_(\"unable to remove %s from index\"), patch->old_name);\n \t}\n \tif (!state->cached) {\n \t\tif (!remove_or_warn(patch->old_mode, patch->old_name) && rmdir_empty) {\n \t\t\tremove_path(patch->old_name);\n \t\t}\n \t}\n+\treturn 0;\n }\n \n static void add_index_file(struct apply_state *state,\n@@ -4274,8 +4275,10 @@ static void write_out_one_result(struct apply_state *state,\n \t\t\t\t int phase)\n {\n \tif (patch->is_delete > 0) {\n-\t\tif (phase == 0)\n-\t\t\tremove_file(state, patch, 1);\n+\t\tif (phase == 0) {\n+\t\t\tif (remove_file(state, patch, 1))\n+\t\t\t\texit(128);\n+\t\t}\n \t\treturn;\n \t}\n \tif (patch->is_new > 0 || patch->is_copy) {\n@@ -4287,8 +4290,10 @@ static void write_out_one_result(struct apply_state *state,\n \t * Rename or modification boils down to the same\n \t * thing: remove the old, write the new\n \t */\n-\tif (phase == 0)\n-\t\tremove_file(state, patch, patch->is_rename);\n+\tif (phase == 0) {\n+\t\tif (remove_file(state, patch, patch->is_rename))\n+\t\t\texit(128);\n+\t}\n \tif (phase == 1)\n \t\tcreate_file(state, patch);\n }\n-- \n2.9.0.172.gfb57a78\n\n"},{"id":"290283","messageId":"20160627182429.31550-27-chriscool@tuxfamily.org","threadId":"42726","inReplyTo":"20160627182429.31550-1-chriscool@tuxfamily.org","subject":"[PATCH v8 26/41] builtin/apply: make try_create_file() return -1 on error","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-27T18:24:14Z","receivedAt":"2016-06-27T18:25:59Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"To libify `git apply` functionality we have to signal errors to the\ncaller instead of die()ing.\n\nTo do that in a compatible manner with the rest of the error handling\nin \"builtin/apply.c\", try_create_file() should return -1 in case of\nerror.\n\nUnfortunately try_create_file() currently returns -1 to signal a\nrecoverable error. To fix that, let's make it return 1 in case of\na recoverable error and -1 in case of an unrecoverable error.\n\nHelped-by: Eric Sunshine <sunshine@sunshineco.com>\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin/apply.c | 41 ++++++++++++++++++++++++++++++-----------\n 1 file changed, 30 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex f54b8c5..651a057 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -4150,38 +4150,45 @@ static int add_index_file(struct apply_state *state,\n \treturn 0;\n }\n \n+/*\n+ * Returns:\n+ *  -1 if an unrecoverable error happened\n+ *   0 if everything went well\n+ *   1 if a recoverable error happened\n+ */\n static int try_create_file(const char *path, unsigned int mode, const char *buf, unsigned long size)\n {\n-\tint fd;\n+\tint fd, res;\n \tstruct strbuf nbuf = STRBUF_INIT;\n \n \tif (S_ISGITLINK(mode)) {\n \t\tstruct stat st;\n \t\tif (!lstat(path, &st) && S_ISDIR(st.st_mode))\n \t\t\treturn 0;\n-\t\treturn mkdir(path, 0777);\n+\t\treturn !!mkdir(path, 0777);\n \t}\n \n \tif (has_symlinks && S_ISLNK(mode))\n \t\t/* Although buf:size is counted string, it also is NUL\n \t\t * terminated.\n \t\t */\n-\t\treturn symlink(buf, path);\n+\t\treturn !!symlink(buf, path);\n \n \tfd = open(path, O_CREAT | O_EXCL | O_WRONLY, (mode & 0100) ? 0777 : 0666);\n \tif (fd < 0)\n-\t\treturn -1;\n+\t\treturn 1;\n \n \tif (convert_to_working_tree(path, buf, size, &nbuf)) {\n \t\tsize = nbuf.len;\n \t\tbuf  = nbuf.buf;\n \t}\n-\twrite_or_die(fd, buf, size);\n+\tres = !write_or_whine_pipe(fd, buf, size, path);\n \tstrbuf_release(&nbuf);\n \n-\tif (close(fd) < 0)\n-\t\tdie_errno(_(\"closing file '%s'\"), path);\n-\treturn 0;\n+\tif (close(fd) < 0 && !res)\n+\t\treturn error(_(\"closing file '%s': %s\"), path, strerror(errno));\n+\n+\treturn res ? -1 : 0;\n }\n \n /*\n@@ -4195,15 +4202,24 @@ static void create_one_file(struct apply_state *state,\n \t\t\t    const char *buf,\n \t\t\t    unsigned long size)\n {\n+\tint res;\n+\n \tif (state->cached)\n \t\treturn;\n-\tif (!try_create_file(path, mode, buf, size))\n+\n+\tres = try_create_file(path, mode, buf, size);\n+\tif (res < 0)\n+\t\texit(128);\n+\tif (!res)\n \t\treturn;\n \n \tif (errno == ENOENT) {\n \t\tif (safe_create_leading_directories(path))\n \t\t\treturn;\n-\t\tif (!try_create_file(path, mode, buf, size))\n+\t\tres = try_create_file(path, mode, buf, size);\n+\t\tif (res < 0)\n+\t\t\texit(128);\n+\t\tif (!res)\n \t\t\treturn;\n \t}\n \n@@ -4222,7 +4238,10 @@ static void create_one_file(struct apply_state *state,\n \t\tfor (;;) {\n \t\t\tchar newpath[PATH_MAX];\n \t\t\tmksnpath(newpath, sizeof(newpath), \"%s~%u\", path, nr);\n-\t\t\tif (!try_create_file(newpath, mode, buf, size)) {\n+\t\t\tres = try_create_file(newpath, mode, buf, size);\n+\t\t\tif (res < 0)\n+\t\t\t\texit(128);\n+\t\t\tif (!res) {\n \t\t\t\tif (!rename(newpath, path))\n \t\t\t\t\treturn;\n \t\t\t\tunlink_or_warn(newpath);\n-- \n2.9.0.172.gfb57a78\n\n"},{"id":"290284","messageId":"20160627182429.31550-33-chriscool@tuxfamily.org","threadId":"42726","inReplyTo":"20160627182429.31550-1-chriscool@tuxfamily.org","subject":"[PATCH v8 32/41] environment: add set_index_file()","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-27T18:24:20Z","receivedAt":"2016-06-27T18:26:00Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Introduce set_index_file() to be able to temporarily change the index file.\n\nIt should be used like this:\n\n    /* Save current index file */\n    old_index_file = get_index_file();\n    set_index_file((char *)tmp_index_file);\n\n    /* Do stuff that will use tmp_index_file as the index file */\n    ...\n\n    /* When finished reset the index file */\n    set_index_file(old_index_file);\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n cache.h       |  1 +\n environment.c | 10 ++++++++++\n 2 files changed, 11 insertions(+)\n\ndiff --git a/cache.h b/cache.h\nindex c73becb..8854365 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -461,6 +461,7 @@ extern int is_inside_work_tree(void);\n extern const char *get_git_dir(void);\n extern const char *get_git_common_dir(void);\n extern char *get_object_directory(void);\n+extern void set_index_file(char *index_file);\n extern char *get_index_file(void);\n extern char *get_graft_file(void);\n extern int set_git_dir(const char *path);\ndiff --git a/environment.c b/environment.c\nindex ca72464..7a53799 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -292,6 +292,16 @@ int odb_pack_keep(char *name, size_t namesz, const unsigned char *sha1)\n \treturn open(name, O_RDWR|O_CREAT|O_EXCL, 0600);\n }\n \n+/*\n+ * Temporarily change the index file.\n+ * Please save the current index file using get_index_file() before changing\n+ * the index file. And when finished, reset it to the saved value.\n+ */\n+void set_index_file(char *index_file)\n+{\n+\tgit_index_file = index_file;\n+}\n+\n char *get_index_file(void)\n {\n \tif (!git_index_file)\n-- \n2.9.0.172.gfb57a78\n\n"},{"id":"290285","messageId":"20160627182429.31550-32-chriscool@tuxfamily.org","threadId":"42726","inReplyTo":"20160627182429.31550-1-chriscool@tuxfamily.org","subject":"[PATCH v8 31/41] apply: make some parsing functions static again","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-27T18:24:19Z","receivedAt":"2016-06-27T18:26:01Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Some parsing functions that were used in both \"apply.c\" and\n\"builtin/apply.c\" are now only used in the former, so they\ncan be made static to \"apply.c\".\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n apply.c | 6 +++---\n apply.h | 5 -----\n 2 files changed, 3 insertions(+), 8 deletions(-)\n\ndiff --git a/apply.c b/apply.c\nindex ababc9a..7bf12a7 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -27,7 +27,7 @@ static void git_apply_config(void)\n \tgit_config(git_default_config, NULL);\n }\n \n-int parse_whitespace_option(struct apply_state *state, const char *option)\n+static int parse_whitespace_option(struct apply_state *state, const char *option)\n {\n \tif (!option) {\n \t\tstate->ws_error_action = warn_on_ws_error;\n@@ -57,8 +57,8 @@ int parse_whitespace_option(struct apply_state *state, const char *option)\n \treturn error(_(\"unrecognized whitespace option '%s'\"), option);\n }\n \n-int parse_ignorewhitespace_option(struct apply_state *state,\n-\t\t\t\t  const char *option)\n+static int parse_ignorewhitespace_option(struct apply_state *state,\n+\t\t\t\t\t\t const char *option)\n {\n \tif (!option || !strcmp(option, \"no\") ||\n \t    !strcmp(option, \"false\") || !strcmp(option, \"never\") ||\ndiff --git a/apply.h b/apply.h\nindex 5ec022c..df44b51 100644\n--- a/apply.h\n+++ b/apply.h\n@@ -97,11 +97,6 @@ struct apply_state {\n \tint applied_after_fixing_ws;\n };\n \n-extern int parse_whitespace_option(struct apply_state *state,\n-\t\t\t\t   const char *option);\n-extern int parse_ignorewhitespace_option(struct apply_state *state,\n-\t\t\t\t\t const char *option);\n-\n extern int apply_option_parse_exclude(const struct option *opt,\n \t\t\t\t      const char *arg, int unset);\n extern int apply_option_parse_include(const struct option *opt,\n-- \n2.9.0.172.gfb57a78\n\n"},{"id":"290286","messageId":"20160627182429.31550-39-chriscool@tuxfamily.org","threadId":"42726","inReplyTo":"20160627182429.31550-1-chriscool@tuxfamily.org","subject":"[PATCH v8 38/41] usage: add get_error_routine() and get_warn_routine()","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-27T18:24:26Z","receivedAt":"2016-06-27T18:26:02Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Let's make it possible to get the current error_routine and warn_routine,\nso that we can store them before using set_error_routine() or\nset_warn_routine() to use new ones.\n\nThis way we will be able put back the original routines, when we are done\nwith using new ones.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n git-compat-util.h |  2 ++\n usage.c           | 10 ++++++++++\n 2 files changed, 12 insertions(+)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex f78706a..92af27d 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -440,7 +440,9 @@ static inline int const_error(void)\n \n extern void set_die_routine(NORETURN_PTR void (*routine)(const char *err, va_list params));\n extern void set_error_routine(void (*routine)(const char *err, va_list params));\n+extern void (*get_error_routine(void))(const char *err, va_list params);\n extern void set_warn_routine(void (*routine)(const char *warn, va_list params));\n+extern void (*get_warn_routine(void))(const char *warn, va_list params);\n extern void set_die_is_recursing_routine(int (*routine)(void));\n extern void set_error_handle(FILE *);\n \ndiff --git a/usage.c b/usage.c\nindex 67e5526..2fd3045 100644\n--- a/usage.c\n+++ b/usage.c\n@@ -70,11 +70,21 @@ void set_error_routine(void (*routine)(const char *err, va_list params))\n \terror_routine = routine;\n }\n \n+void (*get_error_routine(void))(const char *err, va_list params)\n+{\n+\treturn error_routine;\n+}\n+\n void set_warn_routine(void (*routine)(const char *warn, va_list params))\n {\n \twarn_routine = routine;\n }\n \n+void (*get_warn_routine(void))(const char *warn, va_list params)\n+{\n+\treturn warn_routine;\n+}\n+\n void set_die_is_recursing_routine(int (*routine)(void))\n {\n \tdie_is_recursing = routine;\n-- \n2.9.0.172.gfb57a78\n\n"},{"id":"290287","messageId":"20160627182429.31550-36-chriscool@tuxfamily.org","threadId":"42726","inReplyTo":"20160627182429.31550-1-chriscool@tuxfamily.org","subject":"[PATCH v8 35/41] apply: make 'be_silent' incompatible with 'apply_verbosely'","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-27T18:24:23Z","receivedAt":"2016-06-27T18:26:08Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"It should be an error to have both be_silent and apply_verbosely set,\nso let's check that in check_apply_state().\n\nAnd by the way let's not automatically set apply_verbosely when\nbe_silent is set.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n apply.c | 9 +++++++--\n 1 file changed, 7 insertions(+), 2 deletions(-)\n\ndiff --git a/apply.c b/apply.c\nindex 802fa79..1435f85 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -122,8 +122,11 @@ int check_apply_state(struct apply_state *state, int force_apply)\n \t\t\treturn error(_(\"--3way outside a repository\"));\n \t\tstate->check_index = 1;\n \t}\n-\tif (state->apply_with_reject)\n-\t\tstate->apply = state->apply_verbosely = 1;\n+\tif (state->apply_with_reject) {\n+\t\tstate->apply = 1;\n+\t\tif (!state->be_silent)\n+\t\t\tstate->apply_verbosely = 1;\n+\t}\n \tif (!force_apply && (state->diffstat || state->numstat || state->summary || state->check || state->fake_ancestor))\n \t\tstate->apply = 0;\n \tif (state->check_index && is_not_gitdir)\n@@ -135,6 +138,8 @@ int check_apply_state(struct apply_state *state, int force_apply)\n \t}\n \tif (state->check_index)\n \t\tstate->unsafe_paths = 0;\n+\tif (state->be_silent && state->apply_verbosely)\n+\t\treturn error(_(\"incompatible internal 'be_silent' and 'apply_verbosely' flags\"));\n \tif (!state->lock_file)\n \t\treturn error(\"BUG: state->lock_file should not be NULL\");\n \n-- \n2.9.0.172.gfb57a78\n\n"},{"id":"290288","messageId":"20160627182429.31550-40-chriscool@tuxfamily.org","threadId":"42726","inReplyTo":"20160627182429.31550-1-chriscool@tuxfamily.org","subject":"[PATCH v8 39/41] apply: change error_routine when be_silent is set","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-27T18:24:27Z","receivedAt":"2016-06-27T18:26:12Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"To avoid printing anything when applying with be_silent set,\nlet's save the existing warn and error routines before\napplying and replace them with a routine that does nothing.\n\nThen after applying, let's restore the saved routines.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n apply.c | 21 ++++++++++++++++++++-\n apply.h |  4 ++++\n 2 files changed, 24 insertions(+), 1 deletion(-)\n\ndiff --git a/apply.c b/apply.c\nindex e2acc18..de86f40 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -109,6 +109,11 @@ void clear_apply_state(struct apply_state *state)\n \t/* &state->fn_table is cleared at the end of apply_patch() */\n }\n \n+static void mute_routine(const char *bla, va_list params)\n+{\n+\t/* do nothing */\n+}\n+\n int check_apply_state(struct apply_state *state, int force_apply)\n {\n \tint is_not_gitdir = !startup_info->have_repository;\n@@ -143,6 +148,13 @@ int check_apply_state(struct apply_state *state, int force_apply)\n \tif (!state->lock_file)\n \t\treturn error(\"BUG: state->lock_file should not be NULL\");\n \n+\tif (state->be_silent) {\n+\t\tstate->saved_error_routine = get_error_routine();\n+\t\tstate->saved_warn_routine = get_warn_routine();\n+\t\tset_error_routine(mute_routine);\n+\t\tset_warn_routine(mute_routine);\n+\t}\n+\n \treturn 0;\n }\n \n@@ -4860,7 +4872,7 @@ int apply_all_patches(struct apply_state *state,\n \t\tstate->newfd = -1;\n \t}\n \n-\treturn !!errs;\n+\tres = !!errs;\n \n end:\n \tif (state->newfd >= 0) {\n@@ -4868,5 +4880,12 @@ end:\n \t\tstate->newfd = -1;\n \t}\n \n+\tif (state->be_silent) {\n+\t\tset_error_routine(state->saved_error_routine);\n+\t\tset_warn_routine(state->saved_warn_routine);\n+\t}\n+\n+\tif (res > -1)\n+\t\treturn res;\n \treturn (res == -1 ? 1 : 128);\n }\ndiff --git a/apply.h b/apply.h\nindex 44bed19..51a930a 100644\n--- a/apply.h\n+++ b/apply.h\n@@ -89,6 +89,10 @@ struct apply_state {\n \t */\n \tstruct string_list fn_table;\n \n+\t/* This is to save some reporting routines */\n+\tvoid (*saved_error_routine)(const char *err, va_list params);\n+\tvoid (*saved_warn_routine)(const char *warn, va_list params);\n+\n \t/* These control whitespace errors */\n \tenum apply_ws_error_action ws_error_action;\n \tenum apply_ws_ignore ws_ignore_action;\n-- \n2.9.0.172.gfb57a78\n\n"},{"id":"290289","messageId":"20160627182429.31550-34-chriscool@tuxfamily.org","threadId":"42726","inReplyTo":"20160627182429.31550-1-chriscool@tuxfamily.org","subject":"[PATCH v8 33/41] write_or_die: use warning() instead of fprintf(stderr, ...)","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-27T18:24:21Z","receivedAt":"2016-06-27T18:26:13Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"In write_or_whine_pipe() and write_or_whine() when write_in_full()\nreturns an error, let's print the errno related error message using\nwarning() instead of fprintf(stderr, ...).\n\nThis makes it possible to change the way it is handled by changing\nthe current warn routine in usage.c.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n write_or_die.c | 6 ++----\n 1 file changed, 2 insertions(+), 4 deletions(-)\n\ndiff --git a/write_or_die.c b/write_or_die.c\nindex 49e80aa..c29f677 100644\n--- a/write_or_die.c\n+++ b/write_or_die.c\n@@ -87,8 +87,7 @@ int write_or_whine_pipe(int fd, const void *buf, size_t count, const char *msg)\n {\n \tif (write_in_full(fd, buf, count) < 0) {\n \t\tcheck_pipe(errno);\n-\t\tfprintf(stderr, \"%s: write error (%s)\\n\",\n-\t\t\tmsg, strerror(errno));\n+\t\twarning(\"%s: write error (%s)\\n\", msg, strerror(errno));\n \t\treturn 0;\n \t}\n \n@@ -98,8 +97,7 @@ int write_or_whine_pipe(int fd, const void *buf, size_t count, const char *msg)\n int write_or_whine(int fd, const void *buf, size_t count, const char *msg)\n {\n \tif (write_in_full(fd, buf, count) < 0) {\n-\t\tfprintf(stderr, \"%s: write error (%s)\\n\",\n-\t\t\tmsg, strerror(errno));\n+\t\twarning(\"%s: write error (%s)\\n\", msg, strerror(errno));\n \t\treturn 0;\n \t}\n \n-- \n2.9.0.172.gfb57a78\n\n"},{"id":"290290","messageId":"20160627182429.31550-41-chriscool@tuxfamily.org","threadId":"42726","inReplyTo":"20160627182429.31550-1-chriscool@tuxfamily.org","subject":"[PATCH v8 40/41] builtin/am: use apply api in run_apply()","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-27T18:24:28Z","receivedAt":"2016-06-27T18:26:14Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"This replaces run_apply() implementation with a new one that\nuses the apply api that has been previously prepared in\napply.c and apply.h.\n\nThis shoud improve performance a lot in certain cases.\n\nAs the previous implementation was creating a new `git apply`\nprocess to apply each patch, it could be slow on systems like\nWindows where it is costly to create new processes.\n\nAlso the new `git apply` process had to read the index from\ndisk, and when the process was done the calling process\ndiscarded its own index and read back from disk the new\nindex that had been created by the `git apply` process.\n\nThis could be very inefficient with big repositories that\nhave big index files, especially when the system decided\nthat it was a good idea to run the `git apply` processes on\na different processor core.\n\nAlso eliminating index reads enables further performance\nimprovements by using:\n\n`git update-index --split-index`\n\nFor example here is a benchmark of a multi hundred commit\nrebase on the Linux kernel on a Debian laptop with SSD:\n\ncommand: git rebase --onto 1993b17 52bef0c 29dde7c\n\nVanilla \"next\" without split index:                1m54.953s\nVanilla \"next\" with split index:                   1m22.476s\nThis series on top of \"next\" without split index:  1m12.034s\nThis series on top of \"next\" with split index:     0m15.678s\n\n(using branch \"next\" from mid April 2016.)\n\nBenchmarked-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin/am.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++------------\n 1 file changed, 73 insertions(+), 18 deletions(-)\n\ndiff --git a/builtin/am.c b/builtin/am.c\nindex 3dfe70b..8647298 100644\n--- a/builtin/am.c\n+++ b/builtin/am.c\n@@ -28,6 +28,7 @@\n #include \"rerere.h\"\n #include \"prompt.h\"\n #include \"mailinfo.h\"\n+#include \"apply.h\"\n \n /**\n  * Returns 1 if the file is empty or does not exist, 0 otherwise.\n@@ -1521,39 +1522,93 @@ static int parse_mail_rebase(struct am_state *state, const char *mail)\n  */\n static int run_apply(const struct am_state *state, const char *index_file)\n {\n-\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\tstruct argv_array apply_paths = ARGV_ARRAY_INIT;\n+\tstruct argv_array apply_opts = ARGV_ARRAY_INIT;\n+\tstruct apply_state apply_state;\n+\tint res, opts_left;\n+\tchar *save_index_file;\n+\tstatic struct lock_file lock_file;\n+\n+\tstruct option am_apply_options[] = {\n+\t\t{ OPTION_CALLBACK, 0, \"whitespace\", &apply_state, N_(\"action\"),\n+\t\t\tN_(\"detect new or modified lines that have whitespace errors\"),\n+\t\t\t0, apply_option_parse_whitespace },\n+\t\t{ OPTION_CALLBACK, 0, \"ignore-space-change\", &apply_state, NULL,\n+\t\t\tN_(\"ignore changes in whitespace when finding context\"),\n+\t\t\tPARSE_OPT_NOARG, apply_option_parse_space_change },\n+\t\t{ OPTION_CALLBACK, 0, \"ignore-whitespace\", &apply_state, NULL,\n+\t\t\tN_(\"ignore changes in whitespace when finding context\"),\n+\t\t\tPARSE_OPT_NOARG, apply_option_parse_space_change },\n+\t\t{ OPTION_CALLBACK, 0, \"directory\", &apply_state, N_(\"root\"),\n+\t\t\tN_(\"prepend <root> to all filenames\"),\n+\t\t\t0, apply_option_parse_directory },\n+\t\t{ OPTION_CALLBACK, 0, \"exclude\", &apply_state, N_(\"path\"),\n+\t\t\tN_(\"don't apply changes matching the given path\"),\n+\t\t\t0, apply_option_parse_exclude },\n+\t\t{ OPTION_CALLBACK, 0, \"include\", &apply_state, N_(\"path\"),\n+\t\t\tN_(\"apply changes matching the given path\"),\n+\t\t\t0, apply_option_parse_include },\n+\t\tOPT_INTEGER('C', NULL, &apply_state.p_context,\n+\t\t\t\tN_(\"ensure at least <n> lines of context match\")),\n+\t\t{ OPTION_CALLBACK, 'p', NULL, &apply_state, N_(\"num\"),\n+\t\t\tN_(\"remove <num> leading slashes from traditional diff paths\"),\n+\t\t\t0, apply_option_parse_p },\n+\t\tOPT_BOOL(0, \"reject\", &apply_state.apply_with_reject,\n+\t\t\tN_(\"leave the rejected hunks in corresponding *.rej files\")),\n+\t\tOPT_END()\n+\t};\n \n-\tcp.git_cmd = 1;\n+\tif (index_file) {\n+\t\tsave_index_file = get_index_file();\n+\t\tset_index_file((char *)index_file);\n+\t}\n+\n+\tif (init_apply_state(&apply_state, NULL, &lock_file))\n+\t\tdie(\"init_apply_state() failed\");\n+\n+\targv_array_push(&apply_opts, \"apply\");\n+\targv_array_pushv(&apply_opts, state->git_apply_opts.argv);\n+\n+\topts_left = parse_options(apply_opts.argc, apply_opts.argv,\n+\t\t\t\t  NULL, am_apply_options, NULL, 0);\n+\n+\tif (opts_left != 0)\n+\t\tdie(\"unknown option passed thru to git apply\");\n \n \tif (index_file)\n-\t\targv_array_pushf(&cp.env_array, \"GIT_INDEX_FILE=%s\", index_file);\n+\t\tapply_state.cached = 1;\n+\telse\n+\t\tapply_state.check_index = 1;\n \n \t/*\n \t * If we are allowed to fall back on 3-way merge, don't give false\n \t * errors during the initial attempt.\n \t */\n-\tif (state->threeway && !index_file) {\n-\t\tcp.no_stdout = 1;\n-\t\tcp.no_stderr = 1;\n-\t}\n+\tif (state->threeway && !index_file)\n+\t\tapply_state.be_silent = 1;\n \n-\targv_array_push(&cp.args, \"apply\");\n+\tif (check_apply_state(&apply_state, 0))\n+\t\tdie(\"check_apply_state() failed\");\n \n-\targv_array_pushv(&cp.args, state->git_apply_opts.argv);\n+\targv_array_push(&apply_paths, am_path(state, \"patch\"));\n+\n+\tres = apply_all_patches(&apply_state, apply_paths.argc, apply_paths.argv, 0);\n \n \tif (index_file)\n-\t\targv_array_push(&cp.args, \"--cached\");\n-\telse\n-\t\targv_array_push(&cp.args, \"--index\");\n+\t\tset_index_file(save_index_file);\n \n-\targv_array_push(&cp.args, am_path(state, \"patch\"));\n+\targv_array_clear(&apply_paths);\n+\targv_array_clear(&apply_opts);\n+\tclear_apply_state(&apply_state);\n \n-\tif (run_command(&cp))\n-\t\treturn -1;\n+\tif (res)\n+\t\treturn res;\n \n-\t/* Reload index as git-apply will have modified it. */\n-\tdiscard_cache();\n-\tread_cache_from(index_file ? index_file : get_index_file());\n+\tif (index_file) {\n+\t\t/* Reload index as apply_all_patches() will have modified it. */\n+\t\tdiscard_cache();\n+\t\tread_cache_from(index_file);\n+\t}\n \n \treturn 0;\n }\n-- \n2.9.0.172.gfb57a78\n\n"},{"id":"290291","messageId":"20160627182429.31550-35-chriscool@tuxfamily.org","threadId":"42726","inReplyTo":"20160627182429.31550-1-chriscool@tuxfamily.org","subject":"[PATCH v8 34/41] apply: add 'be_silent' variable to 'struct apply_state'","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-27T18:24:22Z","receivedAt":"2016-06-27T18:26:18Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"This variable should prevent anything to be printed on both stderr\nand stdout.\n\nLet's not take care of stdout and apply_verbosely for now though,\nas that will be taken care of in following patches.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n apply.c | 43 +++++++++++++++++++++++++++++--------------\n apply.h |  1 +\n 2 files changed, 30 insertions(+), 14 deletions(-)\n\ndiff --git a/apply.c b/apply.c\nindex 7bf12a7..802fa79 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -1617,8 +1617,9 @@ static void record_ws_error(struct apply_state *state,\n \t\treturn;\n \n \terr = whitespace_error_string(result);\n-\tfprintf(stderr, \"%s:%d: %s.\\n%.*s\\n\",\n-\t\tstate->patch_input_file, linenr, err, len, line);\n+\tif (!state->be_silent)\n+\t\tfprintf(stderr, \"%s:%d: %s.\\n%.*s\\n\",\n+\t\t\tstate->patch_input_file, linenr, err, len, line);\n \tfree(err);\n }\n \n@@ -1813,7 +1814,7 @@ static int parse_single_patch(struct apply_state *state,\n \t\treturn error(_(\"new file %s depends on old contents\"), patch->new_name);\n \tif (0 < patch->is_delete && newlines)\n \t\treturn error(_(\"deleted file %s still has contents\"), patch->old_name);\n-\tif (!patch->is_delete && !newlines && context)\n+\tif (!patch->is_delete && !newlines && context && !state->be_silent)\n \t\tfprintf_ln(stderr,\n \t\t\t   _(\"** warning: \"\n \t\t\t     \"file %s becomes empty but is not deleted\"),\n@@ -3038,8 +3039,8 @@ static int apply_one_fragment(struct apply_state *state,\n \t\t * Warn if it was necessary to reduce the number\n \t\t * of context lines.\n \t\t */\n-\t\tif ((leading != frag->leading) ||\n-\t\t    (trailing != frag->trailing))\n+\t\tif ((leading != frag->leading ||\n+\t\t     trailing != frag->trailing) && !state->be_silent)\n \t\t\tfprintf_ln(stderr, _(\"Context reduced to (%ld/%ld)\"\n \t\t\t\t\t     \" to apply fragment at %d\"),\n \t\t\t\t   leading, trailing, applied_pos+1);\n@@ -3536,7 +3537,8 @@ static int try_threeway(struct apply_state *state,\n \t\t read_blob_object(&buf, pre_sha1, patch->old_mode))\n \t\treturn error(\"repository lacks the necessary blob to fall back on 3-way merge.\");\n \n-\tfprintf(stderr, \"Falling back to three-way merge...\\n\");\n+\tif (!state->be_silent)\n+\t\tfprintf(stderr, \"Falling back to three-way merge...\\n\");\n \n \timg = strbuf_detach(&buf, &len);\n \tprepare_image(&tmp_image, img, len, 1);\n@@ -3566,7 +3568,9 @@ static int try_threeway(struct apply_state *state,\n \tstatus = three_way_merge(image, patch->new_name,\n \t\t\t\t pre_sha1, our_sha1, post_sha1);\n \tif (status < 0) {\n-\t\tfprintf(stderr, \"Failed to fall back on three-way merge...\\n\");\n+\t\tif (!state->be_silent)\n+\t\t\tfprintf(stderr,\n+\t\t\t\t\"Failed to fall back on three-way merge...\\n\");\n \t\treturn status;\n \t}\n \n@@ -3578,9 +3582,15 @@ static int try_threeway(struct apply_state *state,\n \t\t\thashcpy(patch->threeway_stage[0].hash, pre_sha1);\n \t\thashcpy(patch->threeway_stage[1].hash, our_sha1);\n \t\thashcpy(patch->threeway_stage[2].hash, post_sha1);\n-\t\tfprintf(stderr, \"Applied patch to '%s' with conflicts.\\n\", patch->new_name);\n+\t\tif (!state->be_silent)\n+\t\t\tfprintf(stderr,\n+\t\t\t\t\"Applied patch to '%s' with conflicts.\\n\",\n+\t\t\t\tpatch->new_name);\n \t} else {\n-\t\tfprintf(stderr, \"Applied patch to '%s' cleanly.\\n\", patch->new_name);\n+\t\tif (!state->be_silent)\n+\t\t\tfprintf(stderr,\n+\t\t\t\t\"Applied patch to '%s' cleanly.\\n\",\n+\t\t\t\tpatch->new_name);\n \t}\n \treturn 0;\n }\n@@ -4483,7 +4493,8 @@ static int write_out_one_reject(struct apply_state *state, struct patch *patch)\n \t\t\t    \"Applying patch %%s with %d rejects...\",\n \t\t\t    cnt),\n \t\t    cnt);\n-\tsay_patch_name(stderr, sb.buf, patch);\n+\tif (!state->be_silent)\n+\t\tsay_patch_name(stderr, sb.buf, patch);\n \tstrbuf_release(&sb);\n \n \tcnt = strlen(patch->new_name);\n@@ -4510,10 +4521,12 @@ static int write_out_one_reject(struct apply_state *state, struct patch *patch)\n \t     frag;\n \t     cnt++, frag = frag->next) {\n \t\tif (!frag->rejected) {\n-\t\t\tfprintf_ln(stderr, _(\"Hunk #%d applied cleanly.\"), cnt);\n+\t\t\tif (!state->be_silent)\n+\t\t\t\tfprintf_ln(stderr, _(\"Hunk #%d applied cleanly.\"), cnt);\n \t\t\tcontinue;\n \t\t}\n-\t\tfprintf_ln(stderr, _(\"Rejected hunk #%d.\"), cnt);\n+\t\tif (!state->be_silent)\n+\t\t\tfprintf_ln(stderr, _(\"Rejected hunk #%d.\"), cnt);\n \t\tfprintf(rej, \"%.*s\", frag->size, frag->patch);\n \t\tif (frag->patch[frag->size-1] != '\\n')\n \t\t\tfputc('\\n', rej);\n@@ -4562,8 +4575,10 @@ static int write_out_results(struct apply_state *state, struct patch *list)\n \t\tstruct string_list_item *item;\n \n \t\tstring_list_sort(&cpath);\n-\t\tfor_each_string_list_item(item, &cpath)\n-\t\t\tfprintf(stderr, \"U %s\\n\", item->string);\n+\t\tif (!state->be_silent) {\n+\t\t\tfor_each_string_list_item(item, &cpath)\n+\t\t\t\tfprintf(stderr, \"U %s\\n\", item->string);\n+\t\t}\n \t\tstring_list_clear(&cpath, 0);\n \n \t\trerere(0);\ndiff --git a/apply.h b/apply.h\nindex df44b51..44bed19 100644\n--- a/apply.h\n+++ b/apply.h\n@@ -52,6 +52,7 @@ struct apply_state {\n \tint apply_in_reverse;\n \tint apply_with_reject;\n \tint apply_verbosely;\n+\tint be_silent;\n \tint no_add;\n \tint threeway;\n \tint unidiff_zero;\n-- \n2.9.0.172.gfb57a78\n\n"},{"id":"290292","messageId":"20160627182429.31550-42-chriscool@tuxfamily.org","threadId":"42726","inReplyTo":"20160627182429.31550-1-chriscool@tuxfamily.org","subject":"[PATCH v8 41/41] apply: use error_errno() where possible","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-27T18:24:29Z","receivedAt":"2016-06-27T18:26:21Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"To avoid possible mistakes and to uniformly show the errno\nrelated messages, let's use error_errno() where possible.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n apply.c | 18 +++++++++---------\n 1 file changed, 9 insertions(+), 9 deletions(-)\n\ndiff --git a/apply.c b/apply.c\nindex de86f40..2ac22d3 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -3512,7 +3512,7 @@ static int load_current(struct apply_state *state,\n \tce = active_cache[pos];\n \tif (lstat(name, &st)) {\n \t\tif (errno != ENOENT)\n-\t\t\treturn error(_(\"%s: %s\"), name, strerror(errno));\n+\t\t\treturn error_errno(\"%s\", name);\n \t\tif (checkout_target(&the_index, ce, &st))\n \t\t\treturn -1;\n \t}\n@@ -3671,7 +3671,7 @@ static int check_preimage(struct apply_state *state,\n \t} else if (!state->cached) {\n \t\tstat_ret = lstat(old_name, st);\n \t\tif (stat_ret && errno != ENOENT)\n-\t\t\treturn error(_(\"%s: %s\"), old_name, strerror(errno));\n+\t\t\treturn error_errno(\"%s\", old_name);\n \t}\n \n \tif (state->check_index && !previous) {\n@@ -3693,7 +3693,7 @@ static int check_preimage(struct apply_state *state,\n \t} else if (stat_ret < 0) {\n \t\tif (patch->is_new < 0)\n \t\t\tgoto is_new;\n-\t\treturn error(_(\"%s: %s\"), old_name, strerror(errno));\n+\t\treturn error_errno(\"%s\", old_name);\n \t}\n \n \tif (!state->cached && !previous)\n@@ -3752,7 +3752,7 @@ static int check_to_create(struct apply_state *state,\n \n \t\treturn EXISTS_IN_WORKTREE;\n \t} else if ((errno != ENOENT) && (errno != ENOTDIR)) {\n-\t\treturn error(\"%s: %s\", new_name, strerror(errno));\n+\t\treturn error_errno(\"%s\", new_name);\n \t}\n \treturn 0;\n }\n@@ -4271,9 +4271,9 @@ static int add_index_file(struct apply_state *state,\n \t\tif (!state->cached) {\n \t\t\tif (lstat(path, &st) < 0) {\n \t\t\t\tfree(ce);\n-\t\t\t\treturn error(_(\"unable to stat newly \"\n-\t\t\t\t\t       \"created file '%s': %s\"),\n-\t\t\t\t\t     path, strerror(errno));\n+\t\t\t\treturn error_errno(_(\"unable to stat newly \"\n+\t\t\t\t\t\t     \"created file '%s'\"),\n+\t\t\t\t\t\t   path);\n \t\t\t}\n \t\t\tfill_stat_cache_info(ce, &st);\n \t\t}\n@@ -4327,7 +4327,7 @@ static int try_create_file(const char *path, unsigned int mode, const char *buf,\n \tstrbuf_release(&nbuf);\n \n \tif (close(fd) < 0 && !res)\n-\t\treturn error(_(\"closing file '%s': %s\"), path, strerror(errno));\n+\t\treturn error_errno(_(\"closing file '%s'\"), path);\n \n \treturn res ? -1 : 0;\n }\n@@ -4525,7 +4525,7 @@ static int write_out_one_reject(struct apply_state *state, struct patch *patch)\n \n \trej = fopen(namebuf, \"w\");\n \tif (!rej)\n-\t\treturn error(_(\"cannot open %s: %s\"), namebuf, strerror(errno));\n+\t\treturn error_errno(_(\"cannot open %s\"), namebuf);\n \n \t/* Normal git tools never deal with .rej, so do not pretend\n \t * this is a git patch by saying --git or giving extended\n-- \n2.9.0.172.gfb57a78\n\n"},{"id":"290293","messageId":"20160627182429.31550-38-chriscool@tuxfamily.org","threadId":"42726","inReplyTo":"20160627182429.31550-1-chriscool@tuxfamily.org","subject":"[PATCH v8 37/41] usage: add set_warn_routine()","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-27T18:24:25Z","receivedAt":"2016-06-27T18:26:26Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"There are already set_die_routine() and set_error_routine(),\nso let's add set_warn_routine() as this will be needed in a\nfollowing commit.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n git-compat-util.h | 1 +\n usage.c           | 5 +++++\n 2 files changed, 6 insertions(+)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 49d4029..f78706a 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -440,6 +440,7 @@ static inline int const_error(void)\n \n extern void set_die_routine(NORETURN_PTR void (*routine)(const char *err, va_list params));\n extern void set_error_routine(void (*routine)(const char *err, va_list params));\n+extern void set_warn_routine(void (*routine)(const char *warn, va_list params));\n extern void set_die_is_recursing_routine(int (*routine)(void));\n extern void set_error_handle(FILE *);\n \ndiff --git a/usage.c b/usage.c\nindex 1dad03f..67e5526 100644\n--- a/usage.c\n+++ b/usage.c\n@@ -70,6 +70,11 @@ void set_error_routine(void (*routine)(const char *err, va_list params))\n \terror_routine = routine;\n }\n \n+void set_warn_routine(void (*routine)(const char *warn, va_list params))\n+{\n+\twarn_routine = routine;\n+}\n+\n void set_die_is_recursing_routine(int (*routine)(void))\n {\n \tdie_is_recursing = routine;\n-- \n2.9.0.172.gfb57a78\n\n"},{"id":"290294","messageId":"20160627182429.31550-37-chriscool@tuxfamily.org","threadId":"42726","inReplyTo":"20160627182429.31550-1-chriscool@tuxfamily.org","subject":"[PATCH v8 36/41] apply: don't print on stdout when be_silent is set","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-27T18:24:24Z","receivedAt":"2016-06-27T18:26:30Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"This variable should prevent anything to be printed on both stderr\nand stdout.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n apply.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/apply.c b/apply.c\nindex 1435f85..e2acc18 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -4698,13 +4698,13 @@ static int apply_patch(struct apply_state *state,\n \t\tgoto end;\n \t}\n \n-\tif (state->diffstat)\n+\tif (state->diffstat && !state->be_silent)\n \t\tstat_patch_list(state, list);\n \n-\tif (state->numstat)\n+\tif (state->numstat && !state->be_silent)\n \t\tnumstat_patch_list(state, list);\n \n-\tif (state->summary)\n+\tif (state->summary && !state->be_silent)\n \t\tsummary_patch_list(list);\n \n end:\n-- \n2.9.0.172.gfb57a78\n\n"},{"id":"290295","messageId":"20160627182429.31550-23-chriscool@tuxfamily.org","threadId":"42726","inReplyTo":"20160627182429.31550-1-chriscool@tuxfamily.org","subject":"[PATCH v8 22/41] builtin/apply: make add_index_file() return -1 on error","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-27T18:24:10Z","receivedAt":"2016-06-27T18:26:33Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"To libify `git apply` functionality we have to signal errors to the\ncaller instead of die()ing.\n\nTo do that in a compatible manner with the rest of the error handling\nin \"builtin/apply.c\", add_index_file() should return -1 instead of\ncalling die().\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin/apply.c | 48 +++++++++++++++++++++++++++++++-----------------\n 1 file changed, 31 insertions(+), 17 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex b0fd5f7..eadff4d 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -4099,11 +4099,11 @@ static int remove_file(struct apply_state *state, struct patch *patch, int rmdir\n \treturn 0;\n }\n \n-static void add_index_file(struct apply_state *state,\n-\t\t\t   const char *path,\n-\t\t\t   unsigned mode,\n-\t\t\t   void *buf,\n-\t\t\t   unsigned long size)\n+static int add_index_file(struct apply_state *state,\n+\t\t\t  const char *path,\n+\t\t\t  unsigned mode,\n+\t\t\t  void *buf,\n+\t\t\t  unsigned long size)\n {\n \tstruct stat st;\n \tstruct cache_entry *ce;\n@@ -4111,7 +4111,7 @@ static void add_index_file(struct apply_state *state,\n \tunsigned ce_size = cache_entry_size(namelen);\n \n \tif (!state->update_index)\n-\t\treturn;\n+\t\treturn 0;\n \n \tce = xcalloc(1, ce_size);\n \tmemcpy(ce->name, path, namelen);\n@@ -4122,20 +4122,32 @@ static void add_index_file(struct apply_state *state,\n \t\tconst char *s;\n \n \t\tif (!skip_prefix(buf, \"Subproject commit \", &s) ||\n-\t\t    get_sha1_hex(s, ce->sha1))\n-\t\t\tdie(_(\"corrupt patch for submodule %s\"), path);\n+\t\t    get_sha1_hex(s, ce->sha1)) {\n+\t\t\tfree(ce);\n+\t\t\treturn error(_(\"corrupt patch for submodule %s\"), path);\n+\t\t}\n \t} else {\n \t\tif (!state->cached) {\n-\t\t\tif (lstat(path, &st) < 0)\n-\t\t\t\tdie_errno(_(\"unable to stat newly created file '%s'\"),\n-\t\t\t\t\t  path);\n+\t\t\tif (lstat(path, &st) < 0) {\n+\t\t\t\tfree(ce);\n+\t\t\t\treturn error(_(\"unable to stat newly \"\n+\t\t\t\t\t       \"created file '%s': %s\"),\n+\t\t\t\t\t     path, strerror(errno));\n+\t\t\t}\n \t\t\tfill_stat_cache_info(ce, &st);\n \t\t}\n-\t\tif (write_sha1_file(buf, size, blob_type, ce->sha1) < 0)\n-\t\t\tdie(_(\"unable to create backing store for newly created file %s\"), path);\n+\t\tif (write_sha1_file(buf, size, blob_type, ce->sha1) < 0) {\n+\t\t\tfree(ce);\n+\t\t\treturn error(_(\"unable to create backing store \"\n+\t\t\t\t       \"for newly created file %s\"), path);\n+\t\t}\n \t}\n-\tif (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD) < 0)\n-\t\tdie(_(\"unable to add cache entry for %s\"), path);\n+\tif (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD) < 0) {\n+\t\tfree(ce);\n+\t\treturn error(_(\"unable to add cache entry for %s\"), path);\n+\t}\n+\n+\treturn 0;\n }\n \n static int try_create_file(const char *path, unsigned int mode, const char *buf, unsigned long size)\n@@ -4271,8 +4283,10 @@ static void create_file(struct apply_state *state, struct patch *patch)\n \tif (patch->conflicted_threeway) {\n \t\tif (add_conflicted_stages_file(state, patch))\n \t\t\texit(128);\n-\t} else\n-\t\tadd_index_file(state, path, mode, buf, size);\n+\t} else {\n+\t\tif (add_index_file(state, path, mode, buf, size))\n+\t\t\texit(128);\n+\t}\n }\n \n /* phase zero is to remove, phase one is to create */\n-- \n2.9.0.172.gfb57a78\n\n"},{"id":"290296","messageId":"20160627182429.31550-14-chriscool@tuxfamily.org","threadId":"42726","inReplyTo":"20160627182429.31550-1-chriscool@tuxfamily.org","subject":"[PATCH v8 13/41] builtin/apply: move check_apply_state() to apply.c","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-27T18:24:01Z","receivedAt":"2016-06-27T18:26:44Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"To libify `git apply` functionality we must make check_apply_state()\nusable outside \"builtin/apply.c\".\n\nLet's do that by moving it into \"apply.c\".\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n apply.c         | 32 ++++++++++++++++++++++++++++++++\n apply.h         |  1 +\n builtin/apply.c | 32 --------------------------------\n 3 files changed, 33 insertions(+), 32 deletions(-)\n\ndiff --git a/apply.c b/apply.c\nindex c5a9ee2..84dae3d 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -90,3 +90,35 @@ void clear_apply_state(struct apply_state *state)\n \n \t/* &state->fn_table is cleared at the end of apply_patch() */\n }\n+\n+int check_apply_state(struct apply_state *state, int force_apply)\n+{\n+\tint is_not_gitdir = !startup_info->have_repository;\n+\n+\tif (state->apply_with_reject && state->threeway)\n+\t\treturn error(\"--reject and --3way cannot be used together.\");\n+\tif (state->cached && state->threeway)\n+\t\treturn error(\"--cached and --3way cannot be used together.\");\n+\tif (state->threeway) {\n+\t\tif (is_not_gitdir)\n+\t\t\treturn error(_(\"--3way outside a repository\"));\n+\t\tstate->check_index = 1;\n+\t}\n+\tif (state->apply_with_reject)\n+\t\tstate->apply = state->apply_verbosely = 1;\n+\tif (!force_apply && (state->diffstat || state->numstat || state->summary || state->check || state->fake_ancestor))\n+\t\tstate->apply = 0;\n+\tif (state->check_index && is_not_gitdir)\n+\t\treturn error(_(\"--index outside a repository\"));\n+\tif (state->cached) {\n+\t\tif (is_not_gitdir)\n+\t\t\treturn error(_(\"--cached outside a repository\"));\n+\t\tstate->check_index = 1;\n+\t}\n+\tif (state->check_index)\n+\t\tstate->unsafe_paths = 0;\n+\tif (!state->lock_file)\n+\t\treturn error(\"BUG: state->lock_file should not be NULL\");\n+\n+\treturn 0;\n+}\ndiff --git a/apply.h b/apply.h\nindex e18a18a..53f09b5 100644\n--- a/apply.h\n+++ b/apply.h\n@@ -106,5 +106,6 @@ extern int init_apply_state(struct apply_state *state,\n \t\t\t    const char *prefix,\n \t\t\t    struct lock_file *lock_file);\n extern void clear_apply_state(struct apply_state *state);\n+extern int check_apply_state(struct apply_state *state, int force_apply);\n \n #endif\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex 955e94a..18c5419 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -4551,38 +4551,6 @@ static int option_parse_directory(const struct option *opt,\n \treturn 0;\n }\n \n-static int check_apply_state(struct apply_state *state, int force_apply)\n-{\n-\tint is_not_gitdir = !startup_info->have_repository;\n-\n-\tif (state->apply_with_reject && state->threeway)\n-\t\treturn error(\"--reject and --3way cannot be used together.\");\n-\tif (state->cached && state->threeway)\n-\t\treturn error(\"--cached and --3way cannot be used together.\");\n-\tif (state->threeway) {\n-\t\tif (is_not_gitdir)\n-\t\t\treturn error(_(\"--3way outside a repository\"));\n-\t\tstate->check_index = 1;\n-\t}\n-\tif (state->apply_with_reject)\n-\t\tstate->apply = state->apply_verbosely = 1;\n-\tif (!force_apply && (state->diffstat || state->numstat || state->summary || state->check || state->fake_ancestor))\n-\t\tstate->apply = 0;\n-\tif (state->check_index && is_not_gitdir)\n-\t\treturn error(_(\"--index outside a repository\"));\n-\tif (state->cached) {\n-\t\tif (is_not_gitdir)\n-\t\t\treturn error(_(\"--cached outside a repository\"));\n-\t\tstate->check_index = 1;\n-\t}\n-\tif (state->check_index)\n-\t\tstate->unsafe_paths = 0;\n-\tif (!state->lock_file)\n-\t\treturn error(\"BUG: state->lock_file should not be NULL\");\n-\n-\treturn 0;\n-}\n-\n static int apply_all_patches(struct apply_state *state,\n \t\t\t     int argc,\n \t\t\t     const char **argv,\n-- \n2.9.0.172.gfb57a78\n\n"},{"id":"290297","messageId":"20160627182429.31550-28-chriscool@tuxfamily.org","threadId":"42726","inReplyTo":"20160627182429.31550-1-chriscool@tuxfamily.org","subject":"[PATCH v8 27/41] builtin/apply: make create_one_file() return -1 on error","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-27T18:24:15Z","receivedAt":"2016-06-27T18:26:47Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"To libify `git apply` functionality we have to signal errors to the\ncaller instead of exit()ing.\n\nTo do that in a compatible manner with the rest of the error handling\nin \"builtin/apply.c\", create_one_file() should return -1 instead of\ncalling exit().\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin/apply.c | 36 +++++++++++++++++++++---------------\n 1 file changed, 21 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex 651a057..5c67713 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -4195,32 +4195,36 @@ static int try_create_file(const char *path, unsigned int mode, const char *buf,\n  * We optimistically assume that the directories exist,\n  * which is true 99% of the time anyway. If they don't,\n  * we create them and try again.\n+ *\n+ * Returns:\n+ *   -1 on error\n+ *   0 otherwise\n  */\n-static void create_one_file(struct apply_state *state,\n-\t\t\t    char *path,\n-\t\t\t    unsigned mode,\n-\t\t\t    const char *buf,\n-\t\t\t    unsigned long size)\n+static int create_one_file(struct apply_state *state,\n+\t\t\t   char *path,\n+\t\t\t   unsigned mode,\n+\t\t\t   const char *buf,\n+\t\t\t   unsigned long size)\n {\n \tint res;\n \n \tif (state->cached)\n-\t\treturn;\n+\t\treturn 0;\n \n \tres = try_create_file(path, mode, buf, size);\n \tif (res < 0)\n-\t\texit(128);\n+\t\treturn -1;\n \tif (!res)\n-\t\treturn;\n+\t\treturn 0;\n \n \tif (errno == ENOENT) {\n \t\tif (safe_create_leading_directories(path))\n-\t\t\treturn;\n+\t\t\treturn 0;\n \t\tres = try_create_file(path, mode, buf, size);\n \t\tif (res < 0)\n-\t\t\texit(128);\n+\t\t\treturn -1;\n \t\tif (!res)\n-\t\t\treturn;\n+\t\t\treturn 0;\n \t}\n \n \tif (errno == EEXIST || errno == EACCES) {\n@@ -4240,10 +4244,10 @@ static void create_one_file(struct apply_state *state,\n \t\t\tmksnpath(newpath, sizeof(newpath), \"%s~%u\", path, nr);\n \t\t\tres = try_create_file(newpath, mode, buf, size);\n \t\t\tif (res < 0)\n-\t\t\t\texit(128);\n+\t\t\t\treturn -1;\n \t\t\tif (!res) {\n \t\t\t\tif (!rename(newpath, path))\n-\t\t\t\t\treturn;\n+\t\t\t\t\treturn 0;\n \t\t\t\tunlink_or_warn(newpath);\n \t\t\t\tbreak;\n \t\t\t}\n@@ -4252,7 +4256,8 @@ static void create_one_file(struct apply_state *state,\n \t\t\t++nr;\n \t\t}\n \t}\n-\tdie_errno(_(\"unable to write file '%s' mode %o\"), path, mode);\n+\treturn error_errno(_(\"unable to write file '%s' mode %o\"),\n+\t\t\t   path, mode);\n }\n \n static int add_conflicted_stages_file(struct apply_state *state,\n@@ -4297,7 +4302,8 @@ static int create_file(struct apply_state *state, struct patch *patch)\n \n \tif (!mode)\n \t\tmode = S_IFREG | 0644;\n-\tcreate_one_file(state, path, mode, buf, size);\n+\tif (create_one_file(state, path, mode, buf, size))\n+\t\treturn -1;\n \n \tif (patch->conflicted_threeway)\n \t\treturn add_conflicted_stages_file(state, patch);\n-- \n2.9.0.172.gfb57a78\n\n"},{"id":"290298","messageId":"20160627182429.31550-26-chriscool@tuxfamily.org","threadId":"42726","inReplyTo":"20160627182429.31550-1-chriscool@tuxfamily.org","subject":"[PATCH v8 25/41] builtin/apply: make write_out_results() return -1 on error","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-27T18:24:13Z","receivedAt":"2016-06-27T18:26:48Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"To libify `git apply` functionality we have to signal errors to the\ncaller instead of exit()ing.\n\nTo do that in a compatible manner with the rest of the error handling\nin \"builtin/apply.c\", write_out_results() should return -1 instead of\ncalling exit().\n\nHelped-by: Eric Sunshine <sunshine@sunshineco.com>\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin/apply.c | 27 +++++++++++++++++++++------\n 1 file changed, 21 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex 6ec87e6..f54b8c5 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -4383,6 +4383,12 @@ static int write_out_one_reject(struct apply_state *state, struct patch *patch)\n \treturn -1;\n }\n \n+/*\n+ * Returns:\n+ *  -1 if an error happened\n+ *   0 if the patch applied cleanly\n+ *   1 if the patch did not apply cleanly\n+ */\n static int write_out_results(struct apply_state *state, struct patch *list)\n {\n \tint phase;\n@@ -4396,8 +4402,10 @@ static int write_out_results(struct apply_state *state, struct patch *list)\n \t\t\tif (l->rejected)\n \t\t\t\terrs = 1;\n \t\t\telse {\n-\t\t\t\tif (write_out_one_result(state, l, phase))\n-\t\t\t\t\texit(128);\n+\t\t\t\tif (write_out_one_result(state, l, phase)) {\n+\t\t\t\t\tstring_list_clear(&cpath, 0);\n+\t\t\t\t\treturn -1;\n+\t\t\t\t}\n \t\t\t\tif (phase == 1) {\n \t\t\t\t\tif (write_out_one_reject(state, l))\n \t\t\t\t\t\terrs = 1;\n@@ -4517,10 +4525,17 @@ static int apply_patch(struct apply_state *state,\n \t\t}\n \t}\n \n-\tif (state->apply && write_out_results(state, list)) {\n-\t\t/* with --3way, we still need to write the index out */\n-\t\tres = state->apply_with_reject ? -1 : 1;\n-\t\tgoto end;\n+\tif (state->apply) {\n+\t\tint write_res = write_out_results(state, list);\n+\t\tif (write_res < 0) {\n+\t\t\tres = -128;\n+\t\t\tgoto end;\n+\t\t}\n+\t\tif (write_res > 0) {\n+\t\t\t/* with --3way, we still need to write the index out */\n+\t\t\tres = state->apply_with_reject ? -1 : 1;\n+\t\t\tgoto end;\n+\t\t}\n \t}\n \n \tif (state->fake_ancestor &&\n-- \n2.9.0.172.gfb57a78\n\n"},{"id":"290299","messageId":"20160627182429.31550-16-chriscool@tuxfamily.org","threadId":"42726","inReplyTo":"20160627182429.31550-1-chriscool@tuxfamily.org","subject":"[PATCH v8 15/41] builtin/apply: make parse_traditional_patch() return -1 on error","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-27T18:24:03Z","receivedAt":"2016-06-27T18:26:49Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"To libify `git apply` functionality we have to signal errors to the\ncaller instead of die()ing.\n\nTo do that in a compatible manner with the rest of the error handling\nin \"builtin/apply.c\", parse_traditional_patch() should return -1\ninstead of calling die().\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin/apply.c | 15 +++++++++------\n 1 file changed, 9 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex 29ca524..174dbf3 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -755,10 +755,10 @@ static int has_epoch_timestamp(const char *nameline)\n  * files, we can happily check the index for a match, but for creating a\n  * new file we should try to match whatever \"patch\" does. I have no idea.\n  */\n-static void parse_traditional_patch(struct apply_state *state,\n-\t\t\t\t    const char *first,\n-\t\t\t\t    const char *second,\n-\t\t\t\t    struct patch *patch)\n+static int parse_traditional_patch(struct apply_state *state,\n+\t\t\t\t   const char *first,\n+\t\t\t\t   const char *second,\n+\t\t\t\t   struct patch *patch)\n {\n \tchar *name;\n \n@@ -803,7 +803,9 @@ static void parse_traditional_patch(struct apply_state *state,\n \t\t}\n \t}\n \tif (!name)\n-\t\tdie(_(\"unable to find filename in patch at line %d\"), state->linenr);\n+\t\treturn error(_(\"unable to find filename in patch at line %d\"), state->linenr);\n+\n+\treturn 0;\n }\n \n static int gitdiff_hdrend(struct apply_state *state,\n@@ -1467,7 +1469,8 @@ static int find_header(struct apply_state *state,\n \t\t\tcontinue;\n \n \t\t/* Ok, we'll consider it a patch */\n-\t\tparse_traditional_patch(state, line, line+len, patch);\n+\t\tif (parse_traditional_patch(state, line, line+len, patch))\n+\t\t\treturn -128;\n \t\t*hdrsize = len + nextlen;\n \t\tstate->linenr += 2;\n \t\treturn offset;\n-- \n2.9.0.172.gfb57a78\n\n"},{"id":"290300","messageId":"20160627182429.31550-12-chriscool@tuxfamily.org","threadId":"42726","inReplyTo":"20160627182429.31550-1-chriscool@tuxfamily.org","subject":"[PATCH v8 11/41] apply: make init_apply_state() return -1 instead of exit()ing","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-27T18:23:59Z","receivedAt":"2016-06-27T18:26:51Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"To libify `git apply` functionality we have to signal errors to the\ncaller instead of exit()ing.\n\nTo do that in a compatible manner with the rest of the error handling\nin \"builtin/apply.c\", init_apply_state() should return -1 instead of\ncalling exit().\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n apply.c         | 11 ++++++-----\n apply.h         |  6 +++---\n builtin/apply.c |  3 ++-\n 3 files changed, 11 insertions(+), 9 deletions(-)\n\ndiff --git a/apply.c b/apply.c\nindex 1f31bb4..c5a9ee2 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -55,9 +55,9 @@ int parse_ignorewhitespace_option(struct apply_state *state,\n \treturn error(_(\"unrecognized whitespace ignore option '%s'\"), option);\n }\n \n-void init_apply_state(struct apply_state *state,\n-\t\t      const char *prefix,\n-\t\t      struct lock_file *lock_file)\n+int init_apply_state(struct apply_state *state,\n+\t\t     const char *prefix,\n+\t\t     struct lock_file *lock_file)\n {\n \tmemset(state, 0, sizeof(*state));\n \tstate->prefix = prefix;\n@@ -76,9 +76,10 @@ void init_apply_state(struct apply_state *state,\n \n \tgit_apply_config();\n \tif (apply_default_whitespace && parse_whitespace_option(state, apply_default_whitespace))\n-\t\texit(1);\n+\t\treturn -1;\n \tif (apply_default_ignorewhitespace && parse_ignorewhitespace_option(state, apply_default_ignorewhitespace))\n-\t\texit(1);\n+\t\treturn -1;\n+\treturn 0;\n }\n \n void clear_apply_state(struct apply_state *state)\ndiff --git a/apply.h b/apply.h\nindex 08c0a25..e18a18a 100644\n--- a/apply.h\n+++ b/apply.h\n@@ -102,9 +102,9 @@ extern int parse_whitespace_option(struct apply_state *state,\n extern int parse_ignorewhitespace_option(struct apply_state *state,\n \t\t\t\t\t const char *option);\n \n-extern void init_apply_state(struct apply_state *state,\n-\t\t\t     const char *prefix,\n-\t\t\t     struct lock_file *lock_file);\n+extern int init_apply_state(struct apply_state *state,\n+\t\t\t    const char *prefix,\n+\t\t\t    struct lock_file *lock_file);\n extern void clear_apply_state(struct apply_state *state);\n \n #endif\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex 4fbd993..2a4a9d1 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -4741,7 +4741,8 @@ int cmd_apply(int argc, const char **argv, const char *prefix)\n \t\tOPT_END()\n \t};\n \n-\tinit_apply_state(&state, prefix, &lock_file);\n+\tif (init_apply_state(&state, prefix, &lock_file))\n+\t\texit(128);\n \n \targc = parse_options(argc, argv, state.prefix, builtin_apply_options,\n \t\t\tapply_usage, 0);\n-- \n2.9.0.172.gfb57a78\n\n"},{"id":"290301","messageId":"20160627182429.31550-3-chriscool@tuxfamily.org","threadId":"42726","inReplyTo":"20160627182429.31550-1-chriscool@tuxfamily.org","subject":"[PATCH v8 02/41] apply: move 'struct apply_state' to apply.h","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-27T18:23:50Z","receivedAt":"2016-06-27T18:26:55Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"To libify `git apply` functionality we must make 'struct apply_state'\nusable outside \"builtin/apply.c\".\n\nLet's do that by creating a new \"apply.h\" and moving\n'struct apply_state' there.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n apply.h         | 100 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n builtin/apply.c |  98 +-----------------------------------------------------\n 2 files changed, 101 insertions(+), 97 deletions(-)\n create mode 100644 apply.h\n\ndiff --git a/apply.h b/apply.h\nnew file mode 100644\nindex 0000000..7493a40\n--- /dev/null\n+++ b/apply.h\n@@ -0,0 +1,100 @@\n+#ifndef APPLY_H\n+#define APPLY_H\n+\n+enum apply_ws_error_action {\n+\tnowarn_ws_error,\n+\twarn_on_ws_error,\n+\tdie_on_ws_error,\n+\tcorrect_ws_error\n+};\n+\n+enum apply_ws_ignore {\n+\tignore_ws_none,\n+\tignore_ws_change\n+};\n+\n+/*\n+ * We need to keep track of how symlinks in the preimage are\n+ * manipulated by the patches.  A patch to add a/b/c where a/b\n+ * is a symlink should not be allowed to affect the directory\n+ * the symlink points at, but if the same patch removes a/b,\n+ * it is perfectly fine, as the patch removes a/b to make room\n+ * to create a directory a/b so that a/b/c can be created.\n+ *\n+ * See also \"struct string_list symlink_changes\" in \"struct\n+ * apply_state\".\n+ */\n+#define APPLY_SYMLINK_GOES_AWAY 01\n+#define APPLY_SYMLINK_IN_RESULT 02\n+\n+struct apply_state {\n+\tconst char *prefix;\n+\tint prefix_length;\n+\n+\t/* These are lock_file related */\n+\tstruct lock_file *lock_file;\n+\tint newfd;\n+\n+\t/* These control what gets looked at and modified */\n+\tint apply; /* this is not a dry-run */\n+\tint cached; /* apply to the index only */\n+\tint check; /* preimage must match working tree, don't actually apply */\n+\tint check_index; /* preimage must match the indexed version */\n+\tint update_index; /* check_index && apply */\n+\n+\t/* These control cosmetic aspect of the output */\n+\tint diffstat; /* just show a diffstat, and don't actually apply */\n+\tint numstat; /* just show a numeric diffstat, and don't actually apply */\n+\tint summary; /* just report creation, deletion, etc, and don't actually apply */\n+\n+\t/* These boolean parameters control how the apply is done */\n+\tint allow_overlap;\n+\tint apply_in_reverse;\n+\tint apply_with_reject;\n+\tint apply_verbosely;\n+\tint no_add;\n+\tint threeway;\n+\tint unidiff_zero;\n+\tint unsafe_paths;\n+\n+\t/* Other non boolean parameters */\n+\tconst char *fake_ancestor;\n+\tconst char *patch_input_file;\n+\tint line_termination;\n+\tstruct strbuf root;\n+\tint p_value;\n+\tint p_value_known;\n+\tunsigned int p_context;\n+\n+\t/* Exclude and include path parameters */\n+\tstruct string_list limit_by_name;\n+\tint has_include;\n+\n+\t/* Various \"current state\" */\n+\tint linenr; /* current line number */\n+\tstruct string_list symlink_changes; /* we have to track symlinks */\n+\n+\t/*\n+\t * For \"diff-stat\" like behaviour, we keep track of the biggest change\n+\t * we've seen, and the longest filename. That allows us to do simple\n+\t * scaling.\n+\t */\n+\tint max_change;\n+\tint max_len;\n+\n+\t/*\n+\t * Records filenames that have been touched, in order to handle\n+\t * the case where more than one patches touch the same file.\n+\t */\n+\tstruct string_list fn_table;\n+\n+\t/* These control whitespace errors */\n+\tenum apply_ws_error_action ws_error_action;\n+\tenum apply_ws_ignore ws_ignore_action;\n+\tconst char *whitespace_option;\n+\tint whitespace_error;\n+\tint squelch_whitespace_errors;\n+\tint applied_after_fixing_ws;\n+};\n+\n+#endif\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex c4d9396..ad216d3 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -20,103 +20,7 @@\n #include \"xdiff-interface.h\"\n #include \"ll-merge.h\"\n #include \"rerere.h\"\n-\n-enum apply_ws_error_action {\n-\tnowarn_ws_error,\n-\twarn_on_ws_error,\n-\tdie_on_ws_error,\n-\tcorrect_ws_error\n-};\n-\n-\n-enum apply_ws_ignore {\n-\tignore_ws_none,\n-\tignore_ws_change\n-};\n-\n-/*\n- * We need to keep track of how symlinks in the preimage are\n- * manipulated by the patches.  A patch to add a/b/c where a/b\n- * is a symlink should not be allowed to affect the directory\n- * the symlink points at, but if the same patch removes a/b,\n- * it is perfectly fine, as the patch removes a/b to make room\n- * to create a directory a/b so that a/b/c can be created.\n- *\n- * See also \"struct string_list symlink_changes\" in \"struct\n- * apply_state\".\n- */\n-#define APPLY_SYMLINK_GOES_AWAY 01\n-#define APPLY_SYMLINK_IN_RESULT 02\n-\n-struct apply_state {\n-\tconst char *prefix;\n-\tint prefix_length;\n-\n-\t/* These are lock_file related */\n-\tstruct lock_file *lock_file;\n-\tint newfd;\n-\n-\t/* These control what gets looked at and modified */\n-\tint apply; /* this is not a dry-run */\n-\tint cached; /* apply to the index only */\n-\tint check; /* preimage must match working tree, don't actually apply */\n-\tint check_index; /* preimage must match the indexed version */\n-\tint update_index; /* check_index && apply */\n-\n-\t/* These control cosmetic aspect of the output */\n-\tint diffstat; /* just show a diffstat, and don't actually apply */\n-\tint numstat; /* just show a numeric diffstat, and don't actually apply */\n-\tint summary; /* just report creation, deletion, etc, and don't actually apply */\n-\n-\t/* These boolean parameters control how the apply is done */\n-\tint allow_overlap;\n-\tint apply_in_reverse;\n-\tint apply_with_reject;\n-\tint apply_verbosely;\n-\tint no_add;\n-\tint threeway;\n-\tint unidiff_zero;\n-\tint unsafe_paths;\n-\n-\t/* Other non boolean parameters */\n-\tconst char *fake_ancestor;\n-\tconst char *patch_input_file;\n-\tint line_termination;\n-\tstruct strbuf root;\n-\tint p_value;\n-\tint p_value_known;\n-\tunsigned int p_context;\n-\n-\t/* Exclude and include path parameters */\n-\tstruct string_list limit_by_name;\n-\tint has_include;\n-\n-\t/* Various \"current state\" */\n-\tint linenr; /* current line number */\n-\tstruct string_list symlink_changes; /* we have to track symlinks */\n-\n-\t/*\n-\t * For \"diff-stat\" like behaviour, we keep track of the biggest change\n-\t * we've seen, and the longest filename. That allows us to do simple\n-\t * scaling.\n-\t */\n-\tint max_change;\n-\tint max_len;\n-\n-\t/*\n-\t * Records filenames that have been touched, in order to handle\n-\t * the case where more than one patches touch the same file.\n-\t */\n-\tstruct string_list fn_table;\n-\n-\t/* These control whitespace errors */\n-\tenum apply_ws_error_action ws_error_action;\n-\tenum apply_ws_ignore ws_ignore_action;\n-\tconst char *whitespace_option;\n-\tint whitespace_error;\n-\tint squelch_whitespace_errors;\n-\tint applied_after_fixing_ws;\n-};\n+#include \"apply.h\"\n \n static const char * const apply_usage[] = {\n \tN_(\"git apply [<options>] [<patch>...]\"),\n-- \n2.9.0.172.gfb57a78\n\n"},{"id":"290302","messageId":"20160627182429.31550-5-chriscool@tuxfamily.org","threadId":"42726","inReplyTo":"20160627182429.31550-1-chriscool@tuxfamily.org","subject":"[PATCH v8 04/41] builtin/apply: read_patch_file() return -1 instead of die()ing","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-27T18:23:52Z","receivedAt":"2016-06-27T18:26:56Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"To libify `git apply` functionality we have to signal errors to the\ncaller instead of die()ing. Let's do that by returning -1 instead of\ndie()ing in read_patch_file().\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin/apply.c | 8 +++++---\n 1 file changed, 5 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex 2aea203..ba716c1 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -335,10 +335,10 @@ static void say_patch_name(FILE *output, const char *fmt, struct patch *patch)\n \n #define SLOP (16)\n \n-static void read_patch_file(struct strbuf *sb, int fd)\n+static int read_patch_file(struct strbuf *sb, int fd)\n {\n \tif (strbuf_read(sb, fd, 0) < 0)\n-\t\tdie_errno(\"git apply: failed to read\");\n+\t\treturn error_errno(\"git apply: failed to read\");\n \n \t/*\n \t * Make sure that we have some slop in the buffer\n@@ -347,6 +347,7 @@ static void read_patch_file(struct strbuf *sb, int fd)\n \t */\n \tstrbuf_grow(sb, SLOP);\n \tmemset(sb->buf + sb->len, 0, SLOP);\n+\treturn 0;\n }\n \n static unsigned long linelen(const char *buffer, unsigned long size)\n@@ -4425,7 +4426,8 @@ static int apply_patch(struct apply_state *state,\n \tint res = 0;\n \n \tstate->patch_input_file = filename;\n-\tread_patch_file(&buf, fd);\n+\tif (read_patch_file(&buf, fd))\n+\t\treturn -128;\n \toffset = 0;\n \twhile (offset < buf.len) {\n \t\tstruct patch *patch;\n-- \n2.9.0.172.gfb57a78\n\n"},{"id":"290303","messageId":"20160627182429.31550-9-chriscool@tuxfamily.org","threadId":"42726","inReplyTo":"20160627182429.31550-1-chriscool@tuxfamily.org","subject":"[PATCH v8 08/41] builtin/apply: make parse_whitespace_option() return -1 instead of die()ing","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-27T18:23:56Z","receivedAt":"2016-06-27T18:26:57Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"To libify `git apply` functionality we have to signal errors to the\ncaller instead of die()ing.\n\nTo do that in a compatible manner with the rest of the error handling\nin builtin/apply.c, parse_whitespace_option() should return -1 instead\nof calling die().\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin/apply.c | 23 ++++++++++++-----------\n 1 file changed, 12 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex 65927f1..40e8397 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -27,34 +27,34 @@ static const char * const apply_usage[] = {\n \tNULL\n };\n \n-static void parse_whitespace_option(struct apply_state *state, const char *option)\n+static int parse_whitespace_option(struct apply_state *state, const char *option)\n {\n \tif (!option) {\n \t\tstate->ws_error_action = warn_on_ws_error;\n-\t\treturn;\n+\t\treturn 0;\n \t}\n \tif (!strcmp(option, \"warn\")) {\n \t\tstate->ws_error_action = warn_on_ws_error;\n-\t\treturn;\n+\t\treturn 0;\n \t}\n \tif (!strcmp(option, \"nowarn\")) {\n \t\tstate->ws_error_action = nowarn_ws_error;\n-\t\treturn;\n+\t\treturn 0;\n \t}\n \tif (!strcmp(option, \"error\")) {\n \t\tstate->ws_error_action = die_on_ws_error;\n-\t\treturn;\n+\t\treturn 0;\n \t}\n \tif (!strcmp(option, \"error-all\")) {\n \t\tstate->ws_error_action = die_on_ws_error;\n \t\tstate->squelch_whitespace_errors = 0;\n-\t\treturn;\n+\t\treturn 0;\n \t}\n \tif (!strcmp(option, \"strip\") || !strcmp(option, \"fix\")) {\n \t\tstate->ws_error_action = correct_ws_error;\n-\t\treturn;\n+\t\treturn 0;\n \t}\n-\tdie(_(\"unrecognized whitespace option '%s'\"), option);\n+\treturn error(_(\"unrecognized whitespace option '%s'\"), option);\n }\n \n static void parse_ignorewhitespace_option(struct apply_state *state,\n@@ -4589,7 +4589,8 @@ static int option_parse_whitespace(const struct option *opt,\n {\n \tstruct apply_state *state = opt->value;\n \tstate->whitespace_option = arg;\n-\tparse_whitespace_option(state, arg);\n+\tif (parse_whitespace_option(state, arg))\n+\t\texit(1);\n \treturn 0;\n }\n \n@@ -4623,8 +4624,8 @@ static void init_apply_state(struct apply_state *state,\n \tstrbuf_init(&state->root, 0);\n \n \tgit_apply_config();\n-\tif (apply_default_whitespace)\n-\t\tparse_whitespace_option(state, apply_default_whitespace);\n+\tif (apply_default_whitespace && parse_whitespace_option(state, apply_default_whitespace))\n+\t\texit(1);\n \tif (apply_default_ignorewhitespace)\n \t\tparse_ignorewhitespace_option(state, apply_default_ignorewhitespace);\n }\n-- \n2.9.0.172.gfb57a78\n\n"},{"id":"290304","messageId":"20160627182429.31550-10-chriscool@tuxfamily.org","threadId":"42726","inReplyTo":"20160627182429.31550-1-chriscool@tuxfamily.org","subject":"[PATCH v8 09/41] builtin/apply: make parse_ignorewhitespace_option() return -1 instead of die()ing","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-27T18:23:57Z","receivedAt":"2016-06-27T18:26:58Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"To libify `git apply` functionality we have to signal errors to the\ncaller instead of die()ing.\n\nTo do that in a compatible manner with the rest of the error handling\nin \"builtin/apply.c\", parse_ignorewhitespace_option() should return\n-1 instead of calling die().\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin/apply.c | 14 +++++++-------\n 1 file changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex 40e8397..7d0bf66 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -57,20 +57,20 @@ static int parse_whitespace_option(struct apply_state *state, const char *option\n \treturn error(_(\"unrecognized whitespace option '%s'\"), option);\n }\n \n-static void parse_ignorewhitespace_option(struct apply_state *state,\n-\t\t\t\t\t  const char *option)\n+static int parse_ignorewhitespace_option(struct apply_state *state,\n+\t\t\t\t\t const char *option)\n {\n \tif (!option || !strcmp(option, \"no\") ||\n \t    !strcmp(option, \"false\") || !strcmp(option, \"never\") ||\n \t    !strcmp(option, \"none\")) {\n \t\tstate->ws_ignore_action = ignore_ws_none;\n-\t\treturn;\n+\t\treturn 0;\n \t}\n \tif (!strcmp(option, \"change\")) {\n \t\tstate->ws_ignore_action = ignore_ws_change;\n-\t\treturn;\n+\t\treturn 0;\n \t}\n-\tdie(_(\"unrecognized whitespace ignore option '%s'\"), option);\n+\treturn error(_(\"unrecognized whitespace ignore option '%s'\"), option);\n }\n \n static void set_default_whitespace_mode(struct apply_state *state)\n@@ -4626,8 +4626,8 @@ static void init_apply_state(struct apply_state *state,\n \tgit_apply_config();\n \tif (apply_default_whitespace && parse_whitespace_option(state, apply_default_whitespace))\n \t\texit(1);\n-\tif (apply_default_ignorewhitespace)\n-\t\tparse_ignorewhitespace_option(state, apply_default_ignorewhitespace);\n+\tif (apply_default_ignorewhitespace && parse_ignorewhitespace_option(state, apply_default_ignorewhitespace))\n+\t\texit(1);\n }\n \n static void clear_apply_state(struct apply_state *state)\n-- \n2.9.0.172.gfb57a78\n\n"},{"id":"290305","messageId":"20160627182429.31550-6-chriscool@tuxfamily.org","threadId":"42726","inReplyTo":"20160627182429.31550-1-chriscool@tuxfamily.org","subject":"[PATCH v8 05/41] builtin/apply: make find_header() return -128 instead of die()ing","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-27T18:23:53Z","receivedAt":"2016-06-27T18:27:01Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"To libify `git apply` functionality we have to signal errors to the\ncaller instead of die()ing.\n\nTo do that in a compatible manner with the rest of the error handling\nin builtin/apply.c, let's make find_header() return -128 instead of\ncalling die().\n\nWe could make it return -1, unfortunately find_header() already\nreturns -1 when no header is found.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin/apply.c       | 40 ++++++++++++++++++++++++++++------------\n t/t4254-am-corrupt.sh |  2 +-\n 2 files changed, 29 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex ba716c1..aff0d66 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -1419,6 +1419,14 @@ static int parse_fragment_header(const char *line, int len, struct fragment *fra\n \treturn offset;\n }\n \n+/*\n+ * Find file diff header\n+ *\n+ * Returns:\n+ *  -1 if no header was found\n+ *  -128 in case of error\n+ *   the size of the header in bytes (called \"offset\") otherwise\n+ */\n static int find_header(struct apply_state *state,\n \t\t       const char *line,\n \t\t       unsigned long size,\n@@ -1452,8 +1460,9 @@ static int find_header(struct apply_state *state,\n \t\t\tstruct fragment dummy;\n \t\t\tif (parse_fragment_header(line, len, &dummy) < 0)\n \t\t\t\tcontinue;\n-\t\t\tdie(_(\"patch fragment without header at line %d: %.*s\"),\n-\t\t\t    state->linenr, (int)len-1, line);\n+\t\t\terror(_(\"patch fragment without header at line %d: %.*s\"),\n+\t\t\t\t     state->linenr, (int)len-1, line);\n+\t\t\treturn -128;\n \t\t}\n \n \t\tif (size < len + 6)\n@@ -1468,19 +1477,23 @@ static int find_header(struct apply_state *state,\n \t\t\tif (git_hdr_len <= len)\n \t\t\t\tcontinue;\n \t\t\tif (!patch->old_name && !patch->new_name) {\n-\t\t\t\tif (!patch->def_name)\n-\t\t\t\t\tdie(Q_(\"git diff header lacks filename information when removing \"\n-\t\t\t\t\t       \"%d leading pathname component (line %d)\",\n-\t\t\t\t\t       \"git diff header lacks filename information when removing \"\n-\t\t\t\t\t       \"%d leading pathname components (line %d)\",\n-\t\t\t\t\t       state->p_value),\n-\t\t\t\t\t    state->p_value, state->linenr);\n+\t\t\t\tif (!patch->def_name) {\n+\t\t\t\t\terror(Q_(\"git diff header lacks filename information when removing \"\n+\t\t\t\t\t\t\t\"%d leading pathname component (line %d)\",\n+\t\t\t\t\t\t\t\"git diff header lacks filename information when removing \"\n+\t\t\t\t\t\t\t\"%d leading pathname components (line %d)\",\n+\t\t\t\t\t\t\tstate->p_value),\n+\t\t\t\t\t\t     state->p_value, state->linenr);\n+\t\t\t\t\treturn -128;\n+\t\t\t\t}\n \t\t\t\tpatch->old_name = xstrdup(patch->def_name);\n \t\t\t\tpatch->new_name = xstrdup(patch->def_name);\n \t\t\t}\n-\t\t\tif (!patch->is_delete && !patch->new_name)\n-\t\t\t\tdie(\"git diff header lacks filename information \"\n-\t\t\t\t    \"(line %d)\", state->linenr);\n+\t\t\tif (!patch->is_delete && !patch->new_name) {\n+\t\t\t\terror(\"git diff header lacks filename information \"\n+\t\t\t\t\t     \"(line %d)\", state->linenr);\n+\t\t\t\treturn -128;\n+\t\t\t}\n \t\t\tpatch->is_toplevel_relative = 1;\n \t\t\t*hdrsize = git_hdr_len;\n \t\t\treturn offset;\n@@ -1996,6 +2009,9 @@ static int parse_chunk(struct apply_state *state, char *buffer, unsigned long si\n \tint hdrsize, patchsize;\n \tint offset = find_header(state, buffer, size, &hdrsize, patch);\n \n+\tif (offset == -128)\n+\t\texit(128);\n+\n \tif (offset < 0)\n \t\treturn offset;\n \ndiff --git a/t/t4254-am-corrupt.sh b/t/t4254-am-corrupt.sh\nindex 85716dd..9bd7dd2 100755\n--- a/t/t4254-am-corrupt.sh\n+++ b/t/t4254-am-corrupt.sh\n@@ -29,7 +29,7 @@ test_expect_success 'try to apply corrupted patch' '\n '\n \n test_expect_success 'compare diagnostic; ensure file is still here' '\n-\techo \"fatal: git diff header lacks filename information (line 4)\" >expected &&\n+\techo \"error: git diff header lacks filename information (line 4)\" >expected &&\n \ttest_path_is_file f &&\n \ttest_cmp expected actual\n '\n-- \n2.9.0.172.gfb57a78\n\n"},{"id":"290306","messageId":"20160627182429.31550-19-chriscool@tuxfamily.org","threadId":"42726","inReplyTo":"20160627182429.31550-1-chriscool@tuxfamily.org","subject":"[PATCH v8 18/41] builtin/apply: change die_on_unsafe_path() to check_unsafe_path()","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-27T18:24:06Z","receivedAt":"2016-06-27T18:27:03Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"To libify `git apply` functionality we have to signal errors to the\ncaller instead of die()ing.\n\nTo do that in a compatible manner with the rest of the error handling\nin \"builtin/apply.c\", die_on_unsafe_path() should return a negative\ninteger instead of calling die(), so while doing that let's change\nits name to check_unsafe_path().\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin/apply.c | 32 +++++++++++++++++++++-----------\n 1 file changed, 21 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex b48b526..d3a9da2 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -3704,7 +3704,7 @@ static int path_is_beyond_symlink(struct apply_state *state, const char *name_)\n \treturn ret;\n }\n \n-static void die_on_unsafe_path(struct patch *patch)\n+static int check_unsafe_path(struct patch *patch)\n {\n \tconst char *old_name = NULL;\n \tconst char *new_name = NULL;\n@@ -3716,9 +3716,10 @@ static void die_on_unsafe_path(struct patch *patch)\n \t\tnew_name = patch->new_name;\n \n \tif (old_name && !verify_path(old_name))\n-\t\tdie(_(\"invalid path '%s'\"), old_name);\n+\t\treturn error(_(\"invalid path '%s'\"), old_name);\n \tif (new_name && !verify_path(new_name))\n-\t\tdie(_(\"invalid path '%s'\"), new_name);\n+\t\treturn error(_(\"invalid path '%s'\"), new_name);\n+\treturn 0;\n }\n \n /*\n@@ -3808,8 +3809,8 @@ static int check_patch(struct apply_state *state, struct patch *patch)\n \t\t}\n \t}\n \n-\tif (!state->unsafe_paths)\n-\t\tdie_on_unsafe_path(patch);\n+\tif (!state->unsafe_paths && check_unsafe_path(patch))\n+\t\treturn -128;\n \n \t/*\n \t * An attempt to read from or delete a path that is beyond a\n@@ -3837,10 +3838,14 @@ static int check_patch_list(struct apply_state *state, struct patch *patch)\n \tprepare_symlink_changes(state, patch);\n \tprepare_fn_table(state, patch);\n \twhile (patch) {\n+\t\tint res;\n \t\tif (state->apply_verbosely)\n \t\t\tsay_patch_name(stderr,\n \t\t\t\t       _(\"Checking patch %s...\"), patch);\n-\t\terr |= check_patch(state, patch);\n+\t\tres = check_patch(state, patch);\n+\t\tif (res == -128)\n+\t\t\treturn -128;\n+\t\terr |= res;\n \t\tpatch = patch->next;\n \t}\n \treturn err;\n@@ -4472,11 +4477,16 @@ static int apply_patch(struct apply_state *state,\n \t\tgoto end;\n \t}\n \n-\tif ((state->check || state->apply) &&\n-\t    check_patch_list(state, list) < 0 &&\n-\t    !state->apply_with_reject) {\n-\t\tres = -1;\n-\t\tgoto end;\n+\tif (state->check || state->apply) {\n+\t\tint r = check_patch_list(state, list);\n+\t\tif (r == -128) {\n+\t\t\tres = -128;\n+\t\t\tgoto end;\n+\t\t}\n+\t\tif (r < 0 && !state->apply_with_reject) {\n+\t\t\tres = -1;\n+\t\t\tgoto end;\n+\t\t}\n \t}\n \n \tif (state->apply && write_out_results(state, list)) {\n-- \n2.9.0.172.gfb57a78\n\n"},{"id":"290307","messageId":"20160627182429.31550-18-chriscool@tuxfamily.org","threadId":"42726","inReplyTo":"20160627182429.31550-1-chriscool@tuxfamily.org","subject":"[PATCH v8 17/41] builtin/apply: make gitdiff_*() return -1 on error","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-27T18:24:05Z","receivedAt":"2016-06-27T18:27:06Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"To libify `git apply` functionality we have to signal errors to the\ncaller instead of die()ing.\n\nTo do that in a compatible manner with the rest of the error handling\nin \"builtin/apply.c\", gitdiff_*() functions should return -1 instead\nof calling die().\n\nA previous patch made it possible for gitdiff_*() functions to\nreturn -1 in case of error. Let's take advantage of that to\nmake gitdiff_verify_name() return -1 on error, and to have\ngitdiff_oldname() and gitdiff_newname() directly return\nwhat gitdiff_verify_name() returns.\n\nHelped-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin/apply.c | 40 +++++++++++++++++++++-------------------\n 1 file changed, 21 insertions(+), 19 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex 0bed352..b48b526 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -827,54 +827,56 @@ static int gitdiff_hdrend(struct apply_state *state,\n #define DIFF_OLD_NAME 0\n #define DIFF_NEW_NAME 1\n \n-static void gitdiff_verify_name(struct apply_state *state,\n-\t\t\t\tconst char *line,\n-\t\t\t\tint isnull,\n-\t\t\t\tchar **name,\n-\t\t\t\tint side)\n+static int gitdiff_verify_name(struct apply_state *state,\n+\t\t\t       const char *line,\n+\t\t\t       int isnull,\n+\t\t\t       char **name,\n+\t\t\t       int side)\n {\n \tif (!*name && !isnull) {\n \t\t*name = find_name(state, line, NULL, state->p_value, TERM_TAB);\n-\t\treturn;\n+\t\treturn 0;\n \t}\n \n \tif (*name) {\n \t\tint len = strlen(*name);\n \t\tchar *another;\n \t\tif (isnull)\n-\t\t\tdie(_(\"git apply: bad git-diff - expected /dev/null, got %s on line %d\"),\n-\t\t\t    *name, state->linenr);\n+\t\t\treturn error(_(\"git apply: bad git-diff - expected /dev/null, got %s on line %d\"),\n+\t\t\t\t     *name, state->linenr);\n \t\tanother = find_name(state, line, NULL, state->p_value, TERM_TAB);\n-\t\tif (!another || memcmp(another, *name, len + 1))\n-\t\t\tdie((side == DIFF_NEW_NAME) ?\n+\t\tif (!another || memcmp(another, *name, len + 1)) {\n+\t\t\tfree(another);\n+\t\t\treturn error((side == DIFF_NEW_NAME) ?\n \t\t\t    _(\"git apply: bad git-diff - inconsistent new filename on line %d\") :\n \t\t\t    _(\"git apply: bad git-diff - inconsistent old filename on line %d\"), state->linenr);\n+\t\t}\n \t\tfree(another);\n \t} else {\n \t\t/* expect \"/dev/null\" */\n \t\tif (memcmp(\"/dev/null\", line, 9) || line[9] != '\\n')\n-\t\t\tdie(_(\"git apply: bad git-diff - expected /dev/null on line %d\"), state->linenr);\n+\t\t\treturn error(_(\"git apply: bad git-diff - expected /dev/null on line %d\"), state->linenr);\n \t}\n+\n+\treturn 0;\n }\n \n static int gitdiff_oldname(struct apply_state *state,\n \t\t\t   const char *line,\n \t\t\t   struct patch *patch)\n {\n-\tgitdiff_verify_name(state, line,\n-\t\t\t    patch->is_new, &patch->old_name,\n-\t\t\t    DIFF_OLD_NAME);\n-\treturn 0;\n+\treturn gitdiff_verify_name(state, line,\n+\t\t\t\t   patch->is_new, &patch->old_name,\n+\t\t\t\t   DIFF_OLD_NAME);\n }\n \n static int gitdiff_newname(struct apply_state *state,\n \t\t\t   const char *line,\n \t\t\t   struct patch *patch)\n {\n-\tgitdiff_verify_name(state, line,\n-\t\t\t    patch->is_delete, &patch->new_name,\n-\t\t\t    DIFF_NEW_NAME);\n-\treturn 0;\n+\treturn gitdiff_verify_name(state, line,\n+\t\t\t\t   patch->is_delete, &patch->new_name,\n+\t\t\t\t   DIFF_NEW_NAME);\n }\n \n static int gitdiff_oldmode(struct apply_state *state,\n-- \n2.9.0.172.gfb57a78\n\n"},{"id":"290308","messageId":"20160627182429.31550-11-chriscool@tuxfamily.org","threadId":"42726","inReplyTo":"20160627182429.31550-1-chriscool@tuxfamily.org","subject":"[PATCH v8 10/41] builtin/apply: move init_apply_state() to apply.c","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-27T18:23:58Z","receivedAt":"2016-06-27T18:27:09Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"To libify `git apply` functionality we must make init_apply_state()\nusable outside \"builtin/apply.c\".\n\nLet's do that by moving it into a new \"apply.c\".\n\nHelped-by: Eric Sunshine <sunshine@sunshineco.com>\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n Makefile        |  1 +\n apply.c         | 91 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n apply.h         | 10 +++++++\n builtin/apply.c | 88 -------------------------------------------------------\n 4 files changed, 102 insertions(+), 88 deletions(-)\n create mode 100644 apply.c\n\ndiff --git a/Makefile b/Makefile\nindex de5a030..51f8583 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -680,6 +680,7 @@ LIB_OBJS += abspath.o\n LIB_OBJS += advice.o\n LIB_OBJS += alias.o\n LIB_OBJS += alloc.o\n+LIB_OBJS += apply.o\n LIB_OBJS += archive.o\n LIB_OBJS += archive-tar.o\n LIB_OBJS += archive-zip.o\ndiff --git a/apply.c b/apply.c\nnew file mode 100644\nindex 0000000..1f31bb4\n--- /dev/null\n+++ b/apply.c\n@@ -0,0 +1,91 @@\n+#include \"cache.h\"\n+#include \"lockfile.h\"\n+#include \"apply.h\"\n+\n+static void git_apply_config(void)\n+{\n+\tgit_config_get_string_const(\"apply.whitespace\", &apply_default_whitespace);\n+\tgit_config_get_string_const(\"apply.ignorewhitespace\", &apply_default_ignorewhitespace);\n+\tgit_config(git_default_config, NULL);\n+}\n+\n+int parse_whitespace_option(struct apply_state *state, const char *option)\n+{\n+\tif (!option) {\n+\t\tstate->ws_error_action = warn_on_ws_error;\n+\t\treturn 0;\n+\t}\n+\tif (!strcmp(option, \"warn\")) {\n+\t\tstate->ws_error_action = warn_on_ws_error;\n+\t\treturn 0;\n+\t}\n+\tif (!strcmp(option, \"nowarn\")) {\n+\t\tstate->ws_error_action = nowarn_ws_error;\n+\t\treturn 0;\n+\t}\n+\tif (!strcmp(option, \"error\")) {\n+\t\tstate->ws_error_action = die_on_ws_error;\n+\t\treturn 0;\n+\t}\n+\tif (!strcmp(option, \"error-all\")) {\n+\t\tstate->ws_error_action = die_on_ws_error;\n+\t\tstate->squelch_whitespace_errors = 0;\n+\t\treturn 0;\n+\t}\n+\tif (!strcmp(option, \"strip\") || !strcmp(option, \"fix\")) {\n+\t\tstate->ws_error_action = correct_ws_error;\n+\t\treturn 0;\n+\t}\n+\treturn error(_(\"unrecognized whitespace option '%s'\"), option);\n+}\n+\n+int parse_ignorewhitespace_option(struct apply_state *state,\n+\t\t\t\t  const char *option)\n+{\n+\tif (!option || !strcmp(option, \"no\") ||\n+\t    !strcmp(option, \"false\") || !strcmp(option, \"never\") ||\n+\t    !strcmp(option, \"none\")) {\n+\t\tstate->ws_ignore_action = ignore_ws_none;\n+\t\treturn 0;\n+\t}\n+\tif (!strcmp(option, \"change\")) {\n+\t\tstate->ws_ignore_action = ignore_ws_change;\n+\t\treturn 0;\n+\t}\n+\treturn error(_(\"unrecognized whitespace ignore option '%s'\"), option);\n+}\n+\n+void init_apply_state(struct apply_state *state,\n+\t\t      const char *prefix,\n+\t\t      struct lock_file *lock_file)\n+{\n+\tmemset(state, 0, sizeof(*state));\n+\tstate->prefix = prefix;\n+\tstate->prefix_length = state->prefix ? strlen(state->prefix) : 0;\n+\tstate->lock_file = lock_file;\n+\tstate->newfd = -1;\n+\tstate->apply = 1;\n+\tstate->line_termination = '\\n';\n+\tstate->p_value = 1;\n+\tstate->p_context = UINT_MAX;\n+\tstate->squelch_whitespace_errors = 5;\n+\tstate->ws_error_action = warn_on_ws_error;\n+\tstate->ws_ignore_action = ignore_ws_none;\n+\tstate->linenr = 1;\n+\tstrbuf_init(&state->root, 0);\n+\n+\tgit_apply_config();\n+\tif (apply_default_whitespace && parse_whitespace_option(state, apply_default_whitespace))\n+\t\texit(1);\n+\tif (apply_default_ignorewhitespace && parse_ignorewhitespace_option(state, apply_default_ignorewhitespace))\n+\t\texit(1);\n+}\n+\n+void clear_apply_state(struct apply_state *state)\n+{\n+\tstring_list_clear(&state->limit_by_name, 0);\n+\tstring_list_clear(&state->symlink_changes, 0);\n+\tstrbuf_release(&state->root);\n+\n+\t/* &state->fn_table is cleared at the end of apply_patch() */\n+}\ndiff --git a/apply.h b/apply.h\nindex 7493a40..08c0a25 100644\n--- a/apply.h\n+++ b/apply.h\n@@ -97,4 +97,14 @@ struct apply_state {\n \tint applied_after_fixing_ws;\n };\n \n+extern int parse_whitespace_option(struct apply_state *state,\n+\t\t\t\t   const char *option);\n+extern int parse_ignorewhitespace_option(struct apply_state *state,\n+\t\t\t\t\t const char *option);\n+\n+extern void init_apply_state(struct apply_state *state,\n+\t\t\t     const char *prefix,\n+\t\t\t     struct lock_file *lock_file);\n+extern void clear_apply_state(struct apply_state *state);\n+\n #endif\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex 7d0bf66..4fbd993 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -27,52 +27,6 @@ static const char * const apply_usage[] = {\n \tNULL\n };\n \n-static int parse_whitespace_option(struct apply_state *state, const char *option)\n-{\n-\tif (!option) {\n-\t\tstate->ws_error_action = warn_on_ws_error;\n-\t\treturn 0;\n-\t}\n-\tif (!strcmp(option, \"warn\")) {\n-\t\tstate->ws_error_action = warn_on_ws_error;\n-\t\treturn 0;\n-\t}\n-\tif (!strcmp(option, \"nowarn\")) {\n-\t\tstate->ws_error_action = nowarn_ws_error;\n-\t\treturn 0;\n-\t}\n-\tif (!strcmp(option, \"error\")) {\n-\t\tstate->ws_error_action = die_on_ws_error;\n-\t\treturn 0;\n-\t}\n-\tif (!strcmp(option, \"error-all\")) {\n-\t\tstate->ws_error_action = die_on_ws_error;\n-\t\tstate->squelch_whitespace_errors = 0;\n-\t\treturn 0;\n-\t}\n-\tif (!strcmp(option, \"strip\") || !strcmp(option, \"fix\")) {\n-\t\tstate->ws_error_action = correct_ws_error;\n-\t\treturn 0;\n-\t}\n-\treturn error(_(\"unrecognized whitespace option '%s'\"), option);\n-}\n-\n-static int parse_ignorewhitespace_option(struct apply_state *state,\n-\t\t\t\t\t const char *option)\n-{\n-\tif (!option || !strcmp(option, \"no\") ||\n-\t    !strcmp(option, \"false\") || !strcmp(option, \"never\") ||\n-\t    !strcmp(option, \"none\")) {\n-\t\tstate->ws_ignore_action = ignore_ws_none;\n-\t\treturn 0;\n-\t}\n-\tif (!strcmp(option, \"change\")) {\n-\t\tstate->ws_ignore_action = ignore_ws_change;\n-\t\treturn 0;\n-\t}\n-\treturn error(_(\"unrecognized whitespace ignore option '%s'\"), option);\n-}\n-\n static void set_default_whitespace_mode(struct apply_state *state)\n {\n \tif (!state->whitespace_option && !apply_default_whitespace)\n@@ -4539,13 +4493,6 @@ end:\n \treturn res;\n }\n \n-static void git_apply_config(void)\n-{\n-\tgit_config_get_string_const(\"apply.whitespace\", &apply_default_whitespace);\n-\tgit_config_get_string_const(\"apply.ignorewhitespace\", &apply_default_ignorewhitespace);\n-\tgit_config(git_default_config, NULL);\n-}\n-\n static int option_parse_exclude(const struct option *opt,\n \t\t\t\tconst char *arg, int unset)\n {\n@@ -4604,41 +4551,6 @@ static int option_parse_directory(const struct option *opt,\n \treturn 0;\n }\n \n-static void init_apply_state(struct apply_state *state,\n-\t\t\t     const char *prefix,\n-\t\t\t     struct lock_file *lock_file)\n-{\n-\tmemset(state, 0, sizeof(*state));\n-\tstate->prefix = prefix;\n-\tstate->prefix_length = state->prefix ? strlen(state->prefix) : 0;\n-\tstate->lock_file = lock_file;\n-\tstate->newfd = -1;\n-\tstate->apply = 1;\n-\tstate->line_termination = '\\n';\n-\tstate->p_value = 1;\n-\tstate->p_context = UINT_MAX;\n-\tstate->squelch_whitespace_errors = 5;\n-\tstate->ws_error_action = warn_on_ws_error;\n-\tstate->ws_ignore_action = ignore_ws_none;\n-\tstate->linenr = 1;\n-\tstrbuf_init(&state->root, 0);\n-\n-\tgit_apply_config();\n-\tif (apply_default_whitespace && parse_whitespace_option(state, apply_default_whitespace))\n-\t\texit(1);\n-\tif (apply_default_ignorewhitespace && parse_ignorewhitespace_option(state, apply_default_ignorewhitespace))\n-\t\texit(1);\n-}\n-\n-static void clear_apply_state(struct apply_state *state)\n-{\n-\tstring_list_clear(&state->limit_by_name, 0);\n-\tstring_list_clear(&state->symlink_changes, 0);\n-\tstrbuf_release(&state->root);\n-\n-\t/* &state->fn_table is cleared at the end of apply_patch() */\n-}\n-\n static void check_apply_state(struct apply_state *state, int force_apply)\n {\n \tint is_not_gitdir = !startup_info->have_repository;\n-- \n2.9.0.172.gfb57a78\n\n"},{"id":"290309","messageId":"CAP8UFD2HnDSkEcRaNPiBtuoYz=u9y4YMRc0hB0ZSJGjZxmrnMQ@mail.gmail.com","threadId":"42726","inReplyTo":"20160627182429.31550-1-chriscool@tuxfamily.org","subject":"Re: [PATCH v8 00/41] libify apply and use lib in am, part 2","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-27T18:33:36Z","receivedAt":"2016-06-27T18:33:44Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Mon, Jun 27, 2016 at 8:23 PM, Christian Couder\n<christian.couder@gmail.com> wrote:\n>\n> I will send a diff between this version and the previous one, as a\n> reply to this email.\n\nHere is the diff:\n\ndiff --git a/apply.c b/apply.c\nindex 777798a..2ac22d3 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -1516,8 +1516,8 @@ static int parse_fragment_header(const char\n*line, int len, struct fragment *fra\n  * Find file diff header\n  *\n  * Returns:\n- *  -1 in case of error\n- *  -2 if no header was found\n+ *  -1 if no header was found\n+ *  -128 in case of error\n  *   the size of the header in bytes (called \"offset\") otherwise\n  */\n static int find_header(struct apply_state *state,\n@@ -1553,8 +1553,9 @@ static int find_header(struct apply_state *state,\n                        struct fragment dummy;\n                        if (parse_fragment_header(line, len, &dummy) < 0)\n                                continue;\n-                       return error(_(\"patch fragment without header\nat line %d: %.*s\"),\n+                       error(_(\"patch fragment without header at line\n%d: %.*s\"),\n                                     state->linenr, (int)len-1, line);\n+                       return -128;\n                }\n\n                if (size < len + 6)\n@@ -1567,23 +1568,27 @@ static int find_header(struct apply_state *state,\n                if (!memcmp(\"diff --git \", line, 11)) {\n                        int git_hdr_len = parse_git_header(state,\nline, len, size, patch);\n                        if (git_hdr_len < 0)\n-                               return -1;\n+                               return -128;\n                        if (git_hdr_len <= len)\n                                continue;\n                        if (!patch->old_name && !patch->new_name) {\n-                               if (!patch->def_name)\n-                                       return error(Q_(\"git diff\nheader lacks filename information when removing \"\n+                               if (!patch->def_name) {\n+                                       error(Q_(\"git diff header\nlacks filename information when removing \"\n                                                        \"%d leading\npathname component (line %d)\",\n                                                        \"git diff\nheader lacks filename information when removing \"\n                                                        \"%d leading\npathname components (line %d)\",\n                                                        state->p_value),\n                                                     state->p_value,\nstate->linenr);\n+                                       return -128;\n+                               }\n                                patch->old_name = xstrdup(patch->def_name);\n                                patch->new_name = xstrdup(patch->def_name);\n                        }\n-                       if (!patch->is_delete && !patch->new_name)\n-                               return error(\"git diff header lacks\nfilename information \"\n+                       if (!patch->is_delete && !patch->new_name) {\n+                               error(\"git diff header lacks filename\ninformation \"\n                                             \"(line %d)\", state->linenr);\n+                               return -128;\n+                       }\n                        patch->is_toplevel_relative = 1;\n                        *hdrsize = git_hdr_len;\n                        return offset;\n@@ -1604,12 +1609,12 @@ static int find_header(struct apply_state *state,\n\n                /* Ok, we'll consider it a patch */\n                if (parse_traditional_patch(state, line, line+len, patch))\n-                       return -1;\n+                       return -128;\n                *hdrsize = len + nextlen;\n                state->linenr += 2;\n                return offset;\n        }\n-       return -2;\n+       return -1;\n }\n\n static void record_ws_error(struct apply_state *state,\n@@ -2100,8 +2105,8 @@ static int use_patch(struct apply_state *state,\nstruct patch *p)\n  * Create fragments (i.e. patch hunks) and hang them to the given patch.\n  *\n  * Returns:\n- *   -1 on error,\n- *   -2 if no header was found,\n+ *   -1 if no header was found or parse_binary() failed,\n+ *   -128 on another error,\n  *   the number of bytes consumed otherwise,\n  *     so that the caller can call us again for the next patch.\n  */\n@@ -2128,7 +2133,7 @@ static int parse_chunk(struct apply_state\n*state, char *buffer, unsigned long si\n                                       patch);\n\n        if (patchsize < 0)\n-               return -1;\n+               return -128;\n\n        if (!patchsize) {\n                static const char git_binary[] = \"GIT binary patch\\n\";\n@@ -2172,8 +2177,10 @@ static int parse_chunk(struct apply_state\n*state, char *buffer, unsigned long si\n                 * empty to us here.\n                 */\n                if ((state->apply || state->check) &&\n-                   (!patch->is_binary && !metadata_changes(patch)))\n-                       return error(_(\"patch with only garbage at\nline %d\"), state->linenr);\n+                   (!patch->is_binary && !metadata_changes(patch))) {\n+                       error(_(\"patch with only garbage at line %d\"),\nstate->linenr);\n+                       return -128;\n+               }\n        }\n\n        return offset + hdrsize + patchsize;\n@@ -3781,11 +3788,11 @@ static void prepare_symlink_changes(struct\napply_state *state, struct patch *pat\n                if ((patch->old_name && S_ISLNK(patch->old_mode)) &&\n                    (patch->is_rename || patch->is_delete))\n                        /* the symlink at patch->old_name is removed */\n-                       register_symlink_changes(state,\npatch->old_name, SYMLINK_GOES_AWAY);\n+                       register_symlink_changes(state,\npatch->old_name, APPLY_SYMLINK_GOES_AWAY);\n\n                if (patch->new_name && S_ISLNK(patch->new_mode))\n                        /* the symlink at patch->new_name is created\nor remains */\n-                       register_symlink_changes(state,\npatch->new_name, SYMLINK_IN_RESULT);\n+                       register_symlink_changes(state,\npatch->new_name, APPLY_SYMLINK_IN_RESULT);\n        }\n }\n\n@@ -3800,9 +3807,9 @@ static int path_is_beyond_symlink_1(struct\napply_state *state, struct strbuf *na\n                        break;\n                name->buf[name->len] = '\\0';\n                change = check_symlink_changes(state, name->buf);\n-               if (change & SYMLINK_IN_RESULT)\n+               if (change & APPLY_SYMLINK_IN_RESULT)\n                        return 1;\n-               if (change & SYMLINK_GOES_AWAY)\n+               if (change & APPLY_SYMLINK_GOES_AWAY)\n                        /*\n                         * This cannot be \"return 0\", because we may\n                         * see a new one created at a higher level.\n@@ -3944,7 +3951,7 @@ static int check_patch(struct apply_state\n*state, struct patch *patch)\n        }\n\n        if (!state->unsafe_paths && check_unsafe_path(patch))\n-               return -1;\n+               return -128;\n\n        /*\n         * An attempt to read from or delete a path that is beyond a\n@@ -3972,10 +3979,14 @@ static int check_patch_list(struct apply_state\n*state, struct patch *patch)\n        prepare_symlink_changes(state, patch);\n        prepare_fn_table(state, patch);\n        while (patch) {\n+               int res;\n                if (state->apply_verbosely)\n                        say_patch_name(stderr,\n                                       _(\"Checking patch %s...\"), patch);\n-               err |= check_patch(state, patch);\n+               res = check_patch(state, patch);\n+               if (res == -128)\n+                       return -128;\n+               err |= res;\n                patch = patch->next;\n        }\n        return err;\n@@ -4597,9 +4608,10 @@ static int write_out_results(struct apply_state\n*state, struct patch *list)\n  * Try to apply a patch.\n  *\n  * Returns:\n- *  -1 if an error happened\n+ *  -128 if a bad error happened (like patch unreadable)\n+ *  -1 if patch did not apply and user cannot deal with it\n  *   0 if the patch applied\n- *   1 if the patch did not apply\n+ *   1 if the patch did not apply but user might fix it\n  */\n static int apply_patch(struct apply_state *state,\n                       int fd,\n@@ -4614,7 +4626,7 @@ static int apply_patch(struct apply_state *state,\n\n        state->patch_input_file = filename;\n        if (read_patch_file(&buf, fd))\n-               return -1;\n+               return -128;\n        offset = 0;\n        while (offset < buf.len) {\n                struct patch *patch;\n@@ -4626,8 +4638,8 @@ static int apply_patch(struct apply_state *state,\n                nr = parse_chunk(state, buf.buf + offset, buf.len -\noffset, patch);\n                if (nr < 0) {\n                        free_patch(patch);\n-                       if (nr == -1) {\n-                               res = -1;\n+                       if (nr == -128) {\n+                               res = -128;\n                                goto end;\n                        }\n                        break;\n@@ -4649,7 +4661,8 @@ static int apply_patch(struct apply_state *state,\n        }\n\n        if (!list && !skipped_patch) {\n-               res = error(_(\"unrecognized input\"));\n+               error(_(\"unrecognized input\"));\n+               res = -128;\n                goto end;\n        }\n\n@@ -4661,21 +4674,27 @@ static int apply_patch(struct apply_state *state,\n                state->newfd = hold_locked_index(state->lock_file, 1);\n\n        if (state->check_index && read_cache() < 0) {\n-               res = error(_(\"unable to read index file\"));\n+               error(_(\"unable to read index file\"));\n+               res = -128;\n                goto end;\n        }\n\n-       if ((state->check || state->apply) &&\n-           check_patch_list(state, list) < 0 &&\n-           !state->apply_with_reject) {\n-               res = -1;\n-               goto end;\n+       if (state->check || state->apply) {\n+               int r = check_patch_list(state, list);\n+               if (r == -128) {\n+                       res = -128;\n+                       goto end;\n+               }\n+               if (r < 0 && !state->apply_with_reject) {\n+                       res = -1;\n+                       goto end;\n+               }\n        }\n\n        if (state->apply) {\n                int write_res = write_out_results(state, list);\n                if (write_res < 0) {\n-                       res = -1;\n+                       res = -128;\n                        goto end;\n                }\n                if (write_res > 0) {\n@@ -4687,7 +4706,7 @@ static int apply_patch(struct apply_state *state,\n\n        if (state->fake_ancestor &&\n            build_fake_ancestor(list, state->fake_ancestor)) {\n-               res = -1;\n+               res = -128;\n                goto end;\n        }\n\n@@ -4772,7 +4791,6 @@ int apply_all_patches(struct apply_state *state,\n {\n        int i;\n        int res;\n-       int retval = -1;\n        int errs = 0;\n        int read_stdin = 1;\n\n@@ -4783,7 +4801,7 @@ int apply_all_patches(struct apply_state *state,\n                if (!strcmp(arg, \"-\")) {\n                        res = apply_patch(state, 0, \"<stdin>\", options);\n                        if (res < 0)\n-                               goto rollback_end;\n+                               goto end;\n                        errs |= res;\n                        read_stdin = 0;\n                        continue;\n@@ -4795,21 +4813,22 @@ int apply_all_patches(struct apply_state *state,\n                fd = open(arg, O_RDONLY);\n                if (fd < 0) {\n                        error(_(\"can't open patch '%s': %s\"), arg,\nstrerror(errno));\n-                       goto rollback_end;\n+                       res = -128;\n+                       goto end;\n                }\n                read_stdin = 0;\n                set_default_whitespace_mode(state);\n                res = apply_patch(state, fd, arg, options);\n                close(fd);\n                if (res < 0)\n-                       goto rollback_end;\n+                       goto end;\n                errs |= res;\n        }\n        set_default_whitespace_mode(state);\n        if (read_stdin) {\n                res = apply_patch(state, 0, \"<stdin>\", options);\n                if (res < 0)\n-                       goto rollback_end;\n+                       goto end;\n                errs |= res;\n        }\n\n@@ -4828,7 +4847,8 @@ int apply_all_patches(struct apply_state *state,\n                                 \"%d lines add whitespace errors.\",\n                                 state->whitespace_error),\n                              state->whitespace_error);\n-                       goto rollback_end;\n+                       res = -128;\n+                       goto end;\n                }\n                if (state->applied_after_fixing_ws && state->apply)\n                        warning(\"%d line%s applied after\"\n@@ -4846,14 +4866,15 @@ int apply_all_patches(struct apply_state *state,\n                res = write_locked_index(&the_index, state->lock_file,\nCOMMIT_LOCK);\n                if (res) {\n                        error(_(\"Unable to write new index file\"));\n-                       goto rollback_end;\n+                       res = -128;\n+                       goto end;\n                }\n                state->newfd = -1;\n        }\n\n-       retval = !!errs;\n+       res = !!errs;\n\n-rollback_end:\n+end:\n        if (state->newfd >= 0) {\n                rollback_lock_file(state->lock_file);\n                state->newfd = -1;\n@@ -4864,5 +4885,7 @@ rollback_end:\n                set_warn_routine(state->saved_warn_routine);\n        }\n\n-       return retval;\n+       if (res > -1)\n+               return res;\n+       return (res == -1 ? 1 : 128);\n }\ndiff --git a/apply.h b/apply.h\nindex c6cf33d..51a930a 100644\n--- a/apply.h\n+++ b/apply.h\n@@ -1,14 +1,14 @@\n #ifndef APPLY_H\n #define APPLY_H\n\n-enum ws_error_action {\n+enum apply_ws_error_action {\n        nowarn_ws_error,\n        warn_on_ws_error,\n        die_on_ws_error,\n        correct_ws_error\n };\n\n-enum ws_ignore {\n+enum apply_ws_ignore {\n        ignore_ws_none,\n        ignore_ws_change\n };\n@@ -24,8 +24,8 @@ enum ws_ignore {\n  * See also \"struct string_list symlink_changes\" in \"struct\n  * apply_state\".\n  */\n-#define SYMLINK_GOES_AWAY 01\n-#define SYMLINK_IN_RESULT 02\n+#define APPLY_SYMLINK_GOES_AWAY 01\n+#define APPLY_SYMLINK_IN_RESULT 02\n\n struct apply_state {\n        const char *prefix;\n@@ -94,8 +94,8 @@ struct apply_state {\n        void (*saved_warn_routine)(const char *warn, va_list params);\n\n        /* These control whitespace errors */\n-       enum ws_error_action ws_error_action;\n-       enum ws_ignore ws_ignore_action;\n+       enum apply_ws_error_action ws_error_action;\n+       enum apply_ws_ignore ws_ignore_action;\n        const char *whitespace_option;\n        int whitespace_error;\n        int squelch_whitespace_errors;\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex ddd61de..066cb29 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -87,13 +87,13 @@ int cmd_apply(int argc, const char **argv, const\nchar *prefix)\n        };\n\n        if (init_apply_state(&state, prefix, &lock_file))\n-               exit(1);\n+               exit(128);\n\n        argc = parse_options(argc, argv, state.prefix, builtin_apply_options,\n                        apply_usage, 0);\n\n        if (check_apply_state(&state, force_apply))\n-               exit(1);\n+               exit(128);\n\n        ret = apply_all_patches(&state, argc, argv, options);\n"},{"id":"290439","messageId":"xmqqoa6l6lia.fsf@gitster.mtv.corp.google.com","threadId":"42726","inReplyTo":"20160627182429.31550-34-chriscool@tuxfamily.org","subject":"Re: [PATCH v8 33/41] write_or_die: use warning() instead of fprintf(stderr, ...)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-28T21:39:41Z","receivedAt":"2016-06-28T21:39:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <christian.couder@gmail.com> writes:\n\n> diff --git a/write_or_die.c b/write_or_die.c\n> index 49e80aa..c29f677 100644\n> --- a/write_or_die.c\n> +++ b/write_or_die.c\n> @@ -87,8 +87,7 @@ int write_or_whine_pipe(int fd, const void *buf, size_t count, const char *msg)\n>  {\n>  \tif (write_in_full(fd, buf, count) < 0) {\n>  \t\tcheck_pipe(errno);\n> -\t\tfprintf(stderr, \"%s: write error (%s)\\n\",\n> -\t\t\tmsg, strerror(errno));\n> +\t\twarning(\"%s: write error (%s)\\n\", msg, strerror(errno));\n>  \t\treturn 0;\n>  \t}\n>  \n> @@ -98,8 +97,7 @@ int write_or_whine_pipe(int fd, const void *buf, size_t count, const char *msg)\n>  int write_or_whine(int fd, const void *buf, size_t count, const char *msg)\n>  {\n>  \tif (write_in_full(fd, buf, count) < 0) {\n> -\t\tfprintf(stderr, \"%s: write error (%s)\\n\",\n> -\t\t\tmsg, strerror(errno));\n> +\t\twarning(\"%s: write error (%s)\\n\", msg, strerror(errno));\n>  \t\treturn 0;\n>  \t}\n\nI do not think you call write_or_whine() at all.  As another topic\nin flight removes the last caller of this function, this hunk is\nvery much unwelcome.  The only effect of it is to force me resolve\nunnecessary merge conflicts.\n"},{"id":"290584","messageId":"CAP8UFD2kXp2Ugojq9ktudOB=tpcsm3fxe2XYKtRQ9KdzqTug-g@mail.gmail.com","threadId":"42726","inReplyTo":"xmqqoa6l6lia.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v8 33/41] write_or_die: use warning() instead of fprintf(stderr, ...)","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-06-30T09:50:44Z","receivedAt":"2016-06-30T09:50:59Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Tue, Jun 28, 2016 at 11:39 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Christian Couder <christian.couder@gmail.com> writes:\n>\n>> @@ -98,8 +97,7 @@ int write_or_whine_pipe(int fd, const void *buf, size_t count, const char *msg)\n>>  int write_or_whine(int fd, const void *buf, size_t count, const char *msg)\n>>  {\n>>       if (write_in_full(fd, buf, count) < 0) {\n>> -             fprintf(stderr, \"%s: write error (%s)\\n\",\n>> -                     msg, strerror(errno));\n>> +             warning(\"%s: write error (%s)\\n\", msg, strerror(errno));\n>>               return 0;\n>>       }\n>\n> I do not think you call write_or_whine() at all.  As another topic\n> in flight removes the last caller of this function, this hunk is\n> very much unwelcome.  The only effect of it is to force me resolve\n> unnecessary merge conflicts.\n\nOk, sorry about that. I will remove the hunk.\n"},{"id":"292246","messageId":"xmqqinvs19ng.fsf@gitster.mtv.corp.google.com","threadId":"42726","inReplyTo":"20160627182429.31550-33-chriscool@tuxfamily.org","subject":"Re: [PATCH v8 32/41] environment: add set_index_file()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-26T19:28:35Z","receivedAt":"2016-07-26T19:30:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <christian.couder@gmail.com> writes:\n\n> Introduce set_index_file() to be able to temporarily change the index file.\n>\n> It should be used like this:\n>\n>     /* Save current index file */\n>     old_index_file = get_index_file();\n>     set_index_file((char *)tmp_index_file);\n>\n>     /* Do stuff that will use tmp_index_file as the index file */\n>     ...\n>\n>     /* When finished reset the index file */\n>     set_index_file(old_index_file);\n\nWHY is glaringly missing.\n\n> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n> ---\n\nDoesn't calling this have a global effect that is flowned upon when\nused by a library-ish function?  Who is the expected caller in this\nseries that wants to \"libify\" a part of the system?\n\n>  cache.h       |  1 +\n>  environment.c | 10 ++++++++++\n>  2 files changed, 11 insertions(+)\n>\n> diff --git a/cache.h b/cache.h\n> index c73becb..8854365 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -461,6 +461,7 @@ extern int is_inside_work_tree(void);\n>  extern const char *get_git_dir(void);\n>  extern const char *get_git_common_dir(void);\n>  extern char *get_object_directory(void);\n> +extern void set_index_file(char *index_file);\n>  extern char *get_index_file(void);\n>  extern char *get_graft_file(void);\n>  extern int set_git_dir(const char *path);\n> diff --git a/environment.c b/environment.c\n> index ca72464..7a53799 100644\n> --- a/environment.c\n> +++ b/environment.c\n> @@ -292,6 +292,16 @@ int odb_pack_keep(char *name, size_t namesz, const unsigned char *sha1)\n>  \treturn open(name, O_RDWR|O_CREAT|O_EXCL, 0600);\n>  }\n>  \n> +/*\n> + * Temporarily change the index file.\n> + * Please save the current index file using get_index_file() before changing\n> + * the index file. And when finished, reset it to the saved value.\n> + */\n> +void set_index_file(char *index_file)\n> +{\n> +\tgit_index_file = index_file;\n> +}\n> +\n>  char *get_index_file(void)\n>  {\n>  \tif (!git_index_file)\n"},{"id":"292247","messageId":"xmqqeg6g19cv.fsf@gitster.mtv.corp.google.com","threadId":"42726","inReplyTo":"20160627182429.31550-35-chriscool@tuxfamily.org","subject":"Re: [PATCH v8 34/41] apply: add 'be_silent' variable to 'struct apply_state'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-26T19:34:56Z","receivedAt":"2016-07-26T19:35:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <christian.couder@gmail.com> writes:\n\n> This variable should prevent anything to be printed on both stderr\n> and stdout.\n\nIt is far more important to describe \"why\" this is needed than what\nit does, the latter of which can be read from the patch text.\n\nAnd I do not see any \"why\" here.  Is this \"when the current caller\nwanted to silence us, it spawned us in a separate process and\nredirected our output to /dev/null, but we no longer can do that\nbecause we will change the calling convention to allow direct calls\ninto us\"?\n\nDo we have a precedent to name a switch that we usually call \"quiet\"\nor \"silent\" as \"be_{silent,quiet}\"?  Is there already a \"silent\"\nnearby that records what the end-user gave us (e.g. via \"--silent\"\noption), a new name may be needed, but if that is the motivation,\nI'd probably call it something more specific, \"apply_silently\" or\nsomesuch.\n\n> Let's not take care of stdout and apply_verbosely for now though,\n> as that will be taken care of in following patches.\n>\n> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n> ---\n>  apply.c | 43 +++++++++++++++++++++++++++++--------------\n>  apply.h |  1 +\n>  2 files changed, 30 insertions(+), 14 deletions(-)\n>\n> diff --git a/apply.c b/apply.c\n> index 7bf12a7..802fa79 100644\n> --- a/apply.c\n> +++ b/apply.c\n> @@ -1617,8 +1617,9 @@ static void record_ws_error(struct apply_state *state,\n>  \t\treturn;\n>  \n>  \terr = whitespace_error_string(result);\n> -\tfprintf(stderr, \"%s:%d: %s.\\n%.*s\\n\",\n> -\t\tstate->patch_input_file, linenr, err, len, line);\n> +\tif (!state->be_silent)\n> +\t\tfprintf(stderr, \"%s:%d: %s.\\n%.*s\\n\",\n> +\t\t\tstate->patch_input_file, linenr, err, len, line);\n>  \tfree(err);\n>  }\n>  \n> @@ -1813,7 +1814,7 @@ static int parse_single_patch(struct apply_state *state,\n>  \t\treturn error(_(\"new file %s depends on old contents\"), patch->new_name);\n>  \tif (0 < patch->is_delete && newlines)\n>  \t\treturn error(_(\"deleted file %s still has contents\"), patch->old_name);\n> -\tif (!patch->is_delete && !newlines && context)\n> +\tif (!patch->is_delete && !newlines && context && !state->be_silent)\n>  \t\tfprintf_ln(stderr,\n>  \t\t\t   _(\"** warning: \"\n>  \t\t\t     \"file %s becomes empty but is not deleted\"),\n> @@ -3038,8 +3039,8 @@ static int apply_one_fragment(struct apply_state *state,\n>  \t\t * Warn if it was necessary to reduce the number\n>  \t\t * of context lines.\n>  \t\t */\n> -\t\tif ((leading != frag->leading) ||\n> -\t\t    (trailing != frag->trailing))\n> +\t\tif ((leading != frag->leading ||\n> +\t\t     trailing != frag->trailing) && !state->be_silent)\n>  \t\t\tfprintf_ln(stderr, _(\"Context reduced to (%ld/%ld)\"\n>  \t\t\t\t\t     \" to apply fragment at %d\"),\n>  \t\t\t\t   leading, trailing, applied_pos+1);\n> @@ -3536,7 +3537,8 @@ static int try_threeway(struct apply_state *state,\n>  \t\t read_blob_object(&buf, pre_sha1, patch->old_mode))\n>  \t\treturn error(\"repository lacks the necessary blob to fall back on 3-way merge.\");\n>  \n> -\tfprintf(stderr, \"Falling back to three-way merge...\\n\");\n> +\tif (!state->be_silent)\n> +\t\tfprintf(stderr, \"Falling back to three-way merge...\\n\");\n>  \n>  \timg = strbuf_detach(&buf, &len);\n>  \tprepare_image(&tmp_image, img, len, 1);\n> @@ -3566,7 +3568,9 @@ static int try_threeway(struct apply_state *state,\n>  \tstatus = three_way_merge(image, patch->new_name,\n>  \t\t\t\t pre_sha1, our_sha1, post_sha1);\n>  \tif (status < 0) {\n> -\t\tfprintf(stderr, \"Failed to fall back on three-way merge...\\n\");\n> +\t\tif (!state->be_silent)\n> +\t\t\tfprintf(stderr,\n> +\t\t\t\t\"Failed to fall back on three-way merge...\\n\");\n>  \t\treturn status;\n>  \t}\n>  \n> @@ -3578,9 +3582,15 @@ static int try_threeway(struct apply_state *state,\n>  \t\t\thashcpy(patch->threeway_stage[0].hash, pre_sha1);\n>  \t\thashcpy(patch->threeway_stage[1].hash, our_sha1);\n>  \t\thashcpy(patch->threeway_stage[2].hash, post_sha1);\n> -\t\tfprintf(stderr, \"Applied patch to '%s' with conflicts.\\n\", patch->new_name);\n> +\t\tif (!state->be_silent)\n> +\t\t\tfprintf(stderr,\n> +\t\t\t\t\"Applied patch to '%s' with conflicts.\\n\",\n> +\t\t\t\tpatch->new_name);\n>  \t} else {\n> -\t\tfprintf(stderr, \"Applied patch to '%s' cleanly.\\n\", patch->new_name);\n> +\t\tif (!state->be_silent)\n> +\t\t\tfprintf(stderr,\n> +\t\t\t\t\"Applied patch to '%s' cleanly.\\n\",\n> +\t\t\t\tpatch->new_name);\n>  \t}\n>  \treturn 0;\n>  }\n> @@ -4483,7 +4493,8 @@ static int write_out_one_reject(struct apply_state *state, struct patch *patch)\n>  \t\t\t    \"Applying patch %%s with %d rejects...\",\n>  \t\t\t    cnt),\n>  \t\t    cnt);\n> -\tsay_patch_name(stderr, sb.buf, patch);\n> +\tif (!state->be_silent)\n> +\t\tsay_patch_name(stderr, sb.buf, patch);\n>  \tstrbuf_release(&sb);\n>  \n>  \tcnt = strlen(patch->new_name);\n> @@ -4510,10 +4521,12 @@ static int write_out_one_reject(struct apply_state *state, struct patch *patch)\n>  \t     frag;\n>  \t     cnt++, frag = frag->next) {\n>  \t\tif (!frag->rejected) {\n> -\t\t\tfprintf_ln(stderr, _(\"Hunk #%d applied cleanly.\"), cnt);\n> +\t\t\tif (!state->be_silent)\n> +\t\t\t\tfprintf_ln(stderr, _(\"Hunk #%d applied cleanly.\"), cnt);\n>  \t\t\tcontinue;\n>  \t\t}\n> -\t\tfprintf_ln(stderr, _(\"Rejected hunk #%d.\"), cnt);\n> +\t\tif (!state->be_silent)\n> +\t\t\tfprintf_ln(stderr, _(\"Rejected hunk #%d.\"), cnt);\n>  \t\tfprintf(rej, \"%.*s\", frag->size, frag->patch);\n>  \t\tif (frag->patch[frag->size-1] != '\\n')\n>  \t\t\tfputc('\\n', rej);\n> @@ -4562,8 +4575,10 @@ static int write_out_results(struct apply_state *state, struct patch *list)\n>  \t\tstruct string_list_item *item;\n>  \n>  \t\tstring_list_sort(&cpath);\n> -\t\tfor_each_string_list_item(item, &cpath)\n> -\t\t\tfprintf(stderr, \"U %s\\n\", item->string);\n> +\t\tif (!state->be_silent) {\n> +\t\t\tfor_each_string_list_item(item, &cpath)\n> +\t\t\t\tfprintf(stderr, \"U %s\\n\", item->string);\n> +\t\t}\n>  \t\tstring_list_clear(&cpath, 0);\n>  \n>  \t\trerere(0);\n> diff --git a/apply.h b/apply.h\n> index df44b51..44bed19 100644\n> --- a/apply.h\n> +++ b/apply.h\n> @@ -52,6 +52,7 @@ struct apply_state {\n>  \tint apply_in_reverse;\n>  \tint apply_with_reject;\n>  \tint apply_verbosely;\n> +\tint be_silent;\n>  \tint no_add;\n>  \tint threeway;\n>  \tint unidiff_zero;\n"},{"id":"292248","messageId":"xmqqa8h41989.fsf@gitster.mtv.corp.google.com","threadId":"42726","inReplyTo":"20160627182429.31550-36-chriscool@tuxfamily.org","subject":"Re: [PATCH v8 35/41] apply: make 'be_silent' incompatible with 'apply_verbosely'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-26T19:37:42Z","receivedAt":"2016-07-26T19:39:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <christian.couder@gmail.com> writes:\n\n> It should be an error to have both be_silent and apply_verbosely set,\n> so let's check that in check_apply_state().\n\nDoesn't that suggest that we do not want to have a new be_silent\nfield at all?  Perhaps we used to have apply_verbosely = <yes,no>\nresulting in only two verbosity levels, \"verbose\" and \"normal\", and\nyou want to have another new one \"total silence\" or something?\n\nIf so perhaps it would be more appropriate to rename apply_verbosely\nto apply_verbosity that is no longer a boolean?\n\n"},{"id":"292249","messageId":"xmqq60rs191n.fsf@gitster.mtv.corp.google.com","threadId":"42726","inReplyTo":"20160627182429.31550-37-chriscool@tuxfamily.org","subject":"Re: [PATCH v8 36/41] apply: don't print on stdout when be_silent is set","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-26T19:41:40Z","receivedAt":"2016-07-26T19:41:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <christian.couder@gmail.com> writes:\n\n> This variable should prevent anything to be printed on both stderr\n> and stdout.\n\nYou have to mention that skipping the entire callchain, not just the\n\"printing\" part, is safe.  I can see numstat_patch_list() is\nprobably safe as it does not do any computation other than calling\nprintf() and write_name_quoted(), but other two are not immediately\nobvious that what they compute are only used for their own printing\nand there is no other side effects left to affect what happens after\nthis function returns.\n\n\n> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n> ---\n>  apply.c | 6 +++---\n>  1 file changed, 3 insertions(+), 3 deletions(-)\n>\n> diff --git a/apply.c b/apply.c\n> index 1435f85..e2acc18 100644\n> --- a/apply.c\n> +++ b/apply.c\n> @@ -4698,13 +4698,13 @@ static int apply_patch(struct apply_state *state,\n>  \t\tgoto end;\n>  \t}\n>  \n> -\tif (state->diffstat)\n> +\tif (state->diffstat && !state->be_silent)\n>  \t\tstat_patch_list(state, list);\n>  \n> -\tif (state->numstat)\n> +\tif (state->numstat && !state->be_silent)\n>  \t\tnumstat_patch_list(state, list);\n>  \n> -\tif (state->summary)\n> +\tif (state->summary && !state->be_silent)\n>  \t\tsummary_patch_list(list);\n>  \n>  end:\n"},{"id":"292252","messageId":"xmqq1t2g18pz.fsf@gitster.mtv.corp.google.com","threadId":"42726","inReplyTo":"20160627182429.31550-41-chriscool@tuxfamily.org","subject":"Re: [PATCH v8 40/41] builtin/am: use apply api in run_apply()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-26T19:48:40Z","receivedAt":"2016-07-26T19:50:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <christian.couder@gmail.com> writes:\n\n>  static int run_apply(const struct am_state *state, const char *index_file)\n>  {\n> -\tstruct child_process cp = CHILD_PROCESS_INIT;\n> +\tstruct argv_array apply_paths = ARGV_ARRAY_INIT;\n> +\tstruct argv_array apply_opts = ARGV_ARRAY_INIT;\n> +\tstruct apply_state apply_state;\n> +\tint res, opts_left;\n> +\tchar *save_index_file;\n> +\tstatic struct lock_file lock_file;\n> +\n> +\tstruct option am_apply_options[] = {\n> +\t\t{ OPTION_CALLBACK, 0, \"whitespace\", &apply_state, N_(\"action\"),\n> +\t\t\tN_(\"detect new or modified lines that have whitespace errors\"),\n> +\t\t\t0, apply_option_parse_whitespace },\n> +\t\t{ OPTION_CALLBACK, 0, \"ignore-space-change\", &apply_state, NULL,\n> +\t\t\tN_(\"ignore changes in whitespace when finding context\"),\n> +\t\t\tPARSE_OPT_NOARG, apply_option_parse_space_change },\n> +\t\t{ OPTION_CALLBACK, 0, \"ignore-whitespace\", &apply_state, NULL,\n> +\t\t\tN_(\"ignore changes in whitespace when finding context\"),\n> +\t\t\tPARSE_OPT_NOARG, apply_option_parse_space_change },\n> +\t\t{ OPTION_CALLBACK, 0, \"directory\", &apply_state, N_(\"root\"),\n> +\t\t\tN_(\"prepend <root> to all filenames\"),\n> +\t\t\t0, apply_option_parse_directory },\n> +\t\t{ OPTION_CALLBACK, 0, \"exclude\", &apply_state, N_(\"path\"),\n> +\t\t\tN_(\"don't apply changes matching the given path\"),\n> +\t\t\t0, apply_option_parse_exclude },\n> +\t\t{ OPTION_CALLBACK, 0, \"include\", &apply_state, N_(\"path\"),\n> +\t\t\tN_(\"apply changes matching the given path\"),\n> +\t\t\t0, apply_option_parse_include },\n> +\t\tOPT_INTEGER('C', NULL, &apply_state.p_context,\n> +\t\t\t\tN_(\"ensure at least <n> lines of context match\")),\n> +\t\t{ OPTION_CALLBACK, 'p', NULL, &apply_state, N_(\"num\"),\n> +\t\t\tN_(\"remove <num> leading slashes from traditional diff paths\"),\n> +\t\t\t0, apply_option_parse_p },\n> +\t\tOPT_BOOL(0, \"reject\", &apply_state.apply_with_reject,\n> +\t\t\tN_(\"leave the rejected hunks in corresponding *.rej files\")),\n> +\t\tOPT_END()\n> +\t};\n\nIs this an indication that this step came too prematurely?\n\nCan we avoid having to maintain semi-duplicated options array if\ncmd_apply() is properly refactored before this step?  The resulting\ncode is unmaintainable as long as we have this duplication.\n\n\n"},{"id":"292282","messageId":"xmqqzip4xfmz.fsf@gitster.mtv.corp.google.com","threadId":"42726","inReplyTo":"20160627182429.31550-1-chriscool@tuxfamily.org","subject":"Re: [PATCH v8 00/41] libify apply and use lib in am, part 2","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-26T21:18:12Z","receivedAt":"2016-07-26T21:18:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <christian.couder@gmail.com> writes:\n\n> Sorry if this patch series is still long. I can split it into two or\n> more series if it is prefered.\n> ...\n> Christian Couder (41):\n>   apply: make some names more specific\n>   apply: move 'struct apply_state' to apply.h\n>   builtin/apply: make apply_patch() return -1 or -128 instead of\n>     die()ing\n>   builtin/apply: read_patch_file() return -1 instead of die()ing\n>   builtin/apply: make find_header() return -128 instead of die()ing\n>   builtin/apply: make parse_chunk() return a negative integer on error\n>   builtin/apply: make parse_single_patch() return -1 on error\n>   builtin/apply: make parse_whitespace_option() return -1 instead of\n>     die()ing\n>   builtin/apply: make parse_ignorewhitespace_option() return -1 instead\n>     of die()ing\n>   builtin/apply: move init_apply_state() to apply.c\n>   apply: make init_apply_state() return -1 instead of exit()ing\n>   builtin/apply: make check_apply_state() return -1 instead of die()ing\n>   builtin/apply: move check_apply_state() to apply.c\n>   builtin/apply: make apply_all_patches() return 128 or 1 on error\n>   builtin/apply: make parse_traditional_patch() return -1 on error\n>   builtin/apply: make gitdiff_*() return 1 at end of header\n>   builtin/apply: make gitdiff_*() return -1 on error\n>   builtin/apply: change die_on_unsafe_path() to check_unsafe_path()\n>   builtin/apply: make build_fake_ancestor() return -1 on error\n>   builtin/apply: make remove_file() return -1 on error\n>   builtin/apply: make add_conflicted_stages_file() return -1 on error\n>   builtin/apply: make add_index_file() return -1 on error\n>   builtin/apply: make create_file() return -1 on error\n>   builtin/apply: make write_out_one_result() return -1 on error\n>   builtin/apply: make write_out_results() return -1 on error\n>   builtin/apply: make try_create_file() return -1 on error\n>   builtin/apply: make create_one_file() return -1 on error\n>   builtin/apply: rename option parsing functions\n>   apply: rename and move opt constants to apply.h\n>   Move libified code from builtin/apply.c to apply.{c,h}\n>   apply: make some parsing functions static again\n>   environment: add set_index_file()\n>   write_or_die: use warning() instead of fprintf(stderr, ...)\n>   apply: add 'be_silent' variable to 'struct apply_state'\n>   apply: make 'be_silent' incompatible with 'apply_verbosely'\n>   apply: don't print on stdout when be_silent is set\n>   usage: add set_warn_routine()\n>   usage: add get_error_routine() and get_warn_routine()\n>   apply: change error_routine when be_silent is set\n>   builtin/am: use apply api in run_apply()\n>   apply: use error_errno() where possible\n\nI finally found time to get back to finish reviewing this.\n\nThe early part up to and including \"apply: make some parsing\nfunctions static again\" looked good and we could treat them as a\ncontinuation of the earlier cc/apply-introduce-state topic, which\nhas been merged to the 'master' already.\n\nI got an impression that the patches in the remainder of the series\nwere not as polished as the earlier ones, except for the last one,\nwhich should be reordered and be part of the early preparation step\nif possible.\n\nThanks.\n"},{"id":"292298","messageId":"CAP8UFD0tcPnqBcxC5-4tnGMT4W5b7L=C_riwpTvDfeMUjQzpjA@mail.gmail.com","threadId":"42726","inReplyTo":"xmqqzip4xfmz.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v8 00/41] libify apply and use lib in am, part 2","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-07-27T06:15:07Z","receivedAt":"2016-07-27T06:15:15Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Tue, Jul 26, 2016 at 11:18 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Christian Couder <christian.couder@gmail.com> writes:\n>\n>> Sorry if this patch series is still long. I can split it into two or\n>> more series if it is prefered.\n>> ...\n>> Christian Couder (41):\n>>   apply: make some names more specific\n>>   apply: move 'struct apply_state' to apply.h\n>>   builtin/apply: make apply_patch() return -1 or -128 instead of\n>>     die()ing\n>>   builtin/apply: read_patch_file() return -1 instead of die()ing\n>>   builtin/apply: make find_header() return -128 instead of die()ing\n>>   builtin/apply: make parse_chunk() return a negative integer on error\n>>   builtin/apply: make parse_single_patch() return -1 on error\n>>   builtin/apply: make parse_whitespace_option() return -1 instead of\n>>     die()ing\n>>   builtin/apply: make parse_ignorewhitespace_option() return -1 instead\n>>     of die()ing\n>>   builtin/apply: move init_apply_state() to apply.c\n>>   apply: make init_apply_state() return -1 instead of exit()ing\n>>   builtin/apply: make check_apply_state() return -1 instead of die()ing\n>>   builtin/apply: move check_apply_state() to apply.c\n>>   builtin/apply: make apply_all_patches() return 128 or 1 on error\n>>   builtin/apply: make parse_traditional_patch() return -1 on error\n>>   builtin/apply: make gitdiff_*() return 1 at end of header\n>>   builtin/apply: make gitdiff_*() return -1 on error\n>>   builtin/apply: change die_on_unsafe_path() to check_unsafe_path()\n>>   builtin/apply: make build_fake_ancestor() return -1 on error\n>>   builtin/apply: make remove_file() return -1 on error\n>>   builtin/apply: make add_conflicted_stages_file() return -1 on error\n>>   builtin/apply: make add_index_file() return -1 on error\n>>   builtin/apply: make create_file() return -1 on error\n>>   builtin/apply: make write_out_one_result() return -1 on error\n>>   builtin/apply: make write_out_results() return -1 on error\n>>   builtin/apply: make try_create_file() return -1 on error\n>>   builtin/apply: make create_one_file() return -1 on error\n>>   builtin/apply: rename option parsing functions\n>>   apply: rename and move opt constants to apply.h\n>>   Move libified code from builtin/apply.c to apply.{c,h}\n>>   apply: make some parsing functions static again\n>>   environment: add set_index_file()\n>>   write_or_die: use warning() instead of fprintf(stderr, ...)\n>>   apply: add 'be_silent' variable to 'struct apply_state'\n>>   apply: make 'be_silent' incompatible with 'apply_verbosely'\n>>   apply: don't print on stdout when be_silent is set\n>>   usage: add set_warn_routine()\n>>   usage: add get_error_routine() and get_warn_routine()\n>>   apply: change error_routine when be_silent is set\n>>   builtin/am: use apply api in run_apply()\n>>   apply: use error_errno() where possible\n>\n> I finally found time to get back to finish reviewing this.\n\nGreat, thanks!\n\n> The early part up to and including \"apply: make some parsing\n> functions static again\" looked good and we could treat them as a\n> continuation of the earlier cc/apply-introduce-state topic, which\n> has been merged to the 'master' already.\n\nOk, is it ok for you to just pick up this early part, or do you prefer\nme to resend it (maybe with the last patch on top of it)?\n\n> I got an impression that the patches in the remainder of the series\n> were not as polished as the earlier ones, except for the last one,\n> which should be reordered and be part of the early preparation step\n> if possible.\n\nI can resend just the last patch rebased on top of the early part once\nyou have picked up the early part.\n\nAnd yeah the remainder has not been reviewed much. I will rework it as\nyou suggest in your other emails about specific pathes, and then\nresend it in a \"part 3\" patch series.\n\nThanks,\nChristian.\n"},{"id":"292322","messageId":"CACsJy8Bi00yBb6UPm7_x8BDnwQKCXXpOPbswKtayRofmSOstcg@mail.gmail.com","threadId":"42726","inReplyTo":"xmqqinvs19ng.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v8 32/41] environment: add set_index_file()","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-07-27T15:14:50Z","receivedAt":"2016-07-27T15:15:25Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Jul 26, 2016 at 9:28 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Christian Couder <christian.couder@gmail.com> writes:\n>\n>> Introduce set_index_file() to be able to temporarily change the index file.\n>>\n>> It should be used like this:\n>>\n>>     /* Save current index file */\n>>     old_index_file = get_index_file();\n>>     set_index_file((char *)tmp_index_file);\n>>\n>>     /* Do stuff that will use tmp_index_file as the index file */\n>>     ...\n>>\n>>     /* When finished reset the index file */\n>>     set_index_file(old_index_file);\n>\n> WHY is glaringly missing.\n\nIt's used in a4aaa23 (builtin/am: use apply api in run_apply() -\n2016-06-27) as a straight conversion of \"GIT_INDEX_FILE=xxx git apply\"\n.\n\n> Doesn't calling this have a global effect that is flowned upon when\n> used by a library-ish function?  Who is the expected caller in this\n> series that wants to \"libify\" a part of the system?\n\nI agree we should avoid this. There's a bunch of cache_name_pos() (and\neven read_cache()) in the libified apply.c, those will need to be\nconverted  to take struct index_state* directly (read_index_from or\nindex_name_pos). But at least read-cache.c has supported multiple\nindexes for a long time.\n-- \nDuy\n"},{"id":"292326","messageId":"xmqqa8h3xd4a.fsf@gitster.mtv.corp.google.com","threadId":"42726","inReplyTo":"CAP8UFD0tcPnqBcxC5-4tnGMT4W5b7L=C_riwpTvDfeMUjQzpjA@mail.gmail.com","subject":"Re: [PATCH v8 00/41] libify apply and use lib in am, part 2","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-27T16:24:53Z","receivedAt":"2016-07-27T16:25:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <christian.couder@gmail.com> writes:\n\n>> I finally found time to get back to finish reviewing this.\n>\n> Great, thanks!\n\nNo, thank _you_ for working on this.\n\n>> The early part up to and including \"apply: make some parsing\n>> functions static again\" looked good and we could treat them as a\n>> continuation of the earlier cc/apply-introduce-state topic, which\n>> has been merged to the 'master' already.\n>\n> Ok, is it ok for you to just pick up this early part, or do you prefer\n> me to resend it (maybe with the last patch on top of it)?\n\nEither would be fine for _me_, but as the original thread is now\nabout a month old, a final reroll to give people the last chance to\ncomment on them would not hurt.\n"},{"id":"292513","messageId":"CAP8UFD1SSjSGV+1e+zP4s=p+ohgSz6mct3EhPXqbde_y48ST7g@mail.gmail.com","threadId":"42726","inReplyTo":"CACsJy8Bi00yBb6UPm7_x8BDnwQKCXXpOPbswKtayRofmSOstcg@mail.gmail.com","subject":"Re: [PATCH v8 32/41] environment: add set_index_file()","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-07-29T14:21:12Z","receivedAt":"2016-07-29T14:21:20Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Wed, Jul 27, 2016 at 5:14 PM, Duy Nguyen <pclouds@gmail.com> wrote:\n> On Tue, Jul 26, 2016 at 9:28 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Christian Couder <christian.couder@gmail.com> writes:\n>>\n>>> Introduce set_index_file() to be able to temporarily change the index file.\n>>>\n>>> It should be used like this:\n>>>\n>>>     /* Save current index file */\n>>>     old_index_file = get_index_file();\n>>>     set_index_file((char *)tmp_index_file);\n>>>\n>>>     /* Do stuff that will use tmp_index_file as the index file */\n>>>     ...\n>>>\n>>>     /* When finished reset the index file */\n>>>     set_index_file(old_index_file);\n>>\n>> WHY is glaringly missing.\n>\n> It's used in a4aaa23 (builtin/am: use apply api in run_apply() -\n> 2016-06-27) as a straight conversion of \"GIT_INDEX_FILE=xxx git apply\"\n> .\n\nYeah, I would add something like the following to clarify the WHY:\n\n\"It is intended to be used by builtins commands to temporarily change the\nindex file used by libified code.\n\nThis is useful when libified code uses the global index, but a builtin\ncommand wants another index file to be used instead.\"\n\n>> Doesn't calling this have a global effect that is flowned upon when\n>> used by a library-ish function?  Who is the expected caller in this\n>> series that wants to \"libify\" a part of the system?\n\nThe expected caller is a builtin, like \"builtin/am.c\".\n\n> I agree we should avoid this. There's a bunch of cache_name_pos() (and\n> even read_cache()) in the libified apply.c, those will need to be\n> converted  to take struct index_state* directly (read_index_from or\n> index_name_pos).\n\nThere is a lot of other \"libified\" code that is calling these functions:\n\n$ git grep -l cache_name_pos -- '*.c' | grep -v builtin\napply.c\ndiff.c\ndir.c\nmerge-recursive.c\npathspec.c\nrerere.c\nsha1_name.c\nsubmodule.c\nwt-status.c\n\n$ git grep -l read_cache -- '*.c' | grep -v builtin | egrep -v '^t/'\napply.c\ncheck-racy.c\ndiff.c\ndir.c\nmerge-recursive.c\nmerge.c\nread-cache.c\nrerere.c\nrevision.c\nsequencer.c\nsha1_name.c\nsubmodule.c\n\nand some of that code is perhaps directly and indirectly called by the\nlibified apply code.\n\nAlso I am not sure that read_cache() and cache_name_pos() are the only\nfunctions that should be changed.\n\nSo it looks like it is a very big and different project to make the\ncurrent libified code be explicit about which index it is using.\nAnd by the way perhaps this other project should just remove the\n\"the_index\" global altogether.\n\n> But at least read-cache.c has supported multiple\n> indexes for a long time.\n\nThis is great, but it is unfortunately not the only lib file involved.\n"},{"id":"292517","messageId":"CACsJy8CT2r1MKE_bQotPzKEWi10QB81sNn6w7H23GxRkEYRFfg@mail.gmail.com","threadId":"42726","inReplyTo":"CAP8UFD1SSjSGV+1e+zP4s=p+ohgSz6mct3EhPXqbde_y48ST7g@mail.gmail.com","subject":"Re: [PATCH v8 32/41] environment: add set_index_file()","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-07-29T15:34:55Z","receivedAt":"2016-07-29T15:35:30Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Jul 29, 2016 at 4:21 PM, Christian Couder\n<christian.couder@gmail.com> wrote:\n>> I agree we should avoid this. There's a bunch of cache_name_pos() (and\n>> even read_cache()) in the libified apply.c, those will need to be\n>> converted  to take struct index_state* directly (read_index_from or\n>> index_name_pos).\n>\n> There is a lot of other \"libified\" code that is calling these functions:\n>\n> $ git grep -l cache_name_pos -- '*.c' | grep -v builtin\n> apply.c\n> diff.c\n> dir.c\n> merge-recursive.c\n> pathspec.c\n> rerere.c\n> sha1_name.c\n> submodule.c\n> wt-status.c\n\nIrrelevant?\n\n> $ git grep -l read_cache -- '*.c' | grep -v builtin | egrep -v '^t/'\n> apply.c\n> check-racy.c\n> diff.c\n> dir.c\n> merge-recursive.c\n> merge.c\n> read-cache.c\n> rerere.c\n> revision.c\n> sequencer.c\n> sha1_name.c\n> submodule.c\n>\n> and some of that code is perhaps directly and indirectly called by the\n> libified apply code.\n\nYeah. If the libification movement is going strong, we can start\nconverting and at some point should be able to define\nNO_THE_INDEX_COMPATIBILITY_MACROS globally (and avoid the_index along\nthe way)\n\nWithout that, there is a risk. I looked at 'nm apply.o | grep ' [Uu]\n'' and I don't see any external functions that would potentially\naccess the index, except ll_merge. Again there's a good chance I may\nhave missed something.\n\n> So it looks like it is a very big and different project to make the\n> current libified code be explicit about which index it is using.\n> And by the way perhaps this other project should just remove the\n> \"the_index\" global altogether.\n\nThis is probably the way to go. But it's the boring sort of work that\nnobody wants to do :(\n-- \nDuy\n"},{"id":"292547","messageId":"CAP8UFD3pZedUPWTj1A8q0zBGhR5ATL3CXOCxHf1_4Fuo+Uu4UQ@mail.gmail.com","threadId":"42726","inReplyTo":"CACsJy8CT2r1MKE_bQotPzKEWi10QB81sNn6w7H23GxRkEYRFfg@mail.gmail.com","subject":"Re: [PATCH v8 32/41] environment: add set_index_file()","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-07-29T18:23:28Z","receivedAt":"2016-07-29T18:23:44Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Fri, Jul 29, 2016 at 5:34 PM, Duy Nguyen <pclouds@gmail.com> wrote:\n>\n> Yeah. If the libification movement is going strong, we can start\n> converting and at some point should be able to define\n> NO_THE_INDEX_COMPATIBILITY_MACROS globally (and avoid the_index along\n> the way)\n>\n> Without that, there is a risk. I looked at 'nm apply.o | grep ' [Uu]\n> '' and I don't see any external functions that would potentially\n> access the index, except ll_merge. Again there's a good chance I may\n> have missed something.\n>\n>> So it looks like it is a very big and different project to make the\n>> current libified code be explicit about which index it is using.\n>> And by the way perhaps this other project should just remove the\n>> \"the_index\" global altogether.\n>\n> This is probably the way to go. But it's the boring sort of work that\n> nobody wants to do :(\n\nDo you mean that it might be a source of micro-projects for the next\nGSoC or Outreachy? ;-)\n"},{"id":"292552","messageId":"CACsJy8C=d-PsfEuRw_XoE3=E6+2_a7Pai1oFv0pEuQsqXwsWwA@mail.gmail.com","threadId":"42726","inReplyTo":"CAP8UFD3pZedUPWTj1A8q0zBGhR5ATL3CXOCxHf1_4Fuo+Uu4UQ@mail.gmail.com","subject":"Re: [PATCH v8 32/41] environment: add set_index_file()","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-07-29T18:35:14Z","receivedAt":"2016-07-29T18:35:48Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Jul 29, 2016 at 8:23 PM, Christian Couder\n<christian.couder@gmail.com> wrote:\n> On Fri, Jul 29, 2016 at 5:34 PM, Duy Nguyen <pclouds@gmail.com> wrote:\n>>\n>> Yeah. If the libification movement is going strong, we can start\n>> converting and at some point should be able to define\n>> NO_THE_INDEX_COMPATIBILITY_MACROS globally (and avoid the_index along\n>> the way)\n>>\n>> Without that, there is a risk. I looked at 'nm apply.o | grep ' [Uu]\n>> '' and I don't see any external functions that would potentially\n>> access the index, except ll_merge. Again there's a good chance I may\n>> have missed something.\n>>\n>>> So it looks like it is a very big and different project to make the\n>>> current libified code be explicit about which index it is using.\n>>> And by the way perhaps this other project should just remove the\n>>> \"the_index\" global altogether.\n>>\n>> This is probably the way to go. But it's the boring sort of work that\n>> nobody wants to do :(\n>\n> Do you mean that it might be a source of micro-projects for the next\n> GSoC or Outreachy? ;-)\n\nNo that's what I meant by boring. This is not something to interest\nGSoC candidates, and while it looks simple, sometimes it needs a good\nunderstanding of git overall, and it's definitely not small enough for\na micro project. It's very similar to i18n-izing the code base.\nLuckily Vasco Almeida came out of nowhere and did (still do) that.\nThere may be another Vasco somewhere ;-)\n-- \nDuy\n"},{"id":"292556","messageId":"xmqqwpk4mfui.fsf@gitster.mtv.corp.google.com","threadId":"42726","inReplyTo":"CACsJy8C=d-PsfEuRw_XoE3=E6+2_a7Pai1oFv0pEuQsqXwsWwA@mail.gmail.com","subject":"Re: [PATCH v8 32/41] environment: add set_index_file()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-29T18:58:13Z","receivedAt":"2016-07-29T18:58:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen <pclouds@gmail.com> writes:\n\n> On Fri, Jul 29, 2016 at 8:23 PM, Christian Couder\n> \n>> Do you mean that it might be a source of micro-projects for the next\n>> GSoC or Outreachy? ;-)\n>\n> No that's what I meant by boring. This is not something to interest\n> GSoC candidates, and while it looks simple, sometimes it needs a good\n> understanding of git overall, and it's definitely not small enough for\n> a micro project.\n\nI think \"that's not what I meant\" is what you meant ;-) but anyway,\nI actually view that as part of the same \"libify\" project, and not\nsolving it and building an \"am that makes an internal call to apply\nthat is not libified\" is adding technical debt to the codebase.\n\nIt may be a good trade-off between having \"am that internally calls\napply\" earlier and the additional technical debt, but is not a good\nthing to do to the overall health of the project in the longer term\nto pretend as if this new set_index_file() is part of a good API.\n\nInstead it probably should be marked with \"the libification of\n'apply' took a short-circuit by adding this technical debt; please\ndo not call this function in new codepaths\", or something like that.\n\n\n\n"},{"id":"292650","messageId":"CAP8UFD3ivJNjn0dG_JGRdTNdX6R+L2UZnyQqcrbohthNA9RGbQ@mail.gmail.com","threadId":"42726","inReplyTo":"xmqqa8h3xd4a.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v8 00/41] libify apply and use lib in am, part 2","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-07-30T17:39:05Z","receivedAt":"2016-07-30T17:39:13Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Wed, Jul 27, 2016 at 6:24 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Christian Couder <christian.couder@gmail.com> writes:\n>\n>>> The early part up to and including \"apply: make some parsing\n>>> functions static again\" looked good and we could treat them as a\n>>> continuation of the earlier cc/apply-introduce-state topic, which\n>>> has been merged to the 'master' already.\n>>\n>> Ok, is it ok for you to just pick up this early part, or do you prefer\n>> me to resend it (maybe with the last patch on top of it)?\n>\n> Either would be fine for _me_, but as the original thread is now\n> about a month old, a final reroll to give people the last chance to\n> comment on them would not hurt.\n\nOk, everything that was in v8 has just been rerolled in v9.\n"}]}