Core modules need their updates in their own files [#80526]

user warning: Table 'drupal_head.aggregator_item' doesn't exist query: ALTER TABLE aggregator_item ADD INDEX (fid) in /var/www/html/drupal_head/includes/database.mysql.inc on line 128.

i update CVS HEAD this afternoon and ran update.php. failed on aggregator table update. this install has never had aggregator activated, so tables were never created. there should not be any attempt to update them!

notes from IRC regarding attempted aggregator table updates:

webchick> they should be in aggregator_update_1 or whatever
webchick> in aggregator.install, not system.install
eaton> Ugh, that's broken, didn't realize that someone put those updates in there :P

Comment	File	Size	Author
#11	80526-split-core-install.patch	6.6 KB	forngren
#11
#1	aggregator_install.patch	1.89 KB	webchick
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

webchick

she/they

English

Vancouver 🇨🇦

CreditAttribution: webchick commented 27 August 2006 at 04:05

Priority:	Normal	» Critical
Status:	Active	» Needs work

File	Size
aggregator_install.patch	1.89 KB

Bumping status to critical because it will cause updates to be numbered incorrectly unless we fix this before the next db update code gets put in.

For some reason though, this patch doesn't seem to display any drop-down for aggregator module under update.php. Any ideas?

Comment #2

webchick

she/they

English

Vancouver 🇨🇦

CreditAttribution: webchick commented 27 August 2006 at 14:39

Title:

update.php fails on 'no such table ... aggregator'

» Non-required modules need their updates in their own files

Changing the title, as we now have a locale update in system.install; this is only going to get worse.

Comment #3

Heine CreditAttribution: Heine commented 29 August 2006 at 13:17

The problem scenarios devised in IRC with core module updates in module.install:

Suppose a user of 4.7 has locale disabled, upgrades to 4.8, then enables locale. He/she now has to run update.php again.

Suppose a user of 4.8 had locale enabled at some point; when upgrading to a hypothetical 4.8.1 that does contain a db upgrade for locale the locale table is not updated. When the user later enables locale, he/she has to run update.php again.

If we upgrade tables indiscriminately in system.install we get the problem cited by Harry Slaughter, where fresh 4.8 installs won't have certain tables.

Comment #4

chx CreditAttribution: chx commented 4 October 2006 at 10:31

Title:	Non-required modules need their updates in their own files	» Core patches in the future should use db_table_exists
Status:	Needs work	» Postponed

You were running a HEAD->HEAD update. This is not supported.

Drupal 6 updates will need some consideration, indeed. But I think the installation system could take care of them...

For now, there is no issue because when you go on the supported path then you are upgrading from 4.7 where all tables exists.

Comment #5

chx CreditAttribution: chx commented 4 October 2006 at 10:32

Title:

Core patches in the future should use db_table_exists

» Non-required modules need their updates in their own files

hm mmm i think i will keep the title.

Comment #6

webchick

she/they

English

Vancouver 🇨🇦

CreditAttribution: webchick commented 4 October 2006 at 16:39

Well... it's not as simple as db_table_exists... because if a module is initially disabled when I do the update, but then I enable it later, it still needs to execute all of the updates, no?

I also am not sure about the postponed status. We may need this as early as 5.1, if there are database changes. Seems to make sense to work it out pre-release rather than potentially holding up a bugfix release with this problem.

Comment #7

Zen CreditAttribution: Zen commented 26 January 2007 at 09:12

Version:	x.y.z	» 6.x-dev
Status:	Postponed	» Active

I think it is also important from the POV of modules being spun out of core into contrib (por ejemplo).. Bumping.

Comment #8

forngren CreditAttribution: forngren commented 8 February 2007 at 22:14

Subscribing and putting this on my radar.

Comment #9

kkaefer CreditAttribution: kkaefer commented 9 February 2007 at 11:29

In fact, even required module's update/install stuff can be outsourced to the respective module's install file. It's also run on install/update time.

Comment #10

RobRoy CreditAttribution: RobRoy commented 10 February 2007 at 01:22

Title:

Non-required modules need their updates in their own files

» Core modules need their updates in their own files

Yes, I agree that both required and unrequired modules need this. Eaton brought up a good point on another thread that you may want to have an installation profile that installs a custom menu.module in sites/all/modules instead of core's menu.module if you had some crazy customizations. And besides, it just makes sense and would be easier to manage.

Comment #11

forngren CreditAttribution: forngren commented 1 June 2007 at 18:18

Assigned:	Unassigned	» forngren
Status:	Active	» Needs work

File	Size
80526-split-core-install.patch	6.6 KB

First steps...

Comment #12

pwolanin CreditAttribution: pwolanin commented 2 June 2007 at 01:59

patch looks like a good first step.

Note that I think menu module may be a special case (i.e. should be in system.install this time), since it's moving from required in D5 to non-required in D6, and it needs to coordinate with the system update that installs the new menu tables.

Comment #13

forngren CreditAttribution: forngren commented 17 June 2007 at 20:52

I'm not sure how we should proceed with the updates.

Are updates performed on non active modules? If so, then can/may/could we move the updates. But should we? I see three options here:

1) We move all updates into respective modules. This will most likely create an earth-quake-sized mess of everything and screw up everything but clean installs.
2) We move all updates since 5.x into respective modules. This will create a medium sized mess, which probably is solvable.
3) We announce for a change in 7.x an leave things for know. Just a postponed mess.

How long are updates kept in system.install? Forever?

Comment #14

webchick

she/they

English

Vancouver 🇨🇦

CreditAttribution: webchick commented 2 September 2007 at 06:53

chx and I discussed this tonight. Looks like the most straight-forward approach for our current timeline is going to be to throw db_table_exists() in various places to prevent errors during the upgrade path, and fix this properly in 7.x...