Change the method for copying migrations, do not add scope.

The purpose of this change is to allow copying fail on the same names.
Migrations change database and they should be treated with caution,
if 2 migrations are named the same it's much better to skip migration
and allow user decide if it should be copied or not.

6 files changed, 60 insertions, 30 deletions

diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb
index 9ac18f9939..3fc69d7af0 100644
--- a/activerecord/lib/active_record/migration.rb
+++ b/activerecord/lib/active_record/migration.rb
@@ -387,18 +387,18 @@ module ActiveRecord
       def copy(destination, sources)
         copied = []
 
-        sources.each do |scope, path|
+        sources.each do |name, path|
           destination_migrations = ActiveRecord::Migrator.migrations(destination)
           source_migrations = ActiveRecord::Migrator.migrations(path)
           last = destination_migrations.last
 
           source_migrations.each do |migration|
-            next if destination_migrations.any? { |m| m.name == migration.name && m.scope == scope.to_s }
+            next if destination_migrations.any? { |m| m.name == migration.name }
 
             migration.version = next_migration_number(last ? last.version + 1 : 0).to_i
             last = migration
 
-            new_path = File.join(destination, "#{migration.version}_#{migration.name.underscore}.#{scope}.rb")
+            new_path = File.join(destination, "#{migration.version}_#{migration.name.underscore}.rb")
             FileUtils.cp(migration.filename, new_path)
             copied << new_path
           end
@@ -419,9 +419,9 @@ module ActiveRecord
 
   # MigrationProxy is used to defer loading of the actual migration classes
   # until they are needed
-  class MigrationProxy < Struct.new(:name, :version, :filename, :scope)
+  class MigrationProxy < Struct.new(:name, :version, :filename)
 
-    def initialize(name, version, filename, scope)
+    def initialize(name, version, filename)
       super
       @migration = nil
     end
@@ -510,18 +510,18 @@ module ActiveRecord
         seen = Hash.new false
 
         migrations = files.map do |file|
-          version, name, scope = file.scan(/([0-9]+)_([_a-z0-9]*)\.?([_a-z0-9]*)?.rb/).first
+          version, name = file.scan(/([0-9]+)_([_a-z0-9]*).rb/).first
 
           raise IllegalMigrationNameError.new(file) unless version
           version = version.to_i
           name = name.camelize
 
           raise DuplicateMigrationVersionError.new(version) if seen[version]
-          raise DuplicateMigrationNameError.new(name) if seen[[name, scope]]
+          raise DuplicateMigrationNameError.new(name) if seen[name]
 
-          seen[version] = seen[[name, scope]] = true
+          seen[version] = seen[name] = true
 
-          MigrationProxy.new(name, version, file, scope)
+          MigrationProxy.new(name, version, file)
         end
 
         migrations.sort_by(&:version)
diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb
index 6e8ee95613..9635ab25fc 100644
--- a/activerecord/test/cases/migration_test.rb
+++ b/activerecord/test/cases/migration_test.rb
@@ -1910,9 +1910,9 @@ if ActiveRecord::Base.connection.supports_migrations?
       @existing_migrations = Dir[@migrations_path + "/*.rb"]
 
       copied = ActiveRecord::Migration.copy(@migrations_path, {:bukkits => MIGRATIONS_ROOT + "/to_copy"})
-      assert File.exists?(@migrations_path + "/4_people_have_hobbies.bukkits.rb")
-      assert File.exists?(@migrations_path + "/5_people_have_descriptions.bukkits.rb")
-      assert_equal [@migrations_path + "/4_people_have_hobbies.bukkits.rb", @migrations_path + "/5_people_have_descriptions.bukkits.rb"], copied
+      assert File.exists?(@migrations_path + "/4_people_have_hobbies.rb")
+      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
 
       files_count = Dir[@migrations_path + "/*.rb"].length
       copied = ActiveRecord::Migration.copy(@migrations_path, {:bukkits => MIGRATIONS_ROOT + "/to_copy"})
