Erroneous message that Drupal core needs to be updated
| Project: | CVS deploy |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | dww |
| Status: | duplicate |
I suspect I might just be totally missing something, but here's my problem: CVS Deploy makes Update Status work worse, not better, for my site!
I'm using CVS Deploy 5.x-1.1 and Update Status 5.x-2.2. Attached is a screenshot... should I ever be seeing anything like this?
The first problem is that it is telling me I need to update to Drupal 5.7 even though I am running a version of Drupal 5.8-dev that is more recent. The second problem (less important, but perhaps related) is that the filedate is not correct... it says my Drupal 5.8-dev version is from February 20, but actually I just updated Drupal via CVS today (February 27).
Another piece of info that might help is that I tried going back and installing CVS Deploy 5.x-1.0, and in that case, the problem is different -- it can't find a filedate at all for Drupal core (i.e., it behaves just like CVS Deploy isn't installed). So it seems like some change was introduced in the latest version of CVS Deploy...
I'll try to look at the code later to see what's going on, but no time at the moment ;) Thanks for any help!
| Attachment | Size |
|---|---|
| Screenshot.png | 21.15 KB |
#1
The problem is that the core version string in system.module (5.8-dev) is confusing since there's no release on drupal.org that corresponds to that. In fact, my local copy of cvs_deploy has a quick hack to recognize such version strings and convert them back to "5.x-dev" for the purpose of comparing release with update_status. It seems ugly and evil, but I guess it's better than what we have now. :( Part of the problem is that on admin/build/modules it'd be nice to keep the 5.8-dev since that's more information than just 5.x-dev, but then things are inconsistent between there and admin/logs/updates. :( That's why I haven't yet committed this, but I'll post it here for you to a) confirm it solves your problem and b) solicit input from others on how to best handle this. Also, in 6.x this wouldn't work the same way -- we'd have to always convert to 5.x-dev (even for admin/build/modules) and then you're losing some information if you go this route. That's the other big reason I haven't committed this.
Fundamentally, I just hate -dev releases and trying to support them. ;) Use official releases and everything works great. Use -dev and get what you deserve -- something very rough around the edges. ;)
#2
Thanks for the quick turnaround! Unfortunately, it doesn't solve the problem, though... I applied the patch and checked for new updates, and there was no improvement.
So I looked into the code a bit, and I found the following. This is from update_status_get_projects(), right after the hook in CVS Deploy gets called:
<?phpif (!isset($projects[$info['project']])) {
// Only process this if we haven't done this project, since a single
// project can have multiple modules.
$projects[$info['project']] = array(
'name' => $info['project'],
'info' => $info,
'datestamp' => isset($info['datestamp']) ? $info['datestamp'] : 0,
'modules' => array($file->name => $info['name']),
);
}
else {
$projects[$info['project']]['modules'][$file->name] = $info['name'];
}
?>
So basically, it's setting the project datestamp based on the first module it finds. In my case, that would be the Block module, which is where the February 20 date came from (that was the last time the Block module was changed in CVS, I guess). So even though other modules have more recent timestamps which are being correctly returned by CVS Deploy, they aren't getting used, and Update Status thinks that Drupal is out of date.
What is the right way to fix this? The attached patch works fine for me (note that it's for Update Status, not CVS Deploy). But I'll be interested to hear your opinion... looking through the code, I'm truly amazed at all the work that must have gone into figuring out those gigantic blocks of logic! So it's very possible that my patch would break something in a different case ;)
#3
Actually, there are still cases where even my patch won't work. What if the latest CVS update is to the Search module, for example (which I don't have enabled on my site)? I think in that case it will always tell me my Drupal installation is out of date, no matter what I do.
So... you're probably right that some kind of ugly workaround is additionally necessary ;)
Would the following be too simplistic? If I'm running 5.8-anything and the latest stable version is 5.7, then would it be possible to just automatically say that my site is up to date (simply because 8 is greater than 7)? I'm not sure if that's the right thing to do in all situations, but it makes sense for me.
#4
Bah. :( There's code like that in D6 core's update.module (from #209242: update.module reports bogus results when .info files are newer than the cached available update data) but I guess I never included that logic in my backport to update_status (#208687: Update_status claims 5.6 is insecure; wants 5.5). :( Furthermore, it's only doing that max logic for the info_file_ctime field, not the datestamp itself. So, we should probably include something very much like your patch in #2. However, that's really a separate bug from the problem my patch in #1 is solving, and these should probably just be split out into different issues.
However, I still maintain the root of the problem is trying to use -dev and expecting any of this to work at all. ;)
#5
Actually, there are three separate issues ;) But maybe it's best to leave them together until a general strategy is worked out:
Issue 1: The bug dealt with by your patch in #1. Strangely, I don't seem to be experiencing this problem at all, even though based on your description I should be. Once I apply my patch from #2, everything seems to work OK even though my version string is still 5.8-dev.
Issue 2: The problem where Update Status incorrectly sets the project datestamp based on the first module it finds, rather than looking at all of the modules in the project. From what you're saying, it sounds like this affects Drupal 6 core too. So it should be fixed independently of whatever is best for CVS sites. In that case, I'm not so sure my patch in #2 is the right way to go... maybe it depends on how much stupidity you want to protect against inside the Update module? ;) Specifically, suppose someone downloads the new secure version of Drupal (6.1), but they have some custom code added to a couple core modules from Drupal 6.0, so they decide to keep the 6.0 versions of those modules around. Oops! In order to protect against that, I think you would want to do the exact opposite of my patch in #2 (i.e., set the project datestamp to the oldest datestamp it finds among all the component modules), right?
Issue 3: The original problem I identified, which is partially fixed by the patch in #2, but not totally. It seems like the ideal way to fix this is for CVS Deploy to examine all the modules in an individual project (whether enabled or disabled), and return the most recent datestamp -- since if you're running a dev version, you want to compare the most recent file you've updated via CVS to the datestamp of the dev version from drupal.org. This may not be easy to code, though.
I guess I see why you think supporting dev releases is a pain ;) A suggestion, though: If you can't fully support them, perhaps you should put a note to that effect on the CVS Deploy project page?? I would think that a fair number of people who run their sites off CVS do so in order to be able to easily get updated dev versions. I wouldn't dream of doing this for most contributed modules, but I do tend to run the dev release of Drupal core on live sites, since it only gets bugfixes and it's nice to be able to get those right away. This wasn't my idea: It's actually recommended in the "Pro Drupal Development" book, so I suspect I'm not the only one who does this ;)
#6
Yup. ;)
Re: Issue 1: that's probably because it just sees the -dev part either way, knows it's a dev release, and tries to behave accordingly. I guess that makes my patch above useless. ;)
Re: Issue 2: #166333: Visually display a warning if there are version mismatches within a single project
Re: Issue 3: Inspecting disabled modules is sort of a can of worms: #162788: Include modules that aren't enabled
Sorry, I have no time to work on this any more before DrupalCon, and probably not for a little while after that, either.
Meanwhile, yeah, don't expect miracles from -dev. ;) Cursed "Pro Drupal Development" (just kidding)...
#7
Oops, I guess I should have done a little more searching before posting my last comment. So I guess the path to fixing the bug involves first doing #166333: Visually display a warning if there are version mismatches within a single project, then #162788: Include modules that aren't enabled, then coming back here.
I'm willing to spend a little time working on this, although I'm not sure I'm going to have time to do it particularly soon either... But I'm interested in an excuse to learn more about the Update Status code anyway (especially since I just posted this Drupal 7 feature request that I'd like to work on: #228275: Make updating Drupal as easy as installing it). I'll put any followup posts on the above issues and will only post back here when (and if) they're fixed.
Thanks for all your help! I'll be at DrupalCon, so perhaps I will meet you in person there.
#8
Adding to "My Issues" (or my tracker, really) for later review. (A.k.a. subscribing.) (Also, in case it's not mentioned, I'm seeing this problem on Drupal 6.x with 6.x-1.0 of the CVS deploy module.)
#9
The problem still exists in the current version.
#10
I moved issue 2 to its own issue: 499828.
#11
We have separate issues for everything that's a problem so I'm marking this as a duplicate.
#499828: Wrong release date on multi-module projects
#166333: Visually display a warning if there are version mismatches within a single project
#162788: Include modules that aren't enabled