Problem/Motivation

The menu_update_7001() function that was added to the Drupal 7.6 release updates the primary and secondary menu names to their Drupal 7 equivalents but doesn't update existing menu blocks that referenced the old menu names. This causes the following error when upgrading a Drupal 6 site to Drupal 7.6+:

* Notice: Undefined index: primary-links in menu_block_view() (line 462 of /var/www/drupal7/modules/menu/menu.module).
* Notice: Undefined index: secondary-links in menu_block_view() (line 462 of /var/www/drupal7/modules/menu/menu.module).

To reproduce:

  1. Upgrade Drupal 6 site to Drupal 7.6+
  2. View administration pages

BUG: See error messages above.

Proposed resolution

  • Update the menu block deltas to reference the Drupal 7 equivalent menu name and.
  • Add tests for the Drupal 6 to Drupal 7 upgrade path.

Workaround:
Manually update the delta field in the blocks table changing 'primary-links' to 'main-menu' and 'secondary-links' to 'secondary-menu'.

Remaining tasks

Reviews needed.

Original report by Alyx Vance

Rename "Primary Links" and "Secondary Links" to their Drupal 7 equivalents.
if menues already exist in the DB, because you have modified manualy in version 7.0 - 7.4 then you get
PDOException: in menu_update_7001() (Zeile 133 von /var/www/web909/html/japan/modules/menu/menu.install).

if you get this exception then do delete in menu_custom table the entries with name main-menu and secondary-menu and be sure thats exist the entries primary-links and secondary-links.
rename in the table menu_links all main-menu and secondary-menu entries to primary-links and secondary-links.
then run update.php again and Drupal should make update correct.

after this you can create new menus in the DB or leave it and change your menue blocks or else

maybe its exist an another way to solve this but i have no idea where

greetings

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

marcingy’s picture

Status: Active » Postponed (maintainer needs more info)

This was not an update added by 7.6 release so I don't understand how you got into the state you describe.

parasolx’s picture

i got this problem after update to 7.6. my site is upgrading from 6.22. this error keep appear in Recent log.

Notice: Undefined index: secondary-links in menu_block_view() (line 462 of /home/***/public_html/modules/menu/menu.module).

then i follow what Alyx said, then the error still appear but it undefined index for secondary-menu.
Notice: Undefined index: secondary-menu in menu_block_view() (line 462 of /home/***/public_html/modules/menu/menu.module).

parasolx’s picture

I solved my problem. First I revert back what i have done (renaming) from secondary-menu to secondary-links.

It comes from {block} table. Which still have previous delta name of secondary-links.

Then what I do is by deleting all rows that contains "secondary-links" under delta column. After that no more error come out.

marcingy’s picture

Version: 7.6 » 7.x-dev
Status: Postponed (maintainer needs more info) » Active
Issue tags: +6x upgrade to 7x

Thanks for the extra information that makes sense now so the issue is related to upgrading from drupal 6 to drupal 7.

marcingy’s picture

Tagging correctly

Frank Ralf’s picture

Just adding some evidence ;-)

I updated today from 7.4 to 7.7 and my former primary links (from a previous 6 installation) disappeared with the following error message:

Notice: Undefined index: primary-links in menu_block_view() (line 462 of [...]\modules\menu\menu.module).

This only happens with the modified Basic 7.x-2.0-rc3 theme I use. Clearing the caches and rebuilding the menus didn't help. The unmodified Basic theme doesn't throw the error, however the menu items created with Taxonomy Menu are missing.

Will try the remedies from this thread.

See #1007910: D6->D7 update doesn't convert 'primary-links' menu to 'main-menu' for the update from 6 to 7 issue.

EDIT
@OP: My "menu_custom" table doesn't contain any entries for primary-links or secondary-links (anymore?).

@#3: Renaming "primary-links" "main-menu" in the "block" table shows the menu again but the error message remains. Deleting the entry lets the error message disappear but also the menu.

Rob C’s picture

Guys, when i do a clean install of drupal 7.7.: (on the configure site page)