@@ -1928,12 +1928,13 @@ if ActiveRecord::Base.connection.supports_migrations?
       @existing_migrations = Dir[@migrations_path + "/*.rb"]
 
       sources = ActiveSupport::OrderedHash.new
-      sources[:bukkits] = sources[:omg] = MIGRATIONS_ROOT + "/to_copy"
+      sources[:bukkits] = MIGRATIONS_ROOT + "/to_copy"
+      sources[:omg] = MIGRATIONS_ROOT + "/to_copy2"
       ActiveRecord::Migration.copy(@migrations_path, sources)
-      assert File.exists?(@migrations_path + "/4_people_have_hobbies.omg.rb")
-      assert File.exists?(@migrations_path + "/5_people_have_descriptions.omg.rb")
-      assert File.exists?(@migrations_path + "/6_people_have_hobbies.bukkits.rb")
-      assert File.exists?(@migrations_path + "/7_people_have_descriptions.bukkits.rb")
+      assert File.exists?(@migrations_path + "/4_people_have_hobbies.rb")
+      assert File.exists?(@migrations_path + "/5_people_have_descriptions.rb")
+      assert File.exists?(@migrations_path + "/6_create_articles.rb")
+      assert File.exists?(@migrations_path + "/7_create_comments.rb")
 
       files_count = Dir[@migrations_path + "/*.rb"].length
       ActiveRecord::Migration.copy(@migrations_path, sources)
@@ -1948,10 +1949,10 @@ if ActiveRecord::Base.connection.supports_migrations?
 
       Time.travel_to(created_at = Time.utc(2010, 7, 26, 10, 10, 10)) do
         copied = ActiveRecord::Migration.copy(@migrations_path, {:bukkits => MIGRATIONS_ROOT + "/to_copy_with_timestamps"})
-        assert File.exists?(@migrations_path + "/20100726101010_people_have_hobbies.bukkits.rb")
-        assert File.exists?(@migrations_path + "/20100726101011_people_have_descriptions.bukkits.rb")
-        expected = [@migrations_path + "/20100726101010_people_have_hobbies.bukkits.rb",
-                    @migrations_path + "/20100726101011_people_have_descriptions.bukkits.rb"]
+        assert File.exists?(@migrations_path + "/20100726101010_people_have_hobbies.rb")
+        assert File.exists?(@migrations_path + "/20100726101011_people_have_descriptions.rb")
+        expected = [@migrations_path + "/20100726101010_people_have_hobbies.rb",
+                    @migrations_path + "/20100726101011_people_have_descriptions.rb"]
         assert_equal expected, copied
 
         files_count = Dir[@migrations_path + "/*.rb"].length
@@ -1968,14 +1969,15 @@ if ActiveRecord::Base.connection.supports_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_timestamps2"
 
       Time.travel_to(created_at = Time.utc(2010, 7, 26, 10, 10, 10)) do
         copied = ActiveRecord::Migration.copy(@migrations_path, sources)
-        assert File.exists?(@migrations_path + "/20100726101010_people_have_hobbies.omg.rb")
-        assert File.exists?(@migrations_path + "/20100726101011_people_have_descriptions.omg.rb")
-        assert File.exists?(@migrations_path + "/20100726101012_people_have_hobbies.bukkits.rb")
-        assert File.exists?(@migrations_path + "/20100726101013_people_have_descriptions.bukkits.rb")
+        assert File.exists?(@migrations_path + "/20100726101010_people_have_hobbies.rb")
+        assert File.exists?(@migrations_path + "/20100726101011_people_have_descriptions.rb")
+        assert File.exists?(@migrations_path + "/20100726101012_create_articles.rb")
+        assert File.exists?(@migrations_path + "/20100726101013_create_comments.rb")
         assert_equal 4, copied.length
 
         files_count = Dir[@migrations_path + "/*.rb"].length
