Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide a PHP API for reasoning about modules, branches, and releases #28

Conversation

GuySartorelli
Copy link
Member

@GuySartorelli GuySartorelli commented Apr 29, 2024

NOTE: Any "supported-dependency" in CMS 4 only which we don't own has been intentionally excluded from the new json file. e.g. we aren't listing composer/installers in there, which was in the file from the 4 branch.

Issue

@GuySartorelli GuySartorelli marked this pull request as draft April 29, 2024 02:36
@GuySartorelli GuySartorelli force-pushed the pulls/main/combine-all-versions branch 4 times, most recently from cf821f6 to 5c48da2 Compare April 30, 2024 00:43
@GuySartorelli GuySartorelli force-pushed the pulls/main/combine-all-versions branch 6 times, most recently from f3683b5 to acb36a9 Compare May 2, 2024 00:59
@GuySartorelli GuySartorelli marked this pull request as ready for review May 2, 2024 03:11
@GuySartorelli GuySartorelli marked this pull request as draft May 2, 2024 03:11
@GuySartorelli GuySartorelli force-pushed the pulls/main/combine-all-versions branch 4 times, most recently from 024e2e8 to 35fb0c9 Compare May 2, 2024 23:28
@GuySartorelli GuySartorelli changed the title NEW Make a unified JSON file for all CMS versions Provide a PHP API for reasoning about modules, branches, and releases May 2, 2024
@GuySartorelli GuySartorelli force-pushed the pulls/main/combine-all-versions branch 3 times, most recently from 154fe19 to 26231d7 Compare May 2, 2024 23:36
@GuySartorelli GuySartorelli marked this pull request as ready for review May 2, 2024 23:38
Comment on lines +12 to +28
/**
* Get the major release line of Silverstripe CMS this branch belongs to.
* @param array $repoMetaData Data from the MetaData class for the given repository
* @param string $branch The branch to check for
* @param stdClass|null $composerJsonContent The decoded composer.json file for the branch we're checking.
* Used to check against dependencies if there's no hardcoded references for the repository in question.
* @param bool $usePhpDepAsFallback If a CMS major release line can't be found, use the PHP dependency to determine
* a likely CMS major release line.
*/
public static function getCmsMajor(array $repoMetaData, string $branch, ?stdClass $composerJsonContent = null, bool $usePhpDepAsFallback = false): string
{
$cmsMajor = static::getCmsMajorFromBranch($repoMetaData, $branch);
if ($cmsMajor == '' && $composerJsonContent !== null) {
$cmsMajor = static::getCmsMajorFromComposerJson($composerJsonContent, $usePhpDepAsFallback);
}
return $cmsMajor;
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is from gha-generate-matrix

Comment on lines +43 to +50
public static function getBranchesForMergeUp(
string $githubRepository,
array $repoMetaData,
string $defaultBranch,
array $repoTags,
array $repoBranches,
?stdClass $composerJson = null
): array {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is from gha-merge-ups

Comment on lines +138 to +152
private static function getCmsMajorFromBranch(array $repoMetaData, string $branch): string
{
$branchMajor = '';
if (preg_match('#^[1-9]+$#', $branch)) {
$branchMajor = $branch;
} elseif (preg_match('#^([1-9]+)\.[0-9]+$#', $branch, $matches)) {
$branchMajor = $matches[1];
}
foreach ($repoMetaData['majorVersionMapping'] ?? [] as $cmsMajor => $repoBranches) {
if (is_numeric($cmsMajor) && in_array($branchMajor, $repoBranches)) {
return $cmsMajor;
}
}
return '';
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is from gha-generate-matrix

Comment on lines +154 to +193
private static function getCmsMajorFromComposerJson(stdClass $composerJsonContent, bool $usePhpDepAsFallback): string
{
foreach (MetaData::getAllRepositoryMetaData() as $categoryData) {
foreach ($categoryData as $repoData) {
$composerName = $repoData['packagist'] ?? null;
if ($composerName === null || !isset($composerJsonContent->require->$composerName)) {
continue;
}
$parser = new VersionParser();
$constraint = $parser->parseConstraints($composerJsonContent->require->$composerName);
$boundedVersion = explode('.', $constraint->getLowerBound()->getVersion());
$composerVersionMajor = $boundedVersion[0];
// If it's a non-numeric branch constraint or something unstable, don't use it
if ($composerVersionMajor === 0) {
continue;
}
foreach ($repoData['majorVersionMapping'] as $cmsMajor => $repoBranches) {
if (is_numeric($cmsMajor) && in_array($composerVersionMajor, $repoBranches)) {
return $cmsMajor;
}
}
}
}
// Fall back on PHP dependency if that's an option
if ($usePhpDepAsFallback && isset($composerJsonContent->require->php)) {
// Loop through in ascending order - the first result that matches is returned.
foreach (MetaData::PHP_VERSIONS_FOR_CMS_RELEASES as $cmsRelease => $phpVersions) {
// Ignore anything that's not a major release
if (!ctype_digit((string) $cmsRelease)) {
continue;
}
// Only look at the lowest-compatible PHP version of each major release,
// since there's some overlap between major releases
if (Semver::satisfies($phpVersions[0], $composerJsonContent->require->php)) {
return $cmsRelease;
}
}
}
return '';
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is from gha-generate-matrix, but I think also includes some logic from gha-merge-ups

Comment on lines +33 to +43
public const PHP_VERSIONS_FOR_CMS_RELEASES = [
'4.9' => ['7.1', '7.2', '7.3', '7.4'],
'4.10' => ['7.3', '7.4', '8.0'],
'4.11' => ['7.4', '8.0', '8.1'],
'4' => ['7.4', '8.0', '8.1'],
'5.0' => ['8.1', '8.2'],
'5.1' => ['8.1', '8.2'],
'5.2' => ['8.1', '8.2', '8.3'],
'5' => ['8.1', '8.2', '8.3'],
'6' => ['8.1', '8.2', '8.3'],
];
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Migrated from gha-generate-matrix so we can use PHP constraints as a fallback for identifying the correct CMS major.
We ignore the minor versions in the logic in this repo, but generate-matrix needs them and it doesn't make sense to maintain the list in two places.

Comment on lines +54 to +59
public const DO_NOT_MERGE_UP_FROM_MAJOR = [
'bringyourownideas/silverstripe-composer-update-checker' => '2',
'silverstripe/silverstripe-graphql' => '3',
'silverstripe/silverstripe-linkfield' => '3',
'tractorcow-farm/silverstripe-fluent' => '4',
];
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Migrated from gha-merge-ups. Originally only had linkfield in here, but due to changes in the logic these others are needed as well - reasoning in the phpdoc comment.

Comment on lines +65 to +67
public const SKIP_FOR_MERGE_UP = [
'silverstripe/cow',
];
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cow is a bit weird - it has branches and tags that make it look like it should have merge-ups, but in reality we only ever touch the master branch.

* @param string $packagistName The full packagist reference for the repository
* e.g. `silverstripe/framework`.
*/
public static function getMetaDataByPackagistName(string $packagistName): array
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only used by gha-generate-matrix's hardcoded.php - but I kept this logic here instead of dumping it in that file in case we find use for it later on, since it's very similar to the method above.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The merge-up scenarios are mostly taken straight from gha-generate-matrix, with a few more added in for scenarios I encountered during development.

The cms major scenarios are adapted from gha-generate-matrix tests, though due to the additional logic that repo has for getting installer versions, I left them intact there as well.

}
}

public function testGetAllRepositoryMetaData(): void
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is mostly about validating the schema of the json file - I found a few issues with the data when implementing these tests, so they've already proved their worth.

"lockstepped": false,
"type": "module",
"majorVersionMapping": {
"5": ["5"],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"5": ["5"],
"4": ["4"],
"5": ["5"],

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

"5": ["3"],
"6": ["4"]
}
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

@GuySartorelli GuySartorelli May 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean this?

"github": "silverstripe/silverstripe-behat-extension",
"packagist": "silverstripe/behat-extension",
"githubId": 6235025,
"majorVersionMapping": {
"4": ["4"],
"5": ["5"],
"6": ["6"]
}
},

Not missing, it's right there.

"6": ["6"]
}
},
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those are both marked as "supported-dependency", so the note in the PR description applies. Copied below for your convenience:

NOTE: Any "supported-dependency" in CMS 4 only which we don't own has been intentionally excluded from the new json file. e.g. we aren't listing composer/installers in there, which was in the file from the 4 branch.

@GuySartorelli GuySartorelli force-pushed the pulls/main/combine-all-versions branch from 26231d7 to 95cafa7 Compare May 6, 2024 02:05
@emteknetnz emteknetnz merged commit cde6b55 into silverstripe:main May 6, 2024
8 checks passed
@emteknetnz emteknetnz deleted the pulls/main/combine-all-versions branch May 6, 2024 06:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants