Support multiple oldnames for formulae

Library/Homebrew/cmd/migrate.rb

@@ -29,14 +29,6 @@ module Homebrew
 
     args.named.to_kegs.each do |keg|
       f = Formulary.from_keg(keg)
-
-      if f.oldname
-        rack = HOMEBREW_CELLAR/f.oldname
-        raise NoSuchKegError, f.oldname if !rack.exist? || rack.subdirs.empty?
-
-        odie "#{rack} is a symlink" if rack.symlink?
-      end
-
       Migrator.migrate_if_needed(f, force: args.force?, dry_run: args.dry_run?)
     end
   end

Library/Homebrew/cmd/update-report.rb

@@ -545,27 +545,20 @@ class Reporter
     Formula.installed.each do |formula|
       next unless Migrator.needs_migration?(formula)
 
-      oldname = formula.oldname
+      oldnames_to_migrate = formula.oldnames.select do |oldname|
         oldname_rack = HOMEBREW_CELLAR/oldname
+        next false unless oldname_rack.exist?
 
         if oldname_rack.subdirs.empty?
           oldname_rack.rmdir_if_possible
-        next
+          next false
         end
 
-      new_name = tap.formula_renames[oldname]
+        true
-      next unless new_name
-
-      new_full_name = "#{tap}/#{new_name}"
-
-      begin
-        f = Formulary.factory(new_full_name)
-      rescue Exception => e # rubocop:disable Lint/RescueException
-        onoe "#{e.message}\n#{e.backtrace&.join("\n")}" if Homebrew::EnvConfig.developer?
-        next
       end
+      next if oldnames_to_migrate.empty?
 
-      Migrator.migrate_if_needed(f, force: force)
+      Migrator.migrate_if_needed(formula, force: force)
     end
   end
 

Library/Homebrew/formula.rb

@@ -222,7 +222,7 @@ class Formula
     @pin = FormulaPin.new(self)
     @follow_installed_alias = true
     @prefix_returns_versioned_prefix = false
-    @oldname_lock = nil
+    @oldname_locks = []
   end
 
   # @private
@@ -491,10 +491,18 @@ class Formula
   delegate resource: :active_spec
 
   # An old name for the formula.
+  # @deprecated Use #{#oldnames} instead.
   def oldname
-    @oldname ||= if tap
+    # odeprecated "Formula#oldname", "Formula#oldnames"
-      formula_renames = tap.formula_renames
+    @oldname ||= oldnames.first
-      formula_renames.to_a.rassoc(name).first if formula_renames.value?(name)
+  end
+
+  # Old names for the formula.
+  def oldnames
+    @oldnames ||= if tap
+      tap.formula_renames.select { |_, oldname| oldname == name }.keys
+    else
+      []
     end
   end
 
@@ -1350,34 +1358,41 @@ class Formula
   def lock
     @lock = FormulaLock.new(name)
     @lock.lock
-    return unless oldname
-    return unless (oldname_rack = HOMEBREW_CELLAR/oldname).exist?
-    return if oldname_rack.resolved_path != rack
 
-    @oldname_lock = FormulaLock.new(oldname)
+    oldnames.each do |oldname|
-    @oldname_lock.lock
+      next unless (oldname_rack = HOMEBREW_CELLAR/oldname).exist?
+      next if oldname_rack.resolved_path != rack
+
+      oldname_lock = FormulaLock.new(oldname)
+      oldname_lock.lock
+      @oldname_locks << oldname_lock
+    end
   end
 
   # @private
   def unlock
     @lock&.unlock
-    @oldname_lock&.unlock
+    @oldname_locks.each(&:unlock)
+  end
+
+  # @private
+  def oldnames_to_migrate
+    oldnames.select do |oldname|
+      old_rack = HOMEBREW_CELLAR/oldname
+      next false unless old_rack.directory?
+      next false if old_rack.subdirs.empty?
+
+      tap == Tab.for_keg(old_rack.subdirs.min).tap
+    end
   end
 
   def migration_needed?
-    return false unless oldname
+    !oldnames_to_migrate.empty? && !rack.exist?
-    return false if rack.exist?
-
-    old_rack = HOMEBREW_CELLAR/oldname
-    return false unless old_rack.directory?
-    return false if old_rack.subdirs.empty?
-
-    tap == Tab.for_keg(old_rack.subdirs.min).tap
   end
 
   # @private
   def outdated_kegs(fetch_head: false)