* Notice: Undefined index: name in block_menu() (line 146 of modules/block/block.module).
* Notice: Undefined index: name in block_menu() (line 165 of modules/block/block.module).
* Notice: Undefined index: name in system_menu() (line 640 of modules/system/system.module).
* Notice: Undefined index: name in block_menu() (line 146 of modules/block/block.module).
* Notice: Undefined index: name in block_menu() (line 165 of modules/block/block.module).
* Notice: Undefined index: name in system_menu() (line 640 of modules/system/system.module).

When i run the updater on a 7.4 to 7.7 site, same deal as above post.

Kinda rename the issue or file a new issue? Sure it's related.

Rob C’s picture

Hmmm, shall i file a new issue? (continuing the installer lead to an...)

Drupal 7.7 The website encountered an unexpected error. Please try again later. after install.

The error i get when installing 7.7 (CLEAN!)

Error
The website encountered an unexpected error. Please try again later.

Kinda renders the whole thing not useable. Kinda Major. No menu's rendered, kinda not working state. Admin pages are also not working.

Frank Ralf’s picture

I would suggest filing a new issue as your problem seems to affect not only the menu system and looks more severe.

sun’s picture

Title: Update menu 7001 » menu_update_7001() breaks when updating from 7.4 or upgrading from D6
Issue tags: +Upgrade path
Rob C’s picture

Nevermind, i got it. (my mistake, when you have a custom frontpage template, beware people)

xmacinfo’s picture

I have not made any updates from 7.4 to 7.7 yet. I will keep an eye on this issue.

parasolx’s picture

what i found that during updating or upgrading 7000 and 7001, there is missing for renaming menu's in {block} table. like what happen to me.

plus, if the site has disable themes that previously activated primary and secondary menu, the error will show since in "delta" it was not rename.

so what i do is just renaming towards the error, and deleted the inactive themes (even i can rename it also, but because i never used the theme anymore -- bartik).

ju1i3’s picture

I just updated to 7.7 (from 7.4) and suddenly this error message appears:

Notice: Undefined index: primary-links in menu_block_view() (line 462 of /home/gardenwi/public_html/modules/menu/menu.module).

I thought my menus/primary links were ok before and I didn't need to make any changes others have mentioned related to them. I'm using basic Bartik theme.

(www.gardenwithindoors.org.uk)

kalmarr’s picture

Kisugi Ai’s picture

@Kalmarr

look at the block table
you have some entrys with Delta primary-links or secondary-links
delete this entrys because you don't need it any more

@marcingy
it's right thats came not with 7.6 i have seen it first time with 7.5-dev
and i have first solved this problem with the release 7.6 because it's was goes up to my live page
and i have changed only this what's i have told in the bug report
i have had no others problems

saparker’s picture

@Alyx Vance
Thanks for the tips. I deleted the primary-links entries in my block table using PhPMyAdmin and it did the trick.
I also needed to move my Main Menu back to "Suckerfish menu" in Blocks, as this had become disabled during the update to 7.7

Kisugi Ai’s picture

yap because you lose the primary and secondary menus

drupal do find the menus in the block table but can't find the menus in the menus tables, and drupal will disable the blocks and give a warning that no menus are there.

you could change the block table and change the delta to main-menu and secondary-menu, it's an another solution but deleting are better because the new menu entrys at the end of table

lyricnz’s picture

Assigned: Unassigned » lyricnz

Okay, this is a real issue - something that I didn't consider in the final patch in #1007910: D6->D7 update doesn't convert 'primary-links' menu to 'main-menu'. Looking now.

lyricnz’s picture

Here's a patch to the menu upgrade test, that enables both the primary-links and secondary-links blocks before the upgrade. It currently fails with several "Undefined index: primary-links" etc errors (as expected).

lyricnz’s picture

Status: Active » Needs review

Testbot please

Status: Needs review » Needs work

The last submitted patch, menu-update-primary-links-blocks-test-1231856.patch, failed testing.

stevieb’s picture

