upgrade system needs to support branches

(Pithy comment to get me subscribed)

And no, I don't have any good ideas. But the end result should probably one of:
- hook_update_N_M() or similar.
- Keep track of which hook_update_N have been executed instead of only N for each module.

How about this:
- store drupal version in the variables table, and:

define('DRUPAL_VERSION', variable_get('drupal_version', 'drupal47');

switch(DRUPAL_VERSION) {
  case 'drupal46':
    // Do drupal46 specific updates;
  case 'drupal47':
    //Do drupal47 specific updates;
  default:
    // Do common updates;
    break;
}

Code needs polishing, but I think will do it..

since drupal never merges code from one branch to the next, there are a bunch of problems/questions that come to mind w/ stefan's (otherwise quite interesting) approach. if the updates.inc file on the HEAD is at update #183, and the last update was #175 when the 4.7 branch was created, what happens if we need to add a new update on the 4.7 branch to deal with a security issue? what number should it be? should we skip to #184? how do we keep track of that? do we commit the HEAD-specific code into the 184 update on DRUPAL-4-7? do we enforce a policy that if someone commits update #X on a branch, they must immediately commit update #X on the trunk (even if the update is just a commented out placeholder stub if there's no work to do for that update on the trunk)? what if the person who committed the update on the branch (killes) doesn't have permission to commit to the trunk. what version's case do you use for trunk-specific (slated to be, but not yet 4.8) code? do we prematurely just call it "drupal_4_8" so it continues to work once we make the DRUPAL-4-8 branch and release?

i haven't thought about it much yet, but i believe drumm's approach (or a derivative of it) will be what we'll have to use... unless there are a series of good answers to the above questions. ;)

Stefan's solution is a good start but it's not complete: some updates may be needed in the 47 branch but not in 46. Yet, when a 4.6 site upgrades to 4.7, it will need those updates, too. So, we need to rewind the update sequence back to the time when the branch was created and do those updates, too.

outline

I write the following assuming:
1) we keep the update functions numbered sequentially.
2) we only support update paths for major branches (DRUPAL-4-6, DRUPAL-4-7, HEAD, etc) and we don't have updates that go into one sub-branch but not the other (i.e in 4.7.1 but not 4.7.0).

What we need to know at update run time:
1) the branch the site is updating up from (from DRUPAL-4-6, DRUPAL-4-7 or CVS, etc)
2) the last version the site has updated up to in that branch.
3) the branch the site is updating up to (to DRUPAL-4-7 or CVS, etc)
4) the first update ID in that branch and maybe the last 2 ones, too (in case someone updates more than one branch at a time, e.g. from DRUPAL-4-5 to DRUPAL-4-7 or CVS).

The first and second information is stored in the database.
The third and fourth are defined at the top of database/update.inc

Each update function declares what branch the update relates to or can include some simple if/else logic using the above information to guide the update process.

Case scenario 1: 4.7 site updates to newer 4.7

Simple: it works as it does now.
update.inc figures out the FROM and TO branches are the same. Just pick up from the DB the last update id run and see if there are new ones.

Case scenario 2: 4.7 site updates to just released 4.8

update.inc finds out that we are dealing with a branch one up compared to the one stored on DB.

First, update.inc finishes all outstanding 4.7 updates if there are any, to make sure the site is running the latest data for that branch.

Then, it rewinds the update process to the first 48 update (say 180) and start again.

If an update (switch / single case situation) deals only with 47, the update is skipped (all is already done).

If an update deals only with 48, the update is run.

If an update was valid for both branches (a 48 fix backported to 47), the update function needs to add some simple logic to figure out whether the update has been run already in the previous branch, keeping in mind that the update number might not be the same in both branches.

Say the fix was in update_181 in branch 47 but in update_192 in branch 48 (since more updates go in), the logic would be:
if you have gone at least as far as update_181 in the branch 47, skip this update, otherwise run it.

Getting organized

I used the values 47 and 48 for the branch values above, for the sake of clarity. The branch values can have any sequential numerical value.
The cvs branch must be assigned a numerical value that will be that of the next branch, whether 4.8 or 5.0.

It doesn't make sense to 'upgrade' a cvs site to 4.8, so this case is not covered here (but could be fairly easily by adapting the logic above).

Answering questions

if the updates.inc file on the HEAD is at update #183, and the last update was #175 when the 4.7 branch was created, what happens if we need to add a new update on the 4.7 branch to deal with a security issue? what number should it be? should we skip to #184?

on HEAD, the update will be #184. On the 4.7 branch backport, it will be numbered sequentially #176.

how do we keep track of that?

There is no problem when a site upgrades within the same branch.
If the 4.7 site later upgrade to the newly released Drupal 4.8, it will start the upgrades from the first of the 4.8 branch, i.e. #175.
When it gets to #184, the code will say: "since you upgraded your 4.7 site at least up to #176, you don't need to run this update.

do we commit the HEAD-specific code into the 184 update on DRUPAL-4-7?

no.

