- Issue created by @project update bot
This is an automated patch generated using Upgrade Status and Drupal Rector. Please see the issue summary for more details. A merge request (MR) is also openend and updated.
It is important that any automated tests available are run and that you manually test the changes.
Drupal 11 Compatibility
According to the Upgrade Status module → , even with these changes, this module is not yet compatible with Drupal 11.
Currently Drupal Rector, version 0.20.1, cannot fix all Drupal 11 compatibility problems.
Therefore these changes did not update the
info.yml
file for Drupal 11 compatibility.The compatibility issues that Upgrade Status found after the Drupal Rector fixes were applied are attached to help you resolve them manually.
Leaving this issue open, even after committing the current patch or merging the MR, will allow the Project Update Bot → to post additional Drupal 11 compatibility fixes as they become available in Drupal Rector.
Debug information
Bot run #11-127659These packages were used to generate the fixes:
- drupal/upgrade_status: 4.1.0
- mglaman/phpstan-drupal: 1.2.9
- palantirnet/drupal-rector: 0.20.1
This is an automated patch generated using Upgrade Status and Drupal Rector. Please see the issue summary for more details. A merge request (MR) is also openend and updated.
It is important that any automated tests available are run and that you manually test the changes.
Drupal 11 Compatibility
According to the Upgrade Status module → , even with these changes, this module is not yet compatible with Drupal 11.
Currently Drupal Rector, version 0.20.2, cannot fix all Drupal 11 compatibility problems.
Therefore, these changes did not update the
info.yml
file for Drupal 11 compatibility.The compatibility issues that Upgrade Status found after the Drupal Rector fixes were applied are attached to help you resolve them manually.
Leaving this issue open, even after committing the current patch or merging the MR, will allow the Project Update Bot → to post additional Drupal 11 compatibility fixes as they become available in Drupal Rector.
Debug information
Bot run #11-188815These packages were used to generate the fixes:
- drupal/upgrade_status: 4.3.2
- mglaman/phpstan-drupal: 1.2.11
- palantirnet/drupal-rector: 0.20.2
- 🇮🇳India chandu7929 Pune
chandu7929 → changed the visibility of the branch project-update-bot-only to hidden.
- 🇮🇳India chandu7929 Pune
Created MR 46 with required change, since all tests are passing, hence requesting review.
- Status changed to Needs work
5 months ago 11:18am 17 July 2024 - 🇮🇳India secretsayan
To ensure compatibility with D11, we should incorporate D11-based testing. Please enable it for verification purposes.
- Status changed to Needs review
5 months ago 8:26am 18 July 2024 - 🇮🇳India chandu7929 Pune
Now all tests are passing, including the next major: https://git.drupalcode.org/issue/xmlsitemap-3435760/-/pipelines/227593, hence I'll remove the changes added for fetching modules from the CVS path and request review for the MR46.
- Status changed to Needs work
5 months ago 6:38am 19 July 2024 - Status changed to Needs review
5 months ago 7:22am 19 July 2024 - Status changed to RTBC
5 months ago 7:52am 19 July 2024 - 🇮🇳India secretsayan
Moving this to RTBC as there is no more actionable items pending and we have verified that this works with the latest upcoming changes of
drupal/robotstxt
in comment #10. Tests will start passing only when https://www.drupal.org/project/robotstxt/issues/3434261 📌 Automated Drupal 11 compatibility fixes for robotstxt RTBC is released. - 🇫🇷France fgm Paris, France
Upstream issue 📌 Automated Drupal 11 compatibility fixes for robotstxt RTBC is now fixed and RobotsTxt 8.1.6 has been released with D11 compatibility: https://www.drupal.org/project/robotstxt/releases/8.x-1.6 →
- 🇫🇷France fgm Paris, France
OTOH, upgrade_status still reports three instances of error
Call to deprecated method renderPlain() of class Drupal\Core\Render\Renderer. Deprecated in drupal:10.3.0 and is removed from drupal:12.0.0. Use Drupal\Core\Render\RendererInterface::renderInIsolation() instead.
But these are only a problem for D12 migration, so should not be holding back the D11 release.
- Status changed to Needs work
4 months ago 1:12pm 28 August 2024 - 🇩🇪Germany Grevil
New additions by @fgm are necessary, but will result in a fatal error for Drupal < 10.3. Use the DeprecationHelper → instead.
- Status changed to Needs review
4 months ago 7:16pm 28 August 2024 - 🇫🇷France fgm Paris, France
Tried the Deprecation Helper, seems to work just fine in 10.2.7 and 11.0.1
- Status changed to Needs work
4 months ago 7:10am 29 August 2024 - 🇩🇪Germany Grevil
Tests are green now.
But I am really unsure about this MR... There are LOADS of unrelated (and seemingly unnecessary) code style changes, which makes reviewing quite cumbersome. Found one more issue, which I commented.
Besides, 18 new phpcs issues were introduced, but the maintainer should decide here in the end.
- 🇫🇷France fgm Paris, France
AIUI, these code style changes were originally added just to make the tests pass, not for any technical purpose. Seeing how tests are now passing, I think it would be good to release this to unblock at least D11 users.
Users of D10.3 and of obsolete versions do not have any more requirement to upgrade XMLSitemap than the core versions which they did not upgrade, so that should not be a problem for them.
- 🇩🇪Germany Grevil
Seems weird to me, that changes like https://git.drupalcode.org/project/xmlsitemap/-/merge_requests/46/diffs#... would fix any tests?
But not really relevant, in the end, the maintainer needs to decide here 🙂
Only https://git.drupalcode.org/project/xmlsitemap/-/merge_requests/46#note_3... left. Afterwards, I can ping the maintainer.
- 🇭🇰Hong Kong hswong3i
This MR could now apply for D11 with:
... "repositories": { "drupal": { "canonical": false, "type": "composer", "url": "https://packages.drupal.org/8" }, "drupal/xmlsitemap": { "canonical": false, "type": "vcs", "url": "https://git.drupalcode.org/issue/xmlsitemap-3435760.git" }, }, ... "require": { "drupal/xmlsitemap": "dev-3435760-automated-drupal-11", } ...
- Status changed to Needs review
4 months ago 9:45am 1 September 2024 - 🇫🇷France fgm Paris, France
@grevil I added a workaround for that default parameter value, but I'm not a big fan of it. Any better suggestion ?
- 🇫🇷France fgm Paris, France
The "previous major" fail is because Drupal 9 (RIP), didn't have the DeprecationHelper anyway so we cannot have a version for D9 to D11.
- Merge request !48Issue #3435760: Automated Drupal 11 compatibility fixes for xmlsitemap → (Open) created by Grevil
- 🇩🇪Germany Grevil
The "previous major" fail is because Drupal 9 (RIP), didn't have the DeprecationHelper so we cannot have a version for D9 to D11 anyway.
Sorry, I didn't know this! Thought they backported it to D9, but hence it has reached its EOL, they probably just ditched the backport.
- 🇩🇪Germany Anybody Porta Westfalica
@maintainers: I'd vote to remove Drupal 9 compatibilty (EOL!) and require ^10.3 so we can remove the if-statements! Please decide how to proceed.
What about the 2.x branch?
- 🇩🇪Germany Grevil
Alright, as the old "3435760-automated-drupal-11" branch contained a LOT of unrelated (and unnecessary) code style changes, I created a new branch "3435760-drupal-11-compatibility-cleaned" containing only the D11 compatibility changes.
I also used version_compare instead of the DeprecationHelper, to keep the D9 compatibility. Please review!
- 🇩🇪Germany Anybody Porta Westfalica
Thanks @Grevil! Maybe it then makes sense
- to merge this into 8.x-1.x (as last change?)
- rebase 2.x on latest 8.x-1.x (if it has no changes yet, but 8.x-1.x has)
- Remove the ^9 compatibility and require ^10.3 in 2.x - so we can remove the if's in 2.x branch
- Release 2.0.0 for the future?
But let's wait for maintainer feedback. Setting priority to major as D11 is out!
- 🇫🇷France fgm Paris, France
100% for removal of 9.x compatibility. Keeping contrib compatible with dead versions just helps prolong the suffering of those still running them and makes code worse for those maintaining modules.
- 🇦🇺Australia acbramley
Left some comments, it seems like almost all of the changes in the xmlsitemap.module file could be reverted.
- 🇩🇪Germany Anybody Porta Westfalica
I think as next step we urgently need a maintainer decision here if we should drop Drupal 9 support and require Drupal ^10.3 at least
(^10.3 || ^11
) which would allow a lot of cleanup. In that case this would also mean we need a 2.x branch for BC, I think?Could anyone ping maintainers? Seems there are many, but many inactive...
- 🇦🇺Australia acbramley
Just realised I reviewed the wrong MR yesterday 🤦♂️
I've reached out on slack to see if any maintainers can comment.
- 🇦🇺Australia acbramley
Latest commit fixes composer issues with next major and they're all green!
- 🇩🇪Germany Grevil
Just realised I reviewed the wrong MR yesterday 🤦♂️
Happens to the best! 😄
I've reached out on slack to see if any maintainers can comment.
Already did that a few weeks ago... no answer yet, but maybe you have better luck! 🙂👍
- 🇩🇪Germany Anybody Porta Westfalica
@acbramley any luck?
Otherwise request maintainership perhaps? - 🇦🇺Australia acbramley
No response unfortunately, I haven't pinged anyone directly though. There are 13 maintainers on this project, has anyone tried contact forms or direct messages?
- 🇦🇺Australia acbramley
I've pinged several maintainers (all the ones I could find in slack) here https://drupal.slack.com/archives/C1BMUQ9U6/p1727322868231979?thread_ts=...
- 🇩🇪Germany Grevil
Oh my apologies, the last couple of weeks I contacted quite a few maintainers in different modules. Seems I mixed it up, I wanted to contact @dave reid regarding this issue (as he is the most recently active maintainer), but it seems I haven't!
I'll contact him now (on slack and via mail).
- 🇺🇸United States nicxvan
I reached out in slack to @dave reid as well.
I'd vote for moving forward with dropping Drupal 9, it's no longer supported, and there is an update path for people still on Drupal 9.
I can't imagine there being a strong objection.
- 🇩🇪Germany Grevil
I'd say somebody creates a maintainer ship issue?
@nicxvan would you be interested in (minimally) maintaining this module?
- 🇩🇪Germany Grevil
Very nice @ivnish!
Please create an issue regarding the maintainership, so we can hopefully get this fixed ASAP!
- 🇩🇪Germany Anybody Porta Westfalica
@fgm @acbramley what about maintainership for you?
Maintainers should be cleaned up then! Unbelievable and risky they're all inactive!
- 🇺🇸United States nicxvan
I actually just realized that I use https://www.drupal.org/project/simple_sitemap → on most of my sites. The site using this was inherited and I didn't realize it was different.
Simple sitemap is already d11 ready and I think is in drupal cms so I'm planning on switching modules.
- 🇫🇷France fgm Paris, France
@anybody : I'm not in a position to take up maintainership of one more module.
- First commit to issue fork.
- 🇮🇳India ankitv18
With above discussion threads on MR!48 for bifurcating the cleaner implementation without managing the version_compare checks for handling the deprecations, I've cut the separate branch i.e 3435760-d11-readiness against 2.x considering the next major release with supporting drupal version ^10.3 || ^11
Please review MR!49 ~~ pipelines are passing for phpunit and we can fix the validate pipeline separately.
cc: @anybody @grevil @acbramley
- 🇮🇳India ankitv18
I would like to suggest to consider both MR as MR!48 with backward compatibility and MR!49 with minimum support for d10.3 for next major release i.e 2.x
- 🇮🇳India vishalkhode
Reviewed MR !49, looks good to me now. Hence, RTBC.
- Status changed to RTBC
about 1 month ago 3:45pm 13 November 2024 - 🇸🇰Slovakia poker10
Re: maintainership. I am one of the maintainers of this module, but was maintaining only the 7.x branch. I do not have permission to add another maintainers. So I think we need at least one issue - either from me to get full permissions, or from someone else to get on board. Thanks!
- 🇺🇸United States daddison
@poker10 Do you have permission to cut a release for the 2.x branch? Seems the community has tested this and feels comfortable with this.
- ivnish Kazakhstan
https://www.drupal.org/project/xmlsitemap/maintainers.json →
Looks like poker10 has all permissions except "administer maintainers"
I think he can commit this and create a new release. It won't take long
- 🇸🇰Slovakia poker10
Yes, I can commit this and create a release. I looked at the MR and all changes looks good to me (tests are passing with these changes). I can't tell without a deeper check, if the MR is not missing something else, but if tests are green, probably not.
One thing I am not sure about is the status of the 2.x branch. There seems to be some additional commits in comparision with the 8.x-1.x branch. Yes, I can roll a 2.x-beta, but have anyone tested the 2.x-dev if it is really working with all changes currently committed? Sorry, I do not have time to check it commit by commit now.
Thanks!
- 🇺🇸United States mark_fullmer Tucson
Good call, @poker10. Here's my audit & analysis:
1. The cumulative code changes between the 2.x and 8.x-1.x branch can be seen at https://git.drupalcode.org/project/xmlsitemap/-/compare/2.x..8.x-1.x
- The change committed to 8.x-1.x not present in 2.x is https://git.drupalcode.org/project/xmlsitemap/-/commit/40727c92415802bb1... , which is innocuous enough and could simply be merged into 2.x without issue
- The changes committed to 2.x not present in 8.x-1.x consist of https://git.drupalcode.org/project/xmlsitemap/-/commit/1d59a51562d50b7d3... (see #3257944: PHP 8.1: XmlSitemapWriter::openUri($uri) should either be compatible with XMLWriter::openUri(string $uri): bool → ). This commit removes functions that were deprecated in 8.x-1.x and provides additional PHP 8.1 compatibility with return type hinting.
2. Of note, this MR removes the explicit PHP version constraints from the
composer.json
file andxmlsitemap.info.yml
file that were added in https://git.drupalcode.org/project/xmlsitemap/-/commit/1d59a51562d50b7d3... . I believe this makes sense, since there is no longer a need to specify minimum PHP 8.1 compatibility in the module itself, since its constraint to Drupal 10/11 does the same thing. Conclusion: this change in the MR is fine.3. I functionally tested this MR locally and can confirm that it works the same in our codebase as 8.x-1.5. While my testing certainly didn't comprehensively evaluate all of the options for XML sitemap, the code review above suggests that there should not be any functional or backwards-compatibility breaking change, and since the Drupal core compatibility is set at
^10.3 || ^11
, which have a minimum PHP version requirement of 8.1, there is no risk of incompatibility with sites running on PHP 7.4. Based on the above, the actions to take, I believe, are:
- (Optional, but probably a good idea) Merge 8.x-1.x into 2.x once more, which will simply add the
logo.png
file to 2.x and get the branch history up to date. - Merge the Drupal 11 compatibility changes in this issue into 2.x and release 2.0.0-beta1. The release notes should call out the following:
- The 2.x major version release of XML Sitemap removes functions deprecated in 8.x-1.x four years ago. The functions were likely unused, but developers who have extended XML Sitemap should review #3156088: user_access() does not exist in Drupal 8 anymore → for confirmation.
- 2.x also drops support of Drupal 9 up to Drupal 10.2 (i.e., compatible with Drupal 10.3 and 11). Sites still using lower versions of Drupal core should continue to use 8.x-1.5.
- ivnish Kazakhstan
@mark_fullmer good job!
I think we can just add D11 support to 8.x-1.x and allow people to upgrade their sites to D11
- ivnish Kazakhstan
@poker10 I created https://www.drupal.org/project/projectownership/issues/3488525 📌 Add "administer maintainers" permission to an active maintainer Active
- 🇸🇰Slovakia poker10
@ivnish I would probably not do that in 8.x-1.x, as it is going to drop support for anything below 10.3, which can be disruptive for existing sites?
Thanks for creating the other issue.
- ivnish Kazakhstan
as it is going to drop support for anything below 10.3, which can be disruptive for existing sites
Yes, of course. Just an idea :)
Let's working on 2.x branch
- ivnish Kazakhstan
@poker10, you need to create your own issue to become a maintainer of this module. My request was closed :(
- 🇺🇸United States mark_fullmer Tucson
The commits noted in #64 match the analysis & recommendation from #59. Code review looks good. This is RTBC and ready for a 2.0.0-beta1 release, which calls out the following:
- The 2.x major version release of XML Sitemap removes functions deprecated in 8.x-1.x four years ago. The functions were likely unused, but developers who have extended XML Sitemap should review #3156088: user_access() does not exist in Drupal 8 anymore → for confirmation.
- 2.x also drops support of Drupal 9 up to Drupal 10.2 (i.e., compatible with Drupal 10.3 and 11). Sites still using lower versions of Drupal core should continue to use 8.x-1.5.
-
poker10 →
committed 89de4257 on 2.x authored by
ankitv18 →
Issue #3435760 by chandu7929, grevil, ankitv18, fgm, acbramley, anybody...
-
poker10 →
committed 89de4257 on 2.x authored by
ankitv18 →
- 🇸🇰Slovakia poker10
Merged this to 2.x, thanks everyone! Closing this as fixed - if there will be any other issues, please open a new issue. 8.x-1.x will remain without the D11 support, as that is much easier. And I will create a new 2.0.0-beta1 release today.
- 🇸🇰Slovakia poker10
Here is the new release: https://www.drupal.org/project/xmlsitemap/releases/2.0.0-beta1 →
Automatically closed - issue fixed for 2 weeks with no activity.
- 🇳🇱Netherlands firfin
This is a pretty long thread. But would it be fair to summarize it as: for D11 use version 2?
And in that case can we add a summary to the OP?
Finally, a link to the issues stopping a official / non-beta release would be helpful, so people can help with those.
Because not everyone is capable or able to use beta-releases ( not skilled enough with composer, government compliance, etc.) - 🇸🇰Slovakia poker10
Thanks for the thoughts @firfin. Week or two ago, I updated the XML Sitemap project description (see https://www.drupal.org/project/xmlsitemap → ), so the branches should be described also here. It is true, that the 2.0.x branch is with the D11 support and I do not think we will add it to the previous 8.x-1.x branch. If you feel that this IS should be updated as well, feel free to update it.
I am not aware of any new issues with the 2.0.x branch so far. One of the reasons it was tagged as beta was the fact, that I am not as familiar with the D8+ branches of the module (was previously maintaining only the D7 branch). But if there won't be any new issues, I think that we can tag a stable release soon.