subscribe

elachlan’s picture

Subscribe

lyricnz’s picture

Update tests to check that existing primary-links/secondary-links menu blocks are still visible after the upgrade.

lyricnz’s picture

Status: Needs work » Needs review
lyricnz’s picture

And another patch, same tests as #25, but with an update that performs the upgrade.

lyricnz’s picture

This time, with the patch :/

catch’s picture

Priority: Normal » Major

Subscribing.

lyricnz’s picture

Instead of selecting 'bid' from the list of blocks, and then not using it - patch should probably use COUNT()

catch’s picture

That makes sense, then it'd be possible to skip the foreach().

lyricnz’s picture

Actually, here's a better approach that makes use of the existing function for renaming block modules/deltas. It also takes care of block-roles, and per-user permissions. I just had to extend the existing function to support deleting existing blocks, when a rename would land on top of it.

100 passes, 0 fails, 0 exceptions, and 31 debug messages

sun’s picture

Status: Needs review » Needs work
+++ b/includes/update.inc
@@ -355,6 +355,10 @@ function update_fix_d7_block_deltas(&$sandbox, $renamed_deltas, $moved_deltas) {
+            db_delete($table)
...
             db_update($table)

@@ -372,6 +376,10 @@ function update_fix_d7_block_deltas(&$sandbox, $renamed_deltas, $moved_deltas) {
+            db_delete($table)
...
             db_update($table)

1) Why don't we use db_merge() here?

2) Whatever we do, this cries for inline comments, explaining the WTF.

3) The general rule of thumb is to not touch existing updates. I wonder whether we can omit the update.inc changes?

+++ b/modules/menu/menu.install
@@ -161,6 +163,23 @@ function menu_update_7001() {
+function menu_update_7002(&$sandbox) {
...
+  update_fix_d7_block_deltas($sandbox, $renamed_deltas, $moved_deltas);

I'm not sure whether the function in update.inc is supposed to be "abused" by module updates. Don't think so.

+++ b/modules/menu/menu.install
@@ -161,6 +163,23 @@ function menu_update_7001() {
+  );  ¶
...
+  );  ¶

Trailing white-space.

+++ b/modules/menu/menu.install
@@ -161,6 +163,23 @@ function menu_update_7001() {
+}
+/**

Missing newline.

24 days to next Drupal core point release.

lyricnz’s picture

Status: Needs work » Needs review
FileSize
4.52 KB

1) Isn't db_merge() an insert/update? This is an update, preceeded by a delete if required.
2) Added
3) The bit we're changing in update.inc is a utility function - update_fix_d7_block_deltas(). It's actually called from system_update_7004() already, with a heap of blocks to be moved as arguments. We're just calling it again to move some different blocks.

Removed extra whitespace.

How's this?

Status: Needs review » Needs work

The last submitted patch, menu-update-primary-links-blocks-1231856.patch, failed testing.

lyricnz’s picture

Status: Needs work » Needs review

That test failure is unrelated to this change - probably testbot. Works fine here.

lyricnz’s picture

Tripplefix’s picture

FileSize
816 bytes

My patching failed (but I'm a newbie to testing/patching). First I had to type in every file name that needed to be patched (ok - that was easy). But I cannot deal with this error:

Hunk #1 FAILED at 74
1 out of 1 hunk FAILED -- saving rejects to file modules/simpletest/tests/upgrade/upgrade.menu.test.rej

Find the upgrade.menu.test.rej (but renamed to upgrade.menu.test.rej.txt) attachted.

lyricnz’s picture

#38 : On a mac/*nix machine:

lyricnz:drupal7 simonroberts$ curl -s http://drupal.org/files/issues/menu-update-primary-links-blocks-1231856_1.patch | patch -p1
patching file includes/update.inc
patching file modules/menu/menu.install
patching file modules/simpletest/tests/upgrade/drupal-6.menu.database.php
patching file modules/simpletest/tests/upgrade/upgrade.menu.test
lyricnz:drupal7 simonroberts$ 

On windows, download the patch first, then: patch -p1 < menu-update-primary-links-blocks-1231856_1.patch

xjm’s picture

Tagging.

xmacinfo’s picture

@Tripplefix: For each project there is a Version control tab which gives you instructions to clone, create and apply a patch as well as to revert.

Here are the instructions for core:

https://drupal.org/project/drupal/git-instructions

Tripplefix’s picture

@xmacinfo: Thank you for the link to the instructions - didn't know about it yet, and it is indeed good to know about that.

At my local Drupal User Group, I could ask people this night and learned that this "error" looks only very bad - but is not really bad. Thanks to the Drupal guys from the #dughh.

xmacinfo’s picture

this "error" looks only very bad - but is not really bad.

Nevertheless how good or bad an error is, it must still be corrected.

Cheers!

EggPlant’s picture

Hi- I just upgraded from 6.22 to 7.7 and I'm also getting the below error on every page:
"Notice: Undefined index: primary-links in menu_block_view() (line 462 of /*/htdocs/modules/menu/menu.module).."

