Make the on-disk representation of taps unambiguous

This commit supports "-" and "_" in names of user and repository.

Closes Homebrew/homebrew#28203.

Signed-off-by: Jack Nagel <jacknagel@gmail.com>

6 files changed, 85 insertions, 43 deletions

diff --git a/Library/Homebrew/cmd/info.rb b/Library/Homebrew/cmd/info.rb
index 78e90d727..9deda9dfb 100644
--- a/Library/Homebrew/cmd/info.rb
+++ b/Library/Homebrew/cmd/info.rb
@@ -61,7 +61,7 @@ module Homebrew extend self
   def github_info f
     if f.path.to_s =~ HOMEBREW_TAP_PATH_REGEX
       user = $1
-      repo = "homebrew-#$2"
+      repo = $2
       path = $3
     else
       user = f.path.parent.cd { github_fork }
diff --git a/Library/Homebrew/cmd/search.rb b/Library/Homebrew/cmd/search.rb
index d5ccddacb..991ca58b8 100644
--- a/Library/Homebrew/cmd/search.rb
+++ b/Library/Homebrew/cmd/search.rb
@@ -7,11 +7,6 @@ module Homebrew extend self
 
   SEARCH_ERROR_QUEUE = Queue.new
 
-  # A regular expession to capture the username (one or more char but no `/`,
-  # which has to be escaped like `\/`), repository, followed by an optional `/`
-  # and an optional query.
-  TAP_QUERY_REGEX = /^([^\/]+)\/([^\/]+)\/?(.+)?$/
-
   def search
     if ARGV.include? '--macports'
       exec_browser "http://www.macports.org/ports.php?by=name&substr=#{ARGV.next}"
@@ -27,17 +22,21 @@ module Homebrew extend self
       exec_browser "http://packages.ubuntu.com/search?keywords=#{ARGV.next}&searchon=names&suite=all&section=all"
     elsif (query = ARGV.first).nil?
       puts_columns Formula.names
-    elsif ARGV.first =~ TAP_QUERY_REGEX
+    elsif ARGV.first =~ HOMEBREW_TAP_FORMULA_REGEX
       # So look for user/repo/query or list all formulae by the tap
       # we downcase to avoid case-insensitive filesystem issues.
       user, repo, query = $1.downcase, $2.downcase, $3
-      tap_dir = HOMEBREW_LIBRARY/"Taps/#{user}-#{repo}"
+      tap_dir = HOMEBREW_LIBRARY/"Taps/#{user}/homebrew-#{repo}"
       # If, instead of `user/repo/query` the user wrote `user/repo query`:
       query = ARGV[1] if query.nil?
       if tap_dir.directory?
-        # There is a local tap already:
-        result = Dir["#{tap_dir}/*.rb"].map{ |f| File.basename(f).chomp('.rb') }
-        result = result.grep(query_regexp(query)) unless query.nil?
+        result = ""
+        if query
+          tap_dir.find_formula do |child|
+            basename = child.basename(".rb").to_s
+            result = basename if basename == query
+          end
+        end
       else
         # Search online:
         query = '' if query.nil?
@@ -104,7 +103,7 @@ module Homebrew extend self
   end
 
   def search_tap user, repo, rx
-    return [] if (HOMEBREW_LIBRARY/"Taps/#{user.downcase}-#{repo.downcase}").directory?
+    return [] if (HOMEBREW_LIBRARY/"Taps/#{user.downcase}/homebrew-#{repo.downcase}").directory?
 
     results = []
     GitHub.open "https://api.github.com/repos/#{user}/homebrew-#{repo}/git/trees/HEAD?recursive=1" do |json|
diff --git a/Library/Homebrew/cmd/tap.rb b/Library/Homebrew/cmd/tap.rb
index 4b00c21b6..f2815a960 100644
--- a/Library/Homebrew/cmd/tap.rb
+++ b/Library/Homebrew/cmd/tap.rb
@@ -3,9 +3,11 @@ module Homebrew extend self
   def tap
     if ARGV.empty?
       tapd = HOMEBREW_LIBRARY/"Taps"
-      tapd.children.each do |tap|
-        # only replace the *last* dash: yes, tap filenames suck
-        puts tap.basename.to_s.reverse.sub('-', '/').reverse if (tap/'.git').directory?
+      tapd.children.each do |user|
+        next unless user.directory?
+        user.children.each do |repo|
+          puts "#{user.basename}/#{repo.basename.sub("homebrew-", "")}" if (repo/".git").directory?
+        end
       end if tapd.directory?
     elsif ARGV.first == "--repair"
       repair_taps
