Re: FORMAT_PATCH_NAME_MAX increase

Laszlo Ersek [off-list ref] writes:

would it be possible to

- increase the FORMAT_PATCH_NAME_MAX macro from 64 to, say, 128?

- Or else to introduce a new git-config knob for it?

It's open source, so both are "possible", but you are interested in
learning if these are acceptable project-wide.

I have a small review-helper / interdiff script that matches patches
from adjacent versions of a series against each other, based on subject
line. (Using the formatted file name with the patch number stripped.)
The project in question uses long common prefixes in subjects, and the
current limit of 64 does not always ensure unicity (again, with the
number stripped).

The original motivation of the design is that leading numbers are to
ensure they are unique, and munging and truncating of title are to
ensure that they are safe on the filesystem while being identifiable
for eyeballing.  And the current truncation limit may not work well
for certain projects, failing the "identifiabl" goal.

But I do not think that is true only for your custom script.  For
example, with such a project, output from "git shortlog" or the
labels "gitk" gives to commits would not be very useful, because 2/3
of your display are filled with and wasted by the same long prefix
to the left.  The real issue is not that one particular "length
limit" used by format-patch is too short; it is that the project
convention wastes too many bytes for common things that can be left
unsaid, and if we come up with a general way to address that, we'd
make all of Git that summarises commits more useful, not just
format-patch.

So I think lengthening FORMAT_PATCH_NAME_MAX, whether it is done
unconditionally or conditionally, is rather an ad-hoc hack than
a real improvement.

I cannot offhand guess what other places would suffer from such a
project convention, because I do not work with such a project, but
you may be able to come up with a list of various places in Git
where the commit titles are used, and that if there were a mechanism
to take these commit titles, pass them to your cutomization script,
which abbreviates these "long common prefixes" to a shorter string,
and to use the output from that script instead of the actual commit
title, it would help all of these places.  The list of commits "git
merge" records when merging a side branch may benefit from the same
mechanism.

For example, if the titles of your commit title look like this:

    Subject: RedHat Kernel Maintenance: ipv4: add reference count
    Subject: RedHat Kernel Maintenance: net: llc: add lock_sock
    ...

your custom script may be

    #!/bin/sh
    exec sed -e "s/^RedHat Kernel Maintenance: /RHKM-/"
	... there may be more common patterns to shorten ...

and the resulting output from format-patch might become

	0001-RHKM-ipv4-add-reference-count.patch
	0002-RHKM-net-llc-add-lock_sock.patch

while "git shortlog" output and "gitk" display may also be helped
by the same mechanism.

It's OK for the _initial_ application of such a mechanism were to
affect the names of files format-patch creates and nothing else.
Interested parties can then use the same mechanism in other programs
(like "gitk" and "git shortlog"), as long as that mechansim is
designed well.  And then the end users need to learn that mechanism
only once.

On 05/30/17 03:34, Junio C Hamano wrote:

I cannot offhand guess what other places would suffer from such a
project convention, because I do not work with such a project, but
you may be able to come up with a list of various places in Git
where the commit titles are used, and that if there were a mechanism
to take these commit titles, pass them to your cutomization script,
which abbreviates these "long common prefixes" to a shorter string,
and to use the output from that script instead of the actual commit
title, it would help all of these places.

The problem is that I can't really automate the subject munging. The
concrete subjects in this case were:

OvmfPkg/QemuFwCfgLib: Implement SEV internal function for SEC phase
OvmfPkg/QemuFwCfgLib: Implement SEV internal functions for PEI phase
OvmfPkg/QemuFwCfgLib: Implement SEV internal function for Dxe phase

which got formatted as

0010-OvmfPkg-QemuFwCfgLib-Implement-SEV-internal-function.patch
0011-OvmfPkg-QemuFwCfgLib-Implement-SEV-internal-function.patch
0012-OvmfPkg-QemuFwCfgLib-Implement-SEV-internal-function.patch

and these filenames differ only in the running counter on the left.

The patch subjects themselves aren't overlong (the longest is 68
characters).

At best I could "normalize away" the "OvmfPkg/QemuFwCfgLib" prefix, but
that exact prefix is pretty accidental. Any standalone module (driver or
library instance) in the edk2 project is supposed to be named like this
in patch subjects, so all those prefixes would have to be normalized
somehow.

We generally try to limit subjects (and commit messages in general) to
74 columns. I think for one source of inspiration, we used the kernel
documentation, when setting that limit.
<https://www.kernel.org/doc/Documentation/process/submitting-patches.rst>
says, under section 14, "The canonical patch format",

The canonical patch subject line is::

    Subject: [PATCH 001/123] subsystem: summary phrase

The canonical patch message body contains the following:

  [...]

  - The body of the explanation, line wrapped at 75 columns, which will
    be copied to the permanent changelog to describe this patch.

It does not specify the subject length, but perhaps we can apply the
body line length to the subject as well.

So, even in kernel land, if subjects up to 75 columns are permitted, but
FORMAT_PATCH_NAME_MAX is 64, conflicts are possible, at least in theory,
aren't they? With the numbers stripped, of course.

Thanks,
Laszlo