Do I understand the "fix" correctly? I download/install the patch above (menu-update-primary-links-blocks-1231856.patch), then run update.php?

Thanks

lyricnz’s picture

#46 Yes! Please let us know how it goes. Use the patch from #34

garrettrayj’s picture

The patch from #34 worked fine for me.

catch’s picture

Status: Needs review » Needs work
+function menu_update_7002(&$sandbox) {

This needs to go in a 7.x-extras defgroup.

Also no need for $sandbox parameter.

Is there any risk here for sites upgrading from say 7.2 to 7.8? We don't have upgrade path testing for that yet and I'm losing track of all the possibilities here.

lyricnz’s picture

@catch:

1) The reason for the $sandbox argument is that we call update_fix_d7_block_deltas() which uses it to allow batching of user-specific block visibility settings.

2) The code is already in ( "@defgroup updates-7.x-extra Extra updates for 7.x" just like menu_update_7000() and menu_update_7001(). Is that what you meant?

3) I don't see a specific issue for a multi-step upgrade, it should work like any other hook_update(). #48 reported it worked on a 7.7 update.

catch’s picture

Status: Needs work » Needs review

#1 and #2 is just me being an idiot.

#3 /looks/ fine, but I can't tell from #48 what was upgraded to what.

lyricnz’s picture

Manually tested upgrade from D6 to D7+patch. Instructions/screenshots attached. Will rerun with older D7 first, then upgrade, to satisfy #51

lyricnz’s picture

lyricnz’s picture


lyricnz:drupal-7.7 simonroberts$ curl -s http://drupal.org/files/issues/menu-update-primary-links-blocks-1231856_1.patch | patch -p1
patching file includes/update.inc
patching file modules/menu/menu.install
patching file modules/simpletest/tests/upgrade/drupal-6.menu.database.php
patching file modules/simpletest/tests/upgrade/upgrade.menu.test
lyricnz:drupal-7.7 simonroberts$ drush updatedb
The following updates are pending:

menu module : 
  7002 -   Rename the primarysecondary menu blocks to match previously renamed menus. 

Do you wish to run all pending updates? (y/n): y
Finished performing updates.                                                                                                               [ok]
lyricnz:drupal-7.7 simonroberts$ 
catch’s picture

Status: Needs review » Reviewed & tested by the community

So we have a nice patch. Tests for the bit that can be tested, and lyricnz has run manual tests for the scenarios that can't and posted evidence.

Nothing left to do here.

Kisugi Ai’s picture

okay i have used / teste
http://drupal.org/files/issues/menu-update-primary-links-blocks-1231856_...
git apply menu-update-primary-links-blocks-1231856_1.patch
Status report tolt me DB out of date
then update.php
told me
2 pendings
menu module
•7001 - Rename "Primary Links" and "Secondary Links" to their Drupal 7 equivalents.
•7002 - Rename the primarysecondary menu blocks to match previously renamed menus.
after apply
Failed: PDOException: in menu_update_7001() (line 135 of /drupal7/modules/menu/menu.install).
so breaks still on my old db

