From 5b3bbb76c9d224c08ae498677868b69aab71ff7c Mon Sep 17 00:00:00 2001
From: Markus Reiter <me@reitermark.us>
Date: Sun, 1 Jul 2018 23:35:29 +0200
Subject: [PATCH 1/8] Separate staging from download.

---
 Library/Homebrew/download_strategy.rb         | 250 +++++--------
 Library/Homebrew/extend/pathname.rb           |  41 ---
 Library/Homebrew/formula_installer.rb         |   2 +
 Library/Homebrew/sandbox.rb                   |   9 +
 .../Homebrew/test/download_strategies_spec.rb |  19 -
 .../Homebrew/test/support/fixtures/test.jar   | Bin 0 -> 397 bytes
 .../Homebrew/test/support/fixtures/test.lha   | Bin 0 -> 51 bytes
 .../Homebrew/test/support/fixtures/test.lz    | Bin 0 -> 36 bytes
 Library/Homebrew/test/unpack_strategy_spec.rb | 127 +++++++
 Library/Homebrew/unpack_strategy.rb           | 344 ++++++++++++++++++
 10 files changed, 582 insertions(+), 210 deletions(-)
 create mode 100644 Library/Homebrew/test/support/fixtures/test.jar
 create mode 100644 Library/Homebrew/test/support/fixtures/test.lha
 create mode 100644 Library/Homebrew/test/support/fixtures/test.lz
 create mode 100644 Library/Homebrew/test/unpack_strategy_spec.rb
 create mode 100644 Library/Homebrew/unpack_strategy.rb

diff --git a/Library/Homebrew/download_strategy.rb b/Library/Homebrew/download_strategy.rb
index c7a273901996f..3a8f3d24a6f31 100644
--- a/Library/Homebrew/download_strategy.rb
+++ b/Library/Homebrew/download_strategy.rb
@@ -1,6 +1,7 @@
 require "json"
 require "rexml/document"
 require "time"
+require "unpack_strategy"
 
 class AbstractDownloadStrategy
   extend Forwardable
@@ -45,7 +46,10 @@ def ohai(*args)
   # Unpack {#cached_location} into the current working directory, and possibly
   # chdir into the newly-unpacked directory.
   # Unlike {Resource#stage}, this does not take a block.
-  def stage; end
+  def stage
+    UnpackStrategy.detect(cached_location)
+                  .extract(basename: basename_without_params)
+  end
 
   # @!attribute [r] cached_location
   # The path to the cached file or directory associated with the resource.
@@ -63,22 +67,6 @@ def clear_cache
     rm_rf(cached_location)
   end
 
-  def expand_safe_system_args(args)
-    args = args.dup
-    args.each_with_index do |arg, ii|
-      next unless arg.is_a? Hash
-      if ARGV.verbose?
-        args.delete_at ii
-      else
-        args[ii] = arg[:quiet_flag]
-      end
-      return args
-    end
-    # 2 as default because commands are eg. svn up, git pull
-    args.insert(2, "-q") unless ARGV.verbose?
-    args
-  end
-
   def safe_system(*args)
     if @shutup
       quiet_system(*args) || raise(ErrorDuringExecution.new(args.shift, args))
@@ -87,8 +75,9 @@ def safe_system(*args)
     end
   end
 
-  def quiet_safe_system(*args)
-    safe_system(*expand_safe_system_args(args))
+  def basename_without_params
+    # Strip any ?thing=wad out of .c?thing=wad style extensions
+    File.basename(@url)[/[^?]+/]
   end
 end
 
@@ -152,7 +141,7 @@ def last_commit
   private
 
   def cache_tag
-    "__UNKNOWN__"
+    raise NotImplementedError
   end
 
   def cache_filename
@@ -160,7 +149,7 @@ def cache_filename
   end
 
   def repo_valid?
-    true
+    raise NotImplementedError
   end
 
   def clone_repo; end
@@ -177,40 +166,8 @@ def extract_ref(specs)
 
 class AbstractFileDownloadStrategy < AbstractDownloadStrategy
   def stage
-    path = cached_location
-    unpack_dir = Pathname.pwd
-
-    case type = path.compression_type
-    when :zip
-      safe_system "unzip", "-qq", path, "-d", unpack_dir
-      chdir
-    when :gzip_only
-      FileUtils.cp path, unpack_dir, preserve: true
-      safe_system "gunzip", "-q", "-N", unpack_dir/path.basename
-    when :bzip2_only
-      FileUtils.cp path, unpack_dir, preserve: true
-      safe_system "bunzip2", "-q", unpack_dir/path.basename
-    when :gzip, :bzip2, :xz, :compress, :tar
-      if type == :xz && DependencyCollector.tar_needs_xz_dependency?
-        pipe_to_tar "#{HOMEBREW_PREFIX}/opt/xz/bin/xz", unpack_dir
-      else
-        safe_system "tar", "xf", path, "-C", unpack_dir
-      end
-      chdir
-    when :lzip
-      pipe_to_tar "#{HOMEBREW_PREFIX}/opt/lzip/bin/lzip", unpack_dir
-      chdir
-    when :lha
-      safe_system "#{HOMEBREW_PREFIX}/opt/lha/bin/lha", "xq2w=#{unpack_dir}", path
-    when :xar
-      safe_system "xar", "-x", "-f", path, "-C", unpack_dir
-    when :rar
-      safe_system "unrar", "x", "-inul", path, unpack_dir
-    when :p7zip
-      safe_system "7zr", "x", "-y", "-bd", "-bso0", path, "-o#{unpack_dir}"
-    else
-      cp path, unpack_dir/basename_without_params, preserve: true
-    end
+    super
+    chdir
   end
 
   private
@@ -227,22 +184,6 @@ def chdir
     end
   end
 
-  def pipe_to_tar(tool, unpack_dir)
-    path = cached_location
-
-    Utils.popen_read(tool, "-dc", path) do |rd|
-      Utils.popen_write("tar", "xf", "-", "-C", unpack_dir) do |wr|
-        buf = ""
-        wr.write(buf) while rd.read(16384, buf)
-      end
-    end
-  end
-
-  def basename_without_params
-    # Strip any ?thing=wad out of .c?thing=wad style extensions
-    File.basename(@url)[/[^?]+/]
-  end
-
   def ext
     # We need a Pathname because we've monkeypatched extname to support double
     # extensions (e.g. tar.gz).
@@ -384,7 +325,8 @@ def _fetch
 # Useful for installing jars.
 class NoUnzipCurlDownloadStrategy < CurlDownloadStrategy
   def stage
-    cp cached_location, basename_without_params, preserve: true
+    UncompressedUnpackStrategy.new(cached_location)
+                              .extract(basename: basename_without_params)
   end
 end
 
@@ -607,11 +549,6 @@ def fetch
     super
   end
 
-  def stage
-    super
-    safe_system "svn", "export", "--force", cached_location, Dir.pwd
-  end
-
   def source_modified_time
     xml = REXML::Document.new(Utils.popen_read("svn", "info", "--xml", cached_location.to_s))
     Time.parse REXML::XPath.first(xml, "//date/text()").to_s
@@ -647,7 +584,7 @@ def fetch_repo(target, url, revision = nil, ignore_externals = false)
       args << "-r" << revision
     end
     args << "--ignore-externals" if ignore_externals
-    quiet_safe_system(*args)
+    safe_system(*args)
   end
 
   def cache_tag
@@ -692,11 +629,6 @@ def initialize(name, resource)
     @shallow = meta.fetch(:shallow) { true }
   end
 
-  def stage
-    super
-    cp_r File.join(cached_location, "."), Dir.pwd, preserve: true
-  end
-
   def source_modified_time
     Time.parse Utils.popen_read("git", "--git-dir", git_dir, "show", "-s", "--format=%cD")
   end
@@ -929,10 +861,6 @@ def initialize(name, resource)
     end
   end
 
