Remove adding an extension via a URL

Moving this to the component where we need to remove things. 😅

Also, seems like this needs to be postponed on project browser in core. Ideally stable.

I'm not convinced we need to move the crappy old plumbing from 15 years ago into a separate legacy deprecated obsolete project. IMHO, we can simply remove it all once there's a better replacement built on modern foundations.

Clarifying this is about removing the parts of Update Manager that will be replaced with Project Browser (install new stuff), not the parts that will be replaced by Auto Updates (update your existing stuff).

Added admin/theme/install to the Issue Summary

I feel certain I have duplicates of this issue somewhere...

This form has been wrong and broken in its behavior since we made the composer improvements in 8.8, and we no longer support any version of Drupal where what it did was correct. Therefore, it can be removed without replacement even without PB existing yet. We just need to make sure PB is not altering it or something.

Updating proposed resolution accordingly.

Preliminary MR now available. I might have missed some spots, but this is a start. Curious what the bot thinks. 😅

Oh so very satisfying -- I just marked about a dozen issues closed (outdated) given this is now moving forward. 🎉 Most are now child issues of this, although some already had parents and point here as related.

This is great news!

We have functionality to hide this menu and route in Project Browser, so we can take that out when this gets merged.

Is this expected to make it into D11? I would assume it would need a major release.

xjm said in slack it’d have to be 11.1.x, but I don’t really understand why it couldn’t be 11.0.0. I guess it’s too big of a change to happen now that we’re already in rc. Plus all our semi weird rules about a new major version must be identical to the last minor release of the previous major with only deprecated code removed. I still don’t fully understand or support core’s interpretation of semantic versioning, but I don’t want to die on that hill here. 😅

Tests pass, and the MR is all well-deserved deletes. I'm calling it.

I've carried out manual testing in Drupalpod, and confirm the Add module option is no longer available, so confirm RTBC.

@dww I was unsure whether this is considered a new feature, or removal of a deprecated one. If it can go in 11.1, that would imply it's considered a new feature. However, if it's removal deprecated code removal, presumably the feature would have to be marked deprecated in a release (possibly 11.1), and the actual removal would come in 12.0. I would probably consider 11.1 preferable to waiting for 12.

I'd also wonder can this also be backported into 10.4, or does it have to be 11 only?

