Re: [PATCH] format-patch: output header for empty commits

From: John Keeping <john@keeping.me.uk>
Date: Mar 8, 2023, 20:33 UTC
Message-ID: <ZAjxL2MIXCNZgYj/@keeping.me.uk>
In-Reply-To: <xmqqlek9byeb.fsf@gitster.g>

On Mon, Mar 06, 2023 at 09:08:44AM -0800, Junio C Hamano wrote:

Show 40 quoted lines

> John Keeping <john@keeping.me.uk> writes:
> 
> > On Fri, Mar 03, 2023 at 09:13:27AM -0800, Junio C Hamano wrote:
> >> John Keeping <john@keeping.me.uk> writes:
> >> 
> >> > When formatting an empty commit, it is surprising that a totally empty
> >> > file is generated.  Set the flag to always print the header, matching
> >> > the behaviour of git-log.
> >> 
> >> Don't these empty files help send-email as safety against sending
> >> them out?  Unless existing tools depend on the current behaviour in
> >> such a way, I think this is quite a sensible change.
> >
> > Yes, send-email fails trying to send an empty file, but to me this feels
> > more like an accident than an intentional safeguard.  If there were
> > something intentional I'd expect format-patch to fail with --allow-empty
> > as an option to bypass that safety check.
> >
> > Since there are checks in place to avoid unintentionally creating empty
> > commits,...
> 
> Speaking as the original implementer of format-patch, the original
> intention was to forbid such a message to be sent out.  But it was
> designed back in the days when an empty commit were not used as "a
> marker in the history" as widely as these days.  IOW, the original
> intention does not matter all that much when we have to determine if
> the code with the proposed change would negatively affect _today's_
> users.  What the users would see is that they have been protected
> from sending out such a message by mistake (an empty commit may not
> be something you created but you pulled from your colleages), but
> with this change the protection is no longer there.
> 
> Another worry is if the receiving end is prepared to see such a
> "patch".
> 
> Overall, if we were designing format-patch/send-email/am today with
> today's use cases in mind without any existing users of these three
> commands, I think these three would be designed to pass an empty
> commit through the chain unconditionally.  But we do not live in
> such a world, so perhaps some sort of opting in may be appropriate.

Does that mean you want to see format-patch die on empty commits unless --allow-empty is specified?

I think it's in a slightly strange place because it's both a "creation" command and an "inspection" command. Elsewhere the creation commands (like commit or cherry-pick) require --allow-empty but inspection commands (like log or show) always show all commits.

My mental model groups format-patch in the inspection commands and I wouldn't send anything out without inspecting the patch files first (but then I get caught out by this empty commit behaviour when I use format-patch for non-email use and grep doesn't find something I'm sure should be there in an empty commit's message!).