@@ -22,12 +24,12 @@ module Homebrew extend self
     user = "homebrew" if user == "Homebrew"
 
     # we downcase to avoid case-insensitive filesystem issues
-    tapd = HOMEBREW_LIBRARY/"Taps/#{user.downcase}-#{repo.downcase}"
+    tapd = HOMEBREW_LIBRARY/"Taps/#{user.downcase}/homebrew-#{repo.downcase}"
     return false if tapd.directory?
     abort unless system "git clone https://github.com/#{repouser}/homebrew-#{repo} #{tapd}"
 
     files = []
-    tapd.find_formula{ |file| files << tapd.basename.join(file) }
+    tapd.find_formula{ |file| files << tapd.dirname.basename.join(tapd.basename, file) }
     link_tap_formula(files)
     puts "Tapped #{files.length} formula"
 
@@ -88,10 +90,13 @@ module Homebrew extend self
 
     count = 0
     # check symlinks are all set in each tap
-    HOMEBREW_REPOSITORY.join("Library/Taps").children.each do |tap|
-      files = []
-      tap.find_formula{ |file| files << tap.basename.join(file) } if tap.directory?
-      count += link_tap_formula(files)
+    HOMEBREW_REPOSITORY.join("Library/Taps").children.each do |user|
+      next unless user.directory?
+      user.children.each do |repo|
+        files = []
+        repo.find_formula{ |file| files << user.basename.join(repo.basename, file) } if repo.directory?
+        count += link_tap_formula(files)
+      end
     end
 
     puts "Tapped #{count} formula"
@@ -100,7 +105,7 @@ module Homebrew extend self
   private
 
   def tap_args
-    ARGV.first =~ %r{^(\S+)/(homebrew-)?(\w+)$}
+    ARGV.first =~ %r{^([\w_-]+)/(homebrew-)?([\w_-]+)$}
     raise "Invalid tap name" unless $1 && $3
     [$1, $3]
   end
@@ -118,7 +123,7 @@ end
 class Pathname
   def tap_ref
     case self.to_s