-    raise Migrator::MigrationNeededError, self if migration_needed?
+    raise Migrator::MigrationNeededError.new(oldnames_to_migrate.first, name) if migration_needed?
 
     cache_key = "#{full_name}-#{fetch_head}"
     Formula.cache[:outdated_kegs] ||= {}
@@ -1499,7 +1514,7 @@ class Formula
 
   # @private
   def possible_names
-    [name, oldname, *aliases].compact
+    [name, *oldnames, *aliases].compact
   end
 
   def to_s
@@ -2087,7 +2102,8 @@ class Formula
       "name"                     => name,
       "full_name"                => full_name,
       "tap"                      => tap&.name,
-      "oldname"                  => oldname,
+      "oldname"                  => oldnames.first, # deprecated
+      "oldnames"                 => oldnames,
       "aliases"                  => aliases.sort,
       "versioned_formulae"       => versioned_formulae.map(&:name),
       "desc"                     => desc,

Library/Homebrew/formulary.rb

@@ -293,9 +293,9 @@ module Formulary
         self.class.instance_variable_get(:@tap_git_head_string)
       end
 
-      @oldname_string = json_formula["oldname"]
+      @oldnames_array = json_formula["oldnames"] || [json_formula["oldname"]].compact
-      def oldname
+      def oldnames
-        self.class.instance_variable_get(:@oldname_string)
+        self.class.instance_variable_get(:@oldnames_array)
       end
 
       @aliases_array = json_formula["aliases"]

Library/Homebrew/install.rb

@@ -173,7 +173,7 @@ module Homebrew
           # Check if the formula we try to install is the same as installed
           # but not migrated one. If --force is passed then install anyway.
           opoo <<~EOS
-            #{formula.oldname} is already installed, it's just not migrated.
+            #{formula.oldnames_to_migrate.first} is already installed, it's just not migrated.
             To migrate this formula, run:
               brew migrate #{formula}
             Or to force-install it, run:

Library/Homebrew/keg.rb

@@ -167,6 +167,7 @@ class Keg
     @name = path.parent.basename.to_s
     @linked_keg_record = HOMEBREW_LINKED_KEGS/name
     @opt_record = HOMEBREW_PREFIX/"opt/#{name}"
+    @oldname_opt_records = []
     @require_relocation = false
   end
 
@@ -270,7 +271,7 @@ class Keg
     remove_opt_record if optlinked?
     remove_linked_keg_record if linked?
     remove_old_aliases
-    remove_oldname_opt_record
+    remove_oldname_opt_records
   rescue Errno::EACCES, Errno::ENOTEMPTY
     raise if raise_failures
 
@@ -319,13 +320,15 @@ class Keg
     ObserverPathnameExtension.n
   end
 
-  def lock(&block)
+  def lock
     FormulaLock.new(name).with_lock do
-      if oldname_opt_record
+      oldname_locks = oldname_opt_records.map do |record|
-        FormulaLock.new(oldname_opt_record.basename.to_s).with_lock(&block)
+        FormulaLock.new(record.basename.to_s)
-      else
-        yield
       end
+      oldname_locks.each(&:lock)
+      yield
+    ensure
+      oldname_locks&.each(&:unlock)
     end
   end
 
@@ -388,11 +391,15 @@ class Keg
     Formulary.from_keg(self)
   end
 
-  def oldname_opt_record
+  def oldname_opt_records
-    @oldname_opt_record ||= if (opt_dir = HOMEBREW_PREFIX/"opt").directory?
+    return @oldname_opt_records unless @oldname_opt_records.empty?
-      opt_dir.subdirs.find do |dir|
+
+    @oldname_opt_records = if (opt_dir = HOMEBREW_PREFIX/"opt").directory?
+      opt_dir.subdirs.select do |dir|
         dir.symlink? && dir != opt_record && path.parent == dir.resolved_path.parent
       end
+    else
+      []
     end
   end
 
@@ -480,13 +487,14 @@ class Keg
 
   def consistent_reproducible_symlink_permissions!; end
 
