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

Do not prevent Git usage if Captain Hook installation is broken #177

Closed Wirone closed 1 year ago

Wirone commented 2 years ago

There is some edge case scenario, that happens sometimes in our organisation:

There is error: .git/hooks/pre-commit: line 7: vendor/bin/captainhook: No such file or directory which prevents commit

In our case it happens because we have huge monolith app and some people does not work with PHP there, sometimes people just want to fix some texts without building whole app. The problem is that sometimes they trigger Captain Hook's scripts installation (we have it on post-autoload-dump) and then for some reason remove vendor dir. In the end they have invalid environment, with Git hooks installed, and vendor/bin/captainhook called, which fails. It leads to problems because often people do not know how to fix it which causes unneeded discussions.

It would be great if Captain Hook was fail-safe and transparent from developer's point of view. Each script or wrapper should not prevent from using Git when CH installation is broken.

PS. similar scenario is when vendor dir is present, vendor/bin/captainhook is present, but only vendor/captainhook/captainhook is missing for some reason:

Warning: fopen(.../vendor/bin/../captainhook/captainhook/bin/captainhook): failed to open stream: No such file or directory in .../vendor/bin/captainhook on line 35
PHP Warning:  include(phpvfscomposer:///.../vendor/bin/../captainhook/captainhook/bin/captainhook): failed to open stream: "Composer\BinProxyWrapper::stream_open" call failed in .../vendor/bin/captainhook on line 112
Wirone commented 2 years ago

I started implementing this with something like this:

diff --git a/src/Hook/Template/Local/Shell.php b/src/Hook/Template/Local/Shell.php
index 006002e4..d7ce30fb 100644
--- a/src/Hook/Template/Local/Shell.php
+++ b/src/Hook/Template/Local/Shell.php
@@ -91,11 +91,15 @@ class Shell extends Template\Local
             $useTTY,
             [
                 '',
-                $executable
+                'if [ -f "' . $executable . '" ] ; then',
+                '    ' . $executable
                     . ' $INTERACTIVE'
                     . ' --configuration=' . $this->configPath
                     . ' --bootstrap=' . $this->bootstrap
                     . ' hook:' . $hook . ' "$@"' . $useStdIn,
+                'else',
+                '    echo "Captain Hook is not installed, run \`composer install\`!";',
+                'fi',
             ]
         );
     }

But it handles only scenario with missing vendor/bin/captainhook. The second one is harder, I don't actually know where I should hook into πŸ™‚

sebastianfeldmann commented 1 year ago

If they delete the vendor folder they can also remove the hook scripts.

My advice, use the PHAR version install it with phive and keep your dev tooling out of Composer :)

Wirone commented 1 year ago

Yes, they can, but I'm talking about people not necessarily knowing about tooling behind the process. In such cases those people are clueless, they get error and ask about it on Slack πŸ™‚. I believe it could be supported, but understand if you don't want it (but then it shouldn't be closed as "completed" 😝).