From 47d15559863c3cab48853ae301560c9592b8d76c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Pineau?= Date: Thu, 15 Oct 2026 10:17:07 +0200 Subject: [PATCH 1/2] Allow gitlib 2 gitlib 2 requires PHP 8.4, so grumphp supports both gitlib 1 and 2: it uses the API of gitlib 2 when it's installed, without its deprecations. - The repository is configured with a CliDriver instead of options. Its "inherit_environment_variables" option has no counterpart: git inherits the environment variables of PHP anyway, and Dotenv and the paths registrar write them to $_ENV or with putenv(). - The diff of the hooks is parsed with its repository. - The changed files are named by File::getName(), the new name of a renamed file with both versions: getNewName() is nullable in gitlib 2. - The specs no longer double the classes of gitlib, which are final in gitlib 2: they run on temporary git repositories instead. - The raw diff of the specs had spaces instead of a tab, and the status of a modification for a new file: gitlib 2 refuses it. --- composer.json | 2 +- composer.lock | 2 +- resources/config/services.yml | 4 +- .../StashUnstagedChangesSubscriberSpec.php | 31 +++++--- spec/Git/GitRepositorySpec.php | 51 ++++++++------ spec/Locator/ChangedFilesSpec.php | 60 ++++++++-------- src/Git/GitRepository.php | 25 ++++--- src/Locator/ChangedFiles.php | 4 +- src/Locator/GitRepositoryLocator.php | 25 +++++-- test/Helpers/TemporaryGitRepository.php | 70 +++++++++++++++++++ 10 files changed, 191 insertions(+), 83 deletions(-) create mode 100644 test/Helpers/TemporaryGitRepository.php diff --git a/composer.json b/composer.json index 0d145f906..24c7afcf6 100644 --- a/composer.json +++ b/composer.json @@ -20,7 +20,7 @@ "amphp/amp": "^3.0", "amphp/parallel": "^2.1", "doctrine/collections": "^1.6.8 || ^2.0 || ^3.0", - "gitonomy/gitlib": "^1.6", + "gitonomy/gitlib": "^1.6 || ^2.0", "laravel/serializable-closure": "^2.0", "monolog/monolog": "^2.0 || ^3.0", "ondram/ci-detector": "^4.0", diff --git a/composer.lock b/composer.lock index efebcae7d..9b3dd441b 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "65cdb516cf7985eaf6d89dc3432f4d52", + "content-hash": "ff00f21df91f786596588545f7c80210", "packages": [ { "name": "amphp/amp", diff --git a/resources/config/services.yml b/resources/config/services.yml index 9efde599f..2fb587cec 100644 --- a/resources/config/services.yml +++ b/resources/config/services.yml @@ -31,9 +31,7 @@ services: GrumPHP\Git\GitRepository: arguments: - '@GrumPHP\Locator\GitRepositoryLocator' - - - logger: '@grumphp.logger' - inherit_environment_variables: true + - '@grumphp.logger' GrumPHP\IO\IOFactory: arguments: diff --git a/spec/Event/Subscriber/StashUnstagedChangesSubscriberSpec.php b/spec/Event/Subscriber/StashUnstagedChangesSubscriberSpec.php index ca6c35923..775324cb6 100644 --- a/spec/Event/Subscriber/StashUnstagedChangesSubscriberSpec.php +++ b/spec/Event/Subscriber/StashUnstagedChangesSubscriberSpec.php @@ -2,8 +2,6 @@ namespace spec\GrumPHP\Event\Subscriber; -use Gitonomy\Git\Diff\Diff; -use Gitonomy\Git\WorkingCopy; use GrumPHP\Collection\FilesCollection; use GrumPHP\Collection\TaskResultCollection; use GrumPHP\Collection\TasksCollection; @@ -15,23 +13,38 @@ use GrumPHP\IO\IOInterface; use GrumPHP\Task\Context\GitPreCommitContext; use GrumPHP\Task\Context\RunContext; +use GrumPHPTest\Helpers\TemporaryGitRepository; use PhpSpec\ObjectBehavior; use Prophecy\Argument; use Symfony\Component\EventDispatcher\EventSubscriberInterface; class StashUnstagedChangesSubscriberSpec extends ObjectBehavior { - function let(GitStashConfig $config, GitRepository $repository, IOInterface $io, WorkingCopy $workingCopy, Diff $unstaged) + /** + * @var TemporaryGitRepository + */ + private $gitRepository; + + function let(GitStashConfig $config, GitRepository $repository, IOInterface $io) { + $this->gitRepository = TemporaryGitRepository::create(); + $this->gitRepository->dumpFile('file1.php', 'committed'); + $this->gitRepository->git('add', '--all'); + $this->gitRepository->git('commit', '--quiet', '--message=Initial commit'); + $this->gitRepository->dumpFile('file1.php', 'unstaged'); + $this->gitRepository->dumpFile('untracked.php'); + $config->ignoreUnstagedChanges()->willReturn(true); - $repository->getWorkingCopy()->willReturn($workingCopy); - $workingCopy->getDiffPending()->willReturn($unstaged); - $unstaged->getFiles()->willReturn(['file1.php']); - $workingCopy->getUntrackedFiles()->willReturn(['untracked.php']); + $repository->getWorkingCopy()->willReturn($this->gitRepository->getRepository()->getWorkingCopy()); $this->beConstructedWith($config, $repository, $io); } + function letGo() + { + $this->gitRepository->remove(); + } + function it_is_initializable() { $this->shouldHaveType(StashUnstagedChangesSubscriber::class); @@ -68,10 +81,10 @@ function it_should_not_run_in_invalid_context(GitRepository $repository) $repository->run(Argument::cetera())->shouldNotBeCalled(); } - function it_should_not_run_when_there_are_no_unstaged_changes(GitRepository $repository, Diff $unstaged) + function it_should_not_run_when_there_are_no_unstaged_changes(GitRepository $repository) { $event = new RunnerEvent(new TasksCollection(), new RunContext(new FilesCollection()), new TaskResultCollection()); - $unstaged->getFiles()->willReturn([]); + $this->gitRepository->git('checkout', '--', 'file1.php'); $this->saveStash($event); $this->popStash($event); diff --git a/spec/Git/GitRepositorySpec.php b/spec/Git/GitRepositorySpec.php index cce5d6155..779fa22d1 100644 --- a/spec/Git/GitRepositorySpec.php +++ b/spec/Git/GitRepositorySpec.php @@ -5,20 +5,31 @@ use Gitonomy\Git\Diff\Diff; use Gitonomy\Git\Diff\File; use Gitonomy\Git\Exception\ProcessException; -use Gitonomy\Git\Repository; use Gitonomy\Git\WorkingCopy; use GrumPHP\Locator\GitRepositoryLocator; +use GrumPHPTest\Helpers\TemporaryGitRepository; use PhpSpec\ObjectBehavior; use GrumPHP\Git\GitRepository; -use Prophecy\Argument; +use Psr\Log\LoggerInterface; use Symfony\Component\Process\Process; class GitRepositorySpec extends ObjectBehavior { - public function let(GitRepositoryLocator $locator, Repository $repository): void + /** + * @var TemporaryGitRepository + */ + private $gitRepository; + + public function let(GitRepositoryLocator $locator, LoggerInterface $logger): void + { + $this->gitRepository = TemporaryGitRepository::create(); + $this->beConstructedWith($locator, $logger); + $locator->locate($logger)->willReturn($this->gitRepository->getRepository()); + } + + public function letGo(): void { - $this->beConstructedWith($locator, []); - $locator->locate([])->willReturn($repository); + $this->gitRepository->remove(); } function it_is_initializable() @@ -26,25 +37,24 @@ function it_is_initializable() $this->shouldHaveType(GitRepository::class); } - public function it_is_lazy(GitRepositoryLocator $locator, Repository $repository): void + public function it_is_lazy(GitRepositoryLocator $locator, LoggerInterface $logger): void { - $repository->run(Argument::cetera())->willReturn('ok'); - $this->run('command', []); - $this->run('command', []); + $this->run('rev-parse', ['--git-dir']); + $this->run('rev-parse', ['--git-dir']); - $locator->locate([])->shouldBeCalledTimes(1); + $locator->locate($logger)->shouldBeCalledTimes(1); } - public function it_can_run_commands(Repository $repository): void + public function it_can_run_commands(): void { - $repository->run('command', [])->willReturn('ok'); - $this->run('command', [])->shouldBe('ok'); + $this->run('rev-parse', ['--is-bare-repository'])->shouldBe("false\n"); } public function it_can_deal_with_errornous_throwing_runs(Process $process): void { $process->getCommandLine()->willReturn(''); $process->getErrorOutput()->willReturn(''); + $process->getExitCode()->willReturn(1); $process->getOutput()->willReturn(''); $this->tryToRunWithFallback( @@ -75,31 +85,30 @@ function () { )->shouldBe('ok'); } - public function it_can_fetch_working_copy(Repository $repository, WorkingCopy $workingCopy): void + public function it_can_fetch_working_copy(): void { - $repository->getWorkingCopy()->willReturn($workingCopy); - $this->getWorkingCopy()->shouldBe($workingCopy); + $this->getWorkingCopy()->shouldBeAnInstanceOf(WorkingCopy::class); } - public function it_can_parse_raw_diff(Repository $repository): void + public function it_can_parse_raw_diff(): void { - $rawDiff = ':100644 100644 c09a0d49 77e5a35c M file.txt + $rawDiff = ":000000 100644 0000000 d95f3ad A\tfile.txt diff --git a/file.txt b/file.txt new file mode 100644 -index 0000000000000000000000000000000000000000..9766475a4185a151dc9d56d614ffb9aaea3bfd42 +index 0000000000000000000000000000000000000000..d95f3ad14dee633a758d2e331151e950dd13e4ed --- /dev/null +++ b/file.txt @@ -0,0 +1 @@ +content -'; +"; $diff = $this->createRawDiff($rawDiff); $diff->shouldBeAnInstanceOf(Diff::class); /** @var File $file */ $file = $diff->getFiles()[0]; $file->isCreation()->shouldBe(true); $file->getName()->shouldBe('file.txt'); - $file->getRepository()->shouldBe($repository); + $file->getRepository()->shouldBe($this->gitRepository->getRepository()); } } diff --git a/spec/Locator/ChangedFilesSpec.php b/spec/Locator/ChangedFilesSpec.php index 88d8ed7f2..582f744da 100644 --- a/spec/Locator/ChangedFilesSpec.php +++ b/spec/Locator/ChangedFilesSpec.php @@ -3,21 +3,27 @@ namespace spec\GrumPHP\Locator; use Gitonomy\Git\Diff\Diff; -use Gitonomy\Git\Diff\File; -use Gitonomy\Git\WorkingCopy; use GrumPHP\Collection\FilesCollection; use GrumPHP\Git\GitRepository; use GrumPHP\Locator\ChangedFiles; use GrumPHP\Util\Filesystem; use GrumPHP\Util\Paths; +use GrumPHPTest\Helpers\TemporaryGitRepository; use PhpSpec\ObjectBehavior; use Prophecy\Argument; -use Prophecy\Prophet; class ChangedFilesSpec extends ObjectBehavior { + /** + * @var TemporaryGitRepository + */ + private $gitRepository; + function let(GitRepository $repository, Filesystem $filesystem, Paths $paths) { + $this->gitRepository = TemporaryGitRepository::create(); + $repository->getWorkingCopy()->willReturn($this->gitRepository->getRepository()->getWorkingCopy()); + $this->beConstructedWith($repository, $filesystem, $paths); $paths->makePathRelativeToProjectDir(Argument::type('string'))->will( function (array $arguments): string { @@ -26,36 +32,32 @@ function (array $arguments): string { ); } - function it_is_initializable() + function letGo() { - $this->shouldHaveType(ChangedFiles::class); + $this->gitRepository->remove(); } - protected function mockFile($name, $isRename = false, $isDelete = false) + function it_is_initializable() { - $prophet = new Prophet(); - $file = $prophet->prophesize(File::class); - $file->getName()->willReturn($name); - $file->getNewName()->willReturn($name); - $file->isRename()->willReturn($isRename); - $file->isDeletion()->willReturn($isDelete); - return $file->reveal(); + $this->shouldHaveType(ChangedFiles::class); } - function it_will_list_all_diffed_files(GitRepository $repository, Filesystem $filesystem, Diff $diff, WorkingCopy $workingCopy) + function it_will_list_all_diffed_files(Filesystem $filesystem) { - $changedFile = $this->mockFile('file1.txt'); - $movedFile = $this->mockFile('file2.txt', true); - $deletedFile = $this->mockFile('file3.txt', false, true); + $this->gitRepository->dumpFile('file1.txt', 'changed'); + $this->gitRepository->dumpFile('moved.txt', 'moved'); + $this->gitRepository->dumpFile('file3.txt', 'deleted'); + $this->gitRepository->git('add', '--all'); + $this->gitRepository->git('commit', '--quiet', '--message=Initial commit'); + $this->gitRepository->dumpFile('file1.txt', 'changed again'); + $this->gitRepository->git('mv', 'moved.txt', 'file2.txt'); + $this->gitRepository->git('rm', '--quiet', 'file3.txt'); + $this->gitRepository->git('add', '--all'); $filesystem->exists('file1.txt')->willReturn(true); $filesystem->exists('file2.txt')->willReturn(true); $filesystem->exists('file3.txt')->willReturn(false); - $repository->getWorkingCopy()->willReturn($workingCopy); - $workingCopy->getDiffStaged()->willReturn($diff); - $diff->getFiles()->willReturn([$changedFile, $movedFile, $deletedFile]); - $result = $this->locateFromGitRepository(); $result->shouldBeAnInstanceOf(FilesCollection::class); $result[0]->getPathname()->shouldBe('file1.txt'); @@ -63,14 +65,12 @@ function it_will_list_all_diffed_files(GitRepository $repository, Filesystem $fi $result->getIterator()->count()->shouldBe(2); } - function it_will_not_list_non_existing_files(GitRepository $repository, Filesystem $filesystem, Diff $diff, WorkingCopy $workingCopy) + function it_will_not_list_non_existing_files(Filesystem $filesystem) { - $changedFile = $this->mockFile('file1.txt'); - $filesystem->exists('file1.txt')->willReturn(false); + $this->gitRepository->dumpFile('file1.txt', 'changed'); + $this->gitRepository->git('add', '--all'); - $repository->getWorkingCopy()->willReturn($workingCopy); - $workingCopy->getDiffStaged()->willReturn($diff); - $diff->getFiles()->willReturn([$changedFile]); + $filesystem->exists('file1.txt')->willReturn(false); $result = $this->locateFromGitRepository(); $result->shouldBeAnInstanceOf(FilesCollection::class); @@ -79,16 +79,16 @@ function it_will_not_list_non_existing_files(GitRepository $repository, Filesyst function it_will_list_all_diffed_files_from_raw_diff_input(GitRepository $repository, Filesystem $filesystem) { - $rawDiff = ':100644 100644 c09a0d49 77e5a35c M file.txt + $rawDiff = ":000000 100644 0000000 d95f3ad A\tfile.txt diff --git a/file.txt b/file.txt new file mode 100644 -index 0000000000000000000000000000000000000000..9766475a4185a151dc9d56d614ffb9aaea3bfd42 +index 0000000000000000000000000000000000000000..d95f3ad14dee633a758d2e331151e950dd13e4ed --- /dev/null +++ b/file.txt @@ -0,0 +1 @@ +content -'; +"; $repository->createRawDiff($rawDiff)->willReturn(Diff::parse($rawDiff)); $filesystem->exists('file.txt')->willReturn(true); diff --git a/src/Git/GitRepository.php b/src/Git/GitRepository.php index fb05b031d..1e3a3f319 100644 --- a/src/Git/GitRepository.php +++ b/src/Git/GitRepository.php @@ -5,10 +5,12 @@ namespace GrumPHP\Git; use Gitonomy\Git\Diff\Diff; +use Gitonomy\Git\Driver\Cli\CliDriver; use Gitonomy\Git\Exception\ProcessException; use Gitonomy\Git\Repository; use Gitonomy\Git\WorkingCopy; use GrumPHP\Locator\GitRepositoryLocator; +use Psr\Log\LoggerInterface; /** * @@ -23,19 +25,19 @@ class GitRepository private $repositoryLocator; /** - * @var array + * @var LoggerInterface */ - private $repositoryOptions; + private $logger; /** * @var ?Repository */ private $repository; - public function __construct(GitRepositoryLocator $repositoryLocator, array $repositoryOptions) + public function __construct(GitRepositoryLocator $repositoryLocator, LoggerInterface $logger) { $this->repositoryLocator = $repositoryLocator; - $this->repositoryOptions = $repositoryOptions; + $this->logger = $logger; } public function getWorkingCopy(): WorkingCopy @@ -66,10 +68,17 @@ public function tryToRunWithFallback(callable $run, string $fallback): string public function createRawDiff(string $rawDiff): Diff { - $diff = Diff::parse($rawDiff); - $diff->setRepository($this->getRepository()); + // gitlib 1 has no drivers, and takes the repository of a diff once parsed. + if (!class_exists(CliDriver::class)) { + $diff = Diff::parse($rawDiff); + /** @psalm-suppress DeprecatedMethod - Only deprecated in gitlib 2, which doesn't run this. */ + $diff->setRepository($this->getRepository()); - return $diff; + return $diff; + } + + /** @psalm-suppress TooManyArguments - Only too many for gitlib 1, which doesn't run this. */ + return Diff::parse($rawDiff, $this->getRepository()); } /** @@ -80,7 +89,7 @@ public function createRawDiff(string $rawDiff): Diff private function getRepository(): Repository { if (!$this->repository) { - $this->repository = $this->repositoryLocator->locate($this->repositoryOptions); + $this->repository = $this->repositoryLocator->locate($this->logger); } return $this->repository; diff --git a/src/Locator/ChangedFiles.php b/src/Locator/ChangedFiles.php index 6f4294078..cc34dfd82 100644 --- a/src/Locator/ChangedFiles.php +++ b/src/Locator/ChangedFiles.php @@ -71,9 +71,7 @@ private function parseFilesFromDiff(Diff $diff): FilesCollection private function makeFileRelativeToProjectDir(File $file): SplFileInfo { - $filePath = $this->paths->makePathRelativeToProjectDir( - $file->isRename() ? $file->getNewName() : $file->getName() - ); + $filePath = $this->paths->makePathRelativeToProjectDir($file->getName()); return new SplFileInfo($filePath, dirname($filePath), $filePath); } diff --git a/src/Locator/GitRepositoryLocator.php b/src/Locator/GitRepositoryLocator.php index 1662632eb..a9ab1d0ce 100644 --- a/src/Locator/GitRepositoryLocator.php +++ b/src/Locator/GitRepositoryLocator.php @@ -4,8 +4,10 @@ namespace GrumPHP\Locator; +use Gitonomy\Git\Driver\Cli\CliDriver; use Gitonomy\Git\Repository; use GrumPHP\Util\Paths; +use Psr\Log\LoggerInterface; class GitRepositoryLocator { @@ -19,16 +21,25 @@ public function __construct(Paths $paths) $this->paths = $paths; } - public function locate(array $options): Repository + public function locate(LoggerInterface $logger): Repository { - return new Repository( - $this->paths->getGitWorktreeDir(), - array_merge( + // gitlib 1 has no drivers: the options of the repository configure git. + if (!class_exists(CliDriver::class)) { + return new Repository( + $this->paths->getGitWorktreeDir(), [ 'working_dir' => $this->paths->getGitWorkingDir(), - ], - $options - ) + 'logger' => $logger, + 'inherit_environment_variables' => true, + ] + ); + } + + /** @psalm-suppress TooManyArguments, InvalidArgument - Only for gitlib 1, which doesn't run this. */ + return new Repository( + $this->paths->getGitWorktreeDir(), + $this->paths->getGitWorkingDir(), + new CliDriver(logger: $logger) ); } } diff --git a/test/Helpers/TemporaryGitRepository.php b/test/Helpers/TemporaryGitRepository.php new file mode 100644 index 000000000..0911708aa --- /dev/null +++ b/test/Helpers/TemporaryGitRepository.php @@ -0,0 +1,70 @@ +dir = $dir; + $this->git('init', '--quiet'); + $this->repository = new Repository($dir); + } + + public static function create(): self + { + $dir = sys_get_temp_dir().DIRECTORY_SEPARATOR.uniqid('grumphp-git-', true); + (new Filesystem())->mkdir($dir); + + return new self($dir); + } + + public function getRepository(): Repository + { + return $this->repository; + } + + public function dumpFile(string $path, string $content = ''): void + { + (new Filesystem())->dumpFile($this->dir.DIRECTORY_SEPARATOR.$path, $content); + } + + public function git(string ...$arguments): void + { + (new Process( + [ + 'git', + '-c', 'commit.gpgSign=false', + '-c', 'maintenance.auto=false', + '-c', 'user.email=grumphp@example.com', + '-c', 'user.name=GrumPHP', + ...$arguments, + ], + $this->dir + ))->mustRun(); + } + + public function remove(): void + { + (new Filesystem())->remove($this->dir); + } +} From 3ba09ab97ae745c557b310b30148b2125184d7b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Pineau?= Date: Thu, 15 Oct 2026 10:17:29 +0200 Subject: [PATCH 2/2] [DO NOT MERGE] Test with the main branch of gitlib as 2.0.0 To drop once gitlib 2.0 is released: the highest dependencies on PHP 8.4 and 8.5 then install it. --- .github/workflows/grumphp.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/workflows/grumphp.yml b/.github/workflows/grumphp.yml index 461936b42..694ea4f8d 100644 --- a/.github/workflows/grumphp.yml +++ b/.github/workflows/grumphp.yml @@ -43,6 +43,13 @@ jobs: path: ${{ steps.composercache.outputs.dir }} key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.json') }} restore-keys: ${{ runner.os }}-composer- + # TODO: remove once gitlib 2.0 is released. + # The main branch of gitlib is installed as 2.0.0, here and in the projects of the E2E tests. + - name: Use the main branch of gitlib as 2.0.0 + if: matrix.composer-deps == 'highest' && (matrix.php-versions == '8.4' || matrix.php-versions == '8.5') + run: | + git clone --depth=1 https://github.com/gitonomy/gitlib.git "$RUNNER_TEMP/gitlib" + composer config --global repositories.gitlib '{"type": "path", "url": "'"$RUNNER_TEMP/gitlib"'", "options": {"symlink": false, "versions": {"gitonomy/gitlib": "2.0.0"}}}' - name: Install dependencies (highest) if: matrix.composer-deps == 'highest' run: |