This is outside our normal BC policy, but has been my strong recommendation for several years ever since Drupal.org switched to making Composer the only recommended way to build a Drupal site. I encouraged @dww to unpostpone the issue because he was asking about a bunch of outstanding maintenance requests related to this dead-end page. (I'm one of the Drupal core release managers, and we have a governance role to decide about technical debt, maintainability, and backwards compatibility questions.)

10.4 should only receive changes that are (a) dependency updates, (b) security or upgrade path fixes, or (c) necessary critical API contrib compatibility. In this case, (c) might apply so that AU only has to take one approach to be added to Drupal 10 as well as 11.

I shared this issue with the other committers to validate my release management recommendation here.

We do need a release note and a CR for it since it's outside the normal BC policy.

Thanks everyone!

This looks great! Happy to see this all finally being removed.

The MR removes some references to system.theme_install but that still exists (update.theme_install is removed, though).

Also this route reference still exists in \Drupal\Core\Updater\Module::postInstallTasks():

      $default_options + [
        '#url' => Url::fromRoute('update.module_install'),
        '#title' => t('Add another module'),
      ],

Crediting @longwave for the careful review, thanks!

Addressed all your MR threads, and the note in comment #15. However, that comment got me looking closely at core/lib/Drupal/Core/Updater and made me open a new MR thread for feedback. See above. Thoughts?

Added a release note to the summary.
Draft CR at https://www.drupal.org/node/3461934 →

Back to 'Needs review', but I suspect we're going to want to make more changes in core/lib/Drupal/Core/Updater/* before this is RTBC again. Awaiting direction from core committers before I push any commits. See https://git.drupalcode.org/project/drupal/-/merge_requests/8781#note_342201

Thanks!
-Derek

1. Cleaned out all the dead code in core/lib/Drupal/Core/Updater and added the deprecations discussed in the MR thread.
2. Did some more grepping of core and removed a few lingering stale docs references to installing new extensions.
3. Did a fresh rebase to the latest 11.x branch code (commit 45bcde63).

Latest pipeline (226079) is all green once more. I think this is RTBC again, but that's for someone else to say. 😅

Thanks,
-Derek

Before seeing the MR, I was assuming this would deprecate all the code and remove the routes, it looks like it's doing a mixture of code removal and deprecation as well as removing the routes. That's probably fine but does complicate backport a bit.

I think we could backport just the route removal to 11.0.0 without any of the code removal or deprecations. It does diverge from the rc policy a bit, but if someone really is using this, it's easier to explain removing the pages in a major release than a minor.

The chances of either people or code actually using this functionality seems incredibly small at this point, and even if they were, it wouldn't affect runtime operation of a site anyway, so not too worried about stretching things to make it easier for automatic updates and project browser (including a 10.4.0 backport if we decide we want to do that).

I worked on a new branch with the minimal changes described by @catch while on a flight. Just landed, but it’s gonna be a little while before I can get my laptop online and open a backport MR. assigning to myself so no one else bothers to duplicate the effort. Stay tuned. Thanks!

My ride is late, so I had a moment to deal with this while still at the airport. 😅

https://git.drupalcode.org/project/drupal/-/merge_requests/8810 is now open for 11.0.x and 10.*.x branches.

@catch, is that what you had in mind? Happy to make any further changes as needed.

Thanks!
-Derek

I think it makes sense to deprecate all 3 forms as well

You mean for the backport MR? No, @catch explicitly said no new deprecations for the backport.

If you mean the primary MR, I don’t think it makes sense to deprecate. We should just remove all this crap per @xjm.

But thanks for reviewing!
-Derek

P.s. I just pinged the parent meta about progress here. Probably worth reconsidering the backports here in light of that discussion.

Also, how would we deprecate a form and routes, if we did want to do that? 😅

@dww in some cases (e.g. where it turns out contrib is extending a controller or event listener or something), we've removed route definitions, service tags or similar, but left the controller class in core, with the usual @deprecated/trigger_error() in there.

Thanks again @dww. The 11.x MR looks great and ready to go to me.

Some notes on the backport MR:

1. As per #26 let's add a trigger_error deprecation to the UpdateManagerInstall form.
2. Let's also backport the help text and comment changes from system.module and module.api.php to keep them in sync.
3. I think we should also add a deprecation notice to update_authorize_run_install() just in case - deprecated in 10.4 for removal in 11.1 I guess.

As route deprecation is not ready it looks like removal is only valid option 📌 Support route aliasing (Symfony 5.4) and allow deprecating the route name Needs work

Moreover it will need follow-ups to update docs (help topics are not affected) and https://www.drupal.org/docs/user_guide/en/extend-chapter.html →

Thanks, y'all! Pushed commits to address the review of the backport MR in #27:

1. Added deprecation to core/modules/update/src/Form/UpdateManagerInstall.php (both a @deprecated in the class comment itself, and a trigger_error() in the constructor.
2. Cherry picked 2 commits from the 11.x MR into the 11.0.x MR to address point 2 about keeping docs in sync.
3. Added deprecation to update_authorize_run_install() (and then fixed the syntax error I introduced 😅)

Should we backport the deprecations in core/lib/Drupal/Core/Updater/*, too? And change them to being deprecated in 10.4.0? Or leave all that alone?

Re: docs edits in #28: those might be better handled via the parent meta, no? There's already a lot of talk of docs that need updating in there. I guess we could open a specific issue to handle the docs only around this install UI as a child of this one. I defer to the release managers...

Anything else before this is ready?

Thanks again,
-Derek

p.s. I updated the CR about the new deprecations in 10.4.x → .

p.p.s. Added a link to the CR into the draft release note. Hope that's cool. Feel free to edit / remove if needed.

p.p.p.s. Crediting @andypost and @catch for reviews.

I only reviewed the 11.0.x/10.4.x MR properly so far, but that looks right to me.

In case it was missed in the middle of #29:

Should we backport the deprecations in core/lib/Drupal/Core/Updater/*, too? And change them to being deprecated in 10.4.0? Or leave all that alone?

@dww I think those are OK to leave as deprecated in 11.1 for removal in 12, we are removing the internals as well in 11.1 as we don't need to support that any more - but I doubt anyone else is calling inside the updater code in 10.4 and needs notifying now so not worth backporting down to 10.4.

As far as I can see nothing else is left to do here, so marking RTBC.

Committed/pushed to 11.x.

I also committed the backport MR to 11.0.x and backported to 10.4.x

This is slightly stretching the backport rules a bit, but should make it easier for project browser and automatic updates to adapt to the change if only 10.3.0 has these routes in it. Given this only prevents installing new modules, it should have minimum if any impact on existing sites.

Good to make progress on this one!

catch → closed merge request !8781

catch → closed merge request !8810

Brilliant work getting this in. Agree it's stretching the rules a little, but I'm super happy that it's getting into 11.0 and 10.4!

Updating the release note a bit. As a reminder, "change record" is not accessible link text. Please make sure release notes have accessible link texts.

Also improved the CR a bit.

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