Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
dantleech opened this issue Jun 10, 2019 · 5 comments
Assignees

Comments

@dantleech
Copy link
Contributor

dantleech commented Jun 10, 2019

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
Copy link
Contributor Author

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
--------------------------------------------------------------------------------

@dantleech dantleech changed the title Subject and body validator false positive with verbose commit. Diff in git commit -v interpreted as part of the body, causing validation failure. Jun 10, 2019
@sebastianfeldmann
Copy link
Collaborator

sebastianfeldmann commented Jun 12, 2019

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
Copy link
Collaborator

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?

@sebastianfeldmann sebastianfeldmann self-assigned this Jun 13, 2019
@dantleech
Copy link
Contributor Author

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
Copy link
Collaborator

sebastianfeldmann commented Jun 13, 2019

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants