This was filed as comment 48 on #949102: Polish UPGRADE.txt by jenyum, and as per comment #50 by EvanDonovan, I'm filing it as a separate issue.
jenyum:
Not sure if this is an installation bug, or a documentation issue, but I followed the instructions in Upgrade.txt to upgrade from 6.x to Drupal 7, on a fairly bare site with an Acquia distribution. The site was automatically taken online, where I had previously set it to offline for maintenance mode. (Also ended up logged out, so had to set update.php access to TRUE) If this is automatic upgrade.txt should indicate it, from the way it is written it sounds like you should expect to have to take the site back online again when you are done. (Which is the process from Drupal 6.) If this is expected behavior I have to say I wasn't crazy about it, since the site had all contributed modules turned off and was in no way ready for traffic. If it is expected, it should be documented.
EvanDonovan:
I think open a new issue against the install system component. Then folks like DamZ, David Rothstein, catch, chx, etc. who are familiar with the upgrade path can confirm, and if it is the case, and if we can't fix it before rc1, then move the issue back to documentation component.
| Comment | File | Size | Author |
|---|---|---|---|
| #22 | 984358_site_offline_21.diff | 2.14 KB | David_Rothstein |
| #18 | 984358_site_offline_18.diff | 2.15 KB | dalin |
| #12 | 984358_site_offline_12.diff | 2.15 KB | dalin |
| #10 | 984358_site_offline_10.diff | 2.05 KB | dalin |
| #8 | 984358_site_offline_8.diff | 854 bytes | dalin |
Comments
Comment #1
jenyum commentedsubscribing. Thanks for making this an issue.
Comment #2
EvanDonovan commentedSubscribing, thanks jhodgdon!
Comment #3
effulgentsia commentedComment #4
David_Rothstein commentedHm, this looks like an ugly bug.
Apparently, the variable which controls the maintenance mode was renamed from 'site_offline' in Drupal 6 to 'maintenance_mode' in Drupal 7, but no one wrote any kind of upgrade path for it?
So basically, it looks like we need to (a) write an upgrade path for this variable, (b) separately make sure the correct thing happens for both sites updating from Drupal 6 as well as sites updating from a Drupal 7 beta release (the latter probably just need the old variable deleted at this point, nothing else), and then, (c) somehow make it all happen early enough so that it works with the code that is responsible for automatically putting your site in maintenance mode before the updates and taking it out of maintenance mode afterwards.
The code for (c) is in these functions (see what it does with $_SESSION['maintenance_mode']):
http://api.drupal.org/api/drupal/includes--update.inc/function/update_ba...
http://api.drupal.org/api/drupal/includes--update.inc/function/update_fi...
Sounds like fun :)
Comment #5
EvanDonovan commented@David_Rothstein: Do you think this should be critical (rc blocker), since it affects the upgrade path? I guess probably not, since it could theoretically be documented as a "gotcha" and there is no data loss. The fact that there needs to be a beta-to-rc upgrade path too is a bummer.
Is this technically an API change due to the ugliness that it will cause in update_batch and update_finished?
Rather than writing an upgrade path for the variable could update_batch just check on variable_get('site_offline', FALSE) at the beginning as well?
Comment #6
David_Rothstein commentedExactly. This is bad, but it doesn't prevent the update from actually working, or leave your site fundamentally broken afterwards. It shouldn't be a RC blocker, more in the "would be very useful to fix before release category", IMO.
Comment #7
jenyum commentedI agree, it really wasn't the biggest of hurdles. However, if it has been verified that it always happens it should be documented in the upgrade documentation until it is fixed.
Comment #8
dalinEasier to just fix the bug rather than creating temporary documentation.
Comment #9
David_Rothstein commentedWorth testing, but I'm not sure I see how that fixes the bug... hasn't the session variable (and therefore 'maintenance_mode' itself) already been set by the time this runs?
Comment #10
dalinAh, I see. Here's another shot. I manually tested upgrading a D6 site and the site remained offline when I was done.
Comment #11
David_Rothstein commentedHm, that little trick looks at first glance like it might work.... interesting.
Probably the variable_del() should happen inside a new update function, though (like in the earlier patch). If a site updated from Drupal 6 to Drupal 7 RC1 and is now updating again after this patch has been committed, we still want to clear the old variable for them.
Comment #12
dalinAh that makes sense. Here we go.
Comment #13
jhodgdonThe orginal report on this issue also says
Comment #14
David_Rothstein commentedThat part is a duplicate of #945112: Unable to stay logged in during upgrade from D6 -> D7 - and @dalin has a patch there too :)
The patch in #12 looks good to me. I haven't tested it so I don't want to mark it RTBC just yet, but will try to test later. One minor issue is that the code comment is wrong - it refers to 'system_update_7089' but it actually should be 'system_update_7069'.
Comment #15
jhodgdonExcellent, thanks.
Comment #16
jpw1116 commentedI noticed a related “feature,” new to 7.0-rc1:
In the past, within every D7 alpha and beta release, I was able to remain in forced Maintenance Mode after updating a module, e.g., Views.
Now I receive this:
Formerly, the Maintenance Mode state remained the same—namely, offline. The new way may be more logical but is a problem otherwise.
Comment #17
jhodgdonThat's not a related "feature", it's this bug exactly. The above patch should fix it. Have you tested the patch, or would you be willing to?
Comment #18
dalinGood catch David. I fixed the typo.
Comment #19
jpw1116 commentedThank you for clarifying, Jennifer. I have never even had 6.x, so I was hesitant to brand my 7.0-rc1 module updating as the same animal.
I haven't had a chance to test out any fix but plan to within 24 hours.
Comment #20
dalinIf you've never had 6.x then your issue is unrelated.
Comment #21
David_Rothstein commentedI did some final thinking about this patch in the following scenarios (and physically tested #1 and #3), and it all seems to work.
drupal_get_installed_schema_version('system') < 7000check, which is why). So all that happens is that the obsolete variable gets deleted, which is what we want.I therefore think this is RTBC but it sounds like there may be another person or two who wanted to test this which is good; we can hopefully mark it RTBC soon.
I rerolled to fix another minor issue (the PHPDoc for the update function referred to the wrong variable name).
Comment #22
David_Rothstein commentedEr, and here is the reroll...
Comment #23
David_Rothstein commentedRelated but minor issue I found while testing this: #987930: The obsolete 'site_offline_message' variable is not always deleted during the D6->D7 upgrade. (The patch will probably conflict with this one, so I didn't write one yet, but it's simple.)
@jpw1116, it sounds like the issue you're seeing might actually be this one: #976328: Update manager should not take you out of maintenance mode unless you asked it to
Comment #24
jpw1116 commentedI wondered about that, David. You're right. Only I was attempting merely to update a module, not install one.
What is the next step in escalating the issue so that upgrading, installing and updating all get covered in the next release?
-John
Comment #25
jhodgdonjpw1116: If you want to make sure that this patch gets added to Drupal before the release, the most helpful thing would probably be to test it, and report back with (a) what you did to test it and (b) what the results were.
You might also go to #976328: Update manager should not take you out of maintenance mode unless you asked it to and comment there with more details, if you think the issue you were seeing more resembles that one.
Comment #26
David_Rothstein commentedI think I'll mark this RTBC. (As described in #21, I reviewed it and tested it relatively thoroughly.)
Comment #27
webchickWow, that's quite the belly-dancing. :) Luckily it's all included in update_fix_d7_requirements() which will be the very first thing to be blown away in D8.
I looked at this awhile for a way it could bust D6 sites that hit update.php and then decide to get cold feet and not execute the update, but it seems sound.
Committed to HEAD. Thanks!