-  def remove_oldname_opt_record
+  def remove_oldname_opt_records
-    return unless oldname_opt_record
+    oldname_opt_records.reject! do |record|
-    return if oldname_opt_record.resolved_path != path
+      return false if record.resolved_path != path
 
-    @oldname_opt_record.unlink
+      record.unlink
-    @oldname_opt_record.parent.rmdir_if_possible
+      record.parent.rmdir_if_possible
-    @oldname_opt_record = nil
+      true
+    end
   end
 
   def tab
@@ -511,10 +519,10 @@ class Keg
       make_relative_symlink(alias_opt_record, path, verbose: verbose, dry_run: dry_run, overwrite: overwrite)
     end
 
-    return unless oldname_opt_record
+    oldname_opt_records.each do |record|
-
+      record.delete
-    oldname_opt_record.delete
+      make_relative_symlink(record, path, verbose: verbose, dry_run: dry_run, overwrite: overwrite)
-    make_relative_symlink(oldname_opt_record, path, verbose: verbose, dry_run: dry_run, overwrite: overwrite)
+    end
   end
 
   def delete_pyc_files!

Library/Homebrew/migrator.rb

@@ -13,41 +13,34 @@ class Migrator
 
   # Error for when a migration is necessary.
   class MigrationNeededError < RuntimeError
-    def initialize(formula)
+    def initialize(oldname, newname)
       super <<~EOS
-        #{formula.oldname} was renamed to #{formula.name} and needs to be migrated by running:
+        #{oldname} was renamed to #{newname} and needs to be migrated by running:
-          brew migrate #{formula.oldname}
+          brew migrate #{oldname}
       EOS
     end
   end
 
-  # Error for when a formula does not replace another formula.
-  class MigratorNoOldnameError < RuntimeError
-    def initialize(formula)
-      super "#{formula.name} doesn't replace any formula."
-    end
-  end
-
   # Error for when the old name's path does not exist.
   class MigratorNoOldpathError < RuntimeError
-    def initialize(formula)
+    def initialize(oldname)
-      super "#{HOMEBREW_CELLAR/formula.oldname} doesn't exist."
+      super "#{HOMEBREW_CELLAR/oldname} doesn't exist."
     end
   end
 
   # Error for when a formula is migrated to a different tap without explicitly using its fully-qualified name.
   class MigratorDifferentTapsError < RuntimeError
-    def initialize(formula, tap)
+    def initialize(formula, oldname, tap)
       msg = if tap.core_tap?
-        "Please try to use #{formula.oldname} to refer to the formula.\n"
+        "Please try to use #{oldname} to refer to the formula.\n"
       elsif tap
-        "Please try to use fully-qualified #{tap}/#{formula.oldname} to refer to the formula.\n"
+        "Please try to use fully-qualified #{tap}/#{oldname} to refer to the formula.\n"
       end
 
       super <<~EOS
-        #{formula.name} from #{formula.tap} is given, but old name #{formula.oldname} was installed from #{tap || "path or url"}.
+        #{formula.name} from #{formula.tap} is given, but old name #{oldname} was installed from #{tap || "path or url"}.
         #{msg}To force migration, run:
-          brew migrate --force #{formula.oldname}
+          brew migrate --force #{oldname}
       EOS
     end
   end
@@ -65,13 +58,13 @@ class Migrator
   attr_reader :old_pin_record
 
   # Path to oldname opt.
-  attr_reader :old_opt_record
+  attr_reader :old_opt_records
 
-  # Oldname linked keg.
+  # Oldname linked kegs.
-  attr_reader :old_linked_keg
+  attr_reader :old_linked_kegs
 
-  # Path to oldname's linked keg.
+  # Oldname linked kegs that were fully linked.
-  attr_reader :old_linked_keg_record
+  attr_reader :old_full_linked_kegs
 
   # Tabs from oldname kegs.
   attr_reader :old_tabs
@@ -97,53 +90,64 @@ class Migrator
   # Path to newname keg that will be linked if old_linked_keg isn't nil.
   attr_reader :new_linked_keg_record
 
-  def self.needs_migration?(formula)
+  def self.oldnames_needing_migration(formula)
-    oldname = formula.oldname
+    formula.oldnames.select do |oldname|
-    return false unless oldname
-
       oldname_rack = HOMEBREW_CELLAR/oldname
