Permalink
Browse files

Ignore origin comment when checking for duplicates on Migration.copy

49ebe51 fixed copying migrations, but existing migrations would still
trigger warnings. The proper way to compare migrations is to ignore
origin lines - if migration is identical it means that we can
silently skip it, regardless where it comes from.
  • Loading branch information...
1 parent 652db2f commit 62d556424adcbf473ec5fe2ed9b460c058a36463 @drogus drogus committed Dec 9, 2011
@@ -461,11 +461,13 @@ def copy(destination, sources, options = {})
source_migrations = ActiveRecord::Migrator.migrations(path)
source_migrations.each do |migration|
- source = File.read(migration.filename).chomp
+ source = File.read(migration.filename)
source = "# This migration comes from #{name} (originally #{migration.version})\n#{source}"
if duplicate = destination_migrations.detect { |m| m.name == migration.name }
- options[:on_skip].call(name, migration) if File.read(duplicate.filename).chomp != source && options[:on_skip]
+ if options[:on_skip] && !migrations_identical?(File.read(duplicate.filename), source)
+ options[:on_skip].call(name, migration)
+ end
next
end
@@ -491,6 +493,22 @@ def next_migration_number(number)
"%.3d" % number
end
end
+
+ def migrations_identical?(a, b)
+ # Due to a bug some of the migrations copied may not have origin comment,
+ # so we need to ignore it.
+ remove_origin_comment(a.chomp) == remove_origin_comment(b.chomp)
+ end
+ private :migrations_identical?
+
+ def remove_origin_comment(migration_source)
+ if migration_source =~ /^# This migration comes from/
+ migration_source = migration_source.to_a[1..-1].join
+ end
+
+ migration_source
+ end
+ private :remove_origin_comment
end
# MigrationProxy is used to defer loading of the actual migration classes
@@ -2203,15 +2203,16 @@ def test_skipping_migrations
@existing_migrations = Dir[@migrations_path + "/*.rb"]
sources = ActiveSupport::OrderedHash.new
- sources[:bukkits] = sources[:omg] = MIGRATIONS_ROOT + "/to_copy_with_timestamps"
+ sources[:bukkits] = MIGRATIONS_ROOT + "/to_copy_with_timestamps"
+ sources[:omg] = MIGRATIONS_ROOT + "/to_copy_with_name_collision"
skipped = []
on_skip = Proc.new { |name, migration| skipped << "#{name} #{migration.name}" }
copied = ActiveRecord::Migration.copy(@migrations_path, sources, :on_skip => on_skip)
assert_equal 2, copied.length
- assert_equal 2, skipped.length
- assert_equal ["bukkits PeopleHaveHobbies", "bukkits PeopleHaveDescriptions"], skipped
+ assert_equal 1, skipped.length
+ assert_equal ["omg PeopleHaveHobbies"], skipped
ensure
clear
end
@@ -2234,6 +2235,31 @@ def test_skip_is_not_called_if_migrations_are_identical
clear
end
+ def test_skip_ignores_origin_comment
+ ActiveRecord::Base.timestamped_migrations = false
+ @migrations_path = MIGRATIONS_ROOT + "/valid"
+ @existing_migrations = Dir[@migrations_path + "/*.rb"]
+
+ sources = ActiveSupport::OrderedHash.new
+ sources[:bukkits] = MIGRATIONS_ROOT + "/to_copy"
+
+ skipped = []
+ on_skip = Proc.new { |name, migration| skipped << "#{name} #{migration.name}" }
+ copied = ActiveRecord::Migration.copy(@migrations_path, sources, :on_skip => on_skip)
+
+ # remove origin comment
+ migration = @migrations_path + "/4_people_have_hobbies.rb"
+ migration_source = File.read(migration).to_a[1..-1].join
+ File.open(migration, "w") { |f| f.write migration_source }
+
+ ActiveRecord::Migration.copy(@migrations_path, sources, :on_skip => on_skip)
+
+ assert_equal 2, copied.length
+ assert_equal 0, skipped.length
+ ensure
+ clear
+ end
+
def test_copying_migrations_to_non_existing_directory
@migrations_path = MIGRATIONS_ROOT + "/non_existing"
@existing_migrations = []
@@ -0,0 +1,9 @@
+class PeopleHaveLastNames < ActiveRecord::Migration
+ def self.up
+ add_column "people", "hobbies", :string
+ end
+
+ def self.down
+ remove_column "people", "hobbies"
+ end
+end

0 comments on commit 62d5564

Please sign in to comment.