From d616cae0025ae8407978bb7d5175000f0bc3a0c8 Mon Sep 17 00:00:00 2001
From: Gilbertsoft <25326036+gilbertsoft@users.noreply.github.com>
Date: Sun, 28 May 2023 16:47:18 +0200
Subject: [PATCH] [FEATURE] Check target branch (#55)
Resolves #30
---
README.md | 12 ++++++
src/Config.php | 35 ++++++++++++++-
src/Gerrit/Entity/ChangeInfo.php | 3 ++
src/Gerrit/RestApi.php | 8 ++++
src/Utility/ComposerUtils.php | 47 ++++++++++++++++++++-
tests/Unit/ConfigTest.php | 7 +++
tests/Unit/Fixtures/composer.json | 5 +++
tests/Unit/Gerrit/Entity/ChangeInfoTest.php | 2 +
tests/Unit/Gerrit/RestApiTest.php | 6 +++
9 files changed, 123 insertions(+), 2 deletions(-)
diff --git a/README.md b/README.md
index 5bff4e7..8ac75c8 100644
--- a/README.md
+++ b/README.md
@@ -119,6 +119,18 @@ next example:
composer typo3:patch:apply https://review.typo3.org/c/Packages/TYPO3.CMS/+/12345
```
+## Verification of the branch
+
+The plugin compares the current installed core version with the target branch
+of the patch to install and asks for confirmation to anyway try to apply the
+patch to the different version.
+
+To disabled the branch check for this project, run:
+
+```bash
+composer config extra.gilbertsoft/typo3-core-patches.ignore-branch true
+```
+
## Detection of merged changes on update or install
When running `composer update` or `composer install`, the plugin detects changes
diff --git a/src/Config.php b/src/Config.php
index 5d03b5b..ddc2468 100644
--- a/src/Config.php
+++ b/src/Config.php
@@ -70,6 +70,11 @@ final class Config implements PersistenceInterface
*/
private const DEFAULT_PATCH_DIRECTORY = 'patches';
+ /**
+ * @var string
+ */
+ private const PLUGIN_IGNORE_BRANCH = 'ignore-branch';
+
private JsonFile $jsonFile;
private ConfigSourceInterface $configSource;
@@ -84,6 +89,8 @@ final class Config implements PersistenceInterface
private Patches $patches;
+ private bool $ignoreBranch = \false;
+
public function __construct(
?JsonFile $jsonFile = null,
?ConfigSourceInterface $configSource = null
@@ -123,6 +130,18 @@ public function setPatchDirectory(string $patchDirectory): self
return $this;
}
+ public function getIgnoreBranch(): bool
+ {
+ return $this->ignoreBranch;
+ }
+
+ public function setIgnoreBranch(bool $ignoreBranch): self
+ {
+ $this->ignoreBranch = $ignoreBranch;
+
+ return $this;
+ }
+
public function getPreferredInstall(): PreferredInstall
{
return $this->preferredInstall;
@@ -143,7 +162,11 @@ private function isEmpty(): bool
return false;
}
- return $this->patchDirectory === '';
+ if ($this->patchDirectory !== '') {
+ return false;
+ }
+
+ return !$this->ignoreBranch;
}
public function load(): self
@@ -232,6 +255,10 @@ public function jsonSerialize(): array
$config[self::PLUGIN_PATCH_DIRECTORY] = $this->patchDirectory;
}
+ if ($this->ignoreBranch) {
+ $config[self::PLUGIN_IGNORE_BRANCH] = $this->ignoreBranch;
+ }
+
return $config;
}
@@ -285,6 +312,12 @@ public function jsonUnserialize(array $json): self
$this->patchDirectory = $patchDirectory;
+ if (!is_bool($ignoreBranch = $packageConfig[self::PLUGIN_IGNORE_BRANCH] ?? null)) {
+ $ignoreBranch = false;
+ }
+
+ $this->ignoreBranch = $ignoreBranch;
+
return $this;
}
}
diff --git a/src/Gerrit/Entity/ChangeInfo.php b/src/Gerrit/Entity/ChangeInfo.php
index 4faf6ec..0d567d7 100644
--- a/src/Gerrit/Entity/ChangeInfo.php
+++ b/src/Gerrit/Entity/ChangeInfo.php
@@ -22,6 +22,8 @@ final class ChangeInfo extends AbstractEntity
{
public string $id = '';
+ public string $branch = '';
+
public string $changeId = '';
public string $subject = '';
@@ -34,6 +36,7 @@ public static function fromJson(string $json): self
$self = new self();
self::assignProperty($self->id, $object, 'id');
+ self::assignProperty($self->branch, $object, 'branch');
self::assignProperty($self->changeId, $object, 'change_id');
self::assignProperty($self->subject, $object, 'subject');
self::assignProperty($self->number, $object, '_number');
diff --git a/src/Gerrit/RestApi.php b/src/Gerrit/RestApi.php
index 86b8357..c5ddadb 100644
--- a/src/Gerrit/RestApi.php
+++ b/src/Gerrit/RestApi.php
@@ -68,6 +68,14 @@ public function getChange(string $changeId): ChangeInfo
return $this->changeInfo[$changeId];
}
+ /**
+ * @see https://review.typo3.org/Documentation/rest-api-changes.html#change-id
+ */
+ public function getBranch(string $changeId): string
+ {
+ return $this->getChange($changeId)->branch;
+ }
+
/**
* @return string The normalized subject
* @throws UnexpectedValueException
diff --git a/src/Utility/ComposerUtils.php b/src/Utility/ComposerUtils.php
index 85bff78..6093b92 100644
--- a/src/Utility/ComposerUtils.php
+++ b/src/Utility/ComposerUtils.php
@@ -19,8 +19,10 @@
use Composer\Factory;
use Composer\IO\IOInterface;
use Composer\Json\JsonFile;
+use Composer\Package\BasePackage;
use Composer\Package\PackageInterface;
use Composer\Semver\Semver;
+use Composer\Semver\VersionParser;
use GsTYPO3\CorePatches\Config;
use GsTYPO3\CorePatches\Config\Patches;
use GsTYPO3\CorePatches\Config\PreferredInstall;
@@ -48,13 +50,16 @@ final class ComposerUtils
private PatchUtils $patchUtils;
+ private VersionParser $versionParser;
+
public function __construct(
Composer $composer,
IOInterface $io,
?Config $config = null,
?Application $application = null,
?RestApi $restApi = null,
- ?PatchUtils $patchUtils = null
+ ?PatchUtils $patchUtils = null,
+ ?VersionParser $versionParser = null
) {
$this->composer = $composer;
$this->io = $io;
@@ -71,6 +76,7 @@ public function __construct(
$this->gerritRestApi = $restApi ?? new RestApi($httpDownloader);
$this->patchUtils = $patchUtils ?? new PatchUtils($this->composer, $this->io);
+ $this->versionParser = $versionParser ?? new VersionParser();
}
/**
@@ -162,6 +168,9 @@ private function createPatches(
$numericId = $this->gerritRestApi->getNumericId($changeId);
$this->io->write(sprintf(' - Numeric ID is %s', $numericId));
+
+ $branch = $this->gerritRestApi->getBranch($changeId);
+ $this->io->write(sprintf(' - Branch is %s', $branch));
} catch (UnexpectedResponseException | InvalidResponseException | UnexpectedValueException $th) {
$this->io->writeError('Error getting change from Gerrit');
$this->io->writeError(sprintf(
@@ -172,6 +181,15 @@ private function createPatches(
continue;
}
+ if (!$this->checkBranch($numericId, $branch)) {
+ $this->io->writeError(sprintf(
+ 'Skipping change %s not matching the installed branch',
+ $numericId
+ ));
+
+ continue;
+ }
+
try {
$patches = $this->patchUtils->create(
$numericId,
@@ -207,6 +225,33 @@ private function createPatches(
return $patchesCount;
}
+ private function checkBranch(int $changeId, string $targetBranch): bool
+ {
+ if ($this->config->load()->getIgnoreBranch()) {
+ return true;
+ }
+
+ if (
+ !$this->composer->getRepositoryManager()->getLocalRepository()->findPackage(
+ 'typo3/cms-core',
+ $this->versionParser->normalizeBranch($targetBranch)
+ ) instanceof BasePackage
+ ) {
+ return $this->io->askConfirmation(
+ sprintf(
+ 'The change %d is related to the branch "%s" but another version appears to be installed.' .
+ ' Applying the patch may result in errors afterwards. Should the patch for this change be' .
+ ' installed anyway? [y,N] ',
+ $changeId,
+ $targetBranch
+ ),
+ false
+ );
+ }
+
+ return true;
+ }
+
private function setSourceInstall(string $packageName): void
{
/*
diff --git a/tests/Unit/ConfigTest.php b/tests/Unit/ConfigTest.php
index fc7b88e..277eb76 100644
--- a/tests/Unit/ConfigTest.php
+++ b/tests/Unit/ConfigTest.php
@@ -59,6 +59,7 @@ public function testLoadWorksProperly(
self::assertCount($expectedAppliedChangesCount, $config->getChanges());
self::assertCount($expectedPreferredInstallChangedCount, $config->getPreferredInstallChanged());
self::assertSame($expectedPatchDirectory, $config->getPatchDirectory());
+ self::assertSame($expectedIgnoreBranch, $config->getIgnoreBranch());
}
/**
@@ -113,6 +114,7 @@ public function configurationLoadProvider(): Iterator
],
'preferred-install-changed' => ['package1', 'package2'],
'patch-directory' => 'patch-dir',
+ 'ignore-branch' => true,
],
],
],
@@ -399,6 +401,7 @@ public function configurationSaveProvider(): Iterator
3 => 'package3',
],
'patch-directory' => 'patch-dir',
+ 'ignore-branch' => true,
],
],
],
@@ -443,6 +446,7 @@ public function configurationSaveProvider(): Iterator
2 => 'package3',
],
'patch-directory' => 'patch-dir',
+ 'ignore-branch' => true,
],
],
],
@@ -489,6 +493,9 @@ public function testGettersAndSetters(): void
self::assertSame($config->getPreferredInstall(), $config->getPreferredInstall());
+ self::assertSame($config, $config->setIgnoreBranch(true));
+ self::assertTrue($config->getIgnoreBranch());
+
self::assertSame($config->getPatches(), $config->getPatches());
}
}
diff --git a/tests/Unit/Fixtures/composer.json b/tests/Unit/Fixtures/composer.json
index 637801f..f286f33 100644
--- a/tests/Unit/Fixtures/composer.json
+++ b/tests/Unit/Fixtures/composer.json
@@ -42,5 +42,10 @@
"typo3/cms-composer-installers": true
},
"discard-changes": true
+ },
+ "extra": {
+ "gilbertsoft/typo3-core-patches": {
+ "ignore-branch": true
+ }
}
}
diff --git a/tests/Unit/Gerrit/Entity/ChangeInfoTest.php b/tests/Unit/Gerrit/Entity/ChangeInfoTest.php
index 85e0708..394aa09 100644
--- a/tests/Unit/Gerrit/Entity/ChangeInfoTest.php
+++ b/tests/Unit/Gerrit/Entity/ChangeInfoTest.php
@@ -26,12 +26,14 @@ public function testFromJson(): void
{
$changeInfo = ChangeInfo::fromJson('{
"id": "idValue",
+ "branch": "branchValue",
"change_id": "changeIdValue",
"subject": "subjectValue",
"_number": 12345
}');
self::assertSame('idValue', $changeInfo->id);
+ self::assertSame('branchValue', $changeInfo->branch);
self::assertSame('changeIdValue', $changeInfo->changeId);
self::assertSame('subjectValue', $changeInfo->subject);
self::assertSame(12345, $changeInfo->number);
diff --git a/tests/Unit/Gerrit/RestApiTest.php b/tests/Unit/Gerrit/RestApiTest.php
index 7742284..943b431 100644
--- a/tests/Unit/Gerrit/RestApiTest.php
+++ b/tests/Unit/Gerrit/RestApiTest.php
@@ -84,6 +84,7 @@ protected function tearDown(): void
public function testGetChange(
string $rawChangeId,
string $id,
+ string $branch,
string $changeId,
string $subject,
string $subjectNormalized,
@@ -92,10 +93,12 @@ public function testGetChange(
$changeInfo = $this->gerritRestApi->getChange($rawChangeId);
self::assertSame($id, $changeInfo->id);
+ self::assertSame($branch, $changeInfo->branch);
self::assertSame($changeId, $changeInfo->changeId);
self::assertSame($subject, $changeInfo->subject);
self::assertSame($number, $changeInfo->number);
+ self::assertSame($branch, $this->gerritRestApi->getBranch($rawChangeId));
self::assertSame($subjectNormalized, $this->gerritRestApi->getSubject($rawChangeId));
self::assertSame($number, $this->gerritRestApi->getNumericId($rawChangeId));
}
@@ -104,6 +107,7 @@ public function testGetChange(
* @return Iterator [
'rawChangeId' => '73021',
'id' => 'Packages%2FTYPO3.CMS~main~I84baa3df4b3a96cacbb3e686e4c85562d67422df',
+ 'branch' => 'main',
'changeId' => 'I84baa3df4b3a96cacbb3e686e4c85562d67422df',
'subject' => '[TASK] Test change for gilbertsoft/typo3-core-patches',
'subjectNormalized' => 'Test change for gilbertsoft/typo3-core-patches',
@@ -123,6 +128,7 @@ public function changeIdsProvider(): Iterator
yield 'change url' => [
'rawChangeId' => 'https://review.typo3.org/c/Packages/TYPO3.CMS/+/73021',
'id' => 'Packages%2FTYPO3.CMS~main~I84baa3df4b3a96cacbb3e686e4c85562d67422df',
+ 'branch' => 'main',
'changeId' => 'I84baa3df4b3a96cacbb3e686e4c85562d67422df',
'subject' => '[TASK] Test change for gilbertsoft/typo3-core-patches',
'subjectNormalized' => 'Test change for gilbertsoft/typo3-core-patches',