Ignore directories and show warning for unexecutable files (#41)

Author: tarmolov

lib/fs-helpers.js

@@ -53,7 +53,18 @@ var helpers = {
         } :
         function (path) {
             return fs.existsSync(path);
-        }
+        },
+
+    /**
+     * @param {String} path
+     * @returns {Boolean}
+     */
+    isExecutable: function (path) {
+        var stats = fs.lstatSync(path);
+        return (stats.mode & 1) ||
+            (stats.mode & 8) && process.getgid && stats.gid === process.getgid() ||
+            (stats.mode & 64) && process.getuid && stats.uid === process.getuid();
+    }
 };
 
 module.exports = helpers;

lib/git-hooks.js

@@ -3,7 +3,6 @@ var util = require('util');
 var spawn = require('child_process').spawn;
 var fs = require('fs');
 var fsHelpers = require('./fs-helpers');
-var exec = require('child_process').exec;
 
 var HOOKS_DIRNAME = 'hooks';
 var HOOKS_OLD_DIRNAME = 'hooks.old';
@@ -26,19 +25,6 @@ var HOOKS = [
     'update'
 ];
 
-/**
- * @param {String[]} hooks List of hook's paths with possible excludes(.gitignore files)
- * @param {function} callback Filtered hooks will be passed in the callback
- */
-function excludeIgnoredPaths(hooks, callback) {
-    exec('git check-ignore ' + hooks.join(' '), function (error, output) {
-        // intentionally ignore errors
-        callback(hooks.filter(function (hookName) {
-            return output.indexOf(hookName) === -1;
-        }));
-    });
-}
-
 module.exports = {
     /**
      * Installs git hooks.
@@ -123,12 +109,22 @@ module.exports = {
 
         if (fsHelpers.exists(hooksDirname)) {
             var list = fs.readdirSync(hooksDirname);
-            var hooks = list.map(function (hookName) {
-                return path.resolve(hooksDirname, hookName);
-            });
-            excludeIgnoredPaths(hooks, function (filteredHooks) {
-                runHooks(filteredHooks, args, callback);
-            });
+            var hooks = list
+                .map(function (hookName) {
+                    return path.resolve(hooksDirname, hookName);
+                })
+                .filter(function (hookPath) {
+                    var isFile = fs.lstatSync(hookPath).isFile();
+                    var isExecutable = fs.lstatSync(hookPath).isFile() && fsHelpers.isExecutable(hookPath);
+
+                    if (isFile && !isExecutable) {
+                        console.warn('[GIT-HOOKS WARNING] Non-executable file ' + hookPath + ' is skipped');
+                    }
+
+                    return isFile && isExecutable;
+                });
+
+            runHooks(hooks, args, callback);
         } else {
             callback(0);
         }
@@ -162,16 +158,6 @@ function runHooks(hooks, args, callback) {
     }
 }
 
-/**
- * @param {fs.Stats} stats
- * @returns {Boolean}
- */
-function isExecutable(stats) {
-    return (stats.mode & 1) ||
-        (stats.mode & 8) && process.getgid && stats.gid === process.getgid() ||
-        (stats.mode & 64) && process.getuid && stats.uid === process.getuid();
-}
-
 /**
  * Spawns hook as a separate process.
  *
@@ -180,11 +166,6 @@ function isExecutable(stats) {
  * @returns {ChildProcess}
  */
 function spawnHook(hookName, args) {
-    var stats = fs.statSync(hookName);
-    var isHookExecutable = stats && stats.isFile() && isExecutable(stats);
-    if (!isHookExecutable) {
-        throw new Error('Cannot execute hook: ' + hookName + '. Please check file permissions.');
-    }
     args = args || [];
     return spawn(hookName, args, {stdio: 'inherit'});
 }

tests/run.test.js

@@ -8,7 +8,6 @@ var GIT_ROOT = SANDBOX_PATH + '.git/';
 var GIT_HOOKS = GIT_ROOT + 'hooks';
 var PRECOMMIT_HOOK_PATH = GIT_HOOKS + '/pre-commit';
 var PROJECT_PRECOMMIT_HOOK = SANDBOX_PATH + '.githooks/pre-commit/';
-var GIT_IGNORE = SANDBOX_PATH + '.gitignore';
 
 function createHook(path, content) {
     fs.writeFileSync(path, '#!/bin/bash\n' + content);
@@ -38,15 +37,39 @@ describe('git-hook runner', function () {
         });
 
         describe('and a hook is unexecutable', function () {
+            var oldConsoleWarn = console.warn;
+            var consoleLogOutput = '';
+            var logFile = SANDBOX_PATH + 'hello.log';
+            beforeEach(function () {
+                var hookPath = PROJECT_PRECOMMIT_HOOK + 'hello';
+                fs.writeFileSync(hookPath, '#!/bin/bash\n' + 'echo hello > ' + logFile);
+                console.warn = function (str) {
+                    consoleLogOutput += str;
+                };
+            });
+
+            afterEach(function () {
+                console.warn = oldConsoleWarn;
+            });
+
+            it('should skip it', function (done) {
+                gitHooks.run(PRECOMMIT_HOOK_PATH, [], function (code) {
+                    code.should.equal(0);
+                    consoleLogOutput.should.match(/^\[GIT-HOOKS WARNING\]/);
+                    fs.existsSync(logFile).should.be.false;
+                    done();
+                });
+            });
+        });
+
+        describe('and directory in hooks directory is found', function () {
             beforeEach(function () {
-                var logFile = SANDBOX_PATH + 'hello.log';
-                fs.writeFileSync(PROJECT_PRECOMMIT_HOOK + 'hello', '#!/bin/bash\n' + 'echo hello > ' + logFile);
+                fsHelpers.makeDir(PROJECT_PRECOMMIT_HOOK + '/zzzz');
             });
 
-            it('should return an error', function (done) {
-                gitHooks.run(PRECOMMIT_HOOK_PATH, [], function (code, error) {
-                    code.should.equal(1);
-                    error.should.be.ok;
+            it('should skip it', function (done) {
+                gitHooks.run(PRECOMMIT_HOOK_PATH, [], function (code) {
+                    code.should.equal(0);
                     done();
                 });
             });
@@ -107,23 +130,5 @@ describe('git-hook runner', function () {
                 });
             });
         });
-
-        describe('do not run git-ignored scripts from hooks directory', function () {
-            var ignoreFilename = 'ignore-me';
-            var ignoreContent = ignoreFilename + '\n*.swp';
-
-            beforeEach(function () {
-                fs.writeFileSync(GIT_IGNORE, ignoreContent);
-                fs.writeFileSync(PROJECT_PRECOMMIT_HOOK + ignoreFilename, 'exit -1');
-                fs.writeFileSync(PROJECT_PRECOMMIT_HOOK + 'test.swp', 'exit -1');
-            });
-
-            it('should ignore file with wrong permissions in hooks directory', function (done) {
-                gitHooks.run(PRECOMMIT_HOOK_PATH, [], function (code) {
-                    code.should.equal(0);
-                    done();
-                });
-            });
-        });
     });
 });