-  def cvspath
-    @cvspath ||= which("cvs", PATH.new("/usr/bin", Formula["cvs"].opt_bin, ENV["PATH"]))
-  end
-
   def source_modified_time
     # Filter CVS's files because the timestamp for each of them is the moment
     # of clone.
@@ -946,10 +874,6 @@ def source_modified_time
     max_mtime
   end
 
-  def stage
-    cp_r File.join(cached_location, "."), Dir.pwd, preserve: true
-  end
-
   private
 
   def cache_tag
@@ -960,16 +884,23 @@ def repo_valid?
     (cached_location/"CVS").directory?
   end
 
+  def quiet_flag
+    "-Q" unless ARGV.verbose?
+  end
+
   def clone_repo
-    HOMEBREW_CACHE.cd do
+    with_cvs_env do
       # Login is only needed (and allowed) with pserver; skip for anoncvs.
-      quiet_safe_system cvspath, { quiet_flag: "-Q" }, "-d", @url, "login" if @url.include? "pserver"
-      quiet_safe_system cvspath, { quiet_flag: "-Q" }, "-d", @url, "checkout", "-d", cache_filename, @module
+      safe_system "cvs", *quiet_flag, "-d", @url, "login" if @url.include? "pserver"
+      safe_system "cvs", *quiet_flag, "-d", @url, "checkout", "-d", cached_location.basename, @module,
+                  chdir: cached_location.dirname
     end
   end
 
   def update
-    cached_location.cd { quiet_safe_system cvspath, { quiet_flag: "-Q" }, "up" }
+    with_cvs_env do
+      safe_system "cvs", *quiet_flag, "update", chdir: cached_location
+    end
   end
 
   def split_url(in_url)
@@ -978,6 +909,12 @@ def split_url(in_url)
     url = parts.join(":")
     [mod, url]
   end
+
+  def with_cvs_env
+    with_env PATH => PATH.new("/usr/bin", Formula["cvs"].opt_bin, ENV["PATH"]) do
+      yield
+    end
+  end
 end
 
 class MercurialDownloadStrategy < VCSDownloadStrategy
