Automated Drupal 11 compatibility fixes for xmlsitemap

Created on 25 March 2024, 9 months ago

Problem/Motivation

Hello project maintainers,

This is an automated issue to help make this module compatible with Drupal 11.

Changes will periodically be added to this issue that remove deprecated API uses. To stop further changes from being posted, change the status to anything other than Active, Needs review, Needs work or Reviewed and tested by the community. Alternatively, you can remove the "ProjectUpdateBotD11" tag from the issue to stop the bot from posting updates.

The changes will be posted by the Project Update Bot official user account. This account will not receive any issue credit contributions for itself or any company.

Proposed resolution

You have a few options for how to use this issue:

  1. Accept automated changes until this issue is closed

    If this issue is left open (status of Active, Needs review, Needs work or Reviewed and tested by the community) and the "ProjectUpdateBotD11" tag is left on this issue, new changes will be posted periodically if new deprecation fixes are needed.

    As the Drupal Rector project improves and is able to fix more deprecated API uses, the changes posted here will cover more of the deprecated API uses in the module.

    Patches and/or merge requests posted by others are ignored by the bot, and general human interactions in the issue do not stop the bot from posting updates, so feel free to use this issue to refine bot changes. The bot will still post new changes then if there is a change in the new generated patch compared to the changes that the bot posted last. Those changes are then up to humans to integrate.

  2. Leave open but stop new automated changes.

    If you want to use this issue as a starting point to remove deprecated API uses but then don't want new automated changes, remove the "ProjectUpdateBotD11" tag from the issue and use it like any other issue (the status does not matter then). If you want to receive automated changes again, add back the "ProjectUpdateBotD11" tag.

  3. Close it and don't use it

    If the maintainers of this project don't find this issue useful, they can close this issue (any status besides Active, Needs review, Needs work and Reviewed and tested by the community) and no more automated changes will be posted here.

    If the issue is reopened, then new automated changes will be posted.

    If you are using another issue(s) to work on Drupal 11 compatibility it would be very useful to other contributors to add those issues as "Related issues" when closing this issue.

Remaining tasks

Using the patches

  1. Apply the latest patch in the comments by Project Update Bot or human contributors that made it better.
  2. Thoroughly test the patch. These patches are automatically generated so they haven't been tested manually or automatically.
  3. Provide feedback about how the testing went. If you can improve the patch, post an updated patch here.

Using the merge request

  1. Review the merge request and test it.
  2. Thoroughly test the changes. These changes are automatically generated so they haven't been tested manually or automatically.
  3. Provide feedback about how the testing went. If you can improve the merge request, create a new branch and merge request and work from there.

Warning: The 'project-update-bot-only' branch will always be overwritten. Do not work in that branch!

Providing feedback

If there are problems with one of the changes posted by the Project Update Bot , such as it does not correctly replace a deprecation, you can file an issue in the Drupal Rector issue queue . For other issues with the bot, for instance if the issue summary created by the bot is unclear, use the Project analysis issue queue .

📌 Task
Status

Needs review

Version

1.5

Component

