#112 [PATCH] Detect duplicate migration names - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#112 ✓resolved

[PATCH] Detect duplicate migration names

Reported by Aslak Hellesøy | May 5th, 2008 @ 10:15 AM

In Rails 1.2.3 (and trunk), if two migrations have the same name (but different versions), the latter will shadow the former, resulting in former migrations not to be run.

This patch fixes this for Rails 1.2.3. I realise the patch won't apply to trunk, but it should be simple to tweak to trunk too (in the #migrations method).

Apologies for not including a test - I didn't have time to investigate how to write one for migrations.

Comments and changes to this ticket

You flagged this item as spam.
Steve Purcell May 5th, 2008 @ 02:37 PM
Good call. You could even just re-use IllegalMigrationNameError here, since it's essentially illegal to reuse a migration name. :)
DHH May 6th, 2008 @ 08:59 PM
- State changed from “new” to “resolved”
We changed the migration names in edge to run off timestamps instead to avoid duplicates.

Aslak Hellesøy May 7th, 2008 @ 07:09 AM

I don't think you have resolved it ;-)

I have created a new patch in Git:

http://github.com/aslakhellesoy/...

The situation where this can happen is rare, but possible:

ruby script\generate migration chunky
# commit it

# days or weeks later
ruby script\generate migration bacon
# Hmm, this should be called chunky (overlooking there is already a chunky
# migration from before). Rename the migration file to (same_timestamp)_chunky.rb 
# and the migration class to Chunky. There are now two Chunky migrations
# with different timestamps.
rake db:migrate
# This should have thrown an error, but it doesn't.

You flagged this item as spam.
Aslak Hellesøy May 7th, 2008 @ 11:32 AM
Steve taught me off line how to make a fix on a branch - I'll do that next time!
DHH May 7th, 2008 @ 03:33 PM
With auto-incrementing ids, collision was guaranteed. With the timestamp ids, you have to be creating the same migration at the very same second for a collision to happen. That's unlikely enough that we don't need more software to deal with, imo.
You flagged this item as spam.
Aslak Hellesøy May 7th, 2008 @ 03:52 PM
This is not about ids or timestamps at all. It's about two migrations with identical class names. The class in the latest file will overwrite the class in the former, and the former migration will never run.
This is possible if a migration is renamed, and Rails will never warn you about it. (I realise migrations should never be renamed, but it happens sometimes with new uncommitted migrations).
It's likely to happen in projects with many migrations if a developer renames an uncommitted migration after it's generated.
I'm not going to persist more, just point to the fact that the patch is only 5 lines (plus tests).
DHH May 7th, 2008 @ 04:14 PM
- State changed from “resolved” to “open”
Ah. I thought you were worried about id clashes. Do you have the patch handy? I don't see it attached here.
You flagged this item as spam.
Steve Purcell May 7th, 2008 @ 04:17 PM
Here's Aslak's patch -- I had it handy...
- 0001-Detect-duplicate-migration-names-when-running-migrat.patch 3.3 KB
You flagged this item as spam.
Aslak Hellesøy May 7th, 2008 @ 09:55 PM
In my second comment there is a link to a changeset in Git, which you can just merge into the master branch. Here is the link again:
http://github.com/aslakhellesoy/...
One of the benefits of Git is that you don't need patch files in an issue tracker anymore. Just pull patches from other people's clones.
A
DHH May 7th, 2008 @ 10:09 PM
Pulling one-off patches is a huge pain in the ass. It's fine for long-running, expansive branches, but for simple fixes it's far too much work. So we continue to use patch files in the issue trackers for anything but stuff like docrails and rack on rails.
You flagged this item as spam.
Aslak Hellesøy May 7th, 2008 @ 10:31 PM
Yeah, I can see how it might be a PITA for small patches. I'll use attachments for future small patches.
You may want to add a note about this in the sidebar here at Lighthouse. Something like "Small patches as attachments in Lighthouse - big contributions in Git clones".
Thanks for taking the time to review this patch - however small it is.
Repository May 11th, 2008 @ 07:39 PM
- State changed from “open” to “resolved”
(from [10fdf44236ea9abfd327fc59d83670d4bcb3e0ca]) Added protection against duplicate migration names (Aslak Hellesøy) [#112 state:resolved]
http://github.com/rails/rails/co...
Ryan Bigg October 9th, 2010 @ 09:42 PM
- Tag cleared.
Automatic cleanup of spam.
Jeff Kreeftmeijer November 8th, 2010 @ 08:24 AM
Automatic cleanup of spam.