From 8a6a6da0c327810a3cec90d33055aeba5fd056a3 Mon Sep 17 00:00:00 2001 From: Hans van Luttikhuizen-Ross Date: Mon, 19 Aug 2024 11:46:57 +0200 Subject: [PATCH 01/10] Enable running in subdirectories --- app/Commands/Audit.php | 3 +-- app/Commands/Install.php | 5 +++-- app/Commands/Run.php | 4 ++-- app/Commands/SkipOnce.php | 2 +- app/Hook.php | 16 +++++++++------- app/Platform.php | 26 +++++++++++++++++--------- tests/Feature/RunTest.php | 4 ++-- tests/Feature/SkipOnceTest.php | 4 ++-- 8 files changed, 37 insertions(+), 27 deletions(-) diff --git a/app/Commands/Audit.php b/app/Commands/Audit.php index 155a876..7ea9af0 100644 --- a/app/Commands/Audit.php +++ b/app/Commands/Audit.php @@ -35,8 +35,7 @@ public function handle(): int ['getGlobalComposerBinDir', Platform::getGlobalComposerBinDir()], ['isWindows', $platform->isWindows() ? 'yes' : 'no'], ['isNotWindows', $platform->isNotWindows() ? 'yes' : 'no'], - ['gitIsInitialized', $platform->gitIsInitialized() ? 'yes' : 'no'], - ['gitIsNotInitialized', $platform->gitIsNotInitialized() ? 'yes' : 'no'], + ['gitIsInitialized', Platform::gitIsInitialized() ? 'yes' : 'no'], ['- global -', ''], ['base_path', base_path()], ['normalized base_path', Platform::normalizePath(base_path())], diff --git a/app/Commands/Install.php b/app/Commands/Install.php index 678db4e..c728c7b 100644 --- a/app/Commands/Install.php +++ b/app/Commands/Install.php @@ -22,7 +22,8 @@ public function __construct( public function handle(): int { - if ($this->platform->gitIsNotInitialized()) { + if (! Platform::gitIsInitialized()) { + $this->info(Platform::getGitDir()); $this->error('Git has not been initialized in this project, aborting...'); return Command::FAILURE; @@ -73,7 +74,7 @@ public function handle(): int if ($this->option('verbose')) { $this->info('Verifying hooks are executable...'); } - exec('chmod +x '.Platform::cwd('.git/hooks').'/*'); + exec('chmod +x '.Platform::getGitDir('hooks').'/*'); exec('chmod +x '.Whisky::base_path('bin/run-hook')); } diff --git a/app/Commands/Run.php b/app/Commands/Run.php index dc206a3..7aea0b3 100644 --- a/app/Commands/Run.php +++ b/app/Commands/Run.php @@ -25,8 +25,8 @@ public function handle(): int return Command::FAILURE; } - if (File::exists(Platform::cwd('.git/hooks/skip-once'))) { - File::delete(Platform::cwd('.git/hooks/skip-once')); + if (File::exists(Platform::getGitDir('hooks/skip-once'))) { + File::delete(Platform::getGitDir('hooks/skip-once')); return Command::SUCCESS; } diff --git a/app/Commands/SkipOnce.php b/app/Commands/SkipOnce.php index 2e04c34..6a9b194 100644 --- a/app/Commands/SkipOnce.php +++ b/app/Commands/SkipOnce.php @@ -14,7 +14,7 @@ class SkipOnce extends Command public function handle(): int { - File::put(Platform::cwd('.git/hooks/skip-once'), ''); + File::put(Platform::getGitDir('hooks/skip-once'), ''); $this->info('Next hook will be skipped.'); $this->line('If the action you\'re about to take has a `pre` and `post` hook'); diff --git a/app/Hook.php b/app/Hook.php index 0dec3bd..68e34f6 100644 --- a/app/Hook.php +++ b/app/Hook.php @@ -29,7 +29,7 @@ public function __construct( public function uninstall(): bool { - $path = Platform::cwd(".git/hooks/{$this->hook}"); + $path = Platform::getGitDir("hooks/{$this->hook}"); if ($this->fileIsMissing()) { // This should be unreachable. @@ -53,7 +53,7 @@ public function uninstall(): bool // use ensureFileExists instead? public function fileExists(): bool { - return File::exists(Platform::cwd(".git/hooks/{$this->hook}")); + return File::exists(Platform::getGitDir("hooks/{$this->hook}")); } public function fileIsMissing(): bool @@ -82,7 +82,7 @@ public function isNotEnabled(): bool */ public function enable(): void { - File::put(Platform::cwd(".git/hooks/{$this->hook}"), '#!/bin/sh'.PHP_EOL); + File::put(Platform::getGitDir("hooks/{$this->hook}"), '#!/bin/sh'.PHP_EOL); } /** @@ -101,7 +101,7 @@ public function ensureExecutable(): void public function isInstalled(): bool { return Str::contains( - File::get(Platform::cwd(".git/hooks/{$this->hook}")), + File::get(Platform::getGitDir("hooks/{$this->hook}")), $this->getSnippets()->toArray(), ); } @@ -112,7 +112,7 @@ public function isInstalled(): bool public function install(): void { File::append( - Platform::cwd(".git/hooks/{$this->hook}"), + Platform::getGitDir("hooks/{$this->hook}"), $this->getSnippets()->first().PHP_EOL, ); } @@ -139,9 +139,11 @@ public function getScripts(): Collection */ public function getSnippets(): Collection { + $cwd = Platform::cwd(); return collect([ - "{$this->bin} run {$this->hook} \"$1\"", + "pushd {$cwd} && {$this->bin} run {$this->hook} \"$1\" && popd", // Legacy Snippets. + "{$this->bin} run {$this->hook} \"$1\"", "{$this->bin} run {$this->hook}", "eval \"$({$this->bin} get-run-cmd {$this->hook})\"", "eval \"$(./vendor/bin/whisky get-run-cmd {$this->hook})\"", @@ -172,7 +174,7 @@ public static function all(int $flags = self::FROM_CONFIG): Collection $result = collect(); if ($flags & self::FROM_GIT) { - $result->push(...collect(File::files(Platform::cwd('.git/hooks'))) + $result->push(...collect(File::files(Platform::getGitDir('hooks'))) ->map(fn (SplFileInfo $file) => $file->getFilename()) ->filter(fn (string $filename) => ! str_ends_with($filename, 'sample')) ); diff --git a/app/Platform.php b/app/Platform.php index 2117221..af89f89 100644 --- a/app/Platform.php +++ b/app/Platform.php @@ -2,8 +2,6 @@ namespace ProjektGopher\Whisky; -use Illuminate\Support\Facades\File; - class Platform { public static function cwd(string $path = ''): string @@ -33,6 +31,21 @@ public static function normalizePath(string $path): string return $path; } + public static function getGitDir(string $path = ''): ?string + { + $output = shell_exec('git rev-parse --git-dir'); + + if ($output === null) { + return null; + } + + if ($path) { + return static::normalizePath(rtrim($output, "\n").'/'.$path); + } + + return static::normalizePath(rtrim($output, "\n")); + } + public static function getGlobalComposerHome(): string { return rtrim(shell_exec('composer -n global config home --quiet'), "\n"); @@ -58,13 +71,8 @@ public function isNotWindows(): bool return ! $this->isWindows(); } - public function gitIsInitialized(): bool - { - return File::exists(Platform::cwd('.git')); - } - - public function gitIsNotInitialized(): bool + public static function gitIsInitialized(): bool { - return ! $this->gitIsInitialized(); + return static::getGitDir() !== null; } } diff --git a/tests/Feature/RunTest.php b/tests/Feature/RunTest.php index db46d02..ab04ece 100644 --- a/tests/Feature/RunTest.php +++ b/tests/Feature/RunTest.php @@ -11,12 +11,12 @@ File::shouldReceive('exists') ->once() - ->with(Platform::cwd('.git/hooks/skip-once')) + ->with(Platform::getGitDir('hooks/skip-once')) ->andReturnTrue(); File::shouldReceive('delete') ->once() - ->with(Platform::cwd('.git/hooks/skip-once')) + ->with(Platform::getGitDir('hooks/skip-once')) ->andReturnTrue(); $tmp_file = Platform::temp_test_path('whisky_test_commit_msg'); diff --git a/tests/Feature/SkipOnceTest.php b/tests/Feature/SkipOnceTest.php index 9cbcffe..6bb2354 100644 --- a/tests/Feature/SkipOnceTest.php +++ b/tests/Feature/SkipOnceTest.php @@ -14,8 +14,8 @@ ->expectsOutputToContain('Next hook will be skipped.') ->assertExitCode(0); - expect(File::exists(Platform::cwd('.git/hooks/skip-once')))->toBeTrue(); + expect(File::exists(Platform::getGitDir('hooks/skip-once')))->toBeTrue(); // Cleanup - File::delete(Platform::cwd('.git/hooks/skip-once')); + File::delete(Platform::getGitDir('hooks/skip-once')); }); From 76dcfa1221a2756f01b9f0df56e64f1531d44350 Mon Sep 17 00:00:00 2001 From: Len Woodward Date: Wed, 4 Sep 2024 13:24:01 -0700 Subject: [PATCH 02/10] rename getGitDir to git_path --- app/Commands/Install.php | 2 +- app/Commands/Run.php | 4 ++-- app/Commands/SkipOnce.php | 2 +- app/Hook.php | 12 ++++++------ app/Platform.php | 4 ++-- tests/Feature/RunTest.php | 4 ++-- tests/Feature/SkipOnceTest.php | 4 ++-- 7 files changed, 16 insertions(+), 16 deletions(-) diff --git a/app/Commands/Install.php b/app/Commands/Install.php index c728c7b..5389d99 100644 --- a/app/Commands/Install.php +++ b/app/Commands/Install.php @@ -74,7 +74,7 @@ public function handle(): int if ($this->option('verbose')) { $this->info('Verifying hooks are executable...'); } - exec('chmod +x '.Platform::getGitDir('hooks').'/*'); + exec('chmod +x '.Platform::git_path('hooks').'/*'); exec('chmod +x '.Whisky::base_path('bin/run-hook')); } diff --git a/app/Commands/Run.php b/app/Commands/Run.php index 7aea0b3..d461977 100644 --- a/app/Commands/Run.php +++ b/app/Commands/Run.php @@ -25,8 +25,8 @@ public function handle(): int return Command::FAILURE; } - if (File::exists(Platform::getGitDir('hooks/skip-once'))) { - File::delete(Platform::getGitDir('hooks/skip-once')); + if (File::exists(Platform::git_path('hooks/skip-once'))) { + File::delete(Platform::git_path('hooks/skip-once')); return Command::SUCCESS; } diff --git a/app/Commands/SkipOnce.php b/app/Commands/SkipOnce.php index 6a9b194..982f023 100644 --- a/app/Commands/SkipOnce.php +++ b/app/Commands/SkipOnce.php @@ -14,7 +14,7 @@ class SkipOnce extends Command public function handle(): int { - File::put(Platform::getGitDir('hooks/skip-once'), ''); + File::put(Platform::git_path('hooks/skip-once'), ''); $this->info('Next hook will be skipped.'); $this->line('If the action you\'re about to take has a `pre` and `post` hook'); diff --git a/app/Hook.php b/app/Hook.php index 68e34f6..31d60bf 100644 --- a/app/Hook.php +++ b/app/Hook.php @@ -29,7 +29,7 @@ public function __construct( public function uninstall(): bool { - $path = Platform::getGitDir("hooks/{$this->hook}"); + $path = Platform::git_path("hooks/{$this->hook}"); if ($this->fileIsMissing()) { // This should be unreachable. @@ -53,7 +53,7 @@ public function uninstall(): bool // use ensureFileExists instead? public function fileExists(): bool { - return File::exists(Platform::getGitDir("hooks/{$this->hook}")); + return File::exists(Platform::git_path("hooks/{$this->hook}")); } public function fileIsMissing(): bool @@ -82,7 +82,7 @@ public function isNotEnabled(): bool */ public function enable(): void { - File::put(Platform::getGitDir("hooks/{$this->hook}"), '#!/bin/sh'.PHP_EOL); + File::put(Platform::git_path("hooks/{$this->hook}"), '#!/bin/sh'.PHP_EOL); } /** @@ -101,7 +101,7 @@ public function ensureExecutable(): void public function isInstalled(): bool { return Str::contains( - File::get(Platform::getGitDir("hooks/{$this->hook}")), + File::get(Platform::git_path("hooks/{$this->hook}")), $this->getSnippets()->toArray(), ); } @@ -112,7 +112,7 @@ public function isInstalled(): bool public function install(): void { File::append( - Platform::getGitDir("hooks/{$this->hook}"), + Platform::git_path("hooks/{$this->hook}"), $this->getSnippets()->first().PHP_EOL, ); } @@ -174,7 +174,7 @@ public static function all(int $flags = self::FROM_CONFIG): Collection $result = collect(); if ($flags & self::FROM_GIT) { - $result->push(...collect(File::files(Platform::getGitDir('hooks'))) + $result->push(...collect(File::files(Platform::git_path('hooks'))) ->map(fn (SplFileInfo $file) => $file->getFilename()) ->filter(fn (string $filename) => ! str_ends_with($filename, 'sample')) ); diff --git a/app/Platform.php b/app/Platform.php index af89f89..1eb61a2 100644 --- a/app/Platform.php +++ b/app/Platform.php @@ -31,7 +31,7 @@ public static function normalizePath(string $path): string return $path; } - public static function getGitDir(string $path = ''): ?string + public static function git_path(string $path = ''): ?string { $output = shell_exec('git rev-parse --git-dir'); @@ -73,6 +73,6 @@ public function isNotWindows(): bool public static function gitIsInitialized(): bool { - return static::getGitDir() !== null; + return static::git_path() !== null; } } diff --git a/tests/Feature/RunTest.php b/tests/Feature/RunTest.php index ab04ece..3829adb 100644 --- a/tests/Feature/RunTest.php +++ b/tests/Feature/RunTest.php @@ -11,12 +11,12 @@ File::shouldReceive('exists') ->once() - ->with(Platform::getGitDir('hooks/skip-once')) + ->with(Platform::git_path('hooks/skip-once')) ->andReturnTrue(); File::shouldReceive('delete') ->once() - ->with(Platform::getGitDir('hooks/skip-once')) + ->with(Platform::git_path('hooks/skip-once')) ->andReturnTrue(); $tmp_file = Platform::temp_test_path('whisky_test_commit_msg'); diff --git a/tests/Feature/SkipOnceTest.php b/tests/Feature/SkipOnceTest.php index 6bb2354..e941778 100644 --- a/tests/Feature/SkipOnceTest.php +++ b/tests/Feature/SkipOnceTest.php @@ -14,8 +14,8 @@ ->expectsOutputToContain('Next hook will be skipped.') ->assertExitCode(0); - expect(File::exists(Platform::getGitDir('hooks/skip-once')))->toBeTrue(); + expect(File::exists(Platform::git_path('hooks/skip-once')))->toBeTrue(); // Cleanup - File::delete(Platform::getGitDir('hooks/skip-once')); + File::delete(Platform::git_path('hooks/skip-once')); }); From fa3040ce70d8963570cef5d802a491eb290b0971 Mon Sep 17 00:00:00 2001 From: Len Woodward Date: Wed, 4 Sep 2024 13:25:33 -0700 Subject: [PATCH 03/10] replace convenience method --- app/Commands/Audit.php | 1 + app/Platform.php | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/app/Commands/Audit.php b/app/Commands/Audit.php index 7ea9af0..208a102 100644 --- a/app/Commands/Audit.php +++ b/app/Commands/Audit.php @@ -36,6 +36,7 @@ public function handle(): int ['isWindows', $platform->isWindows() ? 'yes' : 'no'], ['isNotWindows', $platform->isNotWindows() ? 'yes' : 'no'], ['gitIsInitialized', Platform::gitIsInitialized() ? 'yes' : 'no'], + ['gitIsNotInitialized', Platform::gitIsNotInitialized() ? 'yes' : 'no'], ['- global -', ''], ['base_path', base_path()], ['normalized base_path', Platform::normalizePath(base_path())], diff --git a/app/Platform.php b/app/Platform.php index 1eb61a2..b978862 100644 --- a/app/Platform.php +++ b/app/Platform.php @@ -75,4 +75,9 @@ public static function gitIsInitialized(): bool { return static::git_path() !== null; } + + public static function gitIsNotInitialized(): bool + { + return ! static::gitIsInitialized(); + } } From d71ba21d96b2e541ae829abe69551a483130dcbb Mon Sep 17 00:00:00 2001 From: Len Woodward Date: Wed, 4 Sep 2024 13:27:50 -0700 Subject: [PATCH 04/10] remove extra function call --- app/Commands/Install.php | 1 - 1 file changed, 1 deletion(-) diff --git a/app/Commands/Install.php b/app/Commands/Install.php index 5389d99..ef7064a 100644 --- a/app/Commands/Install.php +++ b/app/Commands/Install.php @@ -23,7 +23,6 @@ public function __construct( public function handle(): int { if (! Platform::gitIsInitialized()) { - $this->info(Platform::getGitDir()); $this->error('Git has not been initialized in this project, aborting...'); return Command::FAILURE; From 3d111ea05a28e1097a20245ec478ef77dd492beb Mon Sep 17 00:00:00 2001 From: Len Woodward Date: Wed, 4 Sep 2024 13:30:33 -0700 Subject: [PATCH 05/10] refactor git_path to use Process facade for testing --- app/Platform.php | 23 +++++++++++++++-------- tests/Feature/InstallTest.php | 15 +++++++++++---- 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/app/Platform.php b/app/Platform.php index b978862..ad28e2b 100644 --- a/app/Platform.php +++ b/app/Platform.php @@ -2,8 +2,12 @@ namespace ProjektGopher\Whisky; +use Illuminate\Support\Facades\Process; + class Platform { + const GIT_DIR_CMD = 'git rev-parse --git-dir'; + public static function cwd(string $path = ''): string { if ($path) { @@ -33,17 +37,20 @@ public static function normalizePath(string $path): string public static function git_path(string $path = ''): ?string { - $output = shell_exec('git rev-parse --git-dir'); - - if ($output === null) { + /** + * We use the `Process` facade here to run this + * command instead of `shell_exec()` because + * it's easier to mock in our test suite. + */ + $output = Process::run(static::GIT_DIR_CMD); + + if ($output->failed()) { return null; } - if ($path) { - return static::normalizePath(rtrim($output, "\n").'/'.$path); - } - - return static::normalizePath(rtrim($output, "\n")); + return empty($path) === true + ? static::normalizePath(rtrim($output->output(), "\n")) + : static::normalizePath(rtrim($output->output(), "\n") . "/{$path}"); } public static function getGlobalComposerHome(): string diff --git a/tests/Feature/InstallTest.php b/tests/Feature/InstallTest.php index 8be21c6..d95b35e 100644 --- a/tests/Feature/InstallTest.php +++ b/tests/Feature/InstallTest.php @@ -1,6 +1,8 @@ artisan('list') @@ -9,10 +11,15 @@ }); it('fails if git is not initialized', function () { - File::shouldReceive('exists') + Process::shouldReceive('run') ->once() - ->with(normalizePath(base_path('.git'))) - ->andReturnFalse(); + ->with(Platform::GIT_DIR_CMD) + ->andReturn(new FakeProcessResult( + command: Platform::GIT_DIR_CMD, + exitCode: 1, + output: '', + errorOutput: 'fatal: not a git repository (or any of the parent directories): .git\n', + )); $this->artisan('install') ->expectsOutputToContain('Git has not been initialized in this project, aborting...') From 2623f4fb7f2ddde69b8717e3fa8412a66fd7bcab Mon Sep 17 00:00:00 2001 From: Len Woodward Date: Wed, 4 Sep 2024 13:30:56 -0700 Subject: [PATCH 06/10] lint --- app/Hook.php | 1 + app/Platform.php | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/app/Hook.php b/app/Hook.php index 31d60bf..a8f43ff 100644 --- a/app/Hook.php +++ b/app/Hook.php @@ -140,6 +140,7 @@ public function getScripts(): Collection public function getSnippets(): Collection { $cwd = Platform::cwd(); + return collect([ "pushd {$cwd} && {$this->bin} run {$this->hook} \"$1\" && popd", // Legacy Snippets. diff --git a/app/Platform.php b/app/Platform.php index ad28e2b..1dbc5a9 100644 --- a/app/Platform.php +++ b/app/Platform.php @@ -50,7 +50,7 @@ public static function git_path(string $path = ''): ?string return empty($path) === true ? static::normalizePath(rtrim($output->output(), "\n")) - : static::normalizePath(rtrim($output->output(), "\n") . "/{$path}"); + : static::normalizePath(rtrim($output->output(), "\n")."/{$path}"); } public static function getGlobalComposerHome(): string From 9aac699242ce44bb1fd8d59091f88ed0b3313782 Mon Sep 17 00:00:00 2001 From: Len Woodward Date: Wed, 4 Sep 2024 13:34:24 -0700 Subject: [PATCH 07/10] use convenience method for readability --- app/Commands/Install.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/Commands/Install.php b/app/Commands/Install.php index ef7064a..322119f 100644 --- a/app/Commands/Install.php +++ b/app/Commands/Install.php @@ -22,7 +22,7 @@ public function __construct( public function handle(): int { - if (! Platform::gitIsInitialized()) { + if (Platform::gitIsNotInitialized()) { $this->error('Git has not been initialized in this project, aborting...'); return Command::FAILURE; From 1f039b8228019b6794c5a40cd0746bbe163fd89b Mon Sep 17 00:00:00 2001 From: Len Woodward Date: Wed, 4 Sep 2024 13:50:23 -0700 Subject: [PATCH 08/10] remove legacy reference --- app/Commands/Install.php | 1 - 1 file changed, 1 deletion(-) diff --git a/app/Commands/Install.php b/app/Commands/Install.php index 322119f..bb9fbc7 100644 --- a/app/Commands/Install.php +++ b/app/Commands/Install.php @@ -74,7 +74,6 @@ public function handle(): int $this->info('Verifying hooks are executable...'); } exec('chmod +x '.Platform::git_path('hooks').'/*'); - exec('chmod +x '.Whisky::base_path('bin/run-hook')); } $this->info('Git hooks installed successfully.'); From 98483678393414be0634cdea4de52f789437fe79 Mon Sep 17 00:00:00 2001 From: Len Woodward Date: Wed, 4 Sep 2024 14:50:16 -0700 Subject: [PATCH 09/10] add reminder --- app/Platform.php | 1 + 1 file changed, 1 insertion(+) diff --git a/app/Platform.php b/app/Platform.php index 1dbc5a9..bae2d74 100644 --- a/app/Platform.php +++ b/app/Platform.php @@ -6,6 +6,7 @@ class Platform { + // Possibly use `git rev-parse --absolute-git-dir` instead for consistency. const GIT_DIR_CMD = 'git rev-parse --git-dir'; public static function cwd(string $path = ''): string From b90933e7240cfba8ac8aa41a9228b0d3a1e5738a Mon Sep 17 00:00:00 2001 From: Len Woodward Date: Fri, 6 Sep 2024 22:37:23 -0700 Subject: [PATCH 10/10] use new git_path method in RunTest --- tests/Feature/RunTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/Feature/RunTest.php b/tests/Feature/RunTest.php index 3829adb..59b58d1 100644 --- a/tests/Feature/RunTest.php +++ b/tests/Feature/RunTest.php @@ -47,7 +47,7 @@ ->andReturnFalse(); File::shouldReceive('exists') - ->with(Platform::cwd('.git/hooks/skip-once')) + ->with(Platform::git_path('hooks/skip-once')) ->andReturnFalse(); $tmp_file = Platform::temp_test_path('whisky_test_commit_msg'); @@ -85,7 +85,7 @@ ->andReturnFalse(); File::shouldReceive('exists') - ->with(Platform::cwd('.git/hooks/skip-once')) + ->with(Platform::git_path('hooks/skip-once')) ->andReturnFalse(); $tmp_file = Platform::temp_test_path('whisky_test_pre_commit'); @@ -115,7 +115,7 @@ ->andReturnFalse(); File::shouldReceive('exists') - ->with(Platform::cwd('.git/hooks/skip-once')) + ->with(Platform::git_path('hooks/skip-once')) ->andReturnFalse(); $tmp_file = Platform::temp_test_path('whisky_test_commit_msg');