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

From: brian m. carlson <sandals@crustytoothpaste.net>
Date: Dec 8, 2015, 01:02 UTC
Message-ID: <20151208010227.GC990758@vauxhall.crustytoothpaste.net>
In-Reply-To: <xmqqr3iym4rg.fsf@gitster.mtv.corp.google.com>

On Mon, Dec 07, 2015 at 11:34:59AM -0800, Junio C Hamano wrote:

Show 6 quoted lines

> 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).

Right. I originally considered removing that line altogether, but other parts of Git rely on that header to process patches correctly.

>  - 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.

I'll reroll with the name --zero-commit, unless somebody comes up with a better suggestion.

Show 8 quoted lines

> > +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.

My idea was to future-proof it against changes in the hash function, although that's in the distant future. I'll reroll with the change you suggested. I might drop in another patch to improve the existing tests to cover the case without this option, as I think we just look for "From " currently.

-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187