Page MenuHomeFreeBSD

Add git-blame ignore file
ClosedPublic

Authored by emaste on Sep 11 2022, 3:58 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Oct 26, 3:32 PM
Unknown Object (File)
Thu, Oct 24, 9:22 PM
Unknown Object (File)
Thu, Oct 24, 9:22 PM
Unknown Object (File)
Thu, Oct 24, 9:22 PM
Unknown Object (File)
Sun, Oct 20, 11:20 PM
Unknown Object (File)
Sun, Oct 20, 11:06 PM
Unknown Object (File)
Thu, Oct 17, 11:04 PM
Unknown Object (File)
Thu, Oct 17, 11:04 PM
Subscribers

Details

Summary

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

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
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.

.git-blame-ignore-revs
4

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
        fi
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
2c9764f36b6f20e9a6c71ce64a21988a394050b6
# unexpand -a everything
014585d12527a3c053778459bbd93cdb37421430
# Remove whitespace at EOL.
7ebcc426efefea9cd16ac6f565e72030c608f7a4
# ipfilter module: Style(9) requires a space after return
701301511f0ca044fc71470ed7842cd2c6fee001
# 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]+$//'
c1f3e4bf21571a88d4269a87f57e3e4ff2ba3ef6
# Remove whitespace at EOL.
f247324df75b7f55b48b92acb3b42a5ae2deac8a
# ixgbe: remove whitespace in function comments
4cdc5e12a849871e4e8062a62a31f28545901d84
# s/filesystem/file system/g as discussed on -developers
ce66ddb76352a2e5f34aacdbe7733d92e60aff17
# Remove trailing whitespace per mdoc lint warning
50d675f7a9fca2f3fba3a23ac41a8ae2c0162e4e
# ipfilter userland: Style(9) requires a space after return
44bc30192139b0b3c95510ab3b35802bcc6d63e4
# First pass of style(9) for #define's.
ddd36f9facc3a776056bb354bb9b13dcf3d9ccf5
# White space cleanup for netinet before branch:
a4f757cd5dc1421d90d72a34853247e0c2c18cce
# Fix style nit noticed by bde.
78fd9f198c7e78f4fe384ec772cc908631bec6ee
# `unexpand -a' should be run _before_ sed 's/^#define /#define^I/g'.
d93389d676c34ab28aa75b3bbc4cd9a8b5dc86e5
# style(9): clean up trailing whitespace
e1d581b289848ffa97c452756dc52d45efb68a01
# Remove all trailing white space from the BBR/Rack fold. Bits left around by emacs (thanks emacs).
3fba40d9f2b3551ec627891640dd63829dc13fba
# 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.
d4f95c889dd2cc01eca256766fe5ccb289e0750a
# Remove trailing whitespace
27cc91fbf84cc6c1cef81df859125f42292a2b69
# - Use "device\t" and "options \t" for consistency.
b3b17597eab61d4ea34f1cfd653c53d284e18c75
# libc: use standard LF line endings, not CRLF
29e54af43ee06ed5636b9f567795cd271efc1ef7

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.