committers-guide: Improve "Fixes:" metadata

[jlduran]

Summary

Use the preferred format for displaying the "Fixes:" metadata in the git
commit message.

[imp]

We were just talking about this.

[emaste]

Amazing, we were discussing the Fixes tag on the srcmgr call just a few minutes ago.

[jhb]

Might be useful to describe how it is canonical? I don't recall seeing anything in the git manpages describing this format for example, so presumably the format doesn't come from git itself?

[emaste]

Here's a proposed patch which aimed to add support to git directly: https://git.vger.kernel.narkive.com/ZeYr3x1N/patch-commit-add-f-fixes-commit-option-to-add-fixes-line

Linux Kernel Summit 2013 decided on a commit message convention to
identify commits containing bugs fixed by a commit: a "Fixes:" line,
included in the standard commit footer (along with "Signed-off-by:" if
present), containing an abbreviated commit hash (at least 12 characters
to keep it valid for a long time) and the subject of the commit (for
human readers). This helps people (or automated tools) determine how
far to backport a commit.

[jlduran]

FreeBSD-telepathy?
I just committed a fix and then realized the preferred style is the one described here (glancing over the commit history and the preferred style of my mentor).
Should I just remove the word "canonical" and use "preferred", or wait until the srcmgr call minutes are published?

[emaste]

A quick survey - Linux kernel is exclusively %h ("%s") over the last year.

In FreeBSD I see 98 Fixes: over the last year.

• 46 are similar to %h ("%s") although with inconsistent short hash length, and some truncate the quoted title to fit within suggested line length limits.
• 2 use a full hash and no message
• 6 use parens and single quotes e.g. Fixes: df62b8a25f47 ('xen: add a handler for the debug interrupt')
• 5 use parens but no quotes e.g. Fixes: 5ad3b09f2fe9 (smb: distinguishable descriptions)
• 3 use no parens or quotes e.g. Fixes: 5caeef02fa53 RAID-Z expansion feature
• 36 use just a short hash (as few as 7 chars of the hash)

A number of the truncated title ones (e.g. Fixes: 5e16809c953f ("tzsetup: symlink /etc/localtime instead of co...")) are mine, but based on this quick survey I'll stop doing that and just use the full title.

Using "preferred" in the commit message avoids the need for us to find a reference for "canonical" and is an easy path. Given the evidence of existing commits in the tree I'd say it's fine to commit now, but if you want to wait for the meeting minutes to appear and reference them that's fine too.

[jlduran]

Thank you for the detailed explanation.
I'll wait for the MoM.
By the way, I have updated my local prepare-commit-msg hook, but maybe it would be useful to update tools/tools/git/hooks/prepare-commit-message as well?

[jhb]

In D47397#1081004, @emaste wrote:

A quick survey - Linux kernel is exclusively %h ("%s") over the last year.

In FreeBSD I see 98 Fixes: over the last year.

• 46 are similar to %h ("%s") although with inconsistent short hash length, and some truncate the quoted title to fit within suggested line length limits.
• 2 use a full hash and no message
• 6 use parens and single quotes e.g. Fixes: df62b8a25f47 ('xen: add a handler for the debug interrupt')
• 5 use parens but no quotes e.g. Fixes: 5ad3b09f2fe9 (smb: distinguishable descriptions)
• 3 use no parens or quotes e.g. Fixes: 5caeef02fa53 RAID-Z expansion feature
• 36 use just a short hash (as few as 7 chars of the hash)

A number of the truncated title ones (e.g. Fixes: 5e16809c953f ("tzsetup: symlink /etc/localtime instead of co...")) are mine, but based on this quick survey I'll stop doing that and just use the full title.

Using "preferred" in the commit message avoids the need for us to find a reference for "canonical" and is an easy path. Given the evidence of existing commits in the tree I'd say it's fine to commit now, but if you want to wait for the meeting minutes to appear and reference them that's fine too.

Hmm, I had 32 commits in the last year that all used the previously documented style. Not quite sure how your script missed those as this is certainly more than 3? I also came up with 552 commits using Fixes in the past year.

git log --grep Fixes:  --author jhb --since="1 year ago" | egrep '^commit|Fixes'
commit a6ec2147418c29b259ff6b4f4274417b55419285
    Fixes:          f46d4971b5af nvmf: Handle shutdowns more gracefully
commit 8c8ebbb045185396083cd3e4d333fe1851930ee7
    Fixes:          71fa171c6480 bhyve: Initialize stack buffer in pci_ahci
commit 47f49dd4bbb4a72e53d31046964ce3c111ee0d12
    Fixes:          ddf0ed09bd8f sdt: Implement SDT probes using hot-patching
commit ad152571b8fd69d93ca0e44d80c9b49308b36eae
    Fixes:          1f903953fbf8 bhyve: Add raw tcp to uart backend
commit 776cd02b891ccd984963c9ec26f9748d213f0b9b
    Fixes:          f44ff2aba2d6 bhyve: Treat the COMMAND register for PCI passthru devices as emulated
commit 102b4e33acef74da5b1e5aee43a324fd395342c8
    Fixes:          169641f7dd9f imgact_elf: Add const to a few struct image_params pointers
commit 8da89a177d7197e985157cc230fa9e401c6efd01
    Fixes:          29d079c96491 libsys: move __libsys_interposer consumers
