From a9a0135c6697c7cdbcce77ce90356f7fddf7faa9 Mon Sep 17 00:00:00 2001
From: Patrick Beuks <code@beuks.net>
Date: Tue, 7 Jan 2025 19:35:37 +0100
Subject: [PATCH 1/2] 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());
+ }
}
From e7a6ac1db679ceadd6b25a7ea89add88ae9b4133 Mon Sep 17 00:00:00 2001
From: Patrick Beuks <code@beuks.net>
Date: Tue, 7 Jan 2025 20:48:50 +0100
Subject: [PATCH 2/2] fix style error
---
tests/Gitonomy/Git/Tests/DiffTest.php | 1 +
1 file changed, 1 insertion(+)
diff --git a/tests/Gitonomy/Git/Tests/DiffTest.php b/tests/Gitonomy/Git/Tests/DiffTest.php
index 36788b8..1c0d819 100644
--- a/tests/Gitonomy/Git/Tests/DiffTest.php
+++ b/tests/Gitonomy/Git/Tests/DiffTest.php
@@ -246,6 +246,7 @@ public function testThrowErrorOnBlobGetWithoutIndex()
} catch(\RuntimeException $exception) {
$this->assertSame('Index is missing to return Blob object.', $exception->getMessage());
}
+
try {
$file->getNewBlob();
} catch(\RuntimeException $exception) {