Re: [PATCH 0/2] format-patch: introduce option to suppress commit hashes

From: brian m. carlson <sandals@crustytoothpaste.net>
Date: Dec 7, 2015, 03:30 UTC
Message-ID: <20151207033020.GA990758@vauxhall.crustytoothpaste.net>
In-Reply-To: <xmqqh9jvnfbp.fsf@gitster.mtv.corp.google.com>

On Sun, Dec 06, 2015 at 06:49:14PM -0800, Junio C Hamano wrote:

Show 13 quoted lines

> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
> 
> > git format-patch is often used to create patches that are then stored in
> > version control or displayed with diff.  Having the commit hash in the
> > "From " line usually just creates diff noise in these cases, so this
> > series introduces --no-hash to set that to all zeros.
> 
> I am somewhat negative on this change that deliberately loses
> information in a way that seems too specific to a single workflow.
> 
> I understand that in that particular workflow, the patch stored as
> payload in a history needs only the diff part and the commit that
> the patch was taken from is deemed irrelevant.

Not exactly. At $DAYJOB, we store pristine-tar files, the contents of the tarball, and patches against the source in Git. We do it because RPM likes patches and doesn't know how to speak to Git, but many Debian source packages are stored similarly. Every source change results in a rebase and reformat of all the patches.

Show 12 quoted lines

> But the reason why that and only that piece of information is
> expendable, while author, subject and log message text are not is
> because...?  The answer to that question would very much be
> project's workflow dependant.  From that point of view, I'd say the
> users are much better off without the addition of this feature, but
> have a custom script in their workflow to remove parts that their
> project and workflow deems unnecessary.  Your project may deem the
> source commit object name unnecessary.  Another one may think the
> author date and name are, too.  Patch e-mail signature (i.e. what
> comes after a line with "-- ") by default depends on the version of
> Git that happened to have been used to prepare the patch, which may
> not be something you would want.

We do want to know who wrote the patch and the reason behind the patch, and so pretty much all the information except for the actual hash of the commit is useful to us. The hash is not useful because we've just rebased the commit on a temporary branch. We do, of course, want --no-signature, which already exists, as you pointed out.

The hash of the source file isn't generally as much of a problem, because the patch tends to change, even incidentally (line numbers and such), when the hash of the file changes. It's also something that we have in our history, whereas the temporary branch we rebased in is not.

Show 10 quoted lines

> Stepping back a bit, why is the history from which the patches are
> taken from irrelevant in the first place?  Perhaps because you
> replayed these patches on top of the same base but did not preserve
> their timestamps?  If this user, i.e. the part of the workflow that
> commits generated patches to version control, finds the "irrelevant"
> change irritating, isn't it fair to expect other users, i.e. other
> parts of the same workflow, also find that unnecessary and
> irrelevant rebasing irritating?  It feels like I am seeing an
> entrance to an X-Y problem whose real solution is to stop doing the
> pointless rebases in the first place.

The history is irrelevant because it happens on a temporary branch which is not pushed anywhere. The patches are the canonical form. As I said, this workflow is not specific to us; it's also used by certain Debian maintainers to handle their source packages.

The rebase is required because we're not sure if the patches all apply to the updated source (e.g. we had Git 2.6.2 and are rebasing them on 2.6.3). If patch 1 needed changes, but patch 2 did not, then patch 2 shows up in the diff even though we don't want it to.

> And if that rebase is not pointless, then I am not sure if it is a
> good thing to discard the information that records which incarnation
> of that constantly rebased source tree the patches were taken from.

I was looking through the Debian BTS for Git and I saw this feature request and thought, "Gee, I've always wanted this functionality, but I never thought to ask for it." This frequent, throwaway rebasing is very common in patch-based workflows.

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