From a9a0135c6697c7cdbcce77ce90356f7fddf7faa9 Mon Sep 17 00:00:00 2001 From: Patrick Beuks Date: Tue, 7 Jan 2025 19:35:37 +0100 Subject: [PATCH] fix error with empty index by including raw information in diff --- src/Gitonomy/Git/Commit.php | 2 +- src/Gitonomy/Git/Diff/File.php | 8 ++ src/Gitonomy/Git/Parser/DiffParser.php | 21 ++++- src/Gitonomy/Git/Repository.php | 2 +- src/Gitonomy/Git/WorkingCopy.php | 4 +- tests/Gitonomy/Git/Tests/DiffTest.php | 108 +++++++++++++++++++++++++ 6 files changed, 139 insertions(+), 6 deletions(-) diff --git a/src/Gitonomy/Git/Commit.php b/src/Gitonomy/Git/Commit.php index 005dab3..573d3b5 100644 --- a/src/Gitonomy/Git/Commit.php +++ b/src/Gitonomy/Git/Commit.php @@ -62,7 +62,7 @@ public function setData(array $data) */ public function getDiff() { - $args = ['-r', '-p', '-m', '-M', '--no-commit-id', '--full-index', $this->revision]; + $args = ['-r', '-p', '--raw', '-m', '-M', '--no-commit-id', '--full-index', $this->revision]; $diff = Diff::parse($this->repository->run('diff-tree', $args)); $diff->setRepository($this->repository); diff --git a/src/Gitonomy/Git/Diff/File.php b/src/Gitonomy/Git/Diff/File.php index d3da24b..bfca155 100644 --- a/src/Gitonomy/Git/Diff/File.php +++ b/src/Gitonomy/Git/Diff/File.php @@ -278,6 +278,10 @@ public function getOldBlob() throw new \LogicException('Can\'t return old Blob on a creation'); } + if ($this->oldIndex === '') { + throw new \RuntimeException('Index is missing to return Blob object.'); + } + return $this->repository->getBlob($this->oldIndex); } @@ -291,6 +295,10 @@ public function getNewBlob() throw new \LogicException('Can\'t return new Blob on a deletion'); } + if ($this->newIndex === '') { + throw new \RuntimeException('Index is missing to return Blob object.'); + } + return $this->repository->getBlob($this->newIndex); } } diff --git a/src/Gitonomy/Git/Parser/DiffParser.php b/src/Gitonomy/Git/Parser/DiffParser.php index ad22bea..6ef34e8 100644 --- a/src/Gitonomy/Git/Parser/DiffParser.php +++ b/src/Gitonomy/Git/Parser/DiffParser.php @@ -22,14 +22,31 @@ class DiffParser extends ParserBase protected function doParse() { $this->files = []; + $indexes = []; + // Diff contains raw information + if (str_starts_with($this->content, ':')) { + while ($this->expects(':')) { + $this->consumeRegexp('/\d{6} \d{6} /'); + $oldIndex = $this->consumeShortHash(); + $this->consume(' '); + $newIndex = $this->consumeShortHash(); + $this->consumeTo("\n"); + $this->consumeNewLine(); + $indexes[] = [$oldIndex, $newIndex]; + } + $this->consumeNewLine(); + } elseif (!$this->isFinished()) { + trigger_error('Using Diff::parse without raw information is deprecated. See https://github.com/gitonomy/gitlib/issues/227.', E_USER_DEPRECATED); + } + $fileIndex = 0; while (!$this->isFinished()) { // 1. title $vars = $this->consumeRegexp("/diff --git \"?(a\/.*?)\"? \"?(b\/.*?)\"?\n/"); $oldName = $vars[1]; $newName = $vars[2]; - $oldIndex = null; - $newIndex = null; + $oldIndex = isset($indexes[$fileIndex]) ? $indexes[$fileIndex][0] : null; + $newIndex = isset($indexes[$fileIndex]) ? $indexes[$fileIndex][1] : null; $oldMode = null; $newMode = null; diff --git a/src/Gitonomy/Git/Repository.php b/src/Gitonomy/Git/Repository.php index fe77a88..296496a 100644 --- a/src/Gitonomy/Git/Repository.php +++ b/src/Gitonomy/Git/Repository.php @@ -416,7 +416,7 @@ public function getDiff($revisions) $revisions = new RevisionList($this, $revisions); } - $args = array_merge(['-r', '-p', '-m', '-M', '--no-commit-id', '--full-index'], $revisions->getAsTextArray()); + $args = array_merge(['-r', '-p', '--raw', '-m', '-M', '--no-commit-id', '--full-index'], $revisions->getAsTextArray()); $diff = Diff::parse($this->run('diff', $args)); $diff->setRepository($this); diff --git a/src/Gitonomy/Git/WorkingCopy.php b/src/Gitonomy/Git/WorkingCopy.php index aed3b85..a94fbbb 100644 --- a/src/Gitonomy/Git/WorkingCopy.php +++ b/src/Gitonomy/Git/WorkingCopy.php @@ -50,7 +50,7 @@ public function getUntrackedFiles() public function getDiffPending() { - $diff = Diff::parse($this->run('diff', ['-r', '-p', '-m', '-M', '--full-index'])); + $diff = Diff::parse($this->run('diff', ['-r', '-p', '--raw', '-m', '-M', '--full-index'])); $diff->setRepository($this->repository); return $diff; @@ -58,7 +58,7 @@ public function getDiffPending() public function getDiffStaged() { - $diff = Diff::parse($this->run('diff', ['-r', '-p', '-m', '-M', '--full-index', '--staged'])); + $diff = Diff::parse($this->run('diff', ['-r', '-p', '--raw', '-m', '-M', '--full-index', '--staged'])); $diff->setRepository($this->repository); return $diff; diff --git a/tests/Gitonomy/Git/Tests/DiffTest.php b/tests/Gitonomy/Git/Tests/DiffTest.php index 2232345..36788b8 100644 --- a/tests/Gitonomy/Git/Tests/DiffTest.php +++ b/tests/Gitonomy/Git/Tests/DiffTest.php @@ -13,6 +13,8 @@ namespace Gitonomy\Git\Tests; use Gitonomy\Git\Diff\Diff; +use Gitonomy\Git\Diff\File; +use Gitonomy\Git\Repository; class DiffTest extends AbstractTest { @@ -150,4 +152,110 @@ public function testWorksWithUmlauts($repository) $files = $repository->getCommit(self::FILE_WITH_UMLAUTS_COMMIT)->getDiff()->getFiles(); $this->assertSame('file_with_umlauts_\303\244\303\266\303\274', $files[0]->getNewName()); } + + public function testDeleteFileWithoutRaw() + { + $deprecationCalled = false; + $self = $this; + set_error_handler(function (int $errno, string $errstr) use ($self, &$deprecationCalled): void { + $deprecationCalled = true; + $self->assertSame('Using Diff::parse without raw information is deprecated. See https://github.com/gitonomy/gitlib/issues/227.', $errstr); + }, E_USER_DEPRECATED); + + $diff = Diff::parse(<<<'DIFF' + diff --git a/test b/test + deleted file mode 100644 + index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..0000000000000000000000000000000000000000 + + DIFF); + $firstFile = $diff->getFiles()[0]; + + restore_exception_handler(); + + $this->assertTrue($deprecationCalled); + $this->assertFalse($firstFile->isCreation()); + // TODO: Enable after #226 is merged + //$this->assertTrue($firstFile->isDeletion()); + //$this->assertFalse($firstFile->isChangeMode()); + $this->assertSame('e69de29bb2d1d6434b8b29ae775ad8c2e48c5391', $firstFile->getOldIndex()); + $this->assertNull($firstFile->getNewIndex()); + } + + public function testModeChangeFileWithoutRaw() + { + $deprecationCalled = false; + $self = $this; + set_error_handler(function (int $errno, string $errstr) use ($self, &$deprecationCalled): void { + $deprecationCalled = true; + $self->assertSame('Using Diff::parse without raw information is deprecated. See https://github.com/gitonomy/gitlib/issues/227.', $errstr); + }, E_USER_DEPRECATED); + + $diff = Diff::parse(<<<'DIFF' + diff --git a/a.out b/a.out + old mode 100755 + new mode 100644 + + DIFF); + $firstFile = $diff->getFiles()[0]; + + restore_exception_handler(); + + $this->assertTrue($deprecationCalled); + $this->assertFalse($firstFile->isCreation()); + $this->assertFalse($firstFile->isDeletion()); + $this->assertTrue($firstFile->isChangeMode()); + $this->assertSame('', $firstFile->getOldIndex()); + $this->assertSame('', $firstFile->getNewIndex()); + } + + public function testModeChangeFileWithRaw() + { + $deprecationCalled = false; + set_error_handler(function (int $errno, string $errstr) use (&$deprecationCalled): void { + $deprecationCalled = true; + }, E_USER_DEPRECATED); + + $diff = Diff::parse(<<<'DIFF' + :100755 100644 d1af4b23d0cc9313e5b2d3ef2fb9696c94afaa81 d1af4b23d0cc9313e5b2d3ef2fb9696c94afaa81 M a.out + + diff --git a/a.out b/a.out + old mode 100755 + new mode 100644 + + DIFF); + $firstFile = $diff->getFiles()[0]; + + restore_exception_handler(); + + $this->assertFalse($deprecationCalled); + $this->assertFalse($firstFile->isCreation()); + $this->assertFalse($firstFile->isDeletion()); + $this->assertTrue($firstFile->isChangeMode()); + $this->assertSame('d1af4b23d0cc9313e5b2d3ef2fb9696c94afaa81', $firstFile->getOldIndex()); + $this->assertSame('d1af4b23d0cc9313e5b2d3ef2fb9696c94afaa81', $firstFile->getNewIndex()); + } + + public function testThrowErrorOnBlobGetWithoutIndex() + { + $repository = $this->getMockBuilder(Repository::class)->disableOriginalConstructor()->getMock(); + $file = new File('oldName', 'newName', '100755', '100644', '', '', false); + $file->setRepository($repository); + + try { + $file->getOldBlob(); + } catch(\RuntimeException $exception) { + $this->assertSame('Index is missing to return Blob object.', $exception->getMessage()); + } + try { + $file->getNewBlob(); + } catch(\RuntimeException $exception) { + $this->assertSame('Index is missing to return Blob object.', $exception->getMessage()); + } + + $this->assertFalse($file->isCreation()); + $this->assertFalse($file->isDeletion()); + $this->assertTrue($file->isChangeMode()); + $this->assertSame('', $file->getOldIndex()); + $this->assertSame('', $file->getNewIndex()); + } }