Re: [PATCH 2/2] format-patch: add an option to suppress commit hash

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 7, 2015, 19:34 UTC
Message-ID: <xmqqr3iym4rg.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <1449440196-991107-3-git-send-email-sandals@crustytoothpaste.net>

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

Show 27 quoted lines

> Oftentimes, patches created by git format-patch will be stored in
> version control or compared with diff.  In these cases, two otherwise
> identical patches can have different commit hashes, leading to diff
> noise.  Teach git format-patch a --no-hash option that instead produces
> an all-zero hash to avoid this diff noise.
>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
>  Documentation/git-format-patch.txt | 4 ++++
>  builtin/log.c                      | 5 +++++
>  log-tree.c                         | 3 ++-
>  revision.h                         | 1 +
>  t/t4014-format-patch.sh            | 6 ++++++
>  5 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
> index 40356491..1266f135 100644
> --- a/Documentation/git-format-patch.txt
> +++ b/Documentation/git-format-patch.txt
> @@ -256,6 +256,10 @@ you can use `--suffix=-patch` to get `0001-description-of-my-change-patch`.
>  	using this option cannot be applied properly, but they are
>  	still useful for code review.
>  
> +--no-hash::
> +  Output an all-zero hash in each patch's From header instead
> +  of the hash of the commit.
> +

Two (big) problems with the option name.

 - "--no-something" would mislead people to think you are removing
   something, not replacing it with something else.  This option
   does the latter (i.e. the first line of your output still has
   40-hex; it's just it no longer has a useful 40-hex).

 - There are many places we use hexadecimal strings in format-patch
   output and you are not removing or replacing all of them, only
   the commit object name on the fake "From " line.  Saying "hash"
   would mislead readers.

> +test_expect_success 'format-patch --no-hash' '
> +	git format-patch --no-hash --stdout v2..v1 >patch2 &&
> +	cnt=$(egrep "^From 0+ Mon Sep 17 00:00:00 2001" patch2 | wc -l) &&

Don't test "any number of '0'"; test 40 '0's. This is because the line format was designed to be usable by things like /etc/magic to detect format-patch output, and we want to notice if/when we break that aspect of our output format.

> +	test $cnt = 3
> +'
> +
>  test_done

Thanks.