@@ -986,30 +923,16 @@ def initialize(name, resource)
     @url = @url.sub(%r{^hg://}, "")
   end
 
-  def hgpath
-    @hgpath ||= which("hg", PATH.new(Formula["mercurial"].opt_bin, ENV["PATH"]))
-  end
-
-  def stage
-    super
-
-    dst = Dir.getwd
-    cached_location.cd do
-      if @ref_type && @ref
-        ohai "Checking out #{@ref_type} #{@ref}" if @ref_type && @ref
-        safe_system hgpath, "archive", "--subrepos", "-y", "-r", @ref, "-t", "files", dst
-      else
-        safe_system hgpath, "archive", "--subrepos", "-y", "-t", "files", dst
-      end
-    end
-  end
-
   def source_modified_time
-    Time.parse Utils.popen_read(hgpath, "tip", "--template", "{date|isodate}", "-R", cached_location.to_s)
+    with_hg_env do
+      Time.parse Utils.popen_read("hg", "tip", "--template", "{date|isodate}", "-R", cached_location.to_s)
+    end
   end
 
   def last_commit
-    Utils.popen_read(hgpath, "parent", "--template", "{node|short}", "-R", cached_location.to_s)
+    with_hg_env do
+      Utils.popen_read("hg", "parent", "--template", "{node|short}", "-R", cached_location.to_s)
+    end
   end
 
   private
@@ -1023,12 +946,29 @@ def repo_valid?
   end
 
   def clone_repo
-    safe_system hgpath, "clone", @url, cached_location
+    with_hg_env do
+      safe_system "hg", "clone", @url, cached_location
+    end
   end
 
   def update
-    cached_location.cd do
-      safe_system hgpath, "pull", "--update"
+    with_hg_env do
+      safe_system "hg", "--cwd", cached_location, "pull", "--update"
+
+      update_args = if @ref_type && @ref
+        ohai "Checking out #{@ref_type} #{@ref}"
+        [@ref]
+      else
+        ["--clean"]
+      end
+
+      safe_system "hg", "--cwd", cached_location, "update", *update_args
+    end
+  end
+
+  def with_hg_env
+    with_env PATH => PATH.new(Formula["mercurial"].opt_bin, ENV["PATH"]) do
+      yield
     end
   end
 end
@@ -1040,25 +980,18 @@ def initialize(name, resource)
     ENV["BZR_HOME"] = HOMEBREW_TEMP
   end
 
-  def bzrpath
-    @bzrpath ||= which("bzr", PATH.new(Formula["bazaar"].opt_bin, ENV["PATH"]))
-  end
-
-  def stage
-    # The export command doesn't work on checkouts
-    # See https://bugs.launchpad.net/bzr/+bug/897511
-    cp_r File.join(cached_location, "."), Dir.pwd, preserve: true
-    rm_r ".bzr"
-  end
-
   def source_modified_time
-    timestamp = Utils.popen_read(bzrpath, "log", "-l", "1", "--timezone=utc", cached_location.to_s)[/^timestamp: (.+)$/, 1]
+    timestamp = with_bazaar_env do
+      Utils.popen_read("bzr", "log", "-l", "1", "--timezone=utc", cached_location.to_s)[/^timestamp: (.+)$/, 1]
+    end
     raise "Could not get any timestamps from bzr!" if timestamp.to_s.empty?
     Time.parse timestamp
   end
 
   def last_commit
-    Utils.popen_read(bzrpath, "revno", cached_location.to_s).chomp
+    with_bazaar_env do
+      Utils.popen_read("bzr", "revno", cached_location.to_s).chomp
+    end
   end
 
   private
@@ -1072,13 +1005,21 @@ def repo_valid?
   end
 
   def clone_repo
-    # "lightweight" means history-less
-    safe_system bzrpath, "checkout", "--lightweight", @url, cached_location
+    with_bazaar_env do
+      # "lightweight" means history-less
+      safe_system "bzr", "checkout", "--lightweight", @url, cached_location
+    end
   end
 
   def update
-    cached_location.cd do
-      safe_system bzrpath, "update"
+    with_bazaar_env do
+      safe_system "bzr", "update", chdir: cached_location
+    end
+  end
+
+  def with_bazaar_env
+    with_env "PATH" => PATH.new(Formula["bazaar"].opt_bin, ENV["PATH"]) do
+      yield
     end
   end
 end
@@ -1089,23 +1030,22 @@ def initialize(name, resource)
     @url = @url.sub(%r{^fossil://}, "")
   end
 
-  def fossilpath
-    @fossilpath ||= which("fossil", PATH.new(Formula["fossil"].opt_bin, ENV["PATH"]))
-  end
-
-  def stage
-    super
-    args = [fossilpath, "open", cached_location]
-    args << @ref if @ref_type && @ref
-    safe_system(*args)
-  end
-
   def source_modified_time
-    Time.parse Utils.popen_read(fossilpath, "info", "tip", "-R", cached_location.to_s)[/^uuid: +\h+ (.+)$/, 1]
+    with_fossil_env do
+      Time.parse Utils.popen_read("fossil", "info", "tip", "-R", cached_location.to_s)[/^uuid: +\h+ (.+)$/, 1]
+    end
   end
 
   def last_commit
-    Utils.popen_read(fossilpath, "info", "tip", "-R", cached_location.to_s)[/^uuid: +(\h+) .+$/, 1]
+    with_fossil_env do
+      Utils.popen_read("fossil", "info", "tip", "-R", cached_location.to_s)[/^uuid: +(\h+) .+$/, 1]
+    end
+  end
+
+  def repo_valid?
+    with_fossil_env do
+      quiet_system "fossil", "branch", "-R", cached_location
+    end
   end
 
   private
@@ -1115,11 +1055,21 @@ def cache_tag
   end
 
   def clone_repo
-    safe_system fossilpath, "clone", @url, cached_location
+    with_fossil_env do
+      safe_system "fossil", "clone", @url, cached_location
+    end
   end
 
   def update
-    safe_system fossilpath, "pull", "-R", cached_location
+    with_fossil_env do
+      safe_system "fossil", "pull", "-R", cached_location
+    end
+  end
+
+  def with_fossil_env
+    with_env "PATH" => PATH.new(Formula["fossil"].opt_bin, ENV["PATH"]) do
+      yield
+    end
   end
 end
 
diff --git a/Library/Homebrew/extend/pathname.rb b/Library/Homebrew/extend/pathname.rb
index a138be3e2567a..3f26cc1833f04 100644
--- a/Library/Homebrew/extend/pathname.rb
+++ b/Library/Homebrew/extend/pathname.rb
@@ -263,47 +263,6 @@ def version
     Version.parse(basename)
   end
 
-  # @private
-  def compression_type
-    case extname
-    when ".jar", ".war"
-      # Don't treat jars or wars as compressed
-      return
-    when ".gz"
-      # If the filename ends with .gz not preceded by .tar
-      # then we want to gunzip but not tar
-      return :gzip_only
-    when ".bz2"
-      return :bzip2_only
-    when ".lha", ".lzh"
-      return :lha
-    end
-
-    # Get enough of the file to detect common file types
-    # POSIX tar magic has a 257 byte offset
-    # magic numbers stolen from /usr/share/file/magic/
-    case open("rb") { |f| f.read(262) }
-    when /^PK\003\004/n         then :zip
-    when /^\037\213/n           then :gzip
-    when /^BZh/n                then :bzip2
-    when /^\037\235/n           then :compress
-    when /^.{257}ustar/n        then :tar
-    when /^\xFD7zXZ\x00/n       then :xz
-    when /^LZIP/n               then :lzip
-    when /^Rar!/n               then :rar
-    when /^7z\xBC\xAF\x27\x1C/n then :p7zip
-    when /^xar!/n               then :xar
-    when /^\xed\xab\xee\xdb/n   then :rpm
-    else
-      # This code so that bad-tarballs and zips produce good error messages
-      # when they don't unarchive properly.
-      case extname
-      when ".tar.gz", ".tgz", ".tar.bz2", ".tbz" then :tar
-      when ".zip" then :zip
-      end
-    end
-  end
-
   # @private
   def text_executable?
     /^#!\s*\S+/ =~ open("r") { |f| f.read(1024) }
diff --git a/Library/Homebrew/formula_installer.rb b/Library/Homebrew/formula_installer.rb
index 9b1e7a00b272b..40c4de5e1c92a 100644
--- a/Library/Homebrew/formula_installer.rb
+++ b/Library/Homebrew/formula_installer.rb
@@ -728,6 +728,8 @@ def build
         sandbox.allow_write_path(ENV["HOME"]) if ARGV.interactive?
         sandbox.allow_write_temp_and_cache
         sandbox.allow_write_log(formula)
+        sandbox.allow_cvs
+        sandbox.allow_fossil
         sandbox.allow_write_xcode
         sandbox.allow_write_cellar(formula)
         sandbox.exec(*args)
diff --git a/Library/Homebrew/sandbox.rb b/Library/Homebrew/sandbox.rb
index bda767970b6bc..6ec24a8367540 100644
--- a/Library/Homebrew/sandbox.rb
+++ b/Library/Homebrew/sandbox.rb
@@ -54,6 +54,15 @@ def allow_write_temp_and_cache
     allow_write_path HOMEBREW_CACHE
   end
 
+  def allow_cvs
+    allow_write_path "/Users/#{ENV["USER"]}/.cvspass"
+  end
+
+  def allow_fossil
+    allow_write_path "/Users/#{ENV["USER"]}/.fossil"
+    allow_write_path "/Users/#{ENV["USER"]}/.fossil-journal"
+  end
+
   def allow_write_cellar(formula)
     allow_write_path formula.rack
     allow_write_path formula.etc
diff --git a/Library/Homebrew/test/download_strategies_spec.rb b/Library/Homebrew/test/download_strategies_spec.rb
index f4857787ef36a..6f410358edd1f 100644
--- a/Library/Homebrew/test/download_strategies_spec.rb
+++ b/Library/Homebrew/test/download_strategies_spec.rb
@@ -9,25 +9,6 @@
   let(:resource) { double(Resource, url: url, mirrors: [], specs: specs, version: nil) }
   let(:args) { %w[foo bar baz] }
 
-  describe "#expand_safe_system_args" do
-    it "works with an explicit quiet flag" do
-      args << { quiet_flag: "--flag" }
-      expanded_args = subject.expand_safe_system_args(args)
-      expect(expanded_args).to eq(%w[foo bar baz --flag])
-    end
-
-    it "adds an implicit quiet flag" do
-      expanded_args = subject.expand_safe_system_args(args)
-      expect(expanded_args).to eq(%w[foo bar -q baz])
-    end
-
-    it "does not mutate the arguments" do
-      result = subject.expand_safe_system_args(args)
-      expect(args).to eq(%w[foo bar baz])
-      expect(result).not_to be args
-    end
-  end
-
   specify "#source_modified_time" do
     FileUtils.mktemp "mtime" do
       FileUtils.touch "foo", mtime: Time.now - 10
diff --git a/Library/Homebrew/test/support/fixtures/test.jar b/Library/Homebrew/test/support/fixtures/test.jar
new file mode 100644
index 0000000000000000000000000000000000000000..15a8adbd567eb307f490597a6e31fd459e00df3e
GIT binary patch
literal 397
zcmWIWW@h1H0D-f!ANqh9P=b>|hQZf0#8KDN&rLrxgp+}JBfn+zb|5aT;AUWC`O3(^
zz#;-v8~`)|M00?Rs5{Dc-xkQT17Z<mBYYkGJl$M_L-c&z;HGWDH0`tRnbSVrx_TFR
zy>+$DojJcb$l!|cgCadIUmZ{1i>0erKJk9x`NAV=@~2y9a&eI8oX_1{pSp#-8?sDy
z0-eIhB*%=)KN3JEfq($RTSpL$<U3Y~?=XA@G7gvDAjSbhfniCbGqQ0gKE-7+D;vlO
NOhC92NS_077yuSXRjB{~

literal 0
HcmV?d00001

diff --git a/Library/Homebrew/test/support/fixtures/test.lha b/Library/Homebrew/test/support/fixtures/test.lha
new file mode 100644
index 0000000000000000000000000000000000000000..4b4ac10d8b310afae19042201b457b1557cfb0e2
GIT binary patch
literal 51
zcmXqF(9Ov(&}D!E346<E1ttcDP*w(p*ZJ%Wj3ueXC9DhqOB&f30!0|UG6K~B0MQ2s
A_5c6?

literal 0
HcmV?d00001

diff --git a/Library/Homebrew/test/support/fixtures/test.lz b/Library/Homebrew/test/support/fixtures/test.lz
new file mode 100644
index 0000000000000000000000000000000000000000..ec607250eb85b946c420050d70e7514ca2b565f6
GIT binary patch
literal 36
acmeZ?@(f_)VQBvU`~Uv~45&Z_$^`(o*#@it

literal 0
HcmV?d00001

diff --git a/Library/Homebrew/test/unpack_strategy_spec.rb b/Library/Homebrew/test/unpack_strategy_spec.rb
new file mode 100644
index 0000000000000..2ffca9852aef4
--- /dev/null
+++ b/Library/Homebrew/test/unpack_strategy_spec.rb
@@ -0,0 +1,127 @@
+require "unpack_strategy"
+
+describe UnpackStrategy, :focus do
+  matcher :be_detected_as_a do |klass|
+    match do |expected|
+      @detected = described_class.detect(expected)
+      @detected.is_a?(klass)
+    end
+
+    failure_message do
+      <<~EOS
+        expected: #{klass}
+        detected: #{@detected}
+      EOS
+    end
+  end
+
+  describe "::detect" do
+    it "correctly detects JAR files" do
+      expect(TEST_FIXTURE_DIR/"test.jar").to be_detected_as_a UncompressedUnpackStrategy
+    end
+
+    it "correctly detects ZIP files" do
+      expect(TEST_FIXTURE_DIR/"cask/MyFancyApp.zip").to be_detected_as_a ZipUnpackStrategy
+    end
+
+    it "correctly detects BZIP2 files" do
+      expect(TEST_FIXTURE_DIR/"cask/container.bz2").to be_detected_as_a Bzip2UnpackStrategy
+    end
+
+    it "correctly detects GZIP files" do
+      expect(TEST_FIXTURE_DIR/"cask/container.gz").to be_detected_as_a GzipUnpackStrategy
+    end
+
+    it "correctly detects compressed TAR files" do
+      expect(TEST_FIXTURE_DIR/"cask/container.tar.gz").to be_detected_as_a TarUnpackStrategy
+    end
+
+    it "correctly detects 7-ZIP files" do
+      expect(TEST_FIXTURE_DIR/"cask/container.7z").to be_detected_as_a P7ZipUnpackStrategy
+    end
+
+    it "correctly detects XAR files" do
+      expect(TEST_FIXTURE_DIR/"cask/container.xar").to be_detected_as_a XarUnpackStrategy
+    end
+
+    it "correctly detects XZ files" do
+      expect(TEST_FIXTURE_DIR/"cask/container.xz").to be_detected_as_a XzUnpackStrategy
+    end
+
+    it "correctly detects RAR files" do
+      expect(TEST_FIXTURE_DIR/"cask/container.rar").to be_detected_as_a RarUnpackStrategy
+    end
+
+    it "correctly detects LZIP files" do
+      expect(TEST_FIXTURE_DIR/"test.lz").to be_detected_as_a LzipUnpackStrategy
+    end
+
+    it "correctly detects LHA files" do
+      expect(TEST_FIXTURE_DIR/"test.lha").to be_detected_as_a LhaUnpackStrategy
+    end
+
+    it "correctly detects Git repositories" do
+      mktmpdir do |repo|
+        system "git", "-C", repo, "init"
+
+        expect(repo).to be_detected_as_a GitUnpackStrategy
+      end
+    end
+
+    it "correctly detects Subversion repositories" do
+      mktmpdir do |path|
+        repo = path/"repo"
+        working_copy = path/"working_copy"
+
+        system "svnadmin", "create", repo
+        system "svn", "checkout", "file://#{repo}", working_copy
+
+        expect(working_copy).to be_detected_as_a SubversionUnpackStrategy
+      end
+    end
+  end
+end
+
+describe GitUnpackStrategy do
+  describe "#extract" do
+    it "correctly extracts a Subversion repository" do
+      mktmpdir do |path|
+        repo = path/"repo"
+
+        repo.mkpath
+
+        system "git", "-C", repo, "init"
+
+        FileUtils.touch repo/"test"
+        system "git", "-C", repo, "add", "test"
+        system "git", "-C", repo, "commit", "-m", "Add `test` file."
+
+        unpack_dir = path/"unpack_dir"
+        GitUnpackStrategy.new(repo).extract(to: unpack_dir)
+        expect(unpack_dir.children(false).map(&:to_s)).to match_array [".git", "test"]
+      end
+    end
+  end
+end
+
+describe SubversionUnpackStrategy do
+  describe "#extract" do
+    it "correctly extracts a Subversion repository" do
+      mktmpdir do |path|
+        repo = path/"repo"
+        working_copy = path/"working_copy"
+
+        system "svnadmin", "create", repo
+        system "svn", "checkout", "file://#{repo}", working_copy
+
+        FileUtils.touch working_copy/"test"
+        system "svn", "add", working_copy/"test"
+        system "svn", "commit", working_copy, "-m", "Add `test` file."
+
+        unpack_dir = path/"unpack_dir"
+        SubversionUnpackStrategy.new(working_copy).extract(to: unpack_dir)
+        expect(unpack_dir.children(false).map(&:to_s)).to match_array ["test"]
+      end
+    end
+  end
+end
diff --git a/Library/Homebrew/unpack_strategy.rb b/Library/Homebrew/unpack_strategy.rb
new file mode 100644
index 0000000000000..f57109c6d7d35
--- /dev/null
+++ b/Library/Homebrew/unpack_strategy.rb
@@ -0,0 +1,344 @@
+class UnpackStrategy
+  # length of the longest regex (currently TarUnpackStrategy)
+  MAX_MAGIC_NUMBER_LENGTH = 262
+  private_constant :MAX_MAGIC_NUMBER_LENGTH
+
+  def self.strategies
+    @strategies ||= [
+      JarUnpackStrategy,
+      ZipUnpackStrategy,
+      XarUnpackStrategy,
+      CompressUnpackStrategy,
+      TarUnpackStrategy,
+      GzipUnpackStrategy,
+      Bzip2UnpackStrategy,
+      XzUnpackStrategy,
+      LzipUnpackStrategy,
+      GitUnpackStrategy,
+      MercurialUnpackStrategy,
+      SubversionUnpackStrategy,
+      CvsUnpackStrategy,
+      FossilUnpackStrategy,
+      BazaarUnpackStrategy,
+      P7ZipUnpackStrategy,
+      RarUnpackStrategy,
+      LhaUnpackStrategy,
+    ].freeze
+  end
+  private_class_method :strategies
+
+  def self.detect(path)
+    magic_number = if path.directory?
+      ""
+    else
+      File.binread(path, MAX_MAGIC_NUMBER_LENGTH)
+    end
+
+    strategy = strategies.detect do |s|
+      s.can_extract?(path: path, magic_number: magic_number)
+    end
+
+    # This is so that bad files produce good error messages.
+    strategy ||= case path.extname
+    when ".tar.gz", ".tgz", ".tar.bz2", ".tbz", ".tar.xz", ".txz"
+      TarUnpackStrategy
+    when ".zip"
+      ZipUnpackStrategy
+    else
+      UncompressedUnpackStrategy
+    end
+
+    strategy.new(path)
+  end
+
+  attr_reader :path
+
+  def initialize(path)
+    @path = Pathname(path).expand_path
+  end
+
+  def extract(to: nil, basename: nil)
+    unpack_dir = Pathname(to || Dir.pwd).expand_path
+    unpack_dir.mkpath
+    extract_to_dir(unpack_dir, basename: basename)
+  end
+
+  private
+
+  def extract_to_dir(_unpack_dir, basename:)
+    raise NotImplementedError
+  end
+end
+
+class DirectoryUnpackStrategy < UnpackStrategy
+  def self.can_extract?(path:, magic_number:)
+    path.directory?
+  end
+
+  private
+
+  def extract_to_dir(unpack_dir, basename:)
+    FileUtils.cp_r path.children, unpack_dir, preserve: true
+  end
+end
+
+class UncompressedUnpackStrategy < UnpackStrategy
+  def self.can_extract?(path:, magic_number:)
+    false
+  end
+
+  private
+
+  def extract_to_dir(unpack_dir, basename:)
+    FileUtils.cp path, unpack_dir/basename, preserve: true
+  end
+end
+
+class JarUnpackStrategy < UncompressedUnpackStrategy
+  def self.can_extract?(path:, magic_number:)
+    return false unless ZipUnpackStrategy.can_extract?(path: path, magic_number: magic_number)
+
+    # Check further if the ZIP is a JAR/WAR.
+    Open3.popen3("unzip", "-l", path) do |stdin, stdout, stderr, wait_thr|
+      stdin.close_write
+      stderr.close_read
+
+      begin
+        return stdout.each_line.any? { |l| l.match?(%r{\s+META-INF/MANIFEST.MF$}) }
+      ensure
+        stdout.close_read
+        wait_thr.kill
+      end
+    end
+  end
+end
+
+class P7ZipUnpackStrategy < UnpackStrategy
+  def self.can_extract?(path:, magic_number:)
+    magic_number.match?(/\A7z\xBC\xAF\x27\x1C/n)
+  end
+
+  private
+
+  def extract_to_dir(unpack_dir, basename:)
+    safe_system "7zr", "x", "-y", "-bd", "-bso0", path, "-o#{unpack_dir}"
+  end
+end
+
+class ZipUnpackStrategy < UnpackStrategy
+  def self.can_extract?(path:, magic_number:)
+    magic_number.match?(/\APK(\003\004|\005\006)/n)
+  end
+
+  private
+
+  def extract_to_dir(unpack_dir, basename:)
+    safe_system "unzip", "-qq", path, "-d", unpack_dir
+  end
+end
+
+class TarUnpackStrategy < UnpackStrategy
+  def self.can_extract?(path:, magic_number:)
+    return true if magic_number.match?(/\A.{257}ustar/n)
+
+    # Check if `tar` can list the contents, then it can also extract it.
+    IO.popen(["tar", "tf", path], err: File::NULL) do |stdout|
+      !stdout.read(1).nil?
+    end
+  end
+
+  private
+
+  def extract_to_dir(unpack_dir, basename:)
+    safe_system "tar", "xf", path, "-C", unpack_dir
+  end
+end
+
+class CompressUnpackStrategy < TarUnpackStrategy
+  def self.can_extract?(path:, magic_number:)
+    magic_number.match?(/\A\037\235/n)
+  end
+end
+
+class XzUnpackStrategy < TarUnpackStrategy
+  def self.can_extract?(path:, magic_number:)
+    magic_number.match?(/\A\xFD7zXZ\x00/n)
+  end
+
+  private
+
+  def extract_to_dir(unpack_dir, basename:)
+    if DependencyCollector.tar_needs_xz_dependency?
+      unpack_path = unpack_dir/path.basename
+      FileUtils.cp path, unpack_path, preserve: true
+
+      safe_system Formula["xz"].opt_bin/"xz", "-d", "-q", "-T0", unpack_path
+    else
+      super
+    end
+  end
+end
+
+class Bzip2UnpackStrategy < UnpackStrategy
+  def self.can_extract?(path:, magic_number:)
+    magic_number.match?(/\ABZh/n)
+  end
+
+  private
+
+  def extract_to_dir(unpack_dir, basename:)
+    unpack_path = unpack_dir/path.basename
+    FileUtils.cp path, unpack_path, preserve: true
+
+    safe_system "bunzip2", "-q", unpack_path
+  end
+end
+
+class GzipUnpackStrategy < UnpackStrategy
+  def self.can_extract?(path:, magic_number:)
+    magic_number.match?(/\A\037\213/n)
+  end
+
+  private
+
+  def extract_to_dir(unpack_dir, basename:)
+    unpack_path = unpack_dir/path.basename
+    FileUtils.cp path, unpack_path, preserve: true
+
+    safe_system "gunzip", "-q", "-N", unpack_path
+  end
+end
+
+class LzipUnpackStrategy < UnpackStrategy
+  def self.can_extract?(path:, magic_number:)
+    magic_number.match?(/\ALZIP/n)
+  end
+
+  private
+
+  def extract_to_dir(unpack_dir, basename:)
+    unpack_path = unpack_dir/path.basename
+    FileUtils.cp path, unpack_path, preserve: true
+
+    safe_system Formula["lzip"].opt_bin/"lzip", "-d", "-q", unpack_path
+  end
+end
+
+class XarUnpackStrategy < UnpackStrategy
+  def self.can_extract?(path:, magic_number:)
+    magic_number.match?(/\Axar!/n)
+  end
+
+  private
+
+  def extract_to_dir(unpack_dir, basename:)
+    safe_system "xar", "-x", "-f", path, "-C", unpack_dir
+  end
+end
+
+class RarUnpackStrategy < UnpackStrategy
+  def self.can_extract?(path:, magic_number:)
+    magic_number.match?(/\ARar!/n)
+  end
+
+  private
+
+  def extract_to_dir(unpack_dir, basename:)
+    safe_system "unrar", "x", "-inul", path, unpack_dir
+  end
+end
+
+class LhaUnpackStrategy < UnpackStrategy
+  def self.can_extract?(path:, magic_number:)
+    magic_number.match?(/\A..-(lh0|lh1|lz4|lz5|lzs|lh\\40|lhd|lh2|lh3|lh4|lh5)-/n)
+  end
+
+  private
+
+  def extract_to_dir(unpack_dir, basename:)
+    safe_system Formula["lha"].opt_bin/"lha", "xq2w=#{unpack_dir}", path
+  end
+end
+
+class GitUnpackStrategy < DirectoryUnpackStrategy
+  def self.can_extract?(path:, magic_number:)
+    super && (path/".git").directory?
+  end
+
+  private
+
+  def extract_to_dir(unpack_dir, basename:)
+    FileUtils.cp_r path.children, unpack_dir, preserve: true
+  end
+end
+
+class SubversionUnpackStrategy < DirectoryUnpackStrategy
+  def self.can_extract?(path:, magic_number:)
+    super && (path/".svn").directory?
+  end
+
+  private
+
+  def extract_to_dir(unpack_dir, basename:)
+    safe_system "svn", "export", "--force", path, unpack_dir
+  end
+end
+
+class CvsUnpackStrategy < DirectoryUnpackStrategy
+  def self.can_extract?(path:, magic_number:)
+    super && (path/"CVS").directory?
+  end
+end
+
+class MercurialUnpackStrategy < DirectoryUnpackStrategy
+  def self.can_extract?(path:, magic_number:)
+    super && (path/".hg").directory?
+  end
+
+  private
+
+  def extract_to_dir(unpack_dir, basename:)
+    with_env "PATH" => PATH.new(Formula["mercurial"].opt_bin, ENV["PATH"]) do
+      safe_system "hg", "--cwd", path, "archive", "--subrepos", "-y", "-t", "files", unpack_dir
+    end
+  end
+end
+
+class FossilUnpackStrategy < UnpackStrategy
+  def self.can_extract?(path:, magic_number:)
+    return false unless magic_number.match?(/\ASQLite format 3\000/n)
+
+    # Fossil database is made up of artifacts, so the `artifact` table must exist.
+    query = "select count(*) from sqlite_master where type = 'view' and name = 'artifact'"
+    Utils.popen_read("sqlite3", path, query).to_i == 1
+  end
+
+  private
+
+  def extract_to_dir(unpack_dir, basename:)
+    args = if @ref_type && @ref
+      [@ref]
+    else
+      []
+    end
+
+    with_env "PATH" => PATH.new(Formula["fossil"].opt_bin, ENV["PATH"]) do
+      safe_system "fossil", "open", path, *args, chdir: unpack_dir
+    end
+  end
+end
+
+class BazaarUnpackStrategy < DirectoryUnpackStrategy
+  def self.can_extract?(path:, magic_number:)
+    super && (path/".bzr").directory?
+  end
+
+  private
+
+  def extract_to_dir(unpack_dir, basename:)
+    super
+
+    # The export command doesn't work on checkouts (see https://bugs.launchpad.net/bzr/+bug/897511).
+    FileUtils.rm_r unpack_dir/".bzr"
+  end
+end

From bf856117ba9776b3227106ace27493fe3fd9d16f Mon Sep 17 00:00:00 2001
From: Markus Reiter <me@reitermark.us>
Date: Sun, 8 Jul 2018 20:08:51 +0200
Subject: [PATCH 2/8] Allow unused keyword arguments.

---
 Library/Homebrew/.rubocop.yml | 3 +++
 Library/Homebrew/style.rb     | 4 ++++
 2 files changed, 7 insertions(+)

diff --git a/Library/Homebrew/.rubocop.yml b/Library/Homebrew/.rubocop.yml
index 9fe6333094c61..43bf51dba0b8e 100644
--- a/Library/Homebrew/.rubocop.yml
+++ b/Library/Homebrew/.rubocop.yml
@@ -31,6 +31,9 @@ Lint/NestedMethodDefinition:
 Lint/ParenthesesAsGroupedExpression:
   Enabled: true
 
+Lint/UnusedMethodArgument:
+  AllowUnusedKeywordArguments: true
+
 # TODO: try to bring down all metrics maximums
 Metrics/AbcSize:
   Max: 250
diff --git a/Library/Homebrew/style.rb b/Library/Homebrew/style.rb
index c17239b3f0fff..7d8ca2b7b5c03 100644
--- a/Library/Homebrew/style.rb
+++ b/Library/Homebrew/style.rb
@@ -30,6 +30,10 @@ def check_style_impl(files, output_type, options = {})
         args << "--parallel"
       end
 
+      if ARGV.verbose?
+        args += ["--extra-details", "--display-cop-names"]
+      end
+
       if ARGV.include?("--rspec")
         Homebrew.install_gem! "rubocop-rspec"
         args += %w[--require rubocop-rspec]

From 8ea7ad22189ee15a3e6850c705b342d1cc388c0c Mon Sep 17 00:00:00 2001
From: Markus Reiter <me@reitermark.us>
Date: Sun, 8 Jul 2018 20:13:41 +0200
Subject: [PATCH 3/8] Fix `basename_without_params`.

---
 Library/Homebrew/download_strategy.rb | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Library/Homebrew/download_strategy.rb b/Library/Homebrew/download_strategy.rb
index 3a8f3d24a6f31..2447777ff3236 100644
--- a/Library/Homebrew/download_strategy.rb
+++ b/Library/Homebrew/download_strategy.rb
@@ -76,6 +76,8 @@ def safe_system(*args)
   end
 
   def basename_without_params
+    return unless @url
+
     # Strip any ?thing=wad out of .c?thing=wad style extensions
     File.basename(@url)[/[^?]+/]
   end

From 633c590aac6ce6a3e1c60692a908cc93ba56dacc Mon Sep 17 00:00:00 2001
From: Markus Reiter <me@reitermark.us>
Date: Mon, 9 Jul 2018 20:04:33 +0200
Subject: [PATCH 4/8] Refactor unpack strategy specs.

---
 Library/Homebrew/test/unpack_strategy_spec.rb | 245 +++++++++++-------
 Library/Homebrew/unpack_strategy.rb           |  15 +-
 2 files changed, 158 insertions(+), 102 deletions(-)

diff --git a/Library/Homebrew/test/unpack_strategy_spec.rb b/Library/Homebrew/test/unpack_strategy_spec.rb
index 2ffca9852aef4..57c571f2dd575 100644
--- a/Library/Homebrew/test/unpack_strategy_spec.rb
+++ b/Library/Homebrew/test/unpack_strategy_spec.rb
@@ -1,127 +1,192 @@
 require "unpack_strategy"
 
-describe UnpackStrategy, :focus do
-  matcher :be_detected_as_a do |klass|
-    match do |expected|
-      @detected = described_class.detect(expected)
-      @detected.is_a?(klass)
-    end
+RSpec.shared_examples "UnpackStrategy::detect" do
+  it "is correctly detected" do
+    expect(UnpackStrategy.detect(path)).to be_a described_class
+  end
+end
 
-    failure_message do
-      <<~EOS
-        expected: #{klass}
-        detected: #{@detected}
-      EOS
+RSpec.shared_examples "#extract" do |children: []|
+  specify "#extract" do
+    mktmpdir do |unpack_dir|
+      described_class.new(path).extract(to: unpack_dir)
+      expect(unpack_dir.children(false).map(&:to_s)).to match_array children
     end
   end
+end
 
-  describe "::detect" do
-    it "correctly detects JAR files" do
-      expect(TEST_FIXTURE_DIR/"test.jar").to be_detected_as_a UncompressedUnpackStrategy
+describe UncompressedUnpackStrategy do
+  let(:path) {
+    (mktmpdir/"test").tap do |path|
+      FileUtils.touch path
     end
+  }
 
-    it "correctly detects ZIP files" do
-      expect(TEST_FIXTURE_DIR/"cask/MyFancyApp.zip").to be_detected_as_a ZipUnpackStrategy
-    end
+  include_examples "UnpackStrategy::detect"
+end
 
-    it "correctly detects BZIP2 files" do
-      expect(TEST_FIXTURE_DIR/"cask/container.bz2").to be_detected_as_a Bzip2UnpackStrategy
-    end
+describe P7ZipUnpackStrategy do
+  let(:path) { TEST_FIXTURE_DIR/"cask/container.7z" }
 
-    it "correctly detects GZIP files" do
-      expect(TEST_FIXTURE_DIR/"cask/container.gz").to be_detected_as_a GzipUnpackStrategy
-    end
+  include_examples "UnpackStrategy::detect"
+end
 
-    it "correctly detects compressed TAR files" do
-      expect(TEST_FIXTURE_DIR/"cask/container.tar.gz").to be_detected_as_a TarUnpackStrategy
-    end
+describe XarUnpackStrategy do
+  let(:path) { TEST_FIXTURE_DIR/"cask/container.xar" }
 
-    it "correctly detects 7-ZIP files" do
-      expect(TEST_FIXTURE_DIR/"cask/container.7z").to be_detected_as_a P7ZipUnpackStrategy
-    end
+  include_examples "UnpackStrategy::detect"
+  include_examples "#extract", children: ["container"]
+end
 
-    it "correctly detects XAR files" do
-      expect(TEST_FIXTURE_DIR/"cask/container.xar").to be_detected_as_a XarUnpackStrategy
-    end
+describe XzUnpackStrategy do
+  let(:path) { TEST_FIXTURE_DIR/"cask/container.xz" }
 
-    it "correctly detects XZ files" do
-      expect(TEST_FIXTURE_DIR/"cask/container.xz").to be_detected_as_a XzUnpackStrategy
-    end
+  include_examples "UnpackStrategy::detect"
+end
 
-    it "correctly detects RAR files" do
-      expect(TEST_FIXTURE_DIR/"cask/container.rar").to be_detected_as_a RarUnpackStrategy
-    end
+describe RarUnpackStrategy do
+  let(:path) { TEST_FIXTURE_DIR/"cask/container.rar" }
 
-    it "correctly detects LZIP files" do
-      expect(TEST_FIXTURE_DIR/"test.lz").to be_detected_as_a LzipUnpackStrategy
-    end
+  include_examples "UnpackStrategy::detect"
+end
 
-    it "correctly detects LHA files" do
-      expect(TEST_FIXTURE_DIR/"test.lha").to be_detected_as_a LhaUnpackStrategy
-    end
+describe LzipUnpackStrategy do
+  let(:path) { TEST_FIXTURE_DIR/"test.lz" }
 
-    it "correctly detects Git repositories" do
-      mktmpdir do |repo|
-        system "git", "-C", repo, "init"
+  include_examples "UnpackStrategy::detect"
+end
 
-        expect(repo).to be_detected_as_a GitUnpackStrategy
-      end
-    end
+describe LhaUnpackStrategy do
+  let(:path) { TEST_FIXTURE_DIR/"test.lha" }
 
-    it "correctly detects Subversion repositories" do
-      mktmpdir do |path|
-        repo = path/"repo"
-        working_copy = path/"working_copy"
+  include_examples "UnpackStrategy::detect"
+end
 
-        system "svnadmin", "create", repo
-        system "svn", "checkout", "file://#{repo}", working_copy
+describe JarUnpackStrategy do
+  let(:path) { TEST_FIXTURE_DIR/"test.jar" }
 
-        expect(working_copy).to be_detected_as_a SubversionUnpackStrategy
+  include_examples "UnpackStrategy::detect"
+  include_examples "#extract", children: ["test.jar"]
+end
+
+describe ZipUnpackStrategy do
+  let(:path) { TEST_FIXTURE_DIR/"cask/MyFancyApp.zip" }
+
+  include_examples "UnpackStrategy::detect"
+  include_examples "#extract", children: ["MyFancyApp"]
+
+  context "when ZIP archive is corrupted" do
+    let(:path) {
+      (mktmpdir/"test.zip").tap do |path|
+        FileUtils.touch path
       end
-    end
+    }
+
+    include_examples "UnpackStrategy::detect"
   end
 end
 
-describe GitUnpackStrategy do
-  describe "#extract" do
-    it "correctly extracts a Subversion repository" do
-      mktmpdir do |path|
-        repo = path/"repo"
+describe GzipUnpackStrategy do
+  let(:path) { TEST_FIXTURE_DIR/"cask/container.gz" }
 
-        repo.mkpath
+  include_examples "UnpackStrategy::detect"
+  include_examples "#extract", children: ["container"]
+end
+
+describe Bzip2UnpackStrategy do
+  let(:path) { TEST_FIXTURE_DIR/"cask/container.bz2" }
+
+  include_examples "UnpackStrategy::detect"
+  include_examples "#extract", children: ["container"]
+end
 
-        system "git", "-C", repo, "init"
+describe TarUnpackStrategy do
+  let(:path) { TEST_FIXTURE_DIR/"cask/container.tar.gz" }
 
-        FileUtils.touch repo/"test"
-        system "git", "-C", repo, "add", "test"
-        system "git", "-C", repo, "commit", "-m", "Add `test` file."
+  include_examples "UnpackStrategy::detect"
+  include_examples "#extract", children: ["container"]
 
-        unpack_dir = path/"unpack_dir"
-        GitUnpackStrategy.new(repo).extract(to: unpack_dir)
-        expect(unpack_dir.children(false).map(&:to_s)).to match_array [".git", "test"]
+  context "when TAR archive is corrupted" do
+    let(:path) {
+      (mktmpdir/"test.tar").tap do |path|
+        FileUtils.touch path
       end
-    end
+    }
+
+    include_examples "UnpackStrategy::detect"
   end
 end
 
+describe GitUnpackStrategy do
+  let(:repo) {
+    mktmpdir.tap do |repo|
+      system "git", "-C", repo, "init"
+
+      FileUtils.touch repo/"test"
+      system "git", "-C", repo, "add", "test"
+      system "git", "-C", repo, "commit", "-m", "Add `test` file."
+    end
+  }
+  let(:path) { repo }
+
+  include_examples "UnpackStrategy::detect"
+  include_examples "#extract", children: [".git", "test"]
+end
+
 describe SubversionUnpackStrategy do
-  describe "#extract" do
-    it "correctly extracts a Subversion repository" do
-      mktmpdir do |path|
-        repo = path/"repo"
-        working_copy = path/"working_copy"
-
-        system "svnadmin", "create", repo
-        system "svn", "checkout", "file://#{repo}", working_copy
-
-        FileUtils.touch working_copy/"test"
-        system "svn", "add", working_copy/"test"
-        system "svn", "commit", working_copy, "-m", "Add `test` file."
-
-        unpack_dir = path/"unpack_dir"
-        SubversionUnpackStrategy.new(working_copy).extract(to: unpack_dir)
-        expect(unpack_dir.children(false).map(&:to_s)).to match_array ["test"]
-      end
+  let(:repo) {
+    mktmpdir.tap do |repo|
+      system "svnadmin", "create", repo
     end
-  end
+  }
+  let(:working_copy) {
+    mktmpdir.tap do |working_copy|
+      system "svn", "checkout", "file://#{repo}", working_copy
+
+      FileUtils.touch working_copy/"test"
+      system "svn", "add", working_copy/"test"
+      system "svn", "commit", working_copy, "-m", "Add `test` file."
+    end
+  }
+  let(:path) { working_copy }
+
+  include_examples "UnpackStrategy::detect"
+  include_examples "#extract", children: ["test"]
+end
+
+describe CvsUnpackStrategy do
+  let(:repo) {
+    mktmpdir.tap do |repo|
+      FileUtils.touch repo/"test"
+      (repo/"CVS").mkpath
+    end
+  }
+  let(:path) { repo }
+
+  include_examples "UnpackStrategy::detect"
+  include_examples "#extract", children: ["CVS", "test"]
+end
+
+describe BazaarUnpackStrategy do
+  let(:repo) {
+    mktmpdir.tap do |repo|
+      FileUtils.touch repo/"test"
+      (repo/".bzr").mkpath
+    end
+  }
+  let(:path) { repo }
+
+  include_examples "UnpackStrategy::detect"
+  include_examples "#extract", children: ["test"]
+end
+
+describe MercurialUnpackStrategy do
+  let(:repo) {
+    mktmpdir.tap do |repo|
+      (repo/".hg").mkpath
+    end
+  }
+  let(:path) { repo }
+
+  include_examples "UnpackStrategy::detect"
 end
diff --git a/Library/Homebrew/unpack_strategy.rb b/Library/Homebrew/unpack_strategy.rb
index f57109c6d7d35..a23406805f46d 100644
--- a/Library/Homebrew/unpack_strategy.rb
+++ b/Library/Homebrew/unpack_strategy.rb
@@ -31,7 +31,7 @@ def self.detect(path)
     magic_number = if path.directory?
       ""
     else
-      File.binread(path, MAX_MAGIC_NUMBER_LENGTH)
+      File.binread(path, MAX_MAGIC_NUMBER_LENGTH) || ""
     end
 
     strategy = strategies.detect do |s|
@@ -40,7 +40,7 @@ def self.detect(path)
 
     # This is so that bad files produce good error messages.
     strategy ||= case path.extname
-    when ".tar.gz", ".tgz", ".tar.bz2", ".tbz", ".tar.xz", ".txz"
+    when ".tar", ".tar.gz", ".tgz", ".tar.bz2", ".tbz", ".tar.xz", ".txz"
       TarUnpackStrategy
     when ".zip"
       ZipUnpackStrategy
@@ -58,16 +58,11 @@ def initialize(path)
   end
 
   def extract(to: nil, basename: nil)
+    basename ||= path.basename
     unpack_dir = Pathname(to || Dir.pwd).expand_path
     unpack_dir.mkpath
     extract_to_dir(unpack_dir, basename: basename)
   end
-
-  private
-
-  def extract_to_dir(_unpack_dir, basename:)
-    raise NotImplementedError
-  end
 end
 
 class DirectoryUnpackStrategy < UnpackStrategy
@@ -83,10 +78,6 @@ def extract_to_dir(unpack_dir, basename:)
 end
 
 class UncompressedUnpackStrategy < UnpackStrategy
-  def self.can_extract?(path:, magic_number:)
-    false
-  end
-
   private
 
   def extract_to_dir(unpack_dir, basename:)

From ec7e22e16fbaa09ca01b8e2fff84ba5c7613b5b0 Mon Sep 17 00:00:00 2001
From: Markus Reiter <me@reitermark.us>
Date: Mon, 9 Jul 2018 21:20:00 +0200
Subject: [PATCH 5/8] Fix `XzUnpackStrategy`.

---
 Library/Homebrew/unpack_strategy.rb | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/Library/Homebrew/unpack_strategy.rb b/Library/Homebrew/unpack_strategy.rb
index a23406805f46d..ff41938fc0209 100644
--- a/Library/Homebrew/unpack_strategy.rb
+++ b/Library/Homebrew/unpack_strategy.rb
@@ -151,7 +151,7 @@ def self.can_extract?(path:, magic_number:)
   end
 end
 
-class XzUnpackStrategy < TarUnpackStrategy
+class XzUnpackStrategy < UnpackStrategy
   def self.can_extract?(path:, magic_number:)
     magic_number.match?(/\A\xFD7zXZ\x00/n)
   end
@@ -159,13 +159,23 @@ def self.can_extract?(path:, magic_number:)
   private
 
   def extract_to_dir(unpack_dir, basename:)
-    if DependencyCollector.tar_needs_xz_dependency?
-      unpack_path = unpack_dir/path.basename
-      FileUtils.cp path, unpack_path, preserve: true
+    unpack_path = unpack_dir/path.basename
+    FileUtils.cp path, unpack_path, preserve: true
 
-      safe_system Formula["xz"].opt_bin/"xz", "-d", "-q", "-T0", unpack_path
-    else
-      super
+    safe_system Formula["xz"].opt_bin/"xz", "-d", "-q", "-T0", unpack_path
+
+    extract_nested_tar(unpack_dir, basename: basename)
+  end
+
+  def extract_nested_tar(unpack_dir, basename:)
+    return unless DependencyCollector.tar_needs_xz_dependency?
+    return if (children = unpack_dir.children).count != 1
+    return if (tar = children.first).extname != ".tar"
+
+    Dir.mktmpdir do |tmpdir|
+      tmpdir = Pathname(tmpdir)
+      FileUtils.mv tar, tmpdir/tar.basename
+      TarUnpackStrategy.new(tmpdir/tar.basename).extract(to: unpack_dir, basename: basename)
     end
   end
 end

From 5dfa4ded43cb51d1325bf8cd904e345c1a43ec15 Mon Sep 17 00:00:00 2001
From: Markus Reiter <me@reitermark.us>
Date: Mon, 9 Jul 2018 21:47:55 +0200
Subject: [PATCH 6/8] Reuse `UncompressedUnpackStrategy#extract_to_dir`.

---
 Library/Homebrew/unpack_strategy.rb | 33 +++++++++++------------------
 1 file changed, 12 insertions(+), 21 deletions(-)

diff --git a/Library/Homebrew/unpack_strategy.rb b/Library/Homebrew/unpack_strategy.rb
index ff41938fc0209..bc5a2d9e78fea 100644
--- a/Library/Homebrew/unpack_strategy.rb
+++ b/Library/Homebrew/unpack_strategy.rb
@@ -151,7 +151,7 @@ def self.can_extract?(path:, magic_number:)
   end
 end
 
-class XzUnpackStrategy < UnpackStrategy
+class XzUnpackStrategy < UncompressedUnpackStrategy
   def self.can_extract?(path:, magic_number:)
     magic_number.match?(/\A\xFD7zXZ\x00/n)
   end
@@ -159,11 +159,8 @@ def self.can_extract?(path:, magic_number:)
   private
 
   def extract_to_dir(unpack_dir, basename:)
-    unpack_path = unpack_dir/path.basename
-    FileUtils.cp path, unpack_path, preserve: true
-
-    safe_system Formula["xz"].opt_bin/"xz", "-d", "-q", "-T0", unpack_path
-
+    super
+    safe_system Formula["xz"].opt_bin/"xz", "-d", "-q", "-T0", unpack_dir/basename
     extract_nested_tar(unpack_dir, basename: basename)
   end
 
@@ -180,7 +177,7 @@ def extract_nested_tar(unpack_dir, basename:)
   end
 end
 
-class Bzip2UnpackStrategy < UnpackStrategy
+class Bzip2UnpackStrategy < UncompressedUnpackStrategy
   def self.can_extract?(path:, magic_number:)
     magic_number.match?(/\ABZh/n)
   end
@@ -188,14 +185,12 @@ def self.can_extract?(path:, magic_number:)
   private
 
   def extract_to_dir(unpack_dir, basename:)
-    unpack_path = unpack_dir/path.basename
-    FileUtils.cp path, unpack_path, preserve: true
-
-    safe_system "bunzip2", "-q", unpack_path
+    super
+    safe_system "bunzip2", "-q", unpack_dir/basename
   end
 end
 
-class GzipUnpackStrategy < UnpackStrategy
+class GzipUnpackStrategy < UncompressedUnpackStrategy
   def self.can_extract?(path:, magic_number:)
     magic_number.match?(/\A\037\213/n)
   end
@@ -203,14 +198,12 @@ def self.can_extract?(path:, magic_number:)
   private
 
   def extract_to_dir(unpack_dir, basename:)
-    unpack_path = unpack_dir/path.basename
-    FileUtils.cp path, unpack_path, preserve: true
-
-    safe_system "gunzip", "-q", "-N", unpack_path
+    super
+    safe_system "gunzip", "-q", "-N", unpack_dir/basename
   end
 end
 
-class LzipUnpackStrategy < UnpackStrategy
+class LzipUnpackStrategy < UncompressedUnpackStrategy
   def self.can_extract?(path:, magic_number:)
     magic_number.match?(/\ALZIP/n)
   end
@@ -218,10 +211,8 @@ def self.can_extract?(path:, magic_number:)
   private
 
   def extract_to_dir(unpack_dir, basename:)
-    unpack_path = unpack_dir/path.basename
-    FileUtils.cp path, unpack_path, preserve: true
-
-    safe_system Formula["lzip"].opt_bin/"lzip", "-d", "-q", unpack_path
+    super
+    safe_system Formula["lzip"].opt_bin/"lzip", "-d", "-q", unpack_dir/basename
   end
 end
 

From cba55c8fd4ca128083851e6ee72814103bd2c829 Mon Sep 17 00:00:00 2001
From: Markus Reiter <me@reitermark.us>
Date: Mon, 9 Jul 2018 22:11:26 +0200
Subject: [PATCH 7/8] Add `@ref_type` and `@ref` needef for `fossil`.

---
 Library/Homebrew/download_strategy.rb | 2 +-
 Library/Homebrew/unpack_strategy.rb   | 8 +++++---
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/Library/Homebrew/download_strategy.rb b/Library/Homebrew/download_strategy.rb
index 2447777ff3236..d4aa4cb1ae9a0 100644
--- a/Library/Homebrew/download_strategy.rb
+++ b/Library/Homebrew/download_strategy.rb
@@ -47,7 +47,7 @@ def ohai(*args)
   # chdir into the newly-unpacked directory.
   # Unlike {Resource#stage}, this does not take a block.
   def stage
-    UnpackStrategy.detect(cached_location)
+    UnpackStrategy.detect(cached_location, ref_type: @ref_type, ref: @ref)
                   .extract(basename: basename_without_params)
   end
 
diff --git a/Library/Homebrew/unpack_strategy.rb b/Library/Homebrew/unpack_strategy.rb
index bc5a2d9e78fea..248de594c1ef3 100644
--- a/Library/Homebrew/unpack_strategy.rb
+++ b/Library/Homebrew/unpack_strategy.rb
@@ -27,7 +27,7 @@ def self.strategies
   end
   private_class_method :strategies
 
-  def self.detect(path)
+  def self.detect(path, ref_type: nil, ref: nil)
     magic_number = if path.directory?
       ""
     else
@@ -48,13 +48,15 @@ def self.detect(path)
       UncompressedUnpackStrategy
     end
 
-    strategy.new(path)
+    strategy.new(path, ref_type: ref_type, ref: ref)
   end
 
   attr_reader :path
 
-  def initialize(path)
+  def initialize(path, ref_type: nil, ref: nil)
     @path = Pathname(path).expand_path
+    @ref_type = ref_type
+    @ref = ref
   end
 
   def extract(to: nil, basename: nil)

From 9b4e32322a379521e06198bf3e5d23dcd6f63f77 Mon Sep 17 00:00:00 2001
From: Markus Reiter <me@reitermark.us>
Date: Wed, 11 Jul 2018 01:56:49 +0200
Subject: [PATCH 8/8] `xar` is only pre-installed on macOS.

---
 Library/Homebrew/test/unpack_strategy_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Library/Homebrew/test/unpack_strategy_spec.rb b/Library/Homebrew/test/unpack_strategy_spec.rb
index 57c571f2dd575..c24e417309f75 100644
--- a/Library/Homebrew/test/unpack_strategy_spec.rb
+++ b/Library/Homebrew/test/unpack_strategy_spec.rb
@@ -31,7 +31,7 @@
   include_examples "UnpackStrategy::detect"
 end
 
-describe XarUnpackStrategy do
+describe XarUnpackStrategy, :needs_macos do
   let(:path) { TEST_FIXTURE_DIR/"cask/container.xar" }
 
   include_examples "UnpackStrategy::detect"