gitlint icon indicating copy to clipboard operation
gitlint copied to clipboard

Incorrect commit message parsing for git versions < 1.7.2

Open AlexMooney opened this issue 8 years ago • 5 comments

(venv)vagrant@vagrant-amooney:DEVELOPMENT:current> gitlint --version
gitlint, version 0.8.2

(venv)vagrant@vagrant-amooney:DEVELOPMENT:current> git log -1
commit b0be6977c6fb474b3ca270fffb0485f17bfb48c4
Author: Alex Mooney <[email protected]>
Date:   Thu Apr 27 14:44:10 2017 -0400

    This title is a reasonable length

    Oh man though, this body? It just keeps going forever on one line as if someone with a substandard editor wrote it. It is, like, crazy long with no line breaks at all.

(venv)vagrant@vagrant-amooney:DEVELOPMENT:current> gitlint --debug
config-path: /vagrant/code/claims/.gitlint
[GENERAL]
extra-path: None
ignore: body-is-missing
ignore-merge-commits: True
verbosity: 3
debug: True
target: /vagrant/code/claims
[RULES]
  T1: title-max-length
     line-length=72
  T2: title-trailing-whitespace
  T6: title-leading-whitespace
  T3: title-trailing-punctuation
  T4: title-hard-tab
  T5: title-must-not-contain-word
     words=WIP
  T7: title-match-regex
     regex=.*
  B1: body-max-line-length
     line-length=80
  B5: body-min-length
     min-length=20
  B6: body-is-missing
     ignore-merge-commits=True
  B2: body-trailing-whitespace
  B3: body-hard-tab
  B4: body-first-line-empty
  B7: body-changed-file-mention
     files=

DEBUG: gitlint.lint Linting commit b0be6977c6fb474b3ca270fffb0485f17bfb48c4
DEBUG: gitlint.cli Exit Code = 0

AlexMooney avatar Apr 27 '17 18:04 AlexMooney

So I just tried this and it works fine on my machine. I'd honestly be a surprised if this was broken since we have multiple unit tests on it, but you never know of course.

I think this might actually be related to the other issue you're having (https://github.com/jorisroovers/gitlint/pull/29) with the titles coming through as %B. I think it's not that the rules aren't working but rather that the git log message parsing isn't working.

If that's the case, then this is likely due to your platform or git client. What linux flavor are you on? Can you also give me the output of the following 2 commands?

# What git version
git --version
# gitlint invokes this command to parse the message
git log -1 --pretty=%aN,%aE,%ai,%P%n%B

Thanks!

jorisroovers avatar Apr 27 '17 19:04 jorisroovers

You're right. %B is not being parsed on our Redhat machines because of the older version of git. It seems like git log -1 --pretty=%aN,%aE,%ai,%P%n%s%n%n%b might do the trick for supporting older versions. On git version 2.12.2, at least, the two produced identical output.

vagrant@vagrant-amooney:DEVELOPMENT:current> git --version
git version 1.7.1

vagrant@vagrant-amooney:DEVELOPMENT:current> git log -1 --pretty=%aN,%aE,%ai,%P%n%B
Alex Mooney,[email protected],2017-04-27 14:44:10 -0400,b6cfa976edee210158cd9111f3b5eb83170f7303
%B

vagrant@vagrant-amooney:DEVELOPMENT:current> git log -1 --pretty=%aN,%aE,%ai,%P%n%s%n%n%b
Alex Mooney,[email protected],2017-04-27 14:44:10 -0400,b6cfa976edee210158cd9111f3b5eb83170f7303
This title is a reasonable length

Oh man though, this body? It just keeps going forever on one line as if someone with a substandard editor wrote it. It's like crazy long with no linebreaks.

AlexMooney avatar Apr 28 '17 13:04 AlexMooney

I did some more digging into this.

For future reference, this is the official spec from https://git-scm.com/docs/pretty-formats

'%b': body
'%B': raw body (unwrapped subject and body)

The following answer on SO provides a slightly more detailed answer: http://stackoverflow.com/a/28820275

This %B functionality was added in git v1.7.2 in this commit https://github.com/git/git/commit/1367b12ad623e28546ba40c435015d94e7fbb248.

@AlexMooney Obviously, the easiest solution is updating to git v1.7.2 or above. Given that this particular functionality was added in git in 2010 (i.e. over 7 years ago), I don't think that's an unreasonable ask.

I'll spend some more time exploring alternatives, but your example using a combination of %s and %b doesn't work for all use-cases. In a nutshell, git determines the difference between title (%s) and body (%b) based on the first empty line it finds, while gitlint only considers the actual first line to be the title. This means for example that if you add text on the second line (which should typically be empty, gitlint enforces this), that git will still return it as part of %s.

Coding around this is definitely not unsurmountable, it's just that I'm a little wary how this will impact different versions and distributions of git. We should obviously make sure we don't break gitlint for anyone else :) My current thoughts are that we should probably keep using %B and fallback to a combination of %s and %b for git versions prior to v1.7.2.

I'm not sure when I can make time for this, it might be fairly soon, but might also take a few months (this is still a hobby project for me :-)). Appreciate any PRs, but I do think we need to discuss the exact logic a bit more before someone starts digging in.

jorisroovers avatar May 03 '17 10:05 jorisroovers

I'd propose emitting an error if Git is too old. The said version needed for this was released around 12 years ago. It doesn't seem important enough to implement a workaround for such an old version — there hasn't been any interest in this issue for over 5 years already.

webknjaz avatar Nov 21 '22 14:11 webknjaz

Yes, that's exactly what I had in mind at this point and why I've kept this open 👍

jorisroovers avatar Nov 22 '22 10:11 jorisroovers