reason:
i have already the rows main-menu and secondary-menue in the meue_custom table
but why?
ah i remember
it was a block after an update from 6.20 to 7 in the blocks list. or similar http://drupal.org/node/1113510. okay now i looked for the linklist but i couldn't edit this link list
it was en error there page not found i looked in the tables and
i found a link entry called home in the menu_links table with the menu_name main-menu so i have added this menu in the menu_custom.
now if is the menu entry existend at update 7001 then the update.php breaks.

i have deleted the lines in the menu_custom and the patch was working.

now we have one more things to catch
i am so sorry thats i have found out this just now

but why should rename this menus? could we not let this menus existent an add the both others only? just an iddea.

lyricnz’s picture

If update.php said that you had TWO updates pending, then you are not applying this patch to the correct version of Drupal (or you had not run update.php previously).

If you hack around with the database, I'm afraid you're on your own.

The menus were renamed to produce a consistent user-behaviour between versions. *This issue* is around renaming/moving the menu-blocks to follow the change in the menus.

Frank Ralf’s picture

Patch from #34 applied successfully. I only had to add the main-menu of my modified Basic theme manually to its region (again) but that might have been due to mine fiddling around to solve this issue. Many thanks for the effort!

Kisugi Ai’s picture

@lyricnz
hm, i have usually run every time update.php.
but i don't know when whats going wrong.
i will look where whats go wrong, i have still an old 6.22 live table.
and then i tell you again.

deajan’s picture

Did not work for me.
After an upgrade from D6.22 to D7.7, i got "Notice: Undefined index: secondary-links in menu_block_view() (ligne 462..."

After applying patch #34, i ran the update script and got this:

Failed: PDOException : SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicata du champ 'garland-menu-secondary-menu' pour la clef 2: UPDATE {block} SET delta=:db_update_placeholder_0 WHERE (module = :db_condition_placeholder_0) AND (delta = :db_condition_placeholder_1) ; Array ( [:db_update_placeholder_0] => secondary-menu [:db_condition_placeholder_0] => menu [:db_condition_placeholder_1] => secondary-links ) dans update_fix_d7_block_deltas() (ligne 362 dans /home/web/mysite.tld/ftp/www/includes/update.inc).

Anyway i still get the error.
Any advices ? Thanks.

xmacinfo’s picture

Status: Reviewed & tested by the community » Needs work

@deajan: Can you switch you environment to English before posting an error? Ce sera plus facile pour les autres de lire le message d'erreur. :-)

mgifford’s picture

Version: 7.7 » 7.x-dev

I pulled in the patch in #34. It worked for primary menus. There's a problem in the wording as a space is missing here:

7002 - Rename the primarysecondary menu blocks to match previously renamed menus.

My secondary menus are still not showing up, but I'm going to have to look into that a bit further

EDIT: I'm still not finding it. I looked into the SQL as per http://drupal.org/node/1232998#comment-4863542 & ran through the code of the menu.module in 7.7 but still not finding it.

I'm actually looking for a child menu of the primary menu. I've established a menu hierarchy, but with these changes, only the top level is being displayed for me.

EDIT2: If I've got the main-menu working but I want to print out a child menu I should be able to see the array using -> print_r(menu_navigation_links('main-menu', 1));

I can't. It's producing a blank array when there are child menus.

marcingy’s picture

The comment is correct the problem is because of a slash being used which the gui strips out - see #957700: Forward slashes in update comments are removed in update process

rfay’s picture

#34 did resolve the error message for me.

netguistics’s picture

Version: 7.x-dev » 7.7

For those of us running under IIS what is the best way to handle this issue caused by the upgrade from 7.x to 7.7???

rfay’s picture

Version: 7.x-dev » 7.7

@netguistics, did you try the patch in #34 ?

lyricnz’s picture

#60 if you got that error, then it appears that you may not have applied the patch correctly? With Drupal 7.7, and the patch, line 362 of update.inc is actually the delete (before the update):

