mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <>
To: Hu Keping <>
Subject: Re: [PATCH] Lengthening FORMAT_PATCH_NAME_MAX to 80
Date: Thu, 5 Nov 2020 10:01:49 -0500	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On Thu, Nov 05, 2020 at 08:15:48PM +0000, Hu Keping wrote:

> The file name of the patch generated by `git format-patch` usually be
> truncated as there is less than 60 bytes left for the subject of commit
> message exclude the prefix patch number, say "0001-".
> As per the kernel documentation[1], it suggest the length of subject no
> more than 75.
> >
> > [...]
> > For these reasons, the ``summary`` must be no more than 70-75
> > characters, and it must describe both what the patch changes, as well
> > as why the patch might be necessary.
> >
> Considered the prefix patch number "0001-" would take 5 characters, increase

As the code is written now, the length also includes the ".patch"
suffix, as well as an extra byte (maybe for a NUL? Once upon a time I
imagine we used static buffers, but these days it's all in a strbuf).

A simple test with:

  git init
  for i in $(seq 8); do printf 1234567890; done |
  git commit --allow-empty -F -
  git format-patch -1

shows us generating:


So that's only 52 characters, from our constant of 64. Bumping to 80
gives us 66, which is reasonable though probably still involves
occasional truncation. But maybe keeping the total length to 80 (79,
really, because of the extra byte) may be worth doing.

Which is all a long-winded way of saying that your patch seems
reasonable to me.

Looking at the code which uses the constant, I suspect it could also be
made simpler:

  - the PATH_MAX check in open_next_file() seems pointless. Once upon a
    time it mattered for fitting into a PATH_MAX buffer, but these days
    we use a dynamic buffer anyway. We are probably better off to just
    feed the result to the filesystem and see if it complains (since
    either way we are aborting; I'd feel differently if we adjusted our
    truncation size)

  - the logic in fmt_output_subject() could probably be simpler if the
    constant was "here's how long the subject should be", not "here's
    how long the whole thing must be".

But those are both orthogonal to your patch and can be done separately.


  reply	other threads:[~2020-11-05 15:01 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-05 20:15 [PATCH] Lengthening FORMAT_PATCH_NAME_MAX to 80 Hu Keping
2020-11-05 15:01 ` Jeff King [this message]
2020-11-05 21:16   ` Junio C Hamano
2020-11-06  8:51     ` hukeping
2020-11-06 17:45       ` Junio C Hamano
2020-11-06 20:50     ` Junio C Hamano
2020-11-06 21:56       ` [PATCH] format-patch: make output filename configurable Junio C Hamano
2020-11-06 22:05         ` Eric Sunshine
2020-11-09 19:23           ` [PATCH v2] " Junio C Hamano
2020-11-10  0:23             ` Jeff King
2020-11-10  1:43               ` Junio C Hamano
2020-11-10  2:31             ` hukeping
2020-11-10  2:37               ` Junio C Hamano
2020-11-10  4:44                 ` hukeping
2020-11-10  5:40                   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

  List information:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).