Re: [PATCH] Added ability to scip patch hunk with an empty filename.

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 6, 2015, 18:36 UTC
Message-ID: <xmqqy4ityvyv.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1436202974-7624-2-git-send-email-evgen.druzhynin@gmail.com>

Evgen Druzhynin <evgen.druzhynin@gmail.com> writes:

This space is for you to explain why it is a good idea to do this change. What problem does it solve, how often does that problem appear, what other workarounds exist if any and why they are not satisfactory, etc, etc?

> ---

Above this line, we would want you to sign-off your patch, certifying that you read DCO (see Documentation/SubmittingPatches) and you wrote it or otherwise have the right to pass it on as an open-source patch.

Show 12 quoted lines

>  builtin/apply.c | 16 +++++-----------
>  po/bg.po        | 15 ---------------
>  po/ca.po        | 15 ---------------
>  po/de.po        | 15 ---------------
>  po/fr.po        | 15 ---------------
>  po/git.pot      | 11 -----------
>  po/it.po        | 11 -----------
>  po/pt_PT.po     |  7 -------
>  po/ru.po        | 12 ------------
>  po/sv.po        | 16 ----------------
>  po/vi.po        | 12 ------------
>  po/zh_CN.po     | 11 -----------

We do not want anybody who is not doing the i18n/l10n to touch any po/* files. The i18n/l10n team will adjust their translations accordingly when the code changes.

>  12 files changed, 5 insertions(+), 151 deletions(-)

Show 23 quoted lines

> diff --git a/builtin/apply.c b/builtin/apply.c
> index 54aba4e..e4481b4 100644
> --- a/builtin/apply.c
> +++ b/builtin/apply.c
> @@ -1488,17 +1488,11 @@ static int find_header(const char *line, unsigned long size, int *hdrsize, struc
>  			int git_hdr_len = parse_git_header(line, len, size, patch);
>  			if (git_hdr_len <= len)
>  				continue;
> -			if (!patch->old_name && !patch->new_name) {
> -				if (!patch->def_name)
> -					die(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)",
> -					       p_value),
> -					    p_value, linenr);
> -				patch->old_name = xstrdup(patch->def_name);
> -				patch->new_name = xstrdup(patch->def_name);
> -			}
> +
> +			/* pass this hunk if the filename length is zero */
> +			if (!patch->old_name && !patch->new_name) 
> +				return -1;

Please explain why this is a good idea.

This can happen only when (1) the user gave too large a number to the -p option or (2) the original patch was corrupt. In either case, we would want to stop and give the user a chance to correct it, either by specifying the correct -p number, or editing the corrupt patch to state the right path the patch applies to. Silently dropping the patch is never a good thing to do, is it?

Thanks.