lyricnz:htdocs simonroberts$ tar xfz ~/Downloads/drupal-7.7.tar.gz 
lyricnz:htdocs simonroberts$ cd drupal-7.7/
lyricnz:drupal-7.7 simonroberts$ curl -s http://drupal.org/files/issues/menu-update-primary-links-blocks-1231856_1.patch | patch -p1
patching file includes/update.inc
patching file modules/menu/menu.install
patching file modules/simpletest/tests/upgrade/drupal-6.menu.database.php
patching file modules/simpletest/tests/upgrade/upgrade.menu.test
lyricnz:drupal-7.7 simonroberts$ awk '{print FNR " " $0}' < includes/update.inc | grep -C5 ^362
357           if ($block_exists) {
358             // Delete any existing blocks with the new module+delta.
359             db_delete($table)
360               ->condition('module', $module)
361               ->condition('delta', $new_delta)
362               ->execute();
363             // Rename the old block to the new module+delta.
364             db_update($table)
365               ->fields(array('delta' => $new_delta))
366               ->condition('module', $module)
367               ->condition('delta', $old_delta)
lyricnz:drupal-7.7 simonroberts$ 

How did you apply the patch? Can you check your update.inc, and ensure it looks like this?

lyricnz’s picture

#62 it doesn't sound like your issues/questions are related to this specific issue (primary/secondary menus in blocks causing warnings after upgrading from Drupal 6 to 7)?? Suggest raise other issues if they are indeed unrelated.

catch’s picture

Version: 7.7 » 7.x-dev
Status: Needs work » Needs review

@mgifford there's another major in the queue about primary/secondary links from the same menu not working, that's unrelated to this. I'm putting this back to needs review.

mgifford’s picture

@catch I was looking here, but had trouble finding something that I thought was a close fit:

http://drupal.org/project/issues/search/drupal?text=&assigned=&submitted...

It's always trickier when there are two unrelated bugs that appear together. Can you post the link to the specific instance? Any chance of it getting fixed for 7.8?

catch’s picture

jantoine’s picture

#34 resolves the error for me.

jantoine’s picture

Issue summary updated. Not sure if I should remove the 'Issue summary initiative' tag? Feedback appreciated.

xmacinfo’s picture

Status: Needs review » Reviewed & tested by the community

@AntoineSolutions: Can you tell us which version you are upgrading from?

Marking RTBC since at least two people reported #34 fixed their issues.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Wow, GREAT work on this, lyricnz.

Committed and pushed to 7.x.

Tripplefix’s picture

#34 works for me as well - after I learned how to patch correctly (thanks to lyricnz and xmacinfo). I was upgrading from 7.4

EggPlant’s picture

I've attempted to use the patch in #34 but I guess I don't know enough about the patch process (after reading several postings). I tried using Cygwin for the first time but I still got errors when trying to run the patch on my local files. Does anybody know if the fix in #34 will be included in the next Drupal update? If so, I may just wait for that release. Thanks

jantoine’s picture

@EggPlant,

Yes, this patch has been committed and will be included in the next Drupal release.

Cheers,

Antoine

EggPlant’s picture

Great. Thanks a lot.

ju1i3’s picture

I tried changing the Block table in the DB, primary links to main-menu. Now the error message says main-menu instead of primary links!

Notice: Undefined index: main-menu in menu_block_view() (line 462 of /home/gardenwi/public_html/modules/menu/menu.module).

lyricnz’s picture

If you change the DB yourself, you're on your own! This issue/patch would have resolved it for you.

ju1i3’s picture

It was listed as a workaround.

lyricnz’s picture

Just because someone posted it as a suggestion by some random dude, doesn't mean that Core can be expected to work properly afterwards. Perhaps if you revert your change (ie: put it back to how it was), and rerun menu_update_7002() it will come right. It's hard to be sure, as we don't know exactly what you changed.

Edit: I don't mean to be rude, but please... if you hack the database, you really need to know what you're doing.

parasolx’s picture

hacking database is not Drupal way. The method as discuss at first is to find the way how to solve the problem.
currently you need to look at the issues status.. it already state as "fixed".

meaning it already commit into the -dev version until most of issues solve in a period time, there will be a new release version.

as lyricnz said, you need to revert the change you have done into database, then run update. everything will goes perfectly unless you experience other problem than menu.

tamsoftware’s picture

(phew) thanks ! Any idea when this is ?

For the record, I'm seeing the issue appear going from 7.4 to 7.7 on a site created on 6.x, upgraded to 7.0 and then upgraded to 7.2 and 7.4.
It uses a Main Menu on the right side with Taxonomy Menus.

Good news is that it's not yet finished so not online, I can wait a bit.

Thanks to all who worked on the patch

Cheers

Franck

lyricnz’s picture

#85 : The fix was applied in #75 , so on August 16, 2011. It will be included in the next Drupal release (7.8), whenever that is released. Or you can apply the patch from #34 in the meantime.

xjm’s picture

Technically it will probably be in 7.9 (released the same day as 7.8) since it is not a security fix. :)

