Problem/Motivation

Split from #3491731: [META] Remove the ability to update modules and themes via authorize.php

This issue is for 12.x to (finally!) remove authorize.php, the FileTransfer classes, the rest of the Updater classes, everything in System and Update modules related to this, tests for all this code, and any stale references in docs, etc.

Note that "update.module" is slightly a lie for component, since it's also touching "base system", "system.module", etc, but it's ultimately all for the legacy "Update Manager".

At #3521059: Finish deprecating 'Update Manager' related code in Update Status we deprecate update_clear_update_disk_cache(). We stop calling it from hook_cron(), and have update_post_update_clear_disk_cache() to remove it. However, since in theory something could be calling the deprecated code directly and populating these directories, the "most complete" solution is to have another (basically duplicate) post_update in D12 once all the code paths are totally removed, too.

Steps to reproduce

Proposed resolution

  1. Remove everything that was deprecated in any of the related issues in D11:
  2. Add update_post_update_clear_disk_cache_again() or equivalent.

Remaining tasks

  1. Wait for 12.x branch to open
  2. Remove everything
  3. Add update_post_update_clear_disk_cache_again() or equivalent
  4. Decide if we need to split this up (since the diffstat is nearly 5000 lines of code). Per @catch in #12 "Overall if we can just delete code without having to update logic, I'm (very, extremely) happy to commit thousands of lines of deletions in a single issue/MR." Split out Twig/CSS changes from core to #3571595: Remove authorize.php related Twig and CSS from core
  5. Decide if we should remove CSS and authorize-report.html.twig from the stable9 theme, or leave that alone. Currently the MR doesn't touch stable9 and leaves the dead code in there. Perhaps irrelevant given #3560200: [meta] Tasks to deprecate Stable 9. Moved to #3571596: Remove authorize.php related Twig and CSS from stable9
  6. Decide if we should touch core/modules/migrate_drupal/tests/fixtures/drupal7.php at all as part of this. That file still references lots of stuff being removed here, but I assume we're not supposed to touch that, either. Per @catch in #12: "we can leave it in until the fixture is removed which will happen in 12.x too"
  7. Decide what to do with core/tests/fixtures/config_install/testing_config_install/system.authorize.yml and core/tests/fixtures/config_install/multilingual/system.authorize.yml. Remove them. Tests still pass. They should have been removed at #2715145: Remove system.authorize config but were missed at that time.

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3502975

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

dww created an issue. See original summary.

dww’s picture

dww
we/he/they
commented
Issue summary: View changes
Related issues: +#3521059: Finish deprecating 'Update Manager' related code in Update Status, +#3502974: Deprecate authorize.php, the FileTransfer and Updater systems, +#3417136: Remove adding an extension via a URL

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

dww’s picture

dww
we/he/they
commented
Parent issue: #3491731: [META] Remove the ability to update modules and themes via authorize.php » #3553503: [meta] Remove deprecated classes, methods, procedural functions and code paths outside of deprecated modules from Drupal 12
Related issues: +#3491731: [META] Remove the ability to update modules and themes via authorize.php

Re-parenting this to the more active initiative.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Postponed » Active
dww’s picture

dww
we/he/they
commented
Assigned: Unassigned » dww
Issue tags: +Deprecation Removal

I’m going to work on this. It’s a bit more involved than the usual. And this mostly code I wrote, and would love the satisfaction of being the one to actually remove it. Please don’t work on this until I post an MR for review. Thanks!

kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
commented

I agree! Why is deleting code much more satisfying than writing code!? I love it. 🤣

dww opened merge request !14627

dww’s picture

