Perl -- commit-msg hook doesn't stop the commit when exiting 1

Context :

I am using some git-hooks to apply automatically some formatting options on Perl scripts. On a pre-commit hook I clean and reformat my scripts using perltidy. I want to check what the user put on the commit message and if it's empty or it's equivalent to "abort", we want to prevent the commit and undo the formatting modifications.

Problem :

I removed the .sample extension of the git hook and made it executable using chmod u+x .git/hooks/commit-msg but when my script exit 1 the commit isn't stop like it should.

commit-msg

This hook is invoked by git-commit[1] and git-merge[1], and can be bypassed with the --no-verify option. It takes a single parameter, the name of the file that holds the proposed commit log message. Exiting with a non-zero status causes the command to abort.

source : https://git-scm.com/docs/githooks#_commit_msg

#!/usr/bin/perl -w

use strict;
use warnings;

# Get the path to the files in which we have the commit message
my $commit_file = $ARGV[0];

# Read the file and extract the commit message (lines which don't start with #) 
my @commit_msg;
open(my $fh, "<", "$commit_file");
while (my $line = <$fh>) {
    if (substr($line, 0, 1) ne "#") {
        push(@commit_msg, $line);
    }
}

# Check the message isn't empty or we don't have a "abort" line
my $boolean = 0;
foreach my $line (@commit_msg) {
    if ($line ne "abort" && $line ne "") {
        $boolean = 1;
    }
}

if ($boolean == 0) {
    print "We should commit the modifications\n";
    exit 0; # Don't prevent commit
}
else {
    print "We shouldn't commit the modifications\n";
    exit 1; # Prevent commit
}

The script is executable and works ! If I enter "abort" when I commit some stuff, it prints "We shouldn't commit the modifications" but commit them discarding the exit 1...

I hope someone will be able to help ! I am new with git-hook and can't find a solution. Perhaps I miss something on Stackoverflow but I didn't find a post answering this issue.

Best,

Antoine

Edit : I don't commit using : --no-verify

Solution

When I first installed your hook, I wasn’t able to get it to commit anything, but that’s because the hook has too many negatives. Your language teacher’s admonitions to avoid double negatives will help you out when writing software too. The hook attempts to look for a valid condition by testing in the negative sense if a line looks good, set $boolean to 1 if so, but then exit 0 (indicating success) only if $boolean was 0.

The non-descript name $boolean is likely responsible in part. You may have lost track of your intended meaning between setting it and which exit status you wanted to produce. Further, your intent behind the logic will fail as long as the last line of the commit message is valid.

The code below functions in the way you want with git 2.17.1.

#! /usr/bin/perl -w

use strict;
use warnings;

die "Usage: $0 commit-log-message\n" unless @ARGV == 1; # (1)

# Get the path to the files in which we have the commit message
my $commit_file = shift; # (2)

# Read the file and extract the commit message (lines which don't start with #) 
my $commit_msg = "";
open my $fh, "<", $commit_file or die "$0: open $commit_file: $!"; # (3)
while (<$fh>) {        # (4)
    next if /^#/;      # (5)
    $commit_msg .= $_;
}

# Check the message isn't empty or we don't have an "abort" line
my $valid_commit_msg = $commit_msg ne "" && $commit_msg !~ /^abort$/m; # (6)

if ($valid_commit_msg) { # (7)
    print "We should commit the modifications\n";
    exit 0; # Don't prevent commit
}
else {
    print "We shouldn't commit the modifications\n";
    exit 1; # Prevent commit
}

(1) Yes, git is supposed to provide the name of the file with the log message, but sanity check for it in case the code gets copied or otherwise installed in the wrong hook.

(2) Pluck the argument from @ARGV with shift.

(3) Always, always, always check the return value from open. Note that the error message if it does fail contains the name of the program that had the error ($0), what it was trying to do ("open $commit_file"), and the error ($!). Develop this habit. It will save you a lot of frustration one day.

(4) Rather than copying lines into an array, concatenate them all into a single scalar. Use while (<$fh>) { ... } to see each line in $_, which is more idiomatic Perl and declutters your code.

(5) Skipping comment lines then becomes a simple next if /^#/;.

(6) Say what you mean. Rather than the mechanism ($boolean) name your intention. You want to know that a commit message is valid before letting it pass. A valid commit message must meet two conditions:

The commit message is non-empty.
The commit message does not have any line whose sole contents are abort.

Rendered in Perl, this is

my $valid_commit_msg = $commit_msg ne "" && $commit_msg !~ /^abort$/m;

A couple of notes:

The !~ operator inverts the sense of the regex match, i.e., $commit_msg must not contain abort.
The /m switch at the end of the pattern is for multi-line mode. It makes the ^ and $ anchors match at the beginning and end of lines within the target rather than the leftmost and rightmost characters only.

(7) Use $valid_commit_msg as a boolean in a way that reads naturally.

if ($valid_commit_msg) { ... }

is preferable to if ($valid_commit_msg == 0) { ... } because the 0 value was the wrong one, repeating the good value is redundant, and a value hanging out at the end is easy to overlook.