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.

2 files changed, 24 insertions, 3 deletions

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 = []