dww
we/he/they
commented
Assigned: dww » Unassigned
Issue summary: View changes
Status: Active » Needs review
  1. 50 files +20 −4945 (includes -409 from .phpstan-baseline.php) 🎉
  2. The +20 is only for update_post_update_clear_disk_cache_again()
  3. Pipeline is now all green.
  4. Removed all the tests I could find that were related to any of this.
  5. In addition to all the stuff mentioned in the summary, I did a fair bit of recursive case-insensitive grepping for 'authorize', 'Updater', 'FileTransfer' and the like. Found some stragglers that way (API docs, some tests I hadn't noticed before, code in test modules, etc).
  6. Although it's tempting to delete core/lib/Drupal/Core/Archiver too, that's actually still in use by core/modules/config (both in src/Form/ConfigImportForm.php and src/Controller/ConfigController.php).

Updated the issue summary. Ready for review!

dww’s picture

dww
we/he/they
commented
Issue summary: View changes
Related issues: +#3560200: [meta] Tasks to deprecate Stable 9

Found a few more references to authorize.php so diffstat is now: 53 files +22 −4948. The additional +2 are a comment in .htaccess and core/assets/scaffold/files/htaccess that referenced authorize.php but is otherwise still valid.

Added these decisions to remaining tasks:

  • Decide if we need to split this up (since the diffstat is nearly 5000 lines of code).
  • Decide if we should remove CSS and authorize-report.html.twig from the stable9 theme, or leave that alone. Currently the MR doesn't touch stable9 and leaves the dead code in there. Perhaps irrelevant given #3560200: [meta] Tasks to deprecate Stable 9.
  • Decide if we should touch core/modules/migrate_drupal/tests/fixtures/drupal7.php at all as part of this. That file still references lots of stuff being removed here, but I assume we're not supposed to touch that, either.
  • Decide what to do with core/tests/fixtures/config_install/testing_config_install/system.authorize.yml and core/tests/fixtures/config_install/multilingual/system.authorize.yml.
dww’s picture

dww
we/he/they
commented
Issue summary: View changes

Minor summary cleanup.

catch’s picture

catch
he/him
Primary language English
commented

Overall if we can just delete code without having to update logic, I'm (very, extremely) happy to commit thousands of lines of deletions in a single issue/MR.

What this means though is anything tricky we should split out - either to one issue if it's just a bit fiddly, or separate issues if it's genuinely hard to review.

On the remaining bits:

If cruft in core/modules/migrate_drupal/tests/fixtures/drupal7.php doesn't break any tests, we can leave it in until the fixture is removed which will happen in 12.x too.

Decide if we should remove CSS and authorize-report.html.twig from the stable9 theme, or leave that alone.

I think it probably would be OK to remove because these are for an interface that no-one will be able to access any more, but I think we should split the CSS/template changes out to their own issue anyway and discuss it there. And possibly split that issue into two if stable9 needs discussion.

core/tests/fixtures/config_install/testing_config_install/system.authorize.yml and core/tests/fixtures/config_install/multilingual/system.authorize.yml.

These seem like they could just go? But are tests relying on them?

dww’s picture

dww
we/he/they
commented
Issue summary: View changes

Sweet, thanks!

Updating summary accordingly. I believe this is now RTBC, but that's for someone else to say.

Thanks again!
-Derek

kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
commented

I feel like this a release highlight 😆

dww’s picture

dww
we/he/they
commented

@kim.pepper: Thanks for your comments! Feels supportive to know other folks are watching this.

re: #7: Yeah, funny, huh? 😉 I think in cases like this, it's partly the thrill of finally being rid of something we were responsible for, the burden of supporting it all, the issues, bugs, support requests, etc. But the other part, maybe more significant, is that it was "our baby" all that time, and if someone *else* was the one to come in and rip it all out, it'd be a little bit of an ouch. Yeah, I wrote it (decades ago), and I maintained it (such as it was), and now I'm here to finally put it to rest. If that flow was interrupted, and another person finished the loop, I'd feel left out. That's my I'm-not-a-therapist diagnosis, at least. 😂

re: #14: It's a release highlight for old-timers like us, but "removed a bunch of stuff that hasn't really worked for a few years" isn't exactly highlights for 99+% of the folks reading release notes. 😅 The issues for deprecating it and removing the UI for it were already release highlights. I don't think finally removing the cruft from the codebase counts.

Relative to the total size of core, even a 5K line removal is still only a drop. But it'll be a relief to see it gone, that's for sure!

Thanks again,
-Derek

dww’s picture

dww
we/he/they
commented
Issue summary: View changes
Related issues: +#2715145: Remove system.authorize config

Re remaining task #7: Was doing some git and issue archeology for another issue, and found #2715145: Remove system.authorize config. That's where system.authorize.yml was removed. Looks like we missed those test fixture files at that time. But yeah, clearly, this is very dead code, and should finally be put to rest.

catch’s picture

catch
he/him
Primary language English
commented

For release highlights, maybe we could have something about net lines of code removed between 11.4 and 12.0, or between 11.3 and 12.0 would match how https://dbuytaert.github.io/drupal-core-metrics/ keeps track.

For comparison, Drupal 9.0 looks like it removed about 7,000 net lines of code (8 minor releases of Drupal 8 deprecations to delete).

Drupal 10.0.0 removed more or less zero (possibly because we added a couple of new things like views responsive grid?)

Drupal 11.0.0 removed about net 3,000.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs review » Needs work

This is great cleanup! SystemHooks still has a now-obsolete hook:

  /**
   * Implements hook_updater_info().
   */
  #[Hook('updater_info')]
  public function updaterInfo(): array {
    return [
      'module' => [
        'class' => 'Drupal\Core\Updater\Module',
        'name' => $this->t('Update modules'),
        'weight' => 0,
      ],
      'theme' => [
        'class' => 'Drupal\Core\Updater\Theme',
        'name' => $this->t('Update themes'),
        'weight' => 0,
      ],
    ];
  }

Otherwise I can only find the theme layer related bits remaining, they can go in the followups.

dww’s picture

dww
we/he/they
commented
Status: Needs work » Needs review

@catch re: #17: That sounds good, and more highlightable.

@longwave re: #18: Nice find! Dang, I thought I had grep'ed for all that, but I guess I missed that one. Pushed a commit for it.

GitLab seems weird these days. It keeps claiming a merge is blocked by a non-fastforward. But when I rebase, there aren't conflicts. This has happened multiple times in the last few days on multiple issues. I don't remember this being a thing until recently. But I rebased again, just in case.

Thanks!
-Derek

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

@dww yeah that seems like a recent change in GitLab, it says the merge is blocked unless it is rebased onto HEAD. But the committers don't use merges anyway, so as long as it doesn't explicitly say there's a merge conflict, you are all good.

dww’s picture

dww
we/he/they
commented
Related issues: +#3571860: [12.x] Remove deprecated template_preprocess functions from update.module

FYI that I put the remaining 5 deprecated functions in update.module into a separate issue for better scoping: #3571860: [12.x] Remove deprecated template_preprocess functions from update.module. That's also ready for review. With both branches merged locally, there are no longer any hits for 'deprecate' in core/modules/update/*.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Very nice work @dww! Serious rabbit hole of stuff.

I applied the MR and did a search for deprecated and 12.0.0 and believe all instances have been removed.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs work

Sorry this needs a rebase. I think because the preprocess removal landed.

dww’s picture

dww
we/he/they
commented
Status: Needs work » Needs review

Rebased. Trivial merge conflict resolved (both branches removing unused use statements next to each other). Let's see what the bot thinks.

dww’s picture

dww
we/he/they
commented
Status: Needs review » Reviewed & tested by the community

Random fails (including a Functional random in package_manager? and the dreaded InlineBlockTest), but green pipeline now. Gonna go ahead and self-RTBC in this case.

Thanks!
-Derek

dww’s picture

dww
we/he/they
commented
Title: [12.x] Remove all legacy code related to authorize.php and FileTransfer » Remove all legacy code related to authorize.php and FileTransfer

  • longwave committed 09985a50 on main 
    task: #3502975 Remove all legacy code related to authorize.php and...
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Reviewed & tested by the community » Fixed

Goodbye, Update Manager!

Committed and pushed 09985a50b2b to main. Thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

dww closed merge request !14627