apply: report input location in header parsing errors

Several header parsing errors in apply.c still report only line
numbers. When applying more than one input, that does not tell the
user which input the line belongs to.

Report the patch input location for these header parsing errors, and
update the related tests.

While touching parse_git_diff_header(), update the helper state to use
the current header line when reporting these errors.

Signed-off-by: Jialong Wang <jerrywang183@yahoo.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/apply.c b/apply.c
index b7b0a201b38d4fa2c4c9f28b62913bc4152582e3..700809f3e6b5cf916756ecebecb9e1ae3ae00bf9 100644 (file)
--- a/apply.c
+++ b/apply.c
@@ -42,6 +42,7 @@
 
 struct gitdiff_data {
        struct strbuf *root;
+       const char *patch_input_file;
        int linenr;
        int p_value;
 };
@@ -900,7 +901,8 @@ static int parse_traditional_patch(struct apply_state *state,
                }
        }
        if (!name)
-               return error(_("unable to find filename in patch at line %d"), state->linenr);
+               return error(_("unable to find filename in patch at %s:%d"),
+                            state->patch_input_file, state->linenr);
 
        return 0;
 }
@@ -937,20 +939,35 @@ static int gitdiff_verify_name(struct gitdiff_data *state,
 
        if (*name) {
                char *another;
-               if (isnull)
+               if (isnull) {
+                       if (state->patch_input_file)
+                               return error(_("git apply: bad git-diff - expected /dev/null, got %s at %s:%d"),
+                                            *name, state->patch_input_file, state->linenr);
                        return error(_("git apply: bad git-diff - expected /dev/null, got %s on line %d"),
                                     *name, state->linenr);
+               }
                another = find_name(state->root, line, NULL, state->p_value, TERM_TAB);
                if (!another || strcmp(another, *name)) {
                        free(another);
+                       if (state->patch_input_file)
+                               return error((side == DIFF_NEW_NAME) ?
+                                            _("git apply: bad git-diff - inconsistent new filename at %s:%d") :
+                                            _("git apply: bad git-diff - inconsistent old filename at %s:%d"),
+                                            state->patch_input_file, state->linenr);
                        return error((side == DIFF_NEW_NAME) ?
-                           _("git apply: bad git-diff - inconsistent new filename on line %d") :
-                           _("git apply: bad git-diff - inconsistent old filename on line %d"), state->linenr);
+                                    _("git apply: bad git-diff - inconsistent new filename on line %d") :
+                                    _("git apply: bad git-diff - inconsistent old filename on line %d"),
+                                    state->linenr);
                }
                free(another);
        } else {
-               if (!is_dev_null(line))
-                       return error(_("git apply: bad git-diff - expected /dev/null on line %d"), state->linenr);
+               if (!is_dev_null(line)) {
+                       if (state->patch_input_file)
+                               return error(_("git apply: bad git-diff - expected /dev/null at %s:%d"),
+                                            state->patch_input_file, state->linenr);
+                       return error(_("git apply: bad git-diff - expected /dev/null on line %d"),
+                                    state->linenr);
+               }
        }
 
        return 0;
@@ -974,12 +991,19 @@ static int gitdiff_newname(struct gitdiff_data *state,
                                   DIFF_NEW_NAME);
 }
 
-static int parse_mode_line(const char *line, int linenr, unsigned int *mode)
+static int parse_mode_line(const char *line,
+                          const char *patch_input_file,
+                          int linenr,
+                          unsigned int *mode)
 {
        char *end;
        *mode = strtoul(line, &end, 8);
-       if (end == line || !isspace(*end))
+       if (end == line || !isspace(*end)) {
+               if (patch_input_file)
+                       return error(_("invalid mode at %s:%d: %s"),
+                                    patch_input_file, linenr, line);
                return error(_("invalid mode on line %d: %s"), linenr, line);
+       }
        *mode = canon_mode(*mode);
        return 0;
 }
