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.
Going to try my hand at this. We'll see how far I get. :)
Porting Progress (follow along at http://drupalcode.org/project/upgrade_status.git/shortlog/refs/heads/7.x...)
- upgrade_status.compare.inc
upgrade_status.css- upgrade_status.fetch.inc
- upgrade_status.info
upgrade_status.js- upgrade_status.module
- upgrade_status.report.inc
Recommended plan of attack:
Move the custom upgrade_status_moved_into_core() function into the .module or a separate file + commit.Copy the current/latest .compare.inc + .fetch.inc + .report.inc files into upgrade_status + commit.Rename the functions from "update_status_" to "upgrade_status_" + commit.Fork the code in update_status_install() to create a queue.Update the function names being called in upgrade_status.module.Remove the $site_key from _build_fetch_url() + replace the core major version.- From there, simply try to run, use, and fix it in a trial & fail fashion ;)
Comment | File | Size | Author |
---|---|---|---|
#20 | Screen Shot 2014-01-12 at 10.19.08 PM.png | 87.12 KB | webchick |
#8 | update-6.x-update-7.x.diff | 162.79 KB | webchick |
#4 | update-6.x-upgrade-6.x.diff | 96.12 KB | webchick |
Comments
Comment #1
webchickHm. This is harder than I anticipated because I don't quite follow the logic of what functionality in Update Status module this overrides and why, and the D7 code in some places looks quite a bit different (queues and whatnot). I will now attempt to have a conversation with myself to see if that helps. ;)
First, let's start by comparing Update Status 6.x with Upgrade Status 6.x, as well as Update Status 7.x with Update Status 6.x.
Comment #2
webchickSorry, that first one was a stupid diff. :)
Comment #3
webchickWhen reading that diff, the "+" lines are important, since they're in Upgrade Status module. The "-" signs represent stuff that's only in Update Status module. Upgrade Status uses a couple of conventions, which are nice:
Comments that begin this way are places where the upstream code has been modified for Upgrade Status, along with why we're doing that.
Lines that begin with a # sign are the same, but commented-out lines that are not relevant to Upgrade Status module.
Let's start with...
update.compare.inc
Here, we're leaving all of the logic about actually getting the project data alone (i.e. update_get_projects(), _update_process_info_list(), update_get_project_name(), update_process_project_info(), update_project_cache(), and update_filter_project_info() are left alone), because this logic is the same regardless of what results are actually shown in the table.
Where we start overriding things is in upgrade_status_calculate_project_data() (formerly, update_calculate_project_data()). This makes a few logic modifications that are relevant, namely:
Comment #4
webchickMargh. One more try at that bloody diff. :P
Comment #5
webchickupdate.fetch.inc
Here we fork a number of functions:
Additionally, there's what looks like some handy debug code, as well as a timeout workaround we can apparently remove in D7.
Things we leave alone in this file are:
So basically, what I take away from this is anything required to make _update_refresh() work in D7 needs to be forked into this module during the port.
Comment #6
webchickupdate.report.inc
All parts of this file get forked in some way. Namely:
- Different stuff in the header, including a core version form, which is defined later on in upgrade_status_core_version_form() and upgrade_status_core_version_form_submit().
- Makes "Drupal" the first project in the list; hides Upgrade Status.
- Remove logic about notifications.
- Add some extra divs to support Upgrade Status-specific "compact layout" styling. Mainly this involves making the table rows shrunk down and expandable by default.
- Adds Upgrade Status's status constants to the appropriate table row styles: UPGRADE_STATUS_STABLE / UPGRADE_STATUS_CORE (ok / green), UPGRADE_STATUS_DEVELOPMENT (warning / yellow), UPGRADE_STATUS_CORE with a note of some kind (warning / yellow), unknown (red / not ported).
- If it can't find an upgraded version of any kind, it uses the current project's data so at least something can be printed in the table.
- Pull in Upgrade Status's CSS/JS instead of Update Status's, and rename classes accordingly.
Comment #7
webchickAnd finally...
update.module
Ok. That helps. I think I might have a shot at this now.
Comment #8
webchickNow a re-upload of the 6.x -> 7.x Update Status diff, since Git has made me rusty at passing command line ops to diff, so I forgot -p. :P~
Comment #9
webchickNow some notes in reviewing the 6.x -> 7.x Update Status diff. These are not complete, just some things that stood out.
- In general, string concatenation rule changes " . " vs. ". " and some coding standard fixes add a bunch of annoying noise to this diff.
- In general, there's also a lot of added code around support for downloading new/updated modules and themes, all of which is irrelevant to Upgrade Status.
- time() is now REQUEST_TIME.
- All theme function arguments are now array indexes in "variables" (not "arguments")
- update_calculate_project_data() is now split into two functions: update_calculate_project_data() and update_calculate_project_update_status().
- There are some additions to update.css we might need to take into account.
- Update refreshing is now done via batch job, which sends fetch tasks to a queue. This is a big change. I'm not sure how much of that I'll need to fork yet, but possibly all of it. If so, I'll need the code from update.install that establishes such a queue.
- Version is still not passed into _update_build_fetch_url() so we'll need to continue to fork that.
- That icky custom XML parsing class is removed in favour of SimpleXML. Long live PHP 5!
- There's some kind of crazy code in hook_init() which i don't think we'll need.
- There's now support for tracking which sub-modules/sub-themes are enabled/disabled. Handy.
- Release labels (Revoked, Insecure, etc.) got pulled out into a theme_update_status_label(). We'll want to do something similar for Upgrade Status.
- There's now a toggle to check for disabled modules in core. Nice! This will make creating a contrib dashboard more straight-forward. :) For 6.x -> 7.x I needed Update Status Advanced Settings module to do that.
Comment #10
webchickHiding irrelevant crap.
Comment #11
webchickAdding a checklist of sorts into the issue summary. :)
Comment #12
webchickComment #13
sunThat is a fairly accurate summary — I'm very glad to see that the special and controlled technique of explicitly leaving comments on all changes in the forked files allowed you to find and decipher all manipulations. :)
In retrospective though, I did not use the best method for tracking manipulations: VCS file history. I.e., it would have been even more simple for you if there had been a first clean commit of "Copying compare.inc, fetch.inc, report.inc from Update Status module." + all renames/adjustments + all hacks in separate commits.
When I developed upgrade_status, I did not file upstream issues that would allow us to simplify it in the future (because the new major version of Drupal core is stable/API-frozen already, and the code of the next version might look completely different…) — nevertheless, I hope that we can eliminate at least some of the changes in the D7 version.
[For D8, let's hope that the majority of update_status code is converted into OO, since that would allow upgrade_status to simply extend the core classes and only override the necessary methods :)]
Given that you reverse-engineered the changes already, I'd recommend to move forward like this:
Comment #14
webchickAwesome, thanks for the guidance!
Agreed splitting up the commits would be good to do this time. In almost all of the places I had questions, git blame led me to a single by sun: Revamped entire module and synced with Update module commit, which wasn't able to answer them. Your proposed plan of attack sounds good.
For D8, there's #1987888: Convert update_manual_status() to a new style controller (which is more like "refactor the entire Update Status module to be OO" than what it says on the tin) which I'm probably at least semi-qualified to review now ;) and that would hopefully allow for what you're suggesting. If I'm feeling extra plucky, I might even try a D8 port after that patch gets committed, but let's see how the D7 port goes first. :P
Comment #15
webchickAdding recommended plan of attack to the issue summary.
Comment #16
webchickUpgrade Status upgrade update. ;)
Comment #17
webchickAnd another one!
Baby's awake, so the rest of this is going to have to wait. :)
Next steps:
- Get the core version selector appearing on the report.
- Go through both the remainder of report.inc and compare.inc and update "update" to "upgrade_status" where appropriate.
- Make the remainder of the "// US" hacks in all files.
- Figure out how to get the newfangeldy CSS/JS working. (I might punt on this, since I throw out the UI and re-create it anyway in https://drupal.org/sandbox/webchick/1223824.)
- Possibly add tests? I'll take a gander at how Update Manager's tests are done and if they could be applicable here.
Comment #18
webchickOk, now done with the basic port, with the exception that I kicked the compact layout stuff to a separate issue at #2172127: Port compact layout styling to D7 since my CSS/JS skills are not up to snuff. :\
Now it's time for everyone's favourite!
;)
Comment #19
webchickBtw, that should not be read in any way that the module is working in 7.x. ;) Just that all the modifications are there (or I think so anyway). In order to get the module actually working, I'm starting with #2172189: Add automated tests for Upgrade Status module.
Comment #20
webchickWoohoo! :) It's working! :D Definitely not as pretty as the 6.x version, but the data is all there!
I'm sure there are bugs that will be found, and #2172239: Flesh out list of modules that were moved to core in 8.x definitely needs to be done, but I think I can officially mark this fixed and get a dev release rolled so others can test. :)
Comment #21
webchickFixing version now that there's a dev release for 7.x-1.x.
...and now I will stop spamming all inboxes everywhere. :) Thanks, this was really fun!
Comment #22
sunAWESOME! :-)
I didn't look at the code/commits yet, but it's really great to see that you made it work! :)
I'd be interested to work on #2172239: Flesh out list of modules that were moved to core in 8.x — to sorta continue my original blog post from D7 for D8 ;)