Problem/Motivation
Moduels with the contact module enabled may fail on updating to D7. contact_update_7001() and contact_update_7002()require the {role_permission}
table to be already updated, but this requirement is currently not enforced. The dependency chain is:
contact_update_7002() =>
contact_update_7001() =>
system_update_7007() =>
system_update_7000() =>
user_update_7008() =>
user_update_7006()
Proposed resolution
Patch in #11 adds these dependencies to contact.module
.
Remaining tasks
None.
User interface changes
None.
API changes
None.
Original report by @spongecat
Every time I try to update, it fails on #7007 and #7002. Sometimes in just hangs on last two updates. I'm getting really different results, starting every time from exactly the same point. I tested every single build since beta was released. I had one success with dev build from Oct. 10 on local MAMP. Any other attempt with any version, on linux or wamp just fails. Below are error messages.
The following updates returned messages
system module
Update #7007
Failed: PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '4-administer recommender' for key 'PRIMARY': INSERT INTO {role_permission} (rid, permission) VALUES <...snip...>) in system_update_7007() (line 1883 of C:\wamp\www\modules\system\system.install).
contact module
Update #7002
Failed: PDOException: SQLSTATE[42S22]: Column not found: 1054 Unknown column 'module' in 'where clause': SELECT 1 AS expression FROM {role_permission} role_permission WHERE ( (rid = :db_condition_placeholder_0) AND (permission = :db_condition_placeholder_1) AND (module = :db_condition_placeholder_2) ) FOR UPDATE; Array ( [:db_condition_placeholder_0] => 2 [:db_condition_placeholder_1] => access user contact forms [:db_condition_placeholder_2] => contact ) in contact_update_7002() (line 128 of C:\wamp\www\modules\contact\contact.install).
Comment | File | Size | Author |
---|---|---|---|
#11 | 939562-maybe.patch | 826 bytes | bfroehle |
#3 | 939562-contact-update-dependencies.patch | 828 bytes | Stevel |
Comments
Comment #1
catchMoving this to user module. Can you still reproduce?
Comment #2
graytoby CreditAttribution: graytoby commentedNot on beta 2. I'll set up test environment and I'll try latest dev again. As a side note, this only happened on linux and win systems, I had no problems on MAMP.
Comment #3
Stevel CreditAttribution: Stevel commentedThe error in system_update_7007() looks like a corrupt permission table in the drupal 6 install, where a permission appears twice for the same role. Would be easy to fix (by remembering what rid/permission combinations are already processed), but this seems like a case of garbage-in/garbage-out, so not sure it should be fixed.
contact_update_7002() error looks like a unusual but probably correct (according to current hook_update_dependencies) update ordering.
Added a patch for the contact update error.
Comment #4
Dave ReidCorrect component...
And this needs to depend on user_update_7006() not system_update_7007(). If anything user_update_7007() should depend on system_update_7007().
Comment #7
Starminder CreditAttribution: Starminder commentedHelp....I am stuck exactly here, not sure how to move on get the rest of the updates to complete...
Comment #8
Starminder CreditAttribution: Starminder commentedStill stuck here, no idea how to work with the patch file. Would love to get past the errors above. Thanks!
Comment #9
catchThis could go straight into D7 - there's no new update or anything, just the dependency. I haven't verified, but it looks like sites with contact module enabled may not be able to update to D7 at all, if that's the case I agree it's critical.
Instructions to deal with patch files are here http://drupal.org/patch/apply
Also in this case you can just copy the code into contact.install and remove the + from the beginning of each line, but it's worth taking the time to figure out how to apply a patch.
Comment #10
Starminder CreditAttribution: Starminder commentedthanks Catch, I agree i need to learn to do patches and will as soon as i get some of these fires put out.
I slammed the changes into the file (I think in the right place). When I ran update this one didn't come up anymore, but do have a message about some updates no going due to dependency issues.
Remaining core updates that aren't going in are:
taxonomy module
7005 - Migrate {taxonomy_term_node} table to field storage. @todo: This function can possibly be made much faster by wrapping a transaction around all the inserts.
7006 - Add {taxonomy_term_data}.format column.
7007 - Add index on {taxonomy_term_data}.name column to speed up taxonomy_get_term_by_name().
7008 - Change the weight columns to normal int.
7009 - Change {taxonomy_term_data}.format into varchar.
7010 - Change {taxonomy_index}.created to support signed int.
If I can find a way past the above I'll be in much better shape. I posted under taxonomy but no replies yet. Any ideas?
Comment #11
bfroehle CreditAttribution: bfroehle commentedFrom #73874-57: Normalize permissions table, as to why system_update_7007() is in system.install, not user.install.
This isn't true, AFAIK, but oh well.
This is not possible, as system_update_7000() already depends on user_update_7008()! (See user_update_dependencies()).
Note that contact_update_7002() uses the {role_permission} table which finally gets populated in system_update_7007.
But the patch in #3 is STILL not right! As contact_update_7001 also requires the {role_permission} table to be properly populated!
I've added the following requirements:
* contact_update_7001() depends on system_update_7007()
* contact_update_7002() depends on user_update_7006()
(The second requirement here is vacuous, as we already have the dependency chain
contact_update_7002() => contact_update_7001() => system_update_7007() => system_update_7000() => user_update_7008() => user_update_7006() --- but for the sake of transparency I'm including it).
Comment #12
JoshOrndorff CreditAttribution: JoshOrndorff commentedThis post is mostly to subscribe, but I thought I would also mention an issue that I opened over at #1133024: System Update #7018 Fails on Upgrade. That issue is mostly about a system update that failed, but as you can see I get stuck on contact_update_7002 as well. I'm not sure if that helps sort this out or not, but I figured it wouldn't hurt.
It sounds like if I disable and uninstall the contact module, upgrade to D7, and then re-enable the contact module, I can get around this issue. Is that correct?
Comment #13
catch#11 looks good to me.
Either we need someone to test it manually and report back, or we need to find a way to reproduce the bug with simpletest and write a test for it.
I think manual testing and report back is enough for this since the update doesn't run at all without the patch - as opposed to it being a data loss issue or similar where it can be harder to detect regressions, would be good if some of the people reporting this issue could try the patch - see http://drupal.org/patch/apply for instructions if you've not used patches before.
Comment #14
carlos8f CreditAttribution: carlos8f commentedsubscribing
Comment #15
stefanwray CreditAttribution: stefanwray commentedI had the same errors as spongecat - the originator of this thread. Failed on System Update #7007 and failed on Contact Update #7002.
I just applied the patch in #11.
This patch eliminated the Contact Update #7002 failure.
But I'm still getting:
Comment #16
tlangston CreditAttribution: tlangston commentedsubscribe
Comment #17
steinmb CreditAttribution: steinmb commented@stefanwray did you start the migration all over again? To me it seems you are trying to continue from where it failed just with the patch installed, and that does normally not work.
Comment #18
stefanwray CreditAttribution: stefanwray commented@steinmb yes, i went back to the backed up version of the database and tried to re-install D7 again and got that same error
Comment #19
catch@ stefanwray: you have over 6200 rows being inserted by this update function, that seems like a lot of permissions to me. Could you dump the copy of your Drupal 6 {permission} table and upload the contents here?
Comment #20
kreynen CreditAttribution: kreynen commentedThe system #7007 error was caused by duplicate permission strings in the D6 permission table in @stefanwray's case. The cause of the dups in D6 isn't exactly clear, but I wrote this PHP to help him find and remove the dups before the upgrade. Might be helpful if anyone else runs into this in the future. If you find '=> 2' or '=> 3' in the output, you can manually remove one the strings from the perms field of the permission table.
Comment #21
EvanDonovan CreditAttribution: EvanDonovan commentedShouldn't the patch at #11 still be at "needs review"? Why bumped down to "needs work"?
Comment #22
tlangston CreditAttribution: tlangston commentedThanks for the script but after running and finding no duplicates there...ran update and still failing for this error.
Comment #23
David_Rothstein CreditAttribution: David_Rothstein commentedThis issue is filed against the Contact module so let's focus it on that (which the patch in #11 already does).
Anything that touches system module update dependencies is part of a larger issue which we're working on at #717834: The dependencies declared in core's hook_update_dependencies() implementations aren't actually correct.
Anyway, #11 looks good to me, but it's hard to properly test without doing something about #717834: The dependencies declared in core's hook_update_dependencies() implementations aren't actually correct first, so maybe we should focus efforts there and then see if the Contact fix here still works.
Comment #25
xjmTagging issues not yet using summary template.
Comment #26
catch#11: 939562-maybe.patch queued for re-testing.
Comment #27
catchStill passes with the other patch in and the dependencies here are straightforward. I ran a 6-7 upgrade on a site with contact enabled but couldn't reproduce this. The worst this should do is enforce an execution order that already happens by accident, so rtbc.
Comment #27.0
xjmUpdated issue summary.
Comment #28
xjmSummary added.
Comment #29
catch#11: 939562-maybe.patch queued for re-testing.
Comment #30
webchickSounds good, let's try this. Committed and pushed to 7.x. Thanks!
Comment #31.0
(not verified) CreditAttribution: commentedUpdated issue summary.