-    return false if oldname_rack.symlink?
+      next false if oldname_rack.symlink?
-    return false unless oldname_rack.directory?
+      next false unless oldname_rack.directory?
 
       true
     end
+  end
+
+  def self.needs_migration?(formula)
+    !oldnames_needing_migration(formula).empty?
+  end
 
   def self.migrate_if_needed(formula, force:, dry_run: false)
-    return unless Migrator.needs_migration?(formula)
+    oldnames = Migrator.oldnames_needing_migration(formula)
+    return if oldnames.empty?
 
     begin
       if dry_run
-        ohai "Would migrate #{formula.oldname} to #{formula.name}"
+        ohai "Would migrate #{oldnames.to_sentence} to #{formula.name}"
         return
       end
-      migrator = Migrator.new(formula, force: force)
+
+      oldnames.each do |oldname|
+        migrator = Migrator.new(formula, oldname, force: force)
         migrator.migrate
+      end
     rescue => e
       onoe e
     end
   end
 
-  def initialize(formula, force: false)
+  def initialize(formula, oldname, force: false)
-    @oldname = formula.oldname
+    @oldname = oldname
     @newname = formula.name
-    raise MigratorNoOldnameError, formula unless oldname
 
     @formula = formula
-    @old_cellar = HOMEBREW_CELLAR/formula.oldname
+    @old_cellar = HOMEBREW_CELLAR/oldname
-    raise MigratorNoOldpathError, formula unless old_cellar.exist?
+    raise MigratorNoOldpathError, oldname unless old_cellar.exist?
 
     @old_tabs = old_cellar.subdirs.map { |d| Tab.for_keg(Keg.new(d)) }
     @old_tap = old_tabs.first.tap
 
-    raise MigratorDifferentTapsError.new(formula, old_tap) if !force && !from_same_tap_user?
+    raise MigratorDifferentTapsError.new(formula, oldname, old_tap) if !force && !from_same_tap_user?
 
     @new_cellar = HOMEBREW_CELLAR/formula.name
     @new_cellar_existed = @new_cellar.exist?
 
-    if (@old_linked_keg = linked_old_linked_keg)
+    @old_linked_kegs = linked_old_linked_kegs
-      @old_linked_keg_record = old_linked_keg.linked_keg_record if old_linked_keg.linked?
+    @old_full_linked_kegs = []
-      @old_opt_record = old_linked_keg.opt_record if old_linked_keg.optlinked?
+    @old_opt_records = []
-      @new_linked_keg_record = HOMEBREW_CELLAR/"#{newname}/#{File.basename(old_linked_keg)}"
+    old_linked_kegs.each do |old_linked_keg|
+      @old_full_linked_kegs << old_linked_keg if old_linked_keg.linked?
+      @old_opt_records << old_linked_keg.opt_record if old_linked_keg.optlinked?
+    end
+    unless old_linked_kegs.empty?
+      @new_linked_keg_record = HOMEBREW_CELLAR/"#{newname}/#{File.basename(old_linked_kegs.first)}"
     end
 
     @old_pin_record = HOMEBREW_PINNED_KEGS/oldname
@@ -167,7 +171,7 @@ class Migrator
 
     new_tap = if old_tap
       old_tap_user, = old_tap.user
-      if (migrate_tap = old_tap.tap_migrations[formula.oldname])
+      if (migrate_tap = old_tap.tap_migrations[oldname])
         new_tap_user, new_tap_repo = migrate_tap.split("/")
         "#{new_tap_user}/#{new_tap_repo}"
       end
@@ -188,12 +192,12 @@ class Migrator
     end
   end
 
-  def linked_old_linked_keg
+  def linked_old_linked_kegs
     keg_dirs = []
     keg_dirs += new_cellar.subdirs if new_cellar.exist?
     keg_dirs += old_cellar.subdirs
     kegs = keg_dirs.map { |d| Keg.new(d) }
-    kegs.find(&:linked?) || kegs.find(&:optlinked?)
+    kegs.select { |keg| keg.linked? || keg.optlinked? }
   end
 
   def pinned?
@@ -209,7 +213,7 @@ class Migrator
     move_to_new_directory
     link_oldname_cellar
     link_oldname_opt
-    link_newname unless old_linked_keg.nil?
+    link_newname unless old_linked_kegs.empty?
     update_tabs
     return unless formula.outdated?
 