-    when %r{^#{HOMEBREW_LIBRARY}/Taps/([a-z\-_]+)-(\w+)/(.+)}
+    when %r{^#{HOMEBREW_LIBRARY}/Taps/([\w_-]+)/([\w_-]+)/(.+)}
       "#$1/#$2/#{File.basename($3, '.rb')}"
     when %r{^#{HOMEBREW_LIBRARY}/Formula/(.+)}
       "Homebrew/homebrew/#{File.basename($1, '.rb')}"
diff --git a/Library/Homebrew/cmd/untap.rb b/Library/Homebrew/cmd/untap.rb
index 77e8f8688..8d58e2a5b 100644
--- a/Library/Homebrew/cmd/untap.rb
+++ b/Library/Homebrew/cmd/untap.rb
@@ -13,14 +13,15 @@ module Homebrew extend self
     user.downcase!
     repo.downcase!
 
-    tapd = HOMEBREW_LIBRARY/"Taps/#{user}-#{repo}"
+    tapd = HOMEBREW_LIBRARY/"Taps/#{user}/homebrew-#{repo}"
 
     raise "No such tap!" unless tapd.directory?
 
     files = []
-    tapd.find_formula{ |file| files << Pathname.new("#{user}-#{repo}").join(file) }
+    tapd.find_formula{ |file| files << Pathname.new("#{user}/homebrew-#{repo}").join(file) }
     unlink_tap_formula(files)
-    rm_rf tapd
+    tapd.rmtree
+    tapd.dirname.rmdir_if_possible
     puts "Untapped #{files.length} formula"
   end
 
diff --git a/Library/Homebrew/cmd/update.rb b/Library/Homebrew/cmd/update.rb
index b340881df..fcbffb11f 100644
--- a/Library/Homebrew/cmd/update.rb
+++ b/Library/Homebrew/cmd/update.rb
@@ -34,19 +34,23 @@ module Homebrew extend self
     end
     report.merge!(master_updater.report)
 
-    Dir["Library/Taps/*"].each do |tapd|
-      next unless File.directory?(tapd)
-
-      cd tapd do
-        begin
-          updater = Updater.new
-          updater.pull!
-          report.merge!(updater.report) do |key, oldval, newval|
-            oldval.concat(newval)
+    # rename Taps directories
+    # this procedure will be removed in the future if it seems unnecessasry
+    rename_taps_dir_if_necessary
+
+    Dir["Library/Taps/*/"].each do |user|
+      Dir["#{user}*/"].each do |repo|
+        cd repo do
+          begin
+            updater = Updater.new
+            updater.pull!
+            report.merge!(updater.report) do |key, oldval, newval|
+              oldval.concat(newval)
+            end
+          rescue
+            repo =~ %r{^Library/Taps/([\w_-]+)/(homebrew-)?([\w_-]+)}
+            onoe "Failed to update tap: #$1/#$3"
           end
-        rescue
-          tapd =~ %r{^Library/Taps/(\w+)-(\w+)}
-          onoe "Failed to update tap: #$1/#$2"
         end
       end
     end
@@ -92,6 +96,39 @@ module Homebrew extend self
     raise
   end
 
+  def rename_taps_dir_if_necessary
+    need_repair_taps = false
+    Dir["#{HOMEBREW_LIBRARY}/Taps/*/"].each do |tapd|
+      begin
+        tapd_basename = File.basename(tapd)
+
+        if File.directory?(tapd + "/.git")
+          if tapd_basename.include?("-")
+            # only replace the *last* dash: yes, tap filenames suck
+            user, repo = tapd_basename.reverse.sub("-", "/").reverse.split("/")
+
+            FileUtils.mkdir_p("#{HOMEBREW_LIBRARY}/Taps/#{user.downcase}")
+            FileUtils.mv(tapd, "#{HOMEBREW_LIBRARY}/Taps/#{user.downcase}/homebrew-#{repo.downcase}")
+            need_repair_taps = true
+
+            if tapd_basename.count("-") >= 2
+              opoo "Homebrew changed the structure of Taps like <someuser>/<sometap>. "\
+                + "So you may need to rename #{HOMEBREW_LIBRARY}/Taps/#{user.downcase}/homebrew-#{repo.downcase} manually."
+            end
+          else
+            opoo "Homebrew changed the structure of Taps like <someuser>/<sometap>. "\
+              "#{tapd} is incorrect name format. You may need to rename it like <someuser>/<sometap> manually."
+          end
+        end
+      rescue => ex
+        onoe ex.message
+        next # next tap directory
+      end
+    end
+
+    repair_taps if need_repair_taps
+  end
+
   def load_tap_migrations
     require 'tap_migrations'
   rescue LoadError
@@ -184,7 +221,7 @@ class Report < Hash
 
   def tapped_formula_for key
     fetch(key, []).map do |path|
-      case path when %r{^Library/Taps/(\w+-\w+/.*)}
+      case path when %r{^Library/Taps/([\w_-]+/[\w_-]+/.*)}
         relative_path = $1
         if valid_formula_location?(relative_path)
           Pathname.new(relative_path)
@@ -195,7 +232,7 @@ class Report < Hash
 
   def valid_formula_location?(relative_path)
     ruby_file = /\A.*\.rb\Z/
-    parts = relative_path.split('/')[1..-1]
+    parts = relative_path.split('/')[2..-1]
     [
       parts.length == 1 && parts.first =~ ruby_file,
       parts.length == 2 && parts.first == 'Formula' && parts.last =~ ruby_file,
@@ -215,8 +252,8 @@ class Report < Hash
     fetch(key, []).map do |path|
       case path when %r{^Library/Formula}
         File.basename(path, ".rb")
-      when %r{^Library/Taps/(\w+)-(\w+)/(.*)\.rb}
-        "#$1/#$2/#{File.basename(path, '.rb')}"
+      when %r{^Library/Taps/([\w_-]+)/(homebrew-)?([\w_-]+)/(.*)\.rb}
+        "#$1/#$3/#{File.basename(path, '.rb')}"
       end
     end.compact.sort
   end
diff --git a/Library/Homebrew/cmd/versions.rb b/Library/Homebrew/cmd/versions.rb
index 563e1588f..c8a67f32f 100644
--- a/Library/Homebrew/cmd/versions.rb
+++ b/Library/Homebrew/cmd/versions.rb
@@ -66,7 +66,7 @@ class Formula
     def repository
       @repository ||= begin
         if path.to_s =~ HOMEBREW_TAP_DIR_REGEX
-          HOMEBREW_REPOSITORY/"Library/Taps/#$1-#$2"
+          HOMEBREW_REPOSITORY/"Library/Taps/#$1/#$2"
         else
           HOMEBREW_REPOSITORY
         end