From 54004a4759fa660c4294c00f1b4708f514948250 Mon Sep 17 00:00:00 2001 From: Tsukasa OMOTO Date: Thu, 24 Apr 2014 11:26:45 +0900 Subject: [PATCH] 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 --- Library/Homebrew/cmd/info.rb | 2 +- Library/Homebrew/cmd/search.rb | 21 +++--- Library/Homebrew/cmd/tap.rb | 27 +++++--- Library/Homebrew/cmd/untap.rb | 7 +- Library/Homebrew/cmd/update.rb | 67 ++++++++++++++----- Library/Homebrew/cmd/versions.rb | 2 +- Library/Homebrew/formulary.rb | 2 +- Library/Homebrew/global.rb | 4 +- .../test/fixtures/updater_fixture.yaml | 8 +-- Library/Homebrew/test/test_updater.rb | 6 +- Library/Homebrew/test/testing_env.rb | 2 +- 11 files changed, 95 insertions(+), 53 deletions(-) diff --git a/Library/Homebrew/cmd/info.rb b/Library/Homebrew/cmd/info.rb index 78e90d727f..9deda9dfb8 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 d5ccddacb8..991ca58b89 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§ion=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 4b00c21b62..f2815a9601 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 77e8f8688e..8d58e2a5b3 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 b340881df7..fcbffb11f7 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) + # rename Taps directories + # this procedure will be removed in the future if it seems unnecessasry + rename_taps_dir_if_necessary - cd tapd do - begin - updater = Updater.new - updater.pull! - report.merge!(updater.report) do |key, oldval, newval| - oldval.concat(newval) + 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 /. "\ + + "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 /. "\ + "#{tapd} is incorrect name format. You may need to rename it like / 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 563e1588fc..c8a67f32f4 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 diff --git a/Library/Homebrew/formulary.rb b/Library/Homebrew/formulary.rb index e8c47c9aad..bde6188d6b 100644 --- a/Library/Homebrew/formulary.rb +++ b/Library/Homebrew/formulary.rb @@ -159,7 +159,7 @@ class Formulary def initialize tapped_name @tapped_name = tapped_name user, repo, name = tapped_name.split("/", 3).map(&:downcase) - tap = Pathname.new("#{HOMEBREW_LIBRARY}/Taps/#{user}-#{repo}") + tap = Pathname.new("#{HOMEBREW_LIBRARY}/Taps/#{user}/homebrew-#{repo}") path = tap.join("#{name}.rb") if tap.directory? diff --git a/Library/Homebrew/global.rb b/Library/Homebrew/global.rb index dc2379b273..2a001256ee 100644 --- a/Library/Homebrew/global.rb +++ b/Library/Homebrew/global.rb @@ -85,8 +85,8 @@ HOMEBREW_USER_AGENT = "Homebrew #{HOMEBREW_VERSION} (Ruby #{RUBY_VERSION}-#{RUBY HOMEBREW_CURL_ARGS = '-f#LA' -HOMEBREW_TAP_FORMULA_REGEX = %r{^(\w+)/(\w+)/([^/]+)$} -HOMEBREW_TAP_DIR_REGEX = %r{#{HOMEBREW_LIBRARY}/Taps/(\w+)-(\w+)} +HOMEBREW_TAP_FORMULA_REGEX = %r{^([\w_-]+)/([\w_-]+)/([\w_-]+)$} +HOMEBREW_TAP_DIR_REGEX = %r{#{HOMEBREW_LIBRARY}/Taps/([\w_-]+)/([\w_-]+)} HOMEBREW_TAP_PATH_REGEX = Regexp.new(HOMEBREW_TAP_DIR_REGEX.source \ + %r{/(.*)}.source) diff --git a/Library/Homebrew/test/fixtures/updater_fixture.yaml b/Library/Homebrew/test/fixtures/updater_fixture.yaml index 7342793ffc..7c87f751fa 100644 --- a/Library/Homebrew/test/fixtures/updater_fixture.yaml +++ b/Library/Homebrew/test/fixtures/updater_fixture.yaml @@ -27,10 +27,10 @@ update_git_diff_output_with_formulae_changes: | :100644 100644 c0084b02faf7ab04abcffb3d5688e9d7c1ff7ae8 25cae981801e21cde048cd28747241d18093ca71 M bin/brew update_git_diff_output_with_tapped_formulae_changes: | :100644 100644 9dde0708e2db78e1aa697782bd2ab2ddfd8da984 ee0a81cbe3757e9a4b5aab1c41457f65fed9f258 M Library/Contributions/brew_bash_completion.sh - :100644 100644 741f11dcd29ec909a94fa54df4bcdd2c1d5bb47b 7535134e865994e8ad66ba02a924a1e6c1271b9e A Library/Taps/someuser-sometap/Formula/antiword.rb - :100644 100644 741f11dcd29ec909a94fa54df4bcdd2c1d5bb47b 7535134e865994e8ad66ba02a924a1e6c1271b9e A Library/Taps/someuser-sometap/HomebrewFormula/lua.rb - :100644 100644 741f11dcd29ec909a94fa54df4bcdd2c1d5bb47b 7535134e865994e8ad66ba02a924a1e6c1271b9e A Library/Taps/someuser-sometap/custom-formula.rb - :100644 100644 741f11dcd29ec909a94fa54df4bcdd2c1d5bb47b 7535134e865994e8ad66ba02a924a1e6c1271b9e A Library/Taps/someuser-sometap/lib/not-a-formula.rb + :100644 100644 741f11dcd29ec909a94fa54df4bcdd2c1d5bb47b 7535134e865994e8ad66ba02a924a1e6c1271b9e A Library/Taps/someuser/sometap/Formula/antiword.rb + :100644 100644 741f11dcd29ec909a94fa54df4bcdd2c1d5bb47b 7535134e865994e8ad66ba02a924a1e6c1271b9e A Library/Taps/someuser/sometap/HomebrewFormula/lua.rb + :100644 100644 741f11dcd29ec909a94fa54df4bcdd2c1d5bb47b 7535134e865994e8ad66ba02a924a1e6c1271b9e A Library/Taps/someuser/sometap/custom-formula.rb + :100644 100644 741f11dcd29ec909a94fa54df4bcdd2c1d5bb47b 7535134e865994e8ad66ba02a924a1e6c1271b9e A Library/Taps/someuser/sometap/lib/not-a-formula.rb update_git_diff_output_with_removed_formulae: | :000000 100644 0000000000000000000000000000000000000000 e62589998ef688f64aab10d85c3822dfa5cfb31c A Library/Formula/flac123.rb :100644 100644 bd7d76cf2fa9805ff23b9e8f48ecfb0e569aadd3 0cc2863c9be33ae946268407618f74897961873d M Library/Formula/gdal.rb diff --git a/Library/Homebrew/test/test_updater.rb b/Library/Homebrew/test/test_updater.rb index c87d7d4ecb..4b974e33f1 100644 --- a/Library/Homebrew/test/test_updater.rb +++ b/Library/Homebrew/test/test_updater.rb @@ -85,9 +85,9 @@ class UpdaterTests < Test::Unit::TestCase perform_update(fixture('update_git_diff_output_with_tapped_formulae_changes')) assert @updater.expectations_met? assert_equal [ - Pathname('someuser-sometap/Formula/antiword.rb'), - Pathname('someuser-sometap/HomebrewFormula/lua.rb'), - Pathname('someuser-sometap/custom-formula.rb'), + Pathname('someuser/sometap/Formula/antiword.rb'), + Pathname('someuser/sometap/HomebrewFormula/lua.rb'), + Pathname('someuser/sometap/custom-formula.rb'), ], @report.tapped_formula_for(:A) end diff --git a/Library/Homebrew/test/testing_env.rb b/Library/Homebrew/test/testing_env.rb index fc50394915..269fc7445d 100644 --- a/Library/Homebrew/test/testing_env.rb +++ b/Library/Homebrew/test/testing_env.rb @@ -27,7 +27,7 @@ HOMEBREW_WWW = 'http://example.com' HOMEBREW_CURL_ARGS = '-fsLA' HOMEBREW_VERSION = '0.9-test' -HOMEBREW_TAP_FORMULA_REGEX = %r{^(\w+)/(\w+)/([^/]+)$} +HOMEBREW_TAP_FORMULA_REGEX = %r{^([\w_-]+)/([\w_-]+)/([\w_-]+)$} RUBY_BIN = Pathname.new(RbConfig::CONFIG['bindir']) RUBY_PATH = RUBY_BIN + RbConfig::CONFIG['ruby_install_name'] + RbConfig::CONFIG['EXEEXT']