@@ -1992,8 +1994,8 @@ if ActiveRecord::Base.connection.supports_migrations?
 
       Time.travel_to(created_at = Time.utc(2010, 2, 20, 10, 10, 10)) do
         ActiveRecord::Migration.copy(@migrations_path, {:bukkits => MIGRATIONS_ROOT + "/to_copy_with_timestamps"})
-        assert File.exists?(@migrations_path + "/20100301010102_people_have_hobbies.bukkits.rb")
-        assert File.exists?(@migrations_path + "/20100301010103_people_have_descriptions.bukkits.rb")
+        assert File.exists?(@migrations_path + "/20100301010102_people_have_hobbies.rb")
+        assert File.exists?(@migrations_path + "/20100301010103_people_have_descriptions.rb")
 
         files_count = Dir[@migrations_path + "/*.rb"].length
         copied = ActiveRecord::Migration.copy(@migrations_path, {:bukkits => MIGRATIONS_ROOT + "/to_copy_with_timestamps"})
@@ -2010,8 +2012,8 @@ if ActiveRecord::Base.connection.supports_migrations?
 
       Time.travel_to(created_at = Time.utc(2010, 7, 26, 10, 10, 10)) do
         copied = ActiveRecord::Migration.copy(@migrations_path, {:bukkits => MIGRATIONS_ROOT + "/to_copy_with_timestamps"})
-        assert File.exists?(@migrations_path + "/20100726101010_people_have_hobbies.bukkits.rb")
-        assert File.exists?(@migrations_path + "/20100726101011_people_have_descriptions.bukkits.rb")
+        assert File.exists?(@migrations_path + "/20100726101010_people_have_hobbies.rb")
+        assert File.exists?(@migrations_path + "/20100726101011_people_have_descriptions.rb")
         assert_equal 2, copied.length
       end
     ensure
diff --git a/activerecord/test/migrations/to_copy2/1_create_articles.rb b/activerecord/test/migrations/to_copy2/1_create_articles.rb
new file mode 100644
index 0000000000..0f048d90f7
--- /dev/null
+++ b/activerecord/test/migrations/to_copy2/1_create_articles.rb
@@ -0,0 +1,7 @@
+class CreateArticles < ActiveRecord::Migration
+  def self.up
+  end
+
+  def self.down
+  end
+end
diff --git a/activerecord/test/migrations/to_copy2/2_create_comments.rb b/activerecord/test/migrations/to_copy2/2_create_comments.rb
new file mode 100644
index 0000000000..0f048d90f7
--- /dev/null
+++ b/activerecord/test/migrations/to_copy2/2_create_comments.rb
@@ -0,0 +1,7 @@
+class CreateArticles < ActiveRecord::Migration
+  def self.up
+  end
+
+  def self.down
+  end
+end
diff --git a/activerecord/test/migrations/to_copy_with_timestamps2/20090101010101_create_articles.rb b/activerecord/test/migrations/to_copy_with_timestamps2/20090101010101_create_articles.rb
new file mode 100644
index 0000000000..0f048d90f7
--- /dev/null
+++ b/activerecord/test/migrations/to_copy_with_timestamps2/20090101010101_create_articles.rb
@@ -0,0 +1,7 @@
+class CreateArticles < ActiveRecord::Migration
+  def self.up
+  end
+
+  def self.down
+  end
+end
diff --git a/activerecord/test/migrations/to_copy_with_timestamps2/20090101010202_create_comments.rb b/activerecord/test/migrations/to_copy_with_timestamps2/20090101010202_create_comments.rb
new file mode 100644
index 0000000000..2b048edbb5
--- /dev/null
+++ b/activerecord/test/migrations/to_copy_with_timestamps2/20090101010202_create_comments.rb
@@ -0,0 +1,7 @@
+class CreateComments < ActiveRecord::Migration
+  def self.up
+  end
+
+  def self.down
+  end
+end