@@ -232,14 +236,14 @@ class Migrator
     unlock
   end
 
-  # Move everything from `Cellar/oldname` to `Cellar/newname`.
+  def remove_conflicts(directory)
-  def move_to_new_directory
-    return unless old_cellar.exist?
-
-    if new_cellar.exist?
     conflicted = T.let(false, T::Boolean)
-      old_cellar.each_child do |c|
+
-        next unless (new_cellar/c.basename).exist?
+    directory.each_child do |c|
+      if c.directory? && !c.symlink?
+        conflicted ||= remove_conflicts(c)
+      else
+        next unless (new_cellar/c.relative_path_from(old_cellar)).exist?
 
         begin
           FileUtils.rm_rf c
@@ -248,13 +252,36 @@ class Migrator
           onoe "#{new_cellar/c.basename} already exists."
         end
       end
+    end
 
-      odie "Remove #{new_cellar} manually and run `brew migrate #{oldname}`." if conflicted
+    conflicted
+  end
+
+  def merge_directory(directory)
+    directory.each_child do |c|
+      new_path = new_cellar/c.relative_path_from(old_cellar)
+
+      if c.directory? && !c.symlink? && new_path.exist?
+        merge_directory(c)
+        c.unlink
+      else
+        FileUtils.mv(c, new_path)
+      end
+    end
+  end
+
+  # Move everything from `Cellar/oldname` to `Cellar/newname`.
+  def move_to_new_directory
+    return unless old_cellar.exist?
+
+    if new_cellar.exist?
+      conflicted = remove_conflicts(old_cellar)
+      odie "Remove #{new_cellar} and #{old_cellar} manually and run `brew reinstall #{newname}`." if conflicted
     end
 
     oh1 "Moving #{Formatter.identifier(oldname)} versions to #{new_cellar}"
     if new_cellar.exist?
-      FileUtils.mv(old_cellar.children, new_cellar)
+      merge_directory(old_cellar)
     else
       FileUtils.mv(old_cellar, new_cellar)
     end
@@ -302,7 +329,7 @@ class Migrator
     # If old_keg wasn't linked then we just optlink a keg.
     # If old keg wasn't optlinked and linked, we don't call this method at all.
     # If formula is keg-only we also optlink it.
-    if formula.keg_only? || !old_linked_keg_record
+    if formula.keg_only? || old_full_linked_kegs.empty?
       begin
         new_keg.optlink(verbose: verbose?)
       rescue Keg::LinkError => e
@@ -340,11 +367,11 @@ class Migrator
 
   # Link keg to opt if it was linked before migrating.
   def link_oldname_opt
-    return unless old_opt_record
+    old_opt_records.each do |old_opt_record|
-
       old_opt_record.delete if old_opt_record.symlink?
       old_opt_record.make_relative_symlink(new_linked_keg_record)
     end
+  end
 
   # After migration every `INSTALL_RECEIPT.json` has the wrong path to the formula
   # so we must update `INSTALL_RECEIPT`s.
@@ -358,15 +385,17 @@ class Migrator
 
   # Remove `opt/oldname` link if it belongs to newname.
   def unlink_oldname_opt
-    return if old_opt_record.to_s.blank?
-    return unless old_opt_record.symlink?
-    return unless old_opt_record.exist?
     return unless new_linked_keg_record.exist?
-    return if new_linked_keg_record.realpath != old_opt_record.realpath
+
+    old_opt_records.each do |old_opt_record|
+      next unless old_opt_record.symlink?
+      next unless old_opt_record.exist?
+      next if new_linked_keg_record.realpath != old_opt_record.realpath
 
       old_opt_record.unlink
       old_opt_record.parent.rmdir_if_possible
     end
+  end
 
   # Remove `Cellar/oldname` if it exists.
   def link_oldname_cellar
@@ -399,17 +428,16 @@ class Migrator
       new_cellar.subdirs.each do |d|
         newname_keg = Keg.new(d)
         newname_keg.unlink(verbose: verbose?)
-        newname_keg.uninstall if new_cellar_existed
+        newname_keg.uninstall unless new_cellar_existed
       end
     end
 
-    return if old_linked_keg.nil?
+    return if old_linked_kegs.empty?
 
     # The keg used to be linked and when we backup everything we restore
     # Cellar/oldname, the target also gets restored, so we are able to
     # create a keg using its old path
