From 3a8ca018832ee2861dbe8903c8eeba46ad98b3c8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Th=C3=A9o=20FIDRY?=
<5175937+theofidry@users.noreply.github.com>
Date: Wed, 11 Oct 2023 16:26:43 +0200
Subject: [PATCH] feat: Rework the diff command output (#1054)
- Always display a summary of both archives upon equality failure.
- Provide a human-readable diff of the archives when possible.
- Make the message more explicit that if no diff was found for the
archives content, this is true for the selected diff mode, i.e. a
difference may be found with another diff mode.
---
composer.json | 1 +
composer.lock | 134 +++++++++++++-------------
src/Console/Command/Diff.php | 89 ++++++++++++------
src/Phar/PharDiff.php | 5 +
tests/Console/Command/DiffTest.php | 146 +++++++++++++++++++++++------
5 files changed, 249 insertions(+), 126 deletions(-)
diff --git a/composer.json b/composer.json
index 1d5a6c3bd..1ec553207 100644
--- a/composer.json
+++ b/composer.json
@@ -35,6 +35,7 @@
"phpdocumentor/reflection-docblock": "^5.3",
"phpdocumentor/type-resolver": "^1.7",
"psr/log": "^3.0",
+ "sebastian/diff": "^4.0",
"seld/jsonlint": "^1.9",
"symfony/console": "^6.1.7",
"symfony/filesystem": "^6.1.5",
diff --git a/composer.lock b/composer.lock
index 8fb99834a..cab6be326 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": "ff1e129b64f2e02d9c4a3963dbc7d10c",
+ "content-hash": "49fe6f5891e7fa2ae2c999456606c96d",
"packages": [
{
"name": "amphp/amp",
@@ -1778,6 +1778,72 @@
},
"time": "2021-07-14T16:46:02+00:00"
},
+ {
+ "name": "sebastian/diff",
+ "version": "4.0.5",
+ "source": {
+ "type": "git",
+ "url": "https://github.com/sebastianbergmann/diff.git",
+ "reference": "74be17022044ebaaecfdf0c5cd504fc9cd5a7131"
+ },
+ "dist": {
+ "type": "zip",
+ "url": "https://api.github.com/repos/sebastianbergmann/diff/zipball/74be17022044ebaaecfdf0c5cd504fc9cd5a7131",
+ "reference": "74be17022044ebaaecfdf0c5cd504fc9cd5a7131",
+ "shasum": ""
+ },
+ "require": {
+ "php": ">=7.3"
+ },
+ "require-dev": {
+ "phpunit/phpunit": "^9.3",
+ "symfony/process": "^4.2 || ^5"
+ },
+ "type": "library",
+ "extra": {
+ "branch-alias": {
+ "dev-master": "4.0-dev"
+ }
+ },
+ "autoload": {
+ "classmap": [
+ "src/"
+ ]
+ },
+ "notification-url": "https://packagist.org/downloads/",
+ "license": [
+ "BSD-3-Clause"
+ ],
+ "authors": [
+ {
+ "name": "Sebastian Bergmann",
+ "email": "sebastian@phpunit.de"
+ },
+ {
+ "name": "Kore Nordmann",
+ "email": "mail@kore-nordmann.de"
+ }
+ ],
+ "description": "Diff implementation",
+ "homepage": "https://github.com/sebastianbergmann/diff",
+ "keywords": [
+ "diff",
+ "udiff",
+ "unidiff",
+ "unified diff"
+ ],
+ "support": {
+ "issues": "https://github.com/sebastianbergmann/diff/issues",
+ "source": "https://github.com/sebastianbergmann/diff/tree/4.0.5"
+ },
+ "funding": [
+ {
+ "url": "https://github.com/sebastianbergmann",
+ "type": "github"
+ }
+ ],
+ "time": "2023-05-07T05:35:17+00:00"
+ },
{
"name": "seld/jsonlint",
"version": "1.10.0",
@@ -4769,72 +4835,6 @@
],
"time": "2020-10-26T15:52:27+00:00"
},
- {
- "name": "sebastian/diff",
- "version": "4.0.5",
- "source": {
- "type": "git",
- "url": "https://github.com/sebastianbergmann/diff.git",
- "reference": "74be17022044ebaaecfdf0c5cd504fc9cd5a7131"
- },
- "dist": {
- "type": "zip",
- "url": "https://api.github.com/repos/sebastianbergmann/diff/zipball/74be17022044ebaaecfdf0c5cd504fc9cd5a7131",
- "reference": "74be17022044ebaaecfdf0c5cd504fc9cd5a7131",
- "shasum": ""
- },
- "require": {
- "php": ">=7.3"
- },
- "require-dev": {
- "phpunit/phpunit": "^9.3",
- "symfony/process": "^4.2 || ^5"
- },
- "type": "library",
- "extra": {
- "branch-alias": {
- "dev-master": "4.0-dev"
- }
- },
- "autoload": {
- "classmap": [
- "src/"
- ]
- },
- "notification-url": "https://packagist.org/downloads/",
- "license": [
- "BSD-3-Clause"
- ],
- "authors": [
- {
- "name": "Sebastian Bergmann",
- "email": "sebastian@phpunit.de"
- },
- {
- "name": "Kore Nordmann",
- "email": "mail@kore-nordmann.de"
- }
- ],
- "description": "Diff implementation",
- "homepage": "https://github.com/sebastianbergmann/diff",
- "keywords": [
- "diff",
- "udiff",
- "unidiff",
- "unified diff"
- ],
- "support": {
- "issues": "https://github.com/sebastianbergmann/diff/issues",
- "source": "https://github.com/sebastianbergmann/diff/tree/4.0.5"
- },
- "funding": [
- {
- "url": "https://github.com/sebastianbergmann",
- "type": "github"
- }
- ],
- "time": "2023-05-07T05:35:17+00:00"
- },
{
"name": "sebastian/environment",
"version": "5.1.5",
diff --git a/src/Console/Command/Diff.php b/src/Console/Command/Diff.php
index 6bec5a0bf..8be1e90cd 100644
--- a/src/Console/Command/Diff.php
+++ b/src/Console/Command/Diff.php
@@ -22,6 +22,8 @@
use KevinGH\Box\Phar\DiffMode;
use KevinGH\Box\Phar\PharDiff;
use KevinGH\Box\Phar\PharInfo;
+use SebastianBergmann\Diff\Differ;
+use SebastianBergmann\Diff\Output\UnifiedDiffOutputBuilder;
use Symfony\Component\Console\Input\InputArgument;
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\Console\Output\BufferedOutput;
@@ -119,17 +121,18 @@ public function execute(IO $io): int
$io->comment('Comparing the two archives...');
- $pharInfoA = $diff->getPharInfoA();
- $pharInfoB = $diff->getPharInfoB();
-
- if ($pharInfoA->equals($pharInfoB)) {
+ if ($diff->equals()) {
$io->success('The two archives are identical.');
return ExitCode::SUCCESS;
}
- $this->compareArchives($diff, $io);
- $this->compareContents($diff, $diffMode, $io);
+ self::renderSummary($diff->getPharInfoA(), $io);
+ $io->newLine();
+ self::renderSummary($diff->getPharInfoB(), $io);
+
+ $this->renderArchivesDiff($diff, $io);
+ $this->renderContentsDiff($diff, $diffMode, $io);
return ExitCode::FAILURE;
}
@@ -152,24 +155,25 @@ private static function getPaths(IO $io): array
);
}
- private function compareArchives(PharDiff $diff, IO $io): void
+ private function renderArchivesDiff(PharDiff $diff, IO $io): void
{
- $pharInfoA = $diff->getPharInfoA();
- $pharInfoB = $diff->getPharInfoB();
-
- self::renderArchive(
- $diff->getPharInfoA()->getFileName(),
- $pharInfoA,
- $io,
+ $differ = new Differ(
+ new UnifiedDiffOutputBuilder("\n--- PHAR A\n+++ PHAR B\n"),
);
- $io->newLine();
+ $pharA = self::getShortSummary($diff->getPharInfoA(), $io);
+ $pharB = self::getShortSummary($diff->getPharInfoB(), $io);
- self::renderArchive(
- $diff->getPharInfoB()->getFileName(),
- $pharInfoB,
- $io,
+ if ($pharA === $pharB) {
+ return;
+ }
+
+ $result = $differ->diff(
+ $pharA,
+ $pharB,
);
+
+ $io->writeln($result);
}
private static function getDiffMode(IO $io): DiffMode
@@ -216,9 +220,14 @@ private static function getDiffMode(IO $io): DiffMode
return DiffMode::from($io->getOption(self::DIFF_OPTION)->asNonEmptyString());
}
- private function compareContents(PharDiff $diff, DiffMode $diffMode, IO $io): void
+ private function renderContentsDiff(PharDiff $diff, DiffMode $diffMode, IO $io): void
{
- $io->comment('Comparing the two archives contents...');
+ $io->comment(
+ sprintf(
+ 'Comparing the two archives contents (%s diff)...',
+ $diffMode->value,
+ ),
+ );
$checkSumAlgorithm = $io->getOption(self::CHECK_OPTION)->asNullableNonEmptyString() ?? self::DEFAULT_CHECKSUM_ALGO;
@@ -231,7 +240,7 @@ private function compareContents(PharDiff $diff, DiffMode $diffMode, IO $io): vo
$diffResult = $diff->diff($diffMode);
if (null === $diffResult || [[], []] === $diffResult) {
- $io->success('The contents are identical');
+ $io->writeln('No difference could be observed with this mode.');
return;
}
@@ -259,10 +268,12 @@ private function compareContents(PharDiff $diff, DiffMode $diffMode, IO $io): vo
self::renderPaths('-', $diff->getPharInfoA(), $diffResult[0], $io);
self::renderPaths('+', $diff->getPharInfoB(), $diffResult[1], $io);
- $io->error(sprintf(
- '%d file(s) difference',
- count($diffResult[0]) + count($diffResult[1]),
- ));
+ $io->error(
+ sprintf(
+ '%d file(s) difference',
+ count($diffResult[0]) + count($diffResult[1]),
+ ),
+ );
}
/**
@@ -294,19 +305,39 @@ private static function renderPaths(string $symbol, PharInfo $pharInfo, array $p
$io->writeln($lines);
}
- private static function renderArchive(string $fileName, PharInfo $pharInfo, IO $io): void
+ private static function renderSummary(PharInfo $pharInfo, IO $io): void
{
$io->writeln(
sprintf(
'Archive: %s',
- $fileName,
+ $pharInfo->getFileName(),
),
);
+ self::renderShortSummary($pharInfo, $io);
+ }
+
+ private static function getShortSummary(PharInfo $pharInfo, IO $io): string
+ {
+ $output = new BufferedOutput(
+ $io->getVerbosity(),
+ $io->isDecorated(),
+ $io->getOutput()->getFormatter(),
+ );
+
+ self::renderShortSummary(
+ $pharInfo,
+ $io->withOutput($output),
+ );
+
+ return $output->fetch();
+ }
+
+ private static function renderShortSummary(PharInfo $pharInfo, IO $io): void
+ {
PharInfoRenderer::renderCompression($pharInfo, $io);
PharInfoRenderer::renderSignature($pharInfo, $io);
PharInfoRenderer::renderMetadata($pharInfo, $io);
PharInfoRenderer::renderContentsSummary($pharInfo, $io);
- // TODO: checksum
}
}
diff --git a/src/Phar/PharDiff.php b/src/Phar/PharDiff.php
index 6fbbbea12..4ccc5b90d 100644
--- a/src/Phar/PharDiff.php
+++ b/src/Phar/PharDiff.php
@@ -61,6 +61,11 @@ public function getPharInfoB(): PharInfo
return $this->pharInfoB;
}
+ public function equals(): bool
+ {
+ return $this->pharInfoA->equals($this->pharInfoB);
+ }
+
/**
* @return null|string|array{string[], string[]}
*/
diff --git a/tests/Console/Command/DiffTest.php b/tests/Console/Command/DiffTest.php
index e79d37ac2..794aba55b 100644
--- a/tests/Console/Command/DiffTest.php
+++ b/tests/Console/Command/DiffTest.php
@@ -334,7 +334,7 @@ public static function diffPharsProvider(): iterable
}
}
- private static function commonDiffPharsProvider(): iterable
+ private static function commonDiffPharsProvider(DiffMode $diffMode): iterable
{
yield 'same PHAR' => [
self::FIXTURES_DIR.'/simple-phar-foo.phar',
@@ -353,39 +353,54 @@ private static function commonDiffPharsProvider(): iterable
yield 'different data; same content' => [
self::FIXTURES_DIR.'/simple-phar-bar.phar',
self::FIXTURES_DIR.'/simple-phar-bar-compressed.phar',
- <<<'OUTPUT'
+ sprintf(
+ <<<'OUTPUT'
- // Comparing the two archives...
+ // Comparing the two archives...
- Archive: simple-phar-bar.phar
- Archive Compression: None
- Files Compression: None
- Signature: SHA-1
- Signature Hash: 9ADC09F73909EDF14F8A4ABF9758B6FFAD1BBC51
- Metadata: None
- Contents: 1 file (6.64KB)
+ Archive: simple-phar-bar.phar
+ Archive Compression: None
+ Files Compression: None
+ Signature: SHA-1
+ Signature Hash: 9ADC09F73909EDF14F8A4ABF9758B6FFAD1BBC51
+ Metadata: None
+ Contents: 1 file (6.64KB)
- Archive: simple-phar-bar-compressed.phar
- Archive Compression: None
- Files Compression: GZ
- Signature: SHA-1
- Signature Hash: 3A388D86C91C36659A043D52C2DEB64E8848DD1A
- Metadata: None
- Contents: 1 file (6.65KB)
+ Archive: simple-phar-bar-compressed.phar
+ Archive Compression: None
+ Files Compression: GZ
+ Signature: SHA-1
+ Signature Hash: 3A388D86C91C36659A043D52C2DEB64E8848DD1A
+ Metadata: None
+ Contents: 1 file (6.65KB)
- // Comparing the two archives contents...
+ --- PHAR A
+ +++ PHAR B
+ @@ @@
+ Archive Compression: None
+ -Files Compression: None
+ +Files Compression: GZ
+ Signature: SHA-1
+ -Signature Hash: 9ADC09F73909EDF14F8A4ABF9758B6FFAD1BBC51
+ +Signature Hash: 3A388D86C91C36659A043D52C2DEB64E8848DD1A
+ Metadata: None
+ -Contents: 1 file (6.64KB)
+ +Contents: 1 file (6.65KB)
- [OK] The contents are identical
+ // Comparing the two archives contents (%s diff)...
+ No difference could be observed with this mode.
- OUTPUT,
+ OUTPUT,
+ $diffMode->value,
+ ),
ExitCode::FAILURE,
];
}
private static function listDiffPharsProvider(): iterable
{
- yield from self::commonDiffPharsProvider();
+ yield from self::commonDiffPharsProvider(DiffMode::LIST);
yield 'different files' => [
self::FIXTURES_DIR.'/simple-phar-foo.phar',
@@ -410,7 +425,18 @@ private static function listDiffPharsProvider(): iterable
Metadata: None
Contents: 1 file (6.64KB)
- // Comparing the two archives contents...
+ --- PHAR A
+ +++ PHAR B
+ @@ @@
+ Archive Compression: None
+ Files Compression: None
+ Signature: SHA-1
+ -Signature Hash: 311080EF8E479CE18D866B744B7D467880BFBF57
+ +Signature Hash: 9ADC09F73909EDF14F8A4ABF9758B6FFAD1BBC51
+ Metadata: None
+ Contents: 1 file (6.64KB)
+
+ // Comparing the two archives contents (list diff)...
--- Files present in "simple-phar-foo.phar" but not in "simple-phar-bar.phar"
+++ Files present in "simple-phar-bar.phar" but not in "simple-phar-foo.phar"
@@ -449,10 +475,21 @@ private static function listDiffPharsProvider(): iterable
Metadata: None
Contents: 1 file (6.61KB)
- // Comparing the two archives contents...
+ --- PHAR A
+ +++ PHAR B
+ @@ @@
+ Archive Compression: None
+ Files Compression: None
+ Signature: SHA-1
+ -Signature Hash: 9ADC09F73909EDF14F8A4ABF9758B6FFAD1BBC51
+ +Signature Hash: 122A636B8BB0348C9514833D70281EF6306A5BF5
+ Metadata: None
+ -Contents: 1 file (6.64KB)
+ +Contents: 1 file (6.61KB)
- [OK] The contents are identical
+ // Comparing the two archives contents (list diff)...
+ No difference could be observed with this mode.
OUTPUT,
ExitCode::FAILURE,
@@ -461,7 +498,7 @@ private static function listDiffPharsProvider(): iterable
public static function gitDiffPharsProvider(): iterable
{
- yield from self::commonDiffPharsProvider();
+ yield from self::commonDiffPharsProvider(DiffMode::GIT);
yield 'different files' => [
self::FIXTURES_DIR.'/simple-phar-foo.phar',
@@ -539,7 +576,7 @@ public static function gitDiffPharsProvider(): iterable
public static function GNUDiffPharsProvider(): iterable
{
- yield from self::commonDiffPharsProvider();
+ yield from self::commonDiffPharsProvider(DiffMode::GNU);
yield 'different files' => [
self::FIXTURES_DIR.'/simple-phar-foo.phar',
@@ -564,7 +601,18 @@ public static function GNUDiffPharsProvider(): iterable
Metadata: None
Contents: 1 file (6.64KB)
- // Comparing the two archives contents...
+ --- PHAR A
+ +++ PHAR B
+ @@ @@
+ Archive Compression: None
+ Files Compression: None
+ Signature: SHA-1
+ -Signature Hash: 311080EF8E479CE18D866B744B7D467880BFBF57
+ +Signature Hash: 9ADC09F73909EDF14F8A4ABF9758B6FFAD1BBC51
+ Metadata: None
+ Contents: 1 file (6.64KB)
+
+ // Comparing the two archives contents (gnu diff)...
Only in simple-phar-bar.phar: bar.php
Only in simple-phar-foo.phar: foo.php
@@ -597,7 +645,19 @@ public static function GNUDiffPharsProvider(): iterable
Metadata: None
Contents: 1 file (6.61KB)
- // Comparing the two archives contents...
+ --- PHAR A
+ +++ PHAR B
+ @@ @@
+ Archive Compression: None
+ Files Compression: None
+ Signature: SHA-1
+ -Signature Hash: 9ADC09F73909EDF14F8A4ABF9758B6FFAD1BBC51
+ +Signature Hash: 122A636B8BB0348C9514833D70281EF6306A5BF5
+ Metadata: None
+ -Contents: 1 file (6.64KB)
+ +Contents: 1 file (6.61KB)
+
+ // Comparing the two archives contents (gnu diff)...
diff --exclude=.phar_meta.json simple-phar-bar.phar/bar.php simple-phar-baz.phar/bar.php
3c3
@@ -610,9 +670,35 @@ public static function GNUDiffPharsProvider(): iterable
// Comparing the two archives...
- [OK] The two archives are identical
+ Archive: simple-phar-bar.phar
+ Archive Compression: None
+ Files Compression: None
+ Signature: SHA-1
+ Signature Hash: 9ADC09F73909EDF14F8A4ABF9758B6FFAD1BBC51
+ Metadata: None
+ Contents: 1 file (6.64KB)
+
+ Archive: simple-phar-baz.phar
+ Archive Compression: None
+ Files Compression: None
+ Signature: SHA-1
+ Signature Hash: 122A636B8BB0348C9514833D70281EF6306A5BF5
+ Metadata: None
+ Contents: 1 file (6.61KB)
- // Comparing the two archives contents...
+ --- PHAR A
+ +++ PHAR B
+ @@ @@
+ Archive Compression: None
+ Files Compression: None
+ Signature: SHA-1
+ -Signature Hash: 9ADC09F73909EDF14F8A4ABF9758B6FFAD1BBC51
+ +Signature Hash: 122A636B8BB0348C9514833D70281EF6306A5BF5
+ Metadata: None
+ -Contents: 1 file (6.64KB)
+ +Contents: 1 file (6.61KB)
+
+ // Comparing the two archives contents (gnu diff)...
diff '--exclude=.phar_meta.json' simple-phar-bar.phar/bar.php simple-phar-baz.phar/bar.php
3c3