captainhookphp / captainhook

CaptainHook is a very flexible git hook manager for software developers that makes sharing git hooks with your team a breeze.
http://captainhook.info
MIT License
1.01k stars 87 forks source link

Diff in `git commit -v` interpreted as part of the body, causing validation failure. #41

Open dantleech opened 5 years ago

dantleech commented 5 years ago

When commiting with git commit -v

If I enter the commit message with only a subject and a new line, the validator passes:

Fix CS on commit

# Please enter the commit message for your changes. Lines starting
# with '#' will be ignored, and an empty message aborts the commit.
#
# Date:      Mon Jun 10 09:24:29 2019 +0100

If I enter the commit message with only a subject, the validator fails:

Foobar
# Please enter the commit message for your changes. Lines starting
# with '#' will be ignored, and an empty message aborts the commit.
#
# On branch master
# Your branch is ahead of 'origin/master' by 1 commit.
#   (use "git push" to publish your local commits)

I would guess this is because -v shows a diff after the commted text in the message, which GIT ignores, but I guess the Captain just strips the commted text? (?)

Full message with git commit -v from above:

Foobar

# Please enter the commit message for your changes. Lines starting
# with '#' will be ignored, and an empty message aborts the commit.
#
# Date:      Mon Jun 10 09:28:40 2019 +0100
#
# On branch master
# Your branch is ahead of 'origin/master' by 2 commits.
#   (use "git push" to publish your local commits)
#
# Changes to be committed:
#   modified:   captainhook.json
#
# Untracked files:
#   .php_cs.cache
#   .phpunit.result.cache
#   STDERR
#   dot
#   dot.dot
#   example/
#   lsp-client.log
#   maestro.log
#   out
#   out.png
#   out.ps
#   phpactor.log
#   replay.json
#
# ------------------------ >8 ------------------------
# Do not modify or remove the line above.
# Everything below it will be ignored.
diff --git a/captainhook.json b/captainhook.json
index 97b181f..a6a48ff 100644
--- a/captainhook.json
+++ b/captainhook.json
@@ -12,7 +12,7 @@
     "enabled": true,
     "actions": [
       {
-        "action": "./vendor/bin/php-cs-fixer fix"
+        "action": "./vendor/bin/php-cs-fixer fix --dry-run"
       },
       {
         "action": "./vendor/bin/phpstan analyse --level=7 lib"
dantleech commented 5 years ago

The diff can also cause a false positive on the body length:

--------------------------------------------------------------------------------
  Body lines should not exceed 72 characters
    Line 15 of your body exceeds the max line length
  Subject and body have to be separated by a blank line
--------------------------------------------------------------------------------
sebastianfeldmann commented 5 years ago

Yes the Cap'n removes comment lines from your commit message before validating it. Since they will be removed by git that's seemed like a reasonable thing to do.

Somehow this line

# ------------------------ >8 ------------------------

makes git ignore all lines below. I have to check the syntax for this and make the message parser a lot smarter, because right now it thinks that the diff is part of the commit message. That's why it fails.

sebastianfeldmann commented 5 years ago

This is fixed in sebastianfeldmann/git version 2.2.2. If you run composer update you should get the latest version including the scissors (--- >8 ---) fix.

The odd thing I noticed. If I use

git commit 
# exit with empty commit message
Aborting commit due to empty commit message.

everything works now. If I use

git commit -v
# exit with empty message
# the hooks run and fail due to the empty commit message

But if I add a space or an empty line to the commit message it fails like the version without - v. Can you verify this or is it just my machine?

dantleech commented 5 years ago

Empty commiy message (completely deleting ALL content from the file buffer including comments):

======== Execute hook: commit-msg ==============================================

Action: \CaptainHook\App\Hook\Message\Action\Beams
--------------------------------------------------------------------------------
CAPTAINHOOK FOUND 1 PROBLEM IN YOUR COMMIT MESSAGE
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
  Subject line has to start with an upper case letter
--------------------------------------------------------------------------------

In PHP.php line 51:

  Execution failed:                                                            
  Commit message validation failed in /home/daniel/www/dantleech/maestro/vend  
  or/captainhook/captainhook/src/Hook/Message/Action/Book.php line 63          

Having an empty line:

======== Execute hook: commit-msg ==============================================

Action: \CaptainHook\App\Hook\Message\Action\Beams
All rules passed
Aborting commit due to empty commit message.

Just exiting:

======== Execute hook: commit-msg ==============================================

Action: \CaptainHook\App\Hook\Message\Action\Beams
--------------------------------------------------------------------------------
CAPTAINHOOK FOUND 1 PROBLEM IN YOUR COMMIT MESSAGE
--------------------------------------------------------------------------------

# Please enter the commit message for your changes. Lines starting
# with '#' will be ignored, and an empty message aborts the commit.
#
# On branch master
# Your branch is up to date with 'origin/master'.
#
# Changes to be committed:
#       modified:   lib/Task/Task.php
#
# Changes not staged for commit:
#       modified:   captainhook.json
#
# Untracked files:
#       .php_cs.cache
#       .phpunit.result.cache
#       STDERR
#       dot
#       dot.dot
#       example/
#       lsp-client.log
#       maestro.log
#       out
#       out.png
#       out.ps
#       phpactor.log
#       replay.json
#
# ------------------------ >8 ------------------------
# Do not modify or remove the line above.
# Everything below it will be ignored.
diff --git a/lib/Task/Task.php b/lib/Task/Task.php
index 279ba80..d9c6628 100644
--- a/lib/Task/Task.php
+++ b/lib/Task/Task.php
@@ -1,5 +1,6 @@
 <?php

+
 namespace Maestro\Task;

 interface Task
--------------------------------------------------------------------------------
  Subject line has to start with an upper case letter
--------------------------------------------------------------------------------

In PHP.php line 51:

  Execution failed:                                                            
  Commit message validation failed in /home/daniel/www/dantleech/maestro/vend  
  or/captainhook/captainhook/src/Hook/Message/Action/Book.php line 63          
sebastianfeldmann commented 5 years ago

I don't want to scream "wolf" too fast, but this seems to be a git issue. If you run git commit -v somehow the commit is not aborted if the message is empty. But if the message contains a whitespace git will cancel the commit. Seems strange but so far all experiments confirm my suspicions.

For some reason the commit-message hook gets triggered even if the commit message is empty and the whole commit should be canceled by git.