Code

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • 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-127659

    These packages were used to generate the fixes:

    1. drupal/upgrade_status: 4.1.0
    2. mglaman/phpstan-drupal: 1.2.9
    3. 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-188815

    These packages were used to generate the fixes:

    1. drupal/upgrade_status: 4.3.2
    2. mglaman/phpstan-drupal: 1.2.11
    3. palantirnet/drupal-rector: 0.20.2
  • 🇮🇳India chandu7929 Pune

    chandu7929 changed the visibility of the branch project-update-bot-only to hidden.

  • Merge request !46Drupal 11 compatibility fixes. → (Open) created by chandu7929
  • Pipeline finished with Failed
    6 months ago
    Total: 419s
    #219823
  • Pipeline finished with Failed
    6 months ago
    Total: 418s
    #219880
  • Pipeline finished with Failed
    6 months ago
    Total: 471s
    #219894
  • Pipeline finished with Success
    5 months ago
    Total: 405s
    #220474
  • 🇮🇳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
  • 🇮🇳India secretsayan

    To ensure compatibility with D11, we should incorporate D11-based testing. Please enable it for verification purposes.

  • Pipeline finished with Failed
    5 months ago
    Total: 380s
    #226601
  • Pipeline finished with Success
    5 months ago
    Total: 387s
    #226693
  • Pipeline finished with Failed
    5 months ago
    Total: 381s
    #227500
  • Pipeline finished with Failed
    5 months ago
    Total: 504s
    #227505
  • Pipeline finished with Success
    5 months ago
    Total: 426s
    #227541
  • Pipeline finished with Success
    5 months ago
    Total: 381s
    #227593
  • Status changed to Needs review 5 months ago
  • 🇮🇳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.

  • Pipeline finished with Failed
    5 months ago
    Total: 378s
    #227613
  • Status changed to Needs work 5 months ago
  • Status changed to Needs review 5 months ago
  • Status changed to RTBC 5 months ago
  • 🇮🇳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

  • 🇮🇳India chandu7929 Pune

    Now CI is green, can we get this merge and release?

  • 🇫🇷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.

  • Pipeline finished with Success
    4 months ago
    Total: 408s
    #257196
  • Status changed to Needs work 4 months ago
  • 🇩🇪Germany Grevil

    New additions by @fgm are necessary, but will result in a fatal error for Drupal < 10.3. Use the DeprecationHelper instead.

  • Pipeline finished with Failed
    4 months ago
    #267463
  • Status changed to Needs review 4 months ago
  • 🇫🇷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
  • 🇩🇪Germany Grevil

    Tests are currently failing for previous major.

  • Pipeline finished with Success
    4 months ago
    Total: 374s
    #267880
  • 🇩🇪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
  • 🇫🇷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 ?

  • Pipeline finished with Failed
    4 months ago
    Total: 635s
    #270779
  • Pipeline finished with Failed
    4 months ago
    Total: 533s
    #270797
  • 🇫🇷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.

  • 🇩🇪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.

  • Pipeline finished with Failed
    4 months ago
    Total: 313s
    #271476
  • Pipeline finished with Canceled
    4 months ago
    Total: 145s
    #271484
  • Pipeline finished with Canceled
    4 months ago
    Total: 79s
    #271485
  • Pipeline finished with Failed
    4 months ago
    Total: 313s
    #271486
  • 🇩🇪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?

  • Pipeline finished with Success
    4 months ago
    Total: 384s
    #271495
  • 🇩🇪Germany Grevil

    grevil changed the visibility of the branch 3435760-automated-drupal-11 to hidden.

  • 🇩🇪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

    1. to merge this into 8.x-1.x (as last change?)
    2. rebase 2.x on latest 8.x-1.x (if it has no changes yet, but 8.x-1.x has)
    3. Remove the ^9 compatibility and require ^10.3 in 2.x - so we can remove the if's in 2.x branch
    4. 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.

  • 🇩🇪Germany Grevil

    Agreed! Let's see what @dave reid has to say about this.

  • 🇦🇺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.

  • Pipeline finished with Success
    3 months ago
    Total: 409s
    #291775
  • 🇦🇺Australia acbramley

    Latest commit fixes composer issues with next major and they're all green!

  • Pipeline finished with Success
    3 months ago
    #291791
  • 🇩🇪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?

  • ivnish Kazakhstan

    I can minimally maintain

  • 🇩🇪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.
  • Merge request !49Resolve #3435760 "D11 readiness" → (Merged) created by ankitv18
  • Pipeline finished with Failed
    about 2 months ago
    Total: 345s
    #320928
  • Pipeline finished with Failed
    about 2 months ago
    Total: 408s
    #320931
  • Pipeline finished with Failed
    about 2 months ago
    Total: 395s
    #320945
  • Pipeline finished with Success
    about 2 months ago
    Total: 398s
    #323837
  • 🇮🇳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

  • Pipeline finished with Success
    about 2 months ago
    Total: 436s
    #324691
  • 🇮🇳India vishalkhode

    Reviewed MR !49, looks good to me now. Hence, RTBC.

  • Status changed to RTBC about 1 month ago
  • 🇸🇰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

    2. Of note, this MR removes the explicit PHP version constraints from the composer.json file and xmlsitemap.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 :(

  • Pipeline finished with Success
    about 1 month ago
    Total: 607s
    #345528
  • 🇺🇸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.
  • 🇸🇰Slovakia poker10

    Saving credits.

  • Pipeline finished with Skipped
    21 days ago
    #355094
  • ivnish Kazakhstan

    Yeah! Thank you, @poker10

  • 🇸🇰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.

  • 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.

Production build 0.71.5 2024