-    if old_linked_keg_record
+    old_full_linked_kegs.each do |old_linked_keg|
-      begin
       old_linked_keg.link(verbose: verbose?)
     rescue Keg::LinkError
       old_linked_keg.unlink(verbose: verbose?)
@@ -418,7 +446,7 @@ class Migrator
       old_linked_keg.unlink(verbose: verbose?)
       retry
     end
-    else
+    (old_linked_kegs - old_full_linked_kegs).each do |old_linked_keg|
       old_linked_keg.optlink(verbose: verbose?)
     end
   end

Library/Homebrew/test/formula_spec.rb

@@ -269,7 +269,7 @@ describe Formula do
 
   specify "#migration_needed" do
     f = Testball.new("newname")
-    f.instance_variable_set(:@oldname, "oldname")
+    f.instance_variable_set(:@oldnames, ["oldname"])
     f.instance_variable_set(:@tap, CoreTap.instance)
 
     oldname_prefix = (HOMEBREW_CELLAR/"oldname/2.20")

Library/Homebrew/test/keg_spec.rb

@@ -53,21 +53,21 @@ describe Keg do
     expect(keg).not_to be_an_empty_installation
   end
 
-  specify "#oldname_opt_record" do
+  specify "#oldname_opt_records" do
-    expect(keg.oldname_opt_record).to be_nil
+    expect(keg.oldname_opt_records).to be_empty
     oldname_opt_record = HOMEBREW_PREFIX/"opt/oldfoo"
     oldname_opt_record.make_relative_symlink(HOMEBREW_CELLAR/"foo/1.0")
-    expect(keg.oldname_opt_record).to eq(oldname_opt_record)
+    expect(keg.oldname_opt_records).to eq([oldname_opt_record])
   end
 
-  specify "#remove_oldname_opt_record" do
+  specify "#remove_oldname_opt_records" do
     oldname_opt_record = HOMEBREW_PREFIX/"opt/oldfoo"
     oldname_opt_record.make_relative_symlink(HOMEBREW_CELLAR/"foo/2.0")
-    keg.remove_oldname_opt_record
+    keg.remove_oldname_opt_records
     expect(oldname_opt_record).to be_a_symlink
     oldname_opt_record.unlink
     oldname_opt_record.make_relative_symlink(HOMEBREW_CELLAR/"foo/1.0")
-    keg.remove_oldname_opt_record
+    keg.remove_oldname_opt_records
     expect(oldname_opt_record).not_to be_a_symlink
   end
 

Library/Homebrew/test/migrator_spec.rb

@@ -6,7 +6,7 @@ require "tab"
 require "keg"
 
 describe Migrator do
-  subject(:migrator) { described_class.new(new_formula) }
+  subject(:migrator) { described_class.new(new_formula, old_formula.name) }
 
   let(:new_formula) { Testball.new("newname") }
   let(:old_formula) { Testball.new("oldname") }
@@ -55,15 +55,9 @@ describe Migrator do
   end
 
   describe "::new" do
-    it "raises an error if there is no old name" do
-      expect do
-        described_class.new(old_formula)
-      end.to raise_error(Migrator::MigratorNoOldnameError)
-    end
-
     it "raises an error if there is no old path" do
       expect do
-        described_class.new(new_formula)
+        described_class.new(new_formula, "oldname")
       end.to raise_error(Migrator::MigratorNoOldpathError)
     end
 
@@ -76,7 +70,7 @@ describe Migrator do
       tab.write
 
       expect do
-        described_class.new(new_formula)
+        described_class.new(new_formula, "oldname")
       end.to raise_error(Migrator::MigratorDifferentTapsError)
     end
   end
@@ -212,7 +206,7 @@ describe Migrator do
     tab.tabfile = HOMEBREW_CELLAR/"oldname/0.1/INSTALL_RECEIPT.json"
     tab.source["path"] = "/should/be/the/same"
     tab.write
-    migrator = described_class.new(new_formula)
+    migrator = described_class.new(new_formula, "oldname")
     tab.tabfile.delete
     migrator.backup_old_tabs
     expect(Tab.for_keg(old_keg_record).source["path"]).to eq("/should/be/the/same")