For multipass operations, the batch API provides the notion of 'sandbox', which is a safer way of storing persistent data than $_SESSION.

Current HEAD contains 2 multipass updates, both of which still use the 'old' $_SESSION system :
system_update_6021 (Migrate the menu items from the old menu system to the new {menu_links} table)
book_update_6000 (Migrate the existing book hierarchy into the new {book} / {menu_links} structure)

Attached patch changes them to use the new batch API $sandbox. Not critical, obviously (using $_SESSION is less problematic during updates since sensible admins probably avoid browsing their site while an update is running in another tab...), but would be good for educational purposes - eat your own dogfood, that kind of thing...

Side note :
For 'classic' batch operations, the use of $context['sandbox'] is documented in the PHPdoc for batch_set(). The way this translates to multipass update functions (by-reference $sandbox argument), though, is only documented in batch_example.module : http://cvs.drupal.org/viewvc.py/drupal/contributions/docs/developer/exam...
I couldn't find any relevant place in core doc.

CommentFileSizeAuthor
#2 multipass_update_0.patch13.43 KByched
#1 multipass_update.patch13.42 KByched
multipass_update.patch13.17 KByched

Comments

yched’s picture

yched commented
StatusFileSize
new multipass_update.patch13.42 KB

Oops, previous patch missed one replacement.

yched’s picture

yched commented
StatusFileSize
new multipass_update_0.patch13.43 KB

Rerolled

pwolanin’s picture

pwolanin commented

subscribe

pwolanin’s picture

pwolanin commented

I see some minor bug fixes too in the book update (0 vs. FALSE)

samhassell’s picture

samhassell commented

subscribe - this is causing quite a few people to lose their book.module structures when running Drush updates.

http://drupal.org/node/467770
http://drupal.org/node/667298

aren cambre’s picture

aren cambre commented

Subscribing also because of Drush updates issue.

aren cambre’s picture

aren cambre commented
Priority: Normal » Critical

I think this should be critical if we want to consider Drush a serious part of Drupal. This code is causing Drush to mess up navigation, and it is critical for any web content manager to not screw up navigation.

asb’s picture

asb commented

Coming from #467770: Book structure disappears repeatedly...

wtf... last patch is three years old? The patch from #2 is against HEAD from December 19, 2007 - what's the status of this isse?

iss’s picture

iss commented

subscribing due to drush update

yched’s picture

yched commented

"wtf... last patch is three years old?"

2 years old, actually.
It was posted when polishing D5 -> D6 upgrade path, at the end of the D6 dev cycle. Back then, it was only a cosmetic change converting updates functions that were written before batch API landed in D6, and let them use the new batch API syntax for persistency in multipass updates ($sandbox), instead of the 'old' D5 way (based on $_SESSION).

Drush has landed since then, and what was only 'old-style but working' code is now broken with drush, since CLI doesn't have sessions.

If people have come here these last few days and reported that the patch fixed their D5->D6 drush upgrade, then I guess it still applies and is ready. Needs someone to push the RTBC button. Not me, I wrote the patch :-).

samhassell’s picture

samhassell commented
Status: Needs review » Needs work

hrmm.

against drupal-6.15 cvs the system.module patches are failing:

patching file modules/system/system.install
Hunk #1 FAILED at 2201.
Hunk #2 FAILED at 2250.
Hunk #3 FAILED at 2265.
Hunk #4 succeeded at 1780 (offset -502 lines).
Hunk #5 succeeded at 1815 with fuzz 2 (offset -496 lines).
Hunk #6 FAILED at 1828.
Hunk #7 succeeded at 1936 (offset -433 lines).
4 out of 7 hunks FAILED -- saving rejects to file modules/system/system.install.rej
patching file modules/book/book.install

So I removed them and the book patches run fine.

Unfortunately 'drush up' still wipes the book structure :(

samhassell’s picture

samhassell commented

I removed the book_update_6000() function altogether and drush up still kills the menu structure so I guess we are barking up the wrong tree here, at least to some degree.

aren cambre’s picture

aren cambre commented

So it's time to unset #667298: drush update destroys menu_links table entries for book as duplicate?

seanberto’s picture

seanberto commented

subscribing

quentinsf’s picture

quentinsf commented

subscribing

Magnity’s picture

Magnity commented

Subscribe

gilf’s picture

gilf commented

Subscribe

joel_guesclin’s picture

joel_guesclin commented

I do not think this is to do with drush - I have same problem after ordinary upgrade

aren cambre’s picture

aren cambre commented

Joel: you're right, it doesn't. Drush has independently fixed its own bug.

joel_guesclin’s picture

joel_guesclin commented

Could it be anything to do to do with this?

bjorpe’s picture

bjorpe commented

Subscribing - book upgrade problems is a serious showstopper when moving from D5 to D6

edit: is this really solving the problem with book update that people are experiencing? From the description in the original post it seems unlikely to me.

nancydru’s picture

nancydru
she/her/hers
Primary language English
Location Boston
commented

I upgraded books from 5 to 6, but I believe it was before the menu changes in 6.2.

dpearcefl’s picture

dpearcefl commented
Status: Needs work » Postponed (maintainer needs more info)

Is this issue fixed in the latest D6? is there any interest in pursuing this issue?

dpearcefl’s picture

dpearcefl commented
Status: Postponed (maintainer needs more info) » Needs work

Status: Needs work » Closed (outdated)

Automatically closed because Drupal 6 is no longer supported. If the issue verifiably applies to later versions, please reopen with details and update the version.