Handle D9 core SAs and handle semver contrib releases #22
Conversation
Co-authored-by: Neil Drumm <drumm@delocalizedham.com>
| $core_compat = 7; | ||
| break; | ||
| case 'current': | ||
| // Drupal's module API goes no higher than 8. Drupal 9 core advisories are published in this project's 8.x branch. |
There was a problem hiding this comment.
| // Drupal's module API goes no higher than 8. Drupal 9 core advisories are published in this project's 8.x branch. | |
| // packages.drupal.org/8 is used for 8.x-* versions and all future semantically-versioned releases, which might be compatible with 8 and/or 9 & later. |
"Core compatibility" is much more complex than 7, 8, or 9 nowadays. I think this really means which packages.drupal.org composer endpoint this corresponds with.
There was a problem hiding this comment.
Yes exactly. We need a word for that.
build/src/VersionParser.php Outdated
| // Its absence indicates a semver release (or a core release). | ||
| list($core, $version) = empty($result->taxonomy_vocabulary_6) ? [NULL, $version] : explode('-', $version, 2); | ||
| list($major) = explode('.', $version); | ||
| return ">=$major,<$version"; |
There was a problem hiding this comment.
Like core, this should probably also be ">=$major.$minor,<$version" for semver contrib versions.
There was a problem hiding this comment.
I wonder why that wasnt there before. @webflo?
There was a problem hiding this comment.
This is also new for semantic versioning, since the API-compatibility-prefixed version numbers can only be 8.x-1.2, not 8.x-1.2.3.
build/src/VersionParser.php Outdated
| // Its absence indicates a semver release (or a core release). | ||
| list($core, $version) = empty($result->taxonomy_vocabulary_6) ? [NULL, $version] : explode('-', $version, 2); | ||
| list($major, $minor) = explode('.', $version, 2); | ||
| return ">=$major.$minor,<$version"; |
There was a problem hiding this comment.
I believe API-compatibility-prefixed version numbers still need the original handling, ">=$major,<$version"
This really isn't core vs. contrib, it is major.minor vs. major.minor.patch. If this ever handled D7 core, that would also want ">=$major,<$version", since that was also not semver.
There was a problem hiding this comment.
API-compatibility-prefixed version numbers
And whats the best way to identify those? I am happy to change code to split not on core/contrib but something else.
There was a problem hiding this comment.
@drumm and we do have to handle D7 core in this package. Drush doesn't use it but others might.
There was a problem hiding this comment.
Once list($core, $version) = empty($result->taxonomy_vocabulary_6) ? … is done for contrib, using the original version number for core, since there is already no API compatibility prefix - something like:
$version_components = explode('.', $version); if (count($version_components) === 2) { // Only major.minor, either Drupal core 7, or contrib that had an API compatibility prefix. list($major) = $version_components; return ">=$major,<$version"; } elseif (count($version_components) === 3) { // Semver, either Drupal core 8 or later, or contrib using semver. list($major, $minor) = $version_components; return ">=$major.$minor,<$version"; } else { // Should not happen. } There was a problem hiding this comment.
Thanks. Thats now in the PR. I reran the build and now you can compare the PR output versus the last one run by CircleCI - 3b3a391...8.x-v2
- Does semver_example row look right to you?
- I dunno why some packages are being removed.
There was a problem hiding this comment.
semver_example & drupal look okay to me.
I can't see any reason for the other packages to be removed. There are definitely still the same security releases for google_analytics: https://www.drupal.org/project/google_analytics/releases?version=8.x-2.
There was a problem hiding this comment.
I cleared local cache and reran the build. The packages like google_analytics are no longer removed. Dunno what happenned earlier but it was prob a local problem.
weitzman changed the title 2 participants
This is a PR for build-v2 branch. I think thats right, but please confirm.
I mistakenly published the output of this PR to 011d545#diff-b5d0ee8c97c7abd7e3fa29b9a27d1780. You can see the D9 core conflict at the end of the
drupal/corerow. My mistake will presumably get overwritten during next Circle run.I'm working with drumm to publish an SA against a semver release so we can verify that this PR works as intended. There are no real SAs yet against a semver release.