From 652db2fc3e1d62737e4bedb3b7cee313d7400c0a Mon Sep 17 00:00:00 2001 From: Piotr Sarnacki Date: Fri, 9 Dec 2011 00:50:39 +0100 Subject: Fix copying migrations from engines There was a bug in ActiveRecord::Migration.copy method, which prevented adding special comment about the origin of migration. Because of that, the check if migration is identical or if it's not and should be skipped was always saying that migration is skipped, which was causing additional useless warnings about skipped migrations. --- activerecord/lib/active_record/migration.rb | 6 +++--- activerecord/test/cases/migration_test.rb | 21 +++++++++++++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index e53d5e7167..1d450da006 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -461,11 +461,11 @@ module ActiveRecord source_migrations = ActiveRecord::Migrator.migrations(path) source_migrations.each do |migration| - source = File.read(migration.filename) + source = File.read(migration.filename).chomp 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) != source && options[:on_skip] + options[:on_skip].call(name, migration) if File.read(duplicate.filename).chomp != source && options[:on_skip] next end @@ -474,7 +474,7 @@ module ActiveRecord old_path, migration.filename = migration.filename, new_path last = migration - FileUtils.cp(old_path, migration.filename) + File.open(migration.filename, "w") { |f| f.write source } copied << migration options[:on_copy].call(name, migration, old_path) if options[:on_copy] destination_migrations << migration diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index 42d62eca8e..1997fc96ea 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -2103,6 +2103,9 @@ if ActiveRecord::Base.connection.supports_migrations? assert File.exists?(@migrations_path + "/5_people_have_descriptions.rb") assert_equal [@migrations_path + "/4_people_have_hobbies.rb", @migrations_path + "/5_people_have_descriptions.rb"], copied.map(&:filename) + expected = "# This migration comes from bukkits (originally 1)" + assert_equal expected, IO.readlines(@migrations_path + "/4_people_have_hobbies.rb")[0].chomp + files_count = Dir[@migrations_path + "/*.rb"].length copied = ActiveRecord::Migration.copy(@migrations_path, {:bukkits => MIGRATIONS_ROOT + "/to_copy"}) assert_equal files_count, Dir[@migrations_path + "/*.rb"].length @@ -2213,6 +2216,24 @@ if ActiveRecord::Base.connection.supports_migrations? clear end + def test_skip_is_not_called_if_migrations_are_identical + @migrations_path = MIGRATIONS_ROOT + "/valid_with_timestamps" + @existing_migrations = Dir[@migrations_path + "/*.rb"] + + sources = ActiveSupport::OrderedHash.new + sources[:bukkits] = MIGRATIONS_ROOT + "/to_copy_with_timestamps" + + skipped = [] + on_skip = Proc.new { |name, migration| skipped << "#{name} #{migration.name}" } + copied = ActiveRecord::Migration.copy(@migrations_path, sources, :on_skip => on_skip) + 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 = [] -- cgit v1.2.3