do we enforce a policy that if someone commits update #X on a branch, they must immediately commit update #X on the trunk (even if the update is just a commented out placeholder stub if there's no work to do for that update on the trunk)?

no need.

what if the person who committed the update on the branch (killes) doesn't have permission to commit to the trunk. what version's case do you use for trunk-specific (slated to be, but not yet 4.8) code?

If an update is valid both for a branch and for HEAD, the fix for HEAD should be committed first.
If it is later backported, a patch for HEAD must be supplied to add the "don't run if you ran this on a lower branch" logic.

do we prematurely just call it "drupal_4_8" so it continues to work once we make the DRUPAL-4-8 branch and release?

Whatever the value, we need to assign a value to the future release so that we don't have to replace all the "run this upgrade if you run cvs" with "run this upgrade if you run 4.8/5.0".

For the record, the version is defined in system.module:

define('VERSION', '4.8.0 dev');

i wonder this is enough.

Tested the patch from a clean DRUPAL-4-7, upgrading to HEAD with this patch.

update.php gets confused and pre-selects update 110 as the recommended update, but if I manually select 1000 from the list it runs properly and all the updates work.

brave but broken assumption fixed.

DRUPAL-4-7 to HEAD update successful with the new updates. I think finding the subtle 'update.php doesn't detect nonsequential updates' bug is just as important. Nice patch.

Seriously, that update bug affected contrib modules, too. Yucky.

Eaton also tested that the no updates are available is properly selected, too.

last comment for today: the update.php patch is good for 4.7 so that contrib updates won't break.

doh, i hate to do this, so i'm very much in support of this patch. however, i just found another bug in update.php that assume sequential updates which is made more visible and obnoxious by this patch. :( if you happened to *not* be all the way at 182, and you started with a slightly out of date 4.7 that's at, 179 or so, when you run the update, the reporting at the end tells you about 179-182 (w/ queries), 183-999 ("No queries") and then starts telling you about 1000+. attached is a screenshot to show (part of) what i'm talking about.

i'm too tired now to re-roll this patch to fix this other bug, but it should be fixed before this is commited. if chx doesn't beat me to it (which i'm sure he will), i'll try to roll a patch in the next few days. ;)

thanks/sorry,
-derek

D'oh!

Heh! That was a bit too much :)

Typo fixed and it works.

Indeed it does work. +1! Would update again!

yup, patch from #16 is RTBC, alright. ;) thanks, folks!
-derek

I decided to figure out what $k and $v were. Then I removed them since $k == $v due to drupal_map_assoc().

Then I added some extra documentation comments.

This does reset people using the development version to the start of the development version updates, but that is relatively okay.

drat, once again, i hate to do this, but i just realized both chx's (#16) and drumm's (#19) latest patches don't execute the first queries that are needed for each module. :( in my previous test case (where my system.module schema_version is at 178, and my project.module schema_version is at 2), when i run the patched versions of update.php, the first query executed (and reported) for system is only 180, not the 179 that's selected on the update selection screen. :( similarly, project_update_3() never runs or is reported.

i'll take a quick look right now and see if i can find the fence-post bug.

as expected, simple case of > where we needed >=.

whoops, neither drumm's nor my patch handles the 'no updates needed' case at all. :( stay tuned.

ok, much better. ;) if i were more bold, i'd set this to RTBC. however, i'll leave that to someone else...

Slightly re-rolled version. The update that added the node-types table (188) is now properly renumbered to 1005.

I've tested this back and forth several times; this re-roll is just keeping it up with HEAD. I'll be bold -- RTBC.

i just reviewed and tested #24 -- definitely RTBC. thanks for catching that, eaton!

Committed to HEAD.

Please document a best practice in contrib>docs>hooks>hook_update_N().

setting to active because of docs need

if it's only docs, let's call it a 'task', not a bug. when i saw this back as active/bug/critical, i thought something was horribly broken that needed urgent attention. ;)

I think this does not need more code. Some documentation rather. Let's see what we do in a *minor* upgrade:

• Change the size of a field. Does not depend on previous size. No problems with ALTER, then.
• Add an INDEX. DROP INDEX IF EXISTS is your friend.

What else?

forget to mention that i checked pgsql docs for that drop command.

So, some upgrades will run twice, we just need to examine whether they can be ran twice. And my comment says, yes , they can be because they are such-and-such. The question is, what other kind of db upgrades can happen in a minor version?

Another avenue would be to just check if what they're about to update is correct or not but that's pretty hard and does not worth the bothering.

I agree with chx on this. We just need to be wary of brached updates and test them. It shouldn't be hard to finger print major shema changes with a simple select * from foo, fetch array, if not in array.
most minor changes like adding an index should be harmless if ran twice.

I set this to fixed and mail devel on how you can write branched updates.

I got the mail in the dev-list archive, but as drumm said in #27, the outcome of this thread would be good in contrib>docs>hooks>hook_update_N()

Considering that no new tasks will be completed against D5 and that no one has shown any interest in this issue for a long time, I am closing this issue ticket.

This just needed documentation, which we now have:

http://api.drupal.org/api/drupal/modules--system--system.api.php/functio...