Deprecate/remove the ability to update a module from a URL and authorize.php

Deprecation may need route aliases

Could it be issue #2352637: Remove the UI for installing/updating modules from update module if it is not fixed in time for release → ?

@ressa that is very possibly the issue I was thinking of, especially given I opened it. Things would have been easier now if we'd done that at the time instead of won't fixing it.

Hah, not sure how I never saw #2352637: Remove the UI for installing/updating modules from update module if it is not fixed in time for release → until now. But that was right in the era when I was AWOL from Drupal and update.module was considered "unmaintained".

Anyway, yes, we should finally remove all this code. For reference/context, in 📌 Remove adding an extension via a URL Fixed "we" decided to directly remove the routes and code for install-new-stuff, instead of doing a deprecation dance and moving the old plumbing into an abandoned contrib module. I personally vote we do the same here.

It'd be lovely to know for certain we'd be replacing this with the shiny/new functionality from package_manager and update_manager/auto_updates in the same release. But even if we don't know for certain, the chances that the legacy "update manager" provided by update.module are actually still helping sites stay up to date with contrib releases are pretty small. More and more of contrib is depending on composer to manage external dependencies, which this code doesn't handle. So unless you've got just the right combo of installed modules, this feature is as likely to break your site as it is to help keep it running the most secure versions of things.

😂

More accurate title, since authorize.php and the UI from update.module don't use URLs -- that was the install new code stuff that's already gone. Also, update.module handles modules and themes, fwiw.

@catch: Things would have been easier now if we'd done that at the time instead of won't fixing it

How is it the saying goes ... "hindsight is 20/20"? So many things could or should have been ... I am just grateful that Drupal exists in the first place, and that there are all these amazing, gifted people such as you, @dww, and @andypost working towards a common goal of making a great CMS. Which we are winning, by the way :) I'll add the issue as related, to connect them.

Great clean-up!

This is among the most satisfying MRs I've opened against core in a very long time. 😉 This is maybe too bold. I'm immediately removing everything. No deprecations, just gone. 😂 Current diffstat:

47 files +2 −4963

If we want to mark all this crap (the vast majority of which I originally wrote, so I feel okay describing it that way - no offense to anyone) deprecated and only remove it in 12.x, we'll need a whole new MR. But I wanted to see what happened with this approach for now...

I do not really think there is an API/bc risk here as such, we could check a couple of classes against contrib search just in case, so for me this comes down to a product decision about whether we can drop the feature outright.

Given there are only about 10,000 sites already using 11.x, it does seem very unlikely that any 11.x early-adopters would also be using this feature, so possible that literally no-one would notice.

Looking at changes I see no reason to provide BC and deprecations

There are a number of references to authorize.php that need updating

$ grep -r "authorize.php" core
core/lib/Drupal/Core/DrupalKernel.php:      // authorize.php, and others to auto-detect a base path.
core/lib/Drupal/Core/Render/BareHtmlPageRendererInterface.php: * - authorize.php
core/lib/Drupal/Core/File/file.api.php: * manager when they are redirected to the authorize.php script to authorize
core/lib/Drupal/Core/File/file.api.php: *     the authorize.php form.
core/lib/Drupal/Core/File/file.api.php: * @see authorize.php
core/themes/stable9/templates/admin/authorize-report.html.twig: * Theme override for authorize.php operation report templates.
core/themes/stable9/templates/admin/authorize-report.html.twig: * This report displays the results of an operation run via authorize.php.
core/assets/scaffold/files/htaccess:  # Allow access to PHP files in /core (like authorize.php or install.php):

Finally got a green pipeline. 😅 Diffstat is now up to: 56 files +11 −5618. 🎉

I'm 80% sure there are still some lingering bits of cruft sprinkled about. I need to do a more thorough job of grepping for various things. But those could be cleaned up in follow-ups if we miss them on this first attempt.

Adding some open questions to remaining tasks:

• Decide what to do with the administer software updates permission. It's also used for update.php (DB updates), so I think it needs to stay, but TBD.
• Decide if we can remove lib/Drupal/Core/Archiver/* and related tests, too.

Fleshed out UI and API changes sections.

Whoops, hadn't seen #15 when I wrote #16. Some of those are already gone with additional commits I've pushed. I'll cleanup the others now...

Yeah, core/lib/Drupal/Core/File/file.api.php was already fixed. Pushed a commit for 3 of the others, no question.

I'm not totally sure we should be removing the stuff from stable9. Maybe we need a frontend framework manager to signoff on that (commit b07e3fd3)?

Heh, whoops. Looking a bit more closely at everything I deleted, it includes some of these, e.g. in the Updater* world:

  public function postInstall() {
    @trigger_error(__METHOD__ . '() is deprecated in drupal:11.1.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3461934', E_USER_DEPRECATED);
  }

Should we:

1. Scale back the scope of the removals here?
2. Make sure this lands in 11.1.0 and update the https://www.drupal.org/node/3461934 → CR to match? 😉
3. Other?

Thanks,
-Derek

Added #19 to remaining tasks.

I vote 2

Making the title match the MR. Product and/or release managers can change it back if they disagree.

Gonna bump this to major, since it's got big implications for auto_updates / new update_manager, Drupal CMS, and since we should strongly consider doing this before 11.1.0 final.

I want to see support for tar/zip installs go away and move core to composer only. I do not use update module to update sites. With that in mind, given the speed in this issue is moved I feel a need to speak up for "the other side" of this and at least put something on the record for sites that may (for whatever reason I can not understand) be using these features.

I noted in #3285191-50: [meta] Only support Drupal core installs managed by composer → I do not believe features such as this should be removed in minor releases. I suggested in April this should have been done as a deprecation in D10 for removing D11 however to my knowledge the release managers did not make it a release blocker and as such we missed the D11 window. That alone should be reason not to include this at this time.

Going a step further, even if this proceeds to be done in the 11.x branch, the release cycle is already at RC1 for 11.1.x, this is not the time for these sort of changes where their review time will be limited (unsure if core is releasing this week or if they pushed back since RC1 was delayed a week).

Ultimately it is a release manager decision, however from my pov, as much as I see no use for this feature set, this isn't bending things a bit, it is a significant code yank, one that we knew about and could have properly planned for rather than trying to shoe-horn in at the last minute.

We are already trying to squeeze too much into 11.1.0 given that we are already at rc1, so I agree with @cmlara on not committing this to 11.1.x. While this does affect Drupal CMS, given that the UI is still visible and very confusing when Automatic Updates is also installed, there is nothing stopping AU fixing that, even if that is temporary.

We still would like to remove this before Drupal 12, given that we believe few to no users currently use this feature for reasons already explained, so it is likely that this can still land in 11.2.

Makes sense, thanks.

Do we want to remove the routes and deprecate all the underlying functions and classes in 11.2.x, then completely remove it all in 12.0.x? Or are you thinking we directly remove everything in 11.2.x?

Thanks again,
-Derek

I discussed this a bit with @longwave in slack and while I originally thought it was better to commit in 11.1 (when there very few installs compared to 10.x), given we think no-one is using this anyway, even if there are more 11.x installs in six months, there might be the same number (zero!) using the feature, so it might not make a real difference in practice.

So I think we should go ahead with a full removal even though it's bending the bc policy, because we don't think anyone uses the UI, or integrates with the APIs. An early commit to 11.2 is more likely to tell us if we're somehow wrong about that, and then we could revisit.