marcingy’s picture

Status: Fixed » Needs review
FileSize
1.14 KB

Currently the upgrade path is broken if the block module is not enabled as the menu update throws

Base table or view not found for the block table.

I realise this is not a common set up as we are using mongo block rather than the core block module.

The issue was hidden in the tests as uninstallModulesExcept enable block by default.

Patch attached.

lyricnz’s picture

Status: Needs review » Needs work

FYI {block} is created during any normal install, even "minimal" profile. Disabling block module doesn't delete the table, only uninstalling the module completely will do this.

The patch uses module_exists('block') to check whether the update should be performed, which returns whether the block module is currently enabled - leaving the possibility for a "missed update" when this update is applied while block module is disabled (but not uninstalled). Not sure on Drupal's policies regarding updating tables for disabled modules - there is only one use of module_exists in modules/*/*.install, but several uses of db_table_exists()

Suggest that checking directly for the existence of the table(s) might be better. Watch out for old vs. new table names (see update_fix_d7_block_deltas for example).

marcingy’s picture

I realise that it is installed in the minimal profile but as it isn't a required module it can be disabled and then uninstalled. As I say it is an edge case but we can't assume the block module will exist. I'll work on a re-roll with db_table_exists instead and thanks for the pointer.

marcingy’s picture

Status: Needs work » Needs review
FileSize
1.23 KB

Re-roll with db_table_exists instead of module_exists.

lyricnz’s picture

Yup, that looks OK to me. Any chance of a test-case?

marcingy’s picture

The issue is that the block module was required in d6. It is a unique case because of unique site in many ways and can only have this issue because it started life as a d7 and there are no other tests for a d7 -> d7 path.

I'm sure a test could be created but it doesn't appear to be the norm.

catch’s picture

Status: Needs review » Reviewed & tested by the community

There's no 7-7 upgrade path testing, that depends on 7-8 upgrade path testing which don't have yet either.

I'm not sure block module being disabled/uninstalled is all that much of an edge case, you could presumably do that if you were using panels too.

I was wondering why we have to check for multiple block tables, but update_fix_d7_block_deltas() has similar checks.

So RTBC.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

#1164852: Inconsistencies in field language handling has made me totally paranoid to edit any existing update functions, but in this case I don't think we have a choice. And agreed that disabling block module is definitely not outside the realm of possibilities in D7.

Committed and pushed to 7.x. Thanks.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

cpelham’s picture

Status: Closed (fixed) » Active

I just upgraded from 7.5 to 7.8 and update.php failed with this error:

The following updates returned messages

menu module

Update #7001
Failed: PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry 'main-menu' for key 'PRIMARY': UPDATE {menu_custom} SET menu_name=:db_update_placeholder_0, title=:db_update_placeholder_1 WHERE (menu_name = :db_condition_placeholder_0) ; Array ( [:db_update_placeholder_0] => main-menu [:db_update_placeholder_1] => Main menu [:db_condition_placeholder_0] => primary-links ) in menu_update_7001() (line 135 of /home/cpelham/public_html/crsny.org/public/modules/menu/menu.install).