@@ -988,14 +1012,16 @@ static int gitdiff_oldmode(struct gitdiff_data *state,
                           const char *line,
                           struct patch *patch)
 {
-       return parse_mode_line(line, state->linenr, &patch->old_mode);
+       return parse_mode_line(line, state->patch_input_file, state->linenr,
+                              &patch->old_mode);
 }
 
 static int gitdiff_newmode(struct gitdiff_data *state,
                           const char *line,
                           struct patch *patch)
 {
-       return parse_mode_line(line, state->linenr, &patch->new_mode);
+       return parse_mode_line(line, state->patch_input_file, state->linenr,
+                              &patch->new_mode);
 }
 
 static int gitdiff_delete(struct gitdiff_data *state,
@@ -1314,6 +1340,7 @@ static int check_header_line(int linenr, struct patch *patch)
 }
 
 int parse_git_diff_header(struct strbuf *root,
+                         const char *patch_input_file,
                          int *linenr,
                          int p_value,
                          const char *line,
@@ -1345,6 +1372,7 @@ int parse_git_diff_header(struct strbuf *root,
        size -= len;
        (*linenr)++;
        parse_hdr_state.root = root;
+       parse_hdr_state.patch_input_file = patch_input_file;
        parse_hdr_state.linenr = *linenr;
        parse_hdr_state.p_value = p_value;
 
@@ -1382,6 +1410,7 @@ int parse_git_diff_header(struct strbuf *root,
                        int res;
                        if (len < oplen || memcmp(p->str, line, oplen))
                                continue;
+                       parse_hdr_state.linenr = *linenr;
                        res = p->fn(&parse_hdr_state, line + oplen, patch);
                        if (res < 0)
                                return -1;
@@ -1396,12 +1425,20 @@ int parse_git_diff_header(struct strbuf *root,
 done:
        if (!patch->old_name && !patch->new_name) {
                if (!patch->def_name) {
-                       error(Q_("git diff header lacks filename information when removing "
-                                "%d leading pathname component (line %d)",
-                                "git diff header lacks filename information when removing "
-                                "%d leading pathname components (line %d)",
-                                parse_hdr_state.p_value),
-                             parse_hdr_state.p_value, *linenr);
+                       if (patch_input_file)
+                               error(Q_("git diff header lacks filename information when removing "
+                                        "%d leading pathname component at %s:%d",
+                                        "git diff header lacks filename information when removing "
+                                        "%d leading pathname components at %s:%d",
+                                        parse_hdr_state.p_value),
+                                     parse_hdr_state.p_value, patch_input_file, *linenr);
+                       else
+                               error(Q_("git diff header lacks filename information when removing "
+                                        "%d leading pathname component (line %d)",
+                                        "git diff header lacks filename information when removing "
+                                        "%d leading pathname components (line %d)",
+                                        parse_hdr_state.p_value),
+                                     parse_hdr_state.p_value, *linenr);
                        return -128;
                }
                patch->old_name = xstrdup(patch->def_name);
@@ -1409,8 +1446,12 @@ done:
        }
        if ((!patch->new_name && !patch->is_delete) ||
            (!patch->old_name && !patch->is_new)) {
-               error(_("git diff header lacks filename information "
-                       "(line %d)"), *linenr);
+               if (patch_input_file)
+                       error(_("git diff header lacks filename information at %s:%d"),
+                             patch_input_file, *linenr);
+               else
+                       error(_("git diff header lacks filename information (line %d)"),
+                             *linenr);
                return -128;
        }
        patch->is_toplevel_relative = 1;
@@ -1577,8 +1618,9 @@ static int find_header(struct apply_state *state,
                        struct fragment dummy;
                        if (parse_fragment_header(line, len, &dummy) < 0)
                                continue;
-                       error(_("patch fragment without header at line %d: %.*s"),
-                                    state->linenr, (int)len-1, line);
+                       error(_("patch fragment without header at %s:%d: %.*s"),
+                             state->patch_input_file, state->linenr,
+                             (int)len-1, line);
                        return -128;
                }
 
@@ -1590,7 +1632,9 @@ static int find_header(struct apply_state *state,
                 * or mode change, so we handle that specially
                 */
                if (!memcmp("diff --git ", line, 11)) {
-                       int git_hdr_len = parse_git_diff_header(&state->root, &state->linenr,
+                       int git_hdr_len = parse_git_diff_header(&state->root,
+                                                               state->patch_input_file,
+                                                               &state->linenr,
                                                                state->p_value, line, len,
                                                                size, patch);
                        if (git_hdr_len < 0)

diff --git a/apply.h b/apply.h
index 90e887ec0edadf68acf5dd534b7c3e9aa9a917c2..5f2f03d3ede23c273566640749b9a72da405e4be 100644 (file)
--- a/apply.h
+++ b/apply.h
@@ -167,6 +167,7 @@ int check_apply_state(struct apply_state *state, int force_apply);
  * Returns -1 on failure, the length of the parsed header otherwise.
  */
 int parse_git_diff_header(struct strbuf *root,
+                         const char *patch_input_file,
                          int *linenr,
                          int p_value,
                          const char *line,

diff --git a/range-diff.c b/range-diff.c
index 57edff40a85f24506214c57d8f2fcef521f2506f..2712a9a107ab068d7e7145d68e4ab5e42d753cc1 100644 (file)
--- a/range-diff.c
+++ b/range-diff.c
@@ -140,7 +140,7 @@ static int read_patches(const char *range, struct string_list *list,
                        if (eol)
                                *eol = '\n';
                        orig_len = len;
-                       len = parse_git_diff_header(&root, &linenr, 0, line,
+                       len = parse_git_diff_header(&root, NULL, &linenr, 0, line,
                                                    len, size, &patch);
                        if (len < 0) {
                                error(_("could not parse git header '%.*s'"),

diff --git a/t/t4100-apply-stat.sh b/t/t4100-apply-stat.sh
index b19fc9fe50dd0937861625c1253e76136d49fb90..b3d93d8ed6cda77c49727719e1fe8ff2e00dee43 100755 (executable)
--- a/t/t4100-apply-stat.sh
+++ b/t/t4100-apply-stat.sh
@@ -87,4 +87,42 @@ test_expect_success 'applying multiple patches reports the corrupted input' '
        echo "error: corrupt patch at bad.patch:4" >expect &&
        test_cmp expect err
 '
+
+test_expect_success 'applying a patch without a header reports the input' '
+       cat >fragment.patch <<-\EOF &&
+       @@ -1 +1 @@
+       -a
+       +b
+       EOF
+       test_must_fail git apply fragment.patch 2>err &&
+       echo "error: patch fragment without header at fragment.patch:1: @@ -1 +1 @@" >expect &&
+       test_cmp expect err
+'
+
+test_expect_success 'applying a patch with a missing filename reports the input' '
+       cat >missing.patch <<-\EOF &&
+       diff --git a/f b/f
+       index 7898192..6178079 100644
+       --- a/f
+       @@ -1 +1 @@
+       -a
+       +b
+       EOF
+       test_must_fail git apply missing.patch 2>err &&
+       echo "error: git diff header lacks filename information at missing.patch:4" >expect &&
+       test_cmp expect err
+'
+
+test_expect_success 'applying a patch with an invalid mode reports the input' '
+       cat >mode.patch <<-\EOF &&
+       diff --git a/f b/f
+       old mode 10x644
+       EOF
+       test_must_fail git apply mode.patch 2>err &&
+       cat >expect <<-\EOF &&
+       error: invalid mode at mode.patch:2: 10x644
+
+       EOF
+       test_cmp expect err
+'
 test_done

diff --git a/t/t4254-am-corrupt.sh b/t/t4254-am-corrupt.sh
index ae0a56cf5ec2d787432b7e97ac40bbfcb270225f..96ddf3c53a45b93c138836385a31d3bd2b267af0 100755 (executable)
--- a/t/t4254-am-corrupt.sh
+++ b/t/t4254-am-corrupt.sh
@@ -65,9 +65,8 @@ test_expect_success setup '
 test_expect_success 'try to apply corrupted patch' '
        test_when_finished "git am --abort" &&
        test_must_fail git -c advice.amWorkDir=false -c advice.mergeConflict=false am bad-patch.diff 2>actual &&
-       echo "error: git diff header lacks filename information (line 4)" >expected &&
        test_path_is_file f &&
-       test_cmp expected actual
+       test_grep "error: git diff header lacks filename information at .*rebase-apply/patch:4" actual
 '
 
 test_expect_success "NUL in commit message's body" '