Skip to content

Commit

Permalink
[FEATURE] Check target branch (#55)
Browse files Browse the repository at this point in the history
Resolves #30
  • Loading branch information
gilbertsoft authored May 28, 2023
1 parent 674e13b commit d616cae
Show file tree
Hide file tree
Showing 9 changed files with 123 additions and 2 deletions.
12 changes: 12 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
35 changes: 34 additions & 1 deletion src/Config.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}
}
3 changes: 3 additions & 0 deletions src/Gerrit/Entity/ChangeInfo.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ final class ChangeInfo extends AbstractEntity
{
public string $id = '';

public string $branch = '';

public string $changeId = '';

public string $subject = '';
Expand All @@ -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');
Expand Down
8 changes: 8 additions & 0 deletions src/Gerrit/RestApi.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
47 changes: 46 additions & 1 deletion src/Utility/ComposerUtils.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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();
}

/**
Expand Down Expand Up @@ -162,6 +168,9 @@ private function createPatches(

$numericId = $this->gerritRestApi->getNumericId($changeId);
$this->io->write(sprintf(' - Numeric ID is <comment>%s</comment>', $numericId));

$branch = $this->gerritRestApi->getBranch($changeId);
$this->io->write(sprintf(' - Branch is <comment>%s</comment>', $branch));
} catch (UnexpectedResponseException | InvalidResponseException | UnexpectedValueException $th) {
$this->io->writeError('<warning>Error getting change from Gerrit</warning>');
$this->io->writeError(sprintf(
Expand All @@ -172,6 +181,15 @@ private function createPatches(
continue;
}

if (!$this->checkBranch($numericId, $branch)) {
$this->io->writeError(sprintf(
'<warning>Skipping change %s not matching the installed branch</warning>',
$numericId
));

continue;
}

try {
$patches = $this->patchUtils->create(
$numericId,
Expand Down Expand Up @@ -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(
'<info>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?</info> [<comment>y,N</comment>] ',
$changeId,
$targetBranch
),
false
);
}

return true;
}

private function setSourceInstall(string $packageName): void
{
/*
Expand Down
7 changes: 7 additions & 0 deletions tests/Unit/ConfigTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}

/**
Expand Down Expand Up @@ -113,6 +114,7 @@ public function configurationLoadProvider(): Iterator
],
'preferred-install-changed' => ['package1', 'package2'],
'patch-directory' => 'patch-dir',
'ignore-branch' => true,
],
],
],
Expand Down Expand Up @@ -399,6 +401,7 @@ public function configurationSaveProvider(): Iterator
3 => 'package3',
],
'patch-directory' => 'patch-dir',
'ignore-branch' => true,
],
],
],
Expand Down Expand Up @@ -443,6 +446,7 @@ public function configurationSaveProvider(): Iterator
2 => 'package3',
],
'patch-directory' => 'patch-dir',
'ignore-branch' => true,
],
],
],
Expand Down Expand Up @@ -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());
}
}
5 changes: 5 additions & 0 deletions tests/Unit/Fixtures/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,5 +42,10 @@
"typo3/cms-composer-installers": true
},
"discard-changes": true
},
"extra": {
"gilbertsoft/typo3-core-patches": {
"ignore-branch": true
}
}
}
2 changes: 2 additions & 0 deletions tests/Unit/Gerrit/Entity/ChangeInfoTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 6 additions & 0 deletions tests/Unit/Gerrit/RestApiTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ protected function tearDown(): void
public function testGetChange(
string $rawChangeId,
string $id,
string $branch,
string $changeId,
string $subject,
string $subjectNormalized,
Expand All @@ -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));
}
Expand All @@ -104,6 +107,7 @@ public function testGetChange(
* @return Iterator<string, array{
* rawChangeId: string,
* id: string,
* branch: string,
* changeId: string,
* subject: string,
* subjectNormalized: string,
Expand All @@ -115,6 +119,7 @@ public function changeIdsProvider(): Iterator
yield 'numeric change id' => [
'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',
Expand All @@ -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',
Expand Down

0 comments on commit d616cae

Please sign in to comment.