This is the 2nd time in a row that my site has been screwed by a half-baked point release update of Drupal core. Is this what we should expect from now on? I wish to God you would be proactive and start sending out alerts to everyone with a registered Drupal install warning them not to upgrade as long as their are such bugs...instead of leaving us to find them for ourselves... :)

cpelham’s picture

And by the way my site began as a clean 7.0 site. It was never upgraded from 6. And Blocks were/are turned on...

tim.plunkett’s picture

Status: Active » Closed (fixed)

7.8 doesn't have this fix in it, 7.9 or 7.10 will.

cpelham’s picture

Status: Closed (fixed) » Active

So, what do we do in the mean time? The title of the patch in #91 suggests it is for cases where block is not turned on. I do have block turned on. A solution rather than a "Wait 'til next year" would be highly appreciated given that all I did was you know install the latest official version of Drupal. It really shouldn't be broken out of the box... There's not greater marketing screwup than that... I say this as a Drupal lover and everyday user.

marcingy’s picture

Status: Active » Postponed (maintainer needs more info)

I think there is some misunderstanding the original fix went in and then a bug was found when the block module was not enabled which will appear in the next point release of drupal 7. Would it be possible for you to attach a copy of your menu_custom and block tables before applying the update to help us understand how your issue may be being caused.

Plus do you have any contrib modules installed that could be causing a problem, and preventing the core update from working correctly?

cpelham’s picture

Attached are my menu_custom and block tables from when my site was at Drupal 7.5 prior to applying the 7.8 update. I have quite a lot of contrib modules but not sure which if any might interfere. Do you have any particular suspicions?

marcingy’s picture

Cool thanks - looking at your menu table it looks like i18n is might be the root cause of your problem as the menus in question are status that have been set to a status other than the default of 0 by i18n.

Or you have manually added a menu with a machine name of primary-links as d7 by default only installs

$descriptions = array(
    'navigation' => $t('The <em>Navigation</em> menu contains links intended for site visitors. Links are added to the <em>Navigation</em> menu automatically by some modules.'),
    'user-menu' => $t("The <em>User</em> menu contains links related to the user's account, as well as the 'Log out' link."),
    'management' => $t('The <em>Management</em> menu contains links for administrative tasks.'),
    'main-menu' => $t('The <em>Main</em> menu is used on many sites to show the major sections of the site, often in a top navigation bar.'),
  );
lyricnz’s picture

Your DB tables indicate that the site was indeed upgraded from Drupal 6 in the past. Drupal 7 does not provide "primary-links" blocks, yet your 'block' table includes these. The error message pasted above confirms the exact line where it failed (because "primary-links" block exists).

cpelham’s picture

@lyricnz I can see why you would say that it was an upgraded site, but it was not. However, I did at one point make an attempt to build menus in a fresh D7 site to mirror as much as possible a D6 site. I decided that was less treacherous than actually upgrading the site from D6 to D7. Now it is not important to the site at all that there be any menu with the name "primary-links" so can I just go in and re-name that and try running the update.php again?

@marcyngy Yes, our menus are multi-lingual. Is that an issue? I realize that i18n is a contrib project but it is one of the most important to the Drupal project. Do you suggest I set them their status to 0 for a minute, run the update, and then set them back?

cpelham’s picture

Status: Postponed (maintainer needs more info) » Closed (fixed)

OK, I simply deleted that Primary Links menu and renamed the Secondary Links menu and then the update.php ran without errors. So, perhaps there should be a warning in the update.txt or somewhere saying not to use those names or something? Anyway, I'll admit that this really is a fringe case and an easily resolved one (once one knows what to do).

Albert Volkman’s picture

I'd just like to add that if you're using Context, you'll need to update the menu references there as well.

Albert Volkman’s picture

Issue summary: View changes

Update issue summary following issue summary standards.