Re: [PATCH v3 6/7] builtin: patch-id: remove unused diff-tree prefix

[Junio C Hamano <gitster@pobox.com>]

"Jerry Zhang via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Jerry Zhang <Jerry@skydio.com>
>
> From a "git grep" of the repo, no command, including diff-tree itself,
> produces diff output with "diff-tree " prefixed in the header.

I think you are lucky and it is OK in this case, but the "grep" only
tells about the current source, and a bit more due dilligence is in
general needed.

 * f9767222 (Add "git-patch-id" program to generate patch ID's.,
   2005-06-23) introduced the line that prepares us to see
   "diff-tree" prefix.  In that version, "git diff-tree --pretty
   --stdin" did use the prefix (it comes from 'diff-tree.c").

 * 5f1c3f07 (log-tree: separate major part of diff-tree.,
   2006-04-09) moved things around from "diff-tree.c" to
   "log-tree.c", without changing the behaviour.

 * cd2bdc53 (Common option parsing for "git log --diff" and friends,
   2006-04-14) further moved revs->header_prefix set-up to
   revision.c::setup_revisions(), without changing the behaviour.

 * 91539833 (Log message printout cleanups, 2006-04-17) did change
   the things drastically.  Its log message says:

       This does change "git whatchanged" from using "diff-tree" as
       the commit descriptor to "commit", and I changed one of the
       tests to reflect that new reality. Otherwise everything still
       passes, and my other tests look fine too.

As long as nobody keeps output from version of Git before v1.3.0
this change is safe to do ;-)

There may be third-party tools that was written in 2005-2006 that
still emit diff-tree prefix, but I somehow do not think it is likely
anobody would feed such an output to us.  Given how widely Git is
used, I might be overly optimistic, though.

> Thus remove its handling in "patch-id".
>
> Signed-off-by: Jerry Zhang <Jerry@skydio.com>
> ---
>  builtin/patch-id.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/patch-id.c b/builtin/patch-id.c
> index 745fe193a71..c37b8f573b7 100644
> --- a/builtin/patch-id.c
> +++ b/builtin/patch-id.c
> @@ -74,8 +74,8 @@ static int get_one_patchid(struct object_id *next_oid, struct object_id *result,
>  		const char *p = line;
>  		int len;
>  
> -		if (!skip_prefix(line, "diff-tree ", &p) &&
> -		    !skip_prefix(line, "commit ", &p) &&
> +		/* Possibly skip over the prefix added by "log" or "format-patch" */
> +		if (!skip_prefix(line, "commit ", &p) &&
>  		    !skip_prefix(line, "From ", &p) &&
>  		    starts_with(line, "\\ ") && 12 < strlen(line)) {
>  			if (include_whitespace)