Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
On Available Updates we have download link directly to the tar.gz files. This
but this is not recommend way to update Drupal core or projects now, and hasn't been for a long time
We also have 2 links to individual release page. 1 link with the version number, "9.3.12" and one that say "Release notes"
On the release page there are Composer instructions and even for those who still wanted the tar.gz for some reason there links to that also.
furthermore there are also links to " file hashes" if you want the archives. So even in the case where want the archives it is better to go to the project page.
for module and theme pages we even have a warning
Downloads are for manual installation, which is not recommended when using Drupal 8 or later.
Proposed resolution
Remove the "Download" links as this just encourages people to use the tar.gz which is not recommend and could break you site because it does not take into account Composer dependencies.
Also even for people who still want the tar.gz the existing links to the project page provide them while also showing that Composer is the prefered way
Remaining tasks
User interface changes
Release notes snippet
The "Available updates" report (at /admin/reports/updates) no longer includes direct links to download .tar.gz release files. Sites should use Composer to manage releases and dependencies, not direct "tarball" downloads like these.
| Comment | File | Size | Author |
|---|---|---|---|
| webform_release_page.png | 51.82 KB | tedbow | |
| available_updates.png | 107.93 KB | tedbow | |
| project_page.png | 108.5 KB | tedbow |
Issue fork drupal-3279279
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:
changes
Comments
Comment #2
tedbowComment #3
tedbowA beta tester for the Automatic Updates module found this > #3277775: "Reports > Available updates" should link the automatic updates form, not the regular list of available updates
But even without Automatic Updates in core the download link should be removed
Comment #4
dww+1 from me. Those links are a relic of a bygone era.
Thanks,
-Derek
Comment #5
phenaproximaSelf-assigning to implement.
Comment #7
phenaproximaLet's see what breaks.
In the meantime, I'm setting this to "needs review" to validate the approach I took, which is very conservative: I just removed the download link from the templates, not from anywhere else, since comes from quite deep in the update system and it seems pointless to explicitly unset the variable in a preprocess function. (Although maybe it does make sense to explicitly deprecate it -- is there a procedure for that?)
Furthermore, for the purposes of this issue, it seems like scope creep to actually remove the download_link variable from fetched project data. (Maybe there should be a follow-up for that?)
Comment #8
dwwAll the tests that care about download links. 😅
Meanwhile, we shouldn't change stable or stable9 templates for this. Those are supposed to "never" change (except for exceptions for critical bugs, which this is not).
Otherwise, +1 to the basic approach. I agree with removing the links from the templates but leaving the variable processing in place. That's much less disruptive.
Thanks!
-Derek
Comment #9
phenaproximaHmmm, then does that mean people using those themes, or themes based on them, will see invalid download links??
Because that doesn't seem ideal. That seems actively bad to me, and contradicts the entire raison d'etre for this issue. What should we do?
Comment #10
dwwIt means themes built on top of stable get to consciously decide what fixes they want to make, but the "promise" that their base theme won't change out from under them is preserved. This identical point comes up over and over in trying to fix core bugs. By design, we're supposed to "leave stable broken", and sites opt-in to whatever fixes they want to make to their own templates. But we almost never change the stable templates for them.
The download links aren't "invalid", they're just not the recommended way to update your site. But if someone created a theme based on stable, that's up to them to decide, not us.
p.s. It's all a bit of a moot point, since the available updates report is from the admin theme, not front end, and it's even more rare that someone built an admin theme on top of stable. But still, the principles apply. If they did that, our promise is we don't break their templates out from under them. Even if "break" in this case is "fix a bug". I'd give you a list of 20 bug fixes I tried to make where I was advocating "how does leaving stable broken help anyone?", but I basically always lost such debates, and now I'm sticking with the "Thou Shalt Not Change Stable(tm)" line. 😅
Comment #11
dwwRegardless of stable, we still gotta update tests here. NW. 😉
Comment #12
phenaproximaComment #13
dwwInstead of pushing match over NW, I just pushed the "fix" I was asking for. 😉 @tedbow can RTBC this, so I don't mind getting my hands dirty.
Comment #14
dwwRandom fail in
Drupal\Tests\settings_tray\FunctionalJavascript\SettingsTrayBlockFormTest. Re-queued.Comment #15
phenaproximaComment #16
dwwDraft CR to alert folks using stable they might want to change things:
https://www.drupal.org/node/3279359
I don’t think we need a release note snippet for this.
Comment #17
dwwSorry, xpost
Comment #18
tedbowLooks good but I think the tests need updating because they still have logic around the download links existing
Comment #19
phenaproximaOkay, I addressed all of your feedback, @tedbow. There are now assertions that the download URLs don't appear anywhere in the response, in any form. Reassigning to you for review.
Comment #20
dwwThanks! Handful of real test fails again, mostly like this:
NW and unassigned for now. Maybe after breakfast I can work on this, but if Adam wants to before then, please do. 😉
Comment #21
phenaproximaBack to @tedbow for review.
Comment #22
phenaproximaComment #23
tedbowLooks good, thanks @phenaproxima and @dww!
Comment #24
dwwThanks! Not quite RTBC. Composing an MR review now.
Comment #25
phenaproximaReassigning to @dww for review.
Comment #26
dwwThanks! commit 6ffcc360 looks great. I guess my hands are dirty for RTBC, so over to Ted to re-RTBC.
Comment #27
tedbow@phenaproxima and @dww thanks for the work on this! @dww thanks for the very thorough review, based your comment I also searched
core/modules/update/tests/srcfor "download" and my findings matched what you found. Everything left looks correct.RTBC!
Comment #28
phenaproximaComment #29
tedbowI am not sure if this makes sense to backport to 9.x. It would be great to but if you we do I assume won't be able to backport the changes to the base test classes because of policy, correct? I think for 10.0.x it should be ok to change our base test classes. Though I think it is very very unlikely others are extending these base test classes.
On a related point I think it would be good if we could mark all the base test classes in the Update module specifically as @interal
Comment #30
dwwIn Slack, @xjm requested a release note snippet.
Also, the MR branch needed a rebase to apply to 10.0.x.
That rebase was a little funny, since my copy correctly removed the changes to stable earlier in the history. So there were some cherry picks that no longer applied or made any changes. I think it's all set, but a fresh review would be good before we call this RTBC.
Comment #31
tedbow@dww thanks getting this back on track.
I looked through all the MR comments and all of them remained fixed 🎉
Comment #35
xjmCommitted to 10.1.x, 10.0.x, and 9.5.x, and published the CR. Thanks!
Comment #36
xjmAlso linking the instructions for switching to Composer.