commit 09ed116dc5b84e6b3478b42e5b1076604e92f676
    Fixes:          94a86f3f6920 mixer(8): Add tests
commit a4b248b883cc2ade1a394a21ad108b851b4d454c
    Fixes:          4339f1e667ff share/examples/IPv6/USAGE: remove
commit 20cd6bbc1048d985ac75667c23fcae5f5fe98001
    Fixes:          911f0260390e dtrace: move kinst tests to common
commit 66158bda905eea447d2f8beef92556c090c41dbb
    Fixes:          a00f9e4e8181 scsi: Stop installing both cam.4 and CAM.4
commit b313bf154a5afe5b07da7dd3d5c2efc87348a737
    Fixes:          6ac1f02be34a Add GBDE to ObsoleteFiles.inc
commit 0244e0a177a68fc8ff7e8a58fa7a9553956232ec
    Fixes:          838b6caababb openssl: use getrandom(2) instead of probing for getentropy(2)
commit 56b822a17cde5940909633c50623d463191a7852
    Fixes:          871b33ad65ba pci: Consistently use pci_vf_* for suballocated VF memory resources
commit 98056127ddfa36720bcf46edc09843c867784bcb
    Fixes:          0e1246e33461 acpi: Cleanup handling of suballocated resources
    Fixes:          b377ff8110e3 pcib: Refine handling of resources allocated from bridge windows
    Fixes:          d79b6b8ec267 pci_host_generic: Don't rewrite resource start address for translation
    Fixes:          d714e73f7895 vmd: Use bus_generic_rman_* for PCI bus and memory resources
commit ff7de3b4d32b64acb609af8a4e6e0b640dbd2973
    Fixes:          0c3ade2cf13d nextboot: fix nextboot -k on ZFS
commit 75529910f77a1623b83599de0518d39c5fb789df
    Fixes:          1687d77197c0 man filesystems: move driver pages to section four
commit 03713f805a91749eabcfb2e23e86489b2d4034df
    Fixes:          d79b6b8ec267 pci_host_generic: Don't rewrite resource start address for translation
commit 1b38f851d809950cfe112c705029f40edd0579b3
    Fixes:          7485926e09a0 nvme: Firmware revisions in the firmware slot info logpage are ASCII strings
commit 332dbd3a2f08a887014a425d2532af93503588ce
    Fixes:          d79b6b8ec267 pci_host_generic: Don't rewrite resource start address for translation
commit 04eca69323111986b262eef3135d804361b60d17
    Fixes:          5bda26333a8e gpiobus: Use bus_generic_rman_*
commit 61d9bd21e9d6c3f876c4c8549fafaaaa4de75983
    Fixes:          d7c16b333455 powerpc psim: Use bus_generic_rman_*
commit cd9d26ed91c049dd60919c71badced89e18a882a
    Fixes:          5a7e717fb790 powerpc mpc85xx: Use bus_generic_rman_*
commit 66d37dbedfbf2dc94ccf49e6983c3652d5909b91
    Fixes:          d79b6b8ec267 pci_host_generic: Don't rewrite resource start address for translation
commit ccb1b43e20f5a1a0795b14f3ef39d5c2e15b424d
    Fixes:          d7c16b333455 powerpc psim: Use bus_generic_rman_*
commit 68a3ff041129208ea98a3bd5142061176ab4165e
    Fixes:          5a7e717fb790 powerpc mpc85xx: Use bus_generic_rman_*
commit feefc3c71e33d8f97879c4889d5cf1ec82e98cd9
    Fixes:          d79b6b8ec267 pci_host_generic: Don't rewrite resource start address for translation
commit 992f5b16afa73d0ea63c8a886a78ad2eda928e37
    Fixes:          d79b6b8ec267 pci_host_generic: Don't rewrite resource start address for translation
commit 4505c89242025f840023cdf092fdab845586f42d
    Fixes:          3cf553288b96 simplebus: Consistently map SYS_RES_IOPORT to SYS_RES_MEMORY
commit 67b0b907854af42788ec0ae24d5bd4d5c70b72d7
    Fixes:          3933ff56f9b6 busdma: tidy bus_dma_run_filter() functions
commit 5622f52c98cda7df1faef11d847e04500158e107
    Fixes:          658501d25930 aarch64 nexus: Use bus_generic_rman_*_resource
commit 7aa3bf6952b2c7d1b02d3a67e7f51b96fcf52b50
    Fixes:          903873ce1560 Implement and use new mixer(3) library for FreeBSD.

> git log --first-parent --grep Fixes:  --since="1 year ago" --oneline | wc
     552    4120   33496

I do think the commit should have probably said "Linux" somewhere since that is where this convention originated.

[emaste]

Huh, I am not sure what I was looking at. Indeed, git log --first-parent --since=1year --grep 'Fixes' | sed -n 's/^.*Fixes:[[:space:]]*//p' returns over 500 entries.

Of 578 total:

• 183 with no space (generally just a hash, but a few outliers)
• 395 with a space
  • 257 (" ... ")
  • 22 ( ... )
  • 8 " ... "
  • 104 with no quote or parens ...
  • 4 outliers (e.g. PR 275009, 930a7c2ac67e, 96fe7c8ab0f6)

Linux-style may be an appropriate description although this convention is common in other projects too.