While Drupal 7 doesn't handle the data migration from CCK to Fields, it ought to handle the fact that CCK modules can be named the same as D7 ones.

Opening this after #1220602: Call to undefined function _update_7000_field_read_fields() during Forum update 7003, it was very tempting to re-open #934050: Change format into string since that issue introduced the upgrade failure even if it's only just been found now.

Summary of the issue:

1. An update was added for text module to handle the change in filter module.
2. That update can run on Drupal 7 sites before field, text or any other module has been enabled - because there may be an entry for 'text' disabled in the system.

Probable fix is to have text.module declare an update dependency on system_update_7020().

CommentFileSizeAuthor
#14 drupal.update-remove-d6-modules.14.patch2.41 KBsun
#12 drupal.text-update.12.patch10.78 KBsun
#9 drupal.text-update.6-disabled.tests-only.patch1.26 KBsun
#7 drupal.text-update.6.tests-only.patch1.26 KBsun
#6 drupal.text-update.6.patch2.32 KBsun
#3 how_many_bugs_can_one_patch_create.patch1.36 KBcatch
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Priority: Major » Critical

Potentially 240,000 sites unable to upgrade makes this critical.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Issue tags: +D7 upgrade path

Aaannnd tagging.

I don't intend to work on this, #934050: Change format into string should never have been committed when it was.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Active » Needs review
FileSize
how_many_bugs_can_one_patch_create.patch1.36 KB

Well, I said I wouldn't work on it, but rather than RC, the update was actually added during beta, where it makes no fucking sense to add an update to a module newly added during Drupal 7 - since HEAD to HEAD updates were not even supported at that point.

If a text_update_7001() gets added, it will also need to add a dependency on system_update_7020() or otherwise ensure it doesn't get run on CCK upgraded sites. But it might not need to do that depending on what it does, so there is no way to know.

But for now just zeroing out the update should be fine.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Title: Drupal 7 text module update (added during RC) runs on sites that have D6 CCK text module installed » Drupal 7 text module update (added during beta) runs on sites that have D6 CCK text module installed
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Needs review » Active

I mis-remembered, apparently we do support head to head updates from beta onwards.

So back to active.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Active » Needs review
FileSize
drupal.text-update.6.patch2.32 KB

Let's try this.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
FileSize
drupal.text-update.6.tests-only.patch1.26 KB

According to the OP, that test change should be sufficient to make the upgrade fail. So let's verify that assumption with a test-only patch (same as #6).

Status: Needs review » Needs work

The last submitted patch, drupal.text-update.6.tests-only.patch, failed testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Needs work » Needs review
FileSize
drupal.text-update.6-disabled.tests-only.patch1.26 KB

um, fatal error? 'cos it's enabled?

Status: Needs review » Needs work

The last submitted patch, drupal.text-update.6-disabled.tests-only.patch, failed testing.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented

It should be a fatal error - 'call to undefined function' is the problem.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Needs work » Needs review
FileSize
drupal.text-update.12.patch10.78 KB

oh, um, silly me.

Trying whether merging in #717834: The dependencies declared in core's hook_update_dependencies() implementations aren't actually correct makes any difference.

yched’s picture

yched CreditAttribution: yched commented
Status: Needs review » Needs work

Ugh @the confusion with CCK's text module. Blame on Field API patch team for not seeing this coming.

Shouldn't we modify update_fix_d7_requirements() to remove CCK's 'text' and 'number' records from the system table ? (would prevent text upgrades from running altogether during a D6 upgrade)

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Needs work » Needs review
FileSize
drupal.update-remove-d6-modules.14.patch2.41 KB

yeah, @yched's suggestion is probably the way to go.

Also, not really limited to field type modules...

Status: Needs review » Needs work

The last submitted patch, drupal.update-remove-d6-modules.14.patch, failed testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Priority: Critical » Normal
Status: Needs work » Postponed (maintainer needs more info)

Apparently, the fatal error is a db_insert() exception -- the existing D6 dump for upgrade path tests already contains all CCK module records.

Also, I just happened to do a full upgrade from a CCK-enabled D6 site to D7 on the weekend and did not run into the issue vaguely being outlined in the OP.

In turn, I've no idea what the actual problem is and what needs to be fixed. The existing upgrade path tests are covering the scenario being outlined already (text.module being in {system} but disabled). As of now, the only possible cause I can see for this issue would be to have a copy of the D6 module in sites/all/modules/cck/modules/text, which naturally overrides modules/text.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented

...whereas the D6 module should not have been taken into account in the first place, but that's the issue of #1002258: D6 modules satisfy D7 module dependencies.

matmasr’s picture

matmasr CreditAttribution: matmasr commented

@catch.

I wanted to let you know that I reran the update process all over again from D5 to D6 to D7. This time I has NO fatal errors at all. There were several errors that I can post here later if you are interested, but none were show stoppers.

I looked at my content, and as far as I can tell, the integrity of the data looks good. All stories, pages and custom content show fine ???? Very strange.

Anyway, the only problem is with a custom content that I had that had a custom DATE field is not showing in my calendar view. I am getting a SQL error when the view is displayed. I am not sure if this is related to an improper update or something else.

I want to know what information you would like me to provide to help diagnose and close this current open issue?

texas-bronius’s picture

texas-bronius CreditAttribution: texas-bronius commented

#13 above: I think @yched suggested deleting the modules' text and number records from {system}. Having done so in my source D6.26 site's imported db, that got me past this fatal error:
Error: Call to undefined function _update_7000_field_read_fields()
and on to something else now..

heldercor’s picture

heldercor CreditAttribution: heldercor commented

I have a lot of fields from CCK's text module and other module dependencies too. Removing that entry from {system} doesn't sound good.

Any suggestions?

heldercor’s picture

heldercor CreditAttribution: heldercor commented
Issue summary: View changes

Updated issue summary.