Page MenuHomeFreeBSD

Add git-blame ignore file

Authored by emaste on Sep 11 2022, 3:58 PM.
Referenced Files
Unknown Object (File)
Mon, Sep 11, 5:28 AM
Unknown Object (File)
Jul 10 2023, 12:37 AM
Unknown Object (File)
Jun 19 2023, 2:44 PM
Unknown Object (File)
May 12 2023, 8:43 PM
Unknown Object (File)
May 12 2023, 8:10 PM
Unknown Object (File)
May 12 2023, 7:44 PM
Unknown Object (File)
May 10 2023, 9:37 AM
Unknown Object (File)
May 2 2023, 6:55 AM



.git-blame-ignore-revs lists commit hashes that should be skipped by git blame e.g. non-functional whitespace or style cleanup.

Diff Detail

rG FreeBSD src repository
Lint Not Applicable
Tests Not Applicable

Event Timeline

emaste created this revision.
emaste added a subscriber: gbe.

I added two commits as an example; if we decide to go ahead I will do a first pass to collect suitable commits

I'm cautiously almost not pessimistic about this...

It will cut down the noise a bit... but the cost of maintaining this list seems high... and it's unclear the performance impact if we have thousands of items on this list, as seems quite likely...

So if we have at least a semi-sane story for both of these answers that means I'll almost never have to worry about it, it would, to quite Bruce, depessmize my outlook :)

it's unclear the performance impact if we have thousands of items on this list, as seems quite likely...

FWIW it needs to be explicitly specified to git blame or configured

GitHub recently started to parse this by default so I'd guess they are confident it's no a significant performance impact. Anywhere you'd have thousands of entries you're also parsing a significant portion of the pack files so parsing one text file into some sort of hash structure seems unlikely to be a big deal.

I'm not worried about things being missing. Developer should and entries that annoy them, but there's not need to be perfect. I'm a little worried there might be some arguments about edge cases, but I think we can codify better policy if and when it becomes an issue.


Maybe include the git config command to enable this file for the repo?

This revision is now accepted and ready to land.Sep 14 2022, 1:38 PM

Update intro with brooks' feedback

This revision now requires review to proceed.Sep 14 2022, 3:03 PM

I'm sold.
How are you going to generate this file?

This revision is now accepted and ready to land.Sep 14 2022, 4:04 PM

How are you going to generate this file?

I imagine starting with something like this to find the largest whitespace-only changes:

git rev-list --no-merges HEAD | while read hash; do
        if [ -z "$(git diff -w $hash^..$hash)" ]; then
                echo -n "$hash "
                git diff $hash^..$hash | wc -l
done | sort -rnk2

It will (slowly) produce something like:
2c9764f36b6f20e9a6c71ce64a21988a394050b6 34916
701301511f0ca044fc71470ed7842cd2c6fee001 12741
4cdc5e12a849871e4e8062a62a31f28545901d84 7424
44bc30192139b0b3c95510ab3b35802bcc6d63e4 5842
29e54af43ee06ed5636b9f567795cd271efc1ef7 1625
a0358e3d5184950b4316f105eb292cbafdea208b 1454
c2fa905cf6c1cf1938a0353679e3bd0b617ca179 1184
86d69de88d1c7e8c7ed906590652008fc6f44d05 705
efbe2af2ce5e444aa06d6d989c147ecea3f3120f 565
21ab8c75c940dd15343b4af28b18a66f377e670a 415

or instead of rev-list --no-merges can use git log --format=%H --grep 'style(9)', but will have to inspect each change to ensure that it is style-only.

But these are just approaches to find candidates for this file. I suspect generally entries will be added by developers landing on a whitespace commit during git blame

Ok it looks like the top twenty whitespace-only commits are:

# regen syscall files after d51198d63b63
# unexpand -a everything
# Remove whitespace at EOL.
# ipfilter module: Style(9) requires a space after return
# Removed whitespace at end-of-line; no content changes. I simply did cd src/share; find man[1-9] -type f|xargs perl -pi -e 's/[ \t]+$//'
# Remove whitespace at EOL.
# ixgbe: remove whitespace in function comments
# s/filesystem/file system/g as discussed on -developers
# Remove trailing whitespace per mdoc lint warning
# ipfilter userland: Style(9) requires a space after return
# First pass of style(9) for #define's.
# White space cleanup for netinet before branch:
# Fix style nit noticed by bde.
# `unexpand -a' should be run _before_ sed 's/^#define /#define^I/g'.
# style(9): clean up trailing whitespace
# Remove all trailing white space from the BBR/Rack fold. Bits left around by emacs (thanks emacs).
# In kernel config files, it is supposed to be 'options<space><tab>' not 'options<tab><tab>', per long standing (but recently not so strictly enforced) convention.
# Remove trailing whitespace
# - Use "device\t" and "options \t" for consistency.
# libc: use standard LF line endings, not CRLF

Followup question: Should we have the people making typos comment changes add their commits here?

The larger question is 'when should we add things here'? Is the cost such that we want to do the 'top commits' only, or 'every boring commit' or somewhere in between (and where on that scale should we advise people).

I think I lean toward "add them when their absence is annoying" as an initial policy. It might be worth some experiments to see if the result of adding all whitespace-only commits is measurable though.

This revision was automatically updated to reflect the committed changes.