From 528b4b367ed2a4026c321e2f2e503ace4548b9d2 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Fri, 3 Aug 2018 10:50:49 +0200 Subject: [PATCH 1/2] Always `chdir`. --- Library/Homebrew/download_strategy.rb | 30 ++++++++++++++------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/Library/Homebrew/download_strategy.rb b/Library/Homebrew/download_strategy.rb index 9ea88cae70..29791f03f6 100644 --- a/Library/Homebrew/download_strategy.rb +++ b/Library/Homebrew/download_strategy.rb @@ -14,8 +14,8 @@ class AbstractDownloadStrategy end end - attr_reader :meta, :name, :version - attr_reader :shutup + attr_reader :meta, :name, :version, :shutup + private :meta, :name, :version, :shutup def initialize(url, name, version, **meta) @url = url @@ -52,8 +52,22 @@ class AbstractDownloadStrategy .extract_nestedly(basename: basename_without_params, extension_only: true, verbose: ARGV.verbose? && !shutup) + chdir end + def chdir + entries = Dir["*"] + case entries.length + when 0 then raise "Empty archive" + when 1 then begin + Dir.chdir entries.first + rescue + nil + end + end + end + private :chdir + # @!attribute [r] cached_location # The path to the cached file or directory associated with the resource. def cached_location; end @@ -180,18 +194,6 @@ class AbstractFileDownloadStrategy < AbstractDownloadStrategy private - def chdir - entries = Dir["*"] - case entries.length - when 0 then raise "Empty archive" - when 1 then begin - Dir.chdir entries.first - rescue - nil - end - end - end - def ext # We need a Pathname because we've monkeypatched extname to support double # extensions (e.g. tar.gz). From 51fa19496624b693c42ceaa1a7264e6fe7d665a3 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Fri, 3 Aug 2018 10:51:01 +0200 Subject: [PATCH 2/2] Move `cached_location` to `#initialize`. --- Library/Homebrew/download_strategy.rb | 37 +++++-------------- .../Homebrew/test/download_strategies_spec.rb | 4 +- 2 files changed, 12 insertions(+), 29 deletions(-) diff --git a/Library/Homebrew/download_strategy.rb b/Library/Homebrew/download_strategy.rb index 29791f03f6..600dd39e44 100644 --- a/Library/Homebrew/download_strategy.rb +++ b/Library/Homebrew/download_strategy.rb @@ -14,6 +14,7 @@ class AbstractDownloadStrategy end end + attr_reader :cached_location attr_reader :meta, :name, :version, :shutup private :meta, :name, :version, :shutup @@ -68,10 +69,6 @@ class AbstractDownloadStrategy end private :chdir - # @!attribute [r] cached_location - # The path to the cached file or directory associated with the resource. - def cached_location; end - # @!attribute [r] # return most recent modified time for all files in the current working directory after stage. def source_modified_time @@ -108,7 +105,7 @@ class VCSDownloadStrategy < AbstractDownloadStrategy super @ref_type, @ref = extract_ref(meta) @revision = meta[:revision] - @clone = HOMEBREW_CACHE/cache_filename + @cached_location = HOMEBREW_CACHE/"#{name}--#{cache_tag}" end def fetch @@ -146,10 +143,6 @@ class VCSDownloadStrategy < AbstractDownloadStrategy commit != @last_commit end - def cached_location - @clone - end - def head? version.respond_to?(:head?) && version.head? end @@ -166,10 +159,6 @@ class VCSDownloadStrategy < AbstractDownloadStrategy raise NotImplementedError end - def cache_filename - "#{name}--#{cache_tag}" - end - def repo_valid? raise NotImplementedError end @@ -187,6 +176,11 @@ class VCSDownloadStrategy < AbstractDownloadStrategy end class AbstractFileDownloadStrategy < AbstractDownloadStrategy + def initialize(url, name, version, **meta) + super + @cached_location = HOMEBREW_CACHE/"#{name}-#{version}#{ext}" + end + def stage super chdir @@ -209,12 +203,11 @@ class AbstractFileDownloadStrategy < AbstractDownloadStrategy end class CurlDownloadStrategy < AbstractFileDownloadStrategy - attr_reader :mirrors, :tarball_path, :temporary_path + attr_reader :mirrors, :temporary_path def initialize(url, name, version, **meta) super @mirrors = meta.fetch(:mirrors, []) - @tarball_path = HOMEBREW_CACHE/"#{name}-#{version}#{ext}" @temporary_path = Pathname.new("#{cached_location}.incomplete") end @@ -238,10 +231,6 @@ class CurlDownloadStrategy < AbstractFileDownloadStrategy retry end - def cached_location - tarball_path - end - def clear_cache super rm_rf(temporary_path) @@ -366,8 +355,6 @@ end # This strategy extracts local binary packages. class LocalBottleDownloadStrategy < AbstractFileDownloadStrategy - attr_reader :cached_location - def initialize(path) @cached_location = path end @@ -520,11 +507,11 @@ end # url "scp://example.com/src/abc.1.0.tar.gz" # ... class ScpDownloadStrategy < AbstractFileDownloadStrategy - attr_reader :tarball_path, :temporary_path + attr_reader :temporary_path def initialize(url, name, version, **meta) super - @tarball_path = HOMEBREW_CACHE/"#{name}-#{version}#{ext}" + @cached_location = HOMEBREW_CACHE/"#{name}-#{version}#{ext}" @temporary_path = Pathname.new("#{cached_location}.incomplete") parse_url_pattern end @@ -554,10 +541,6 @@ class ScpDownloadStrategy < AbstractFileDownloadStrategy end end - def cached_location - tarball_path - end - def clear_cache super rm_rf(temporary_path) diff --git a/Library/Homebrew/test/download_strategies_spec.rb b/Library/Homebrew/test/download_strategies_spec.rb index a3a24b8d8b..317eda99b7 100644 --- a/Library/Homebrew/test/download_strategies_spec.rb +++ b/Library/Homebrew/test/download_strategies_spec.rb @@ -234,8 +234,8 @@ describe CurlDownloadStrategy do expect(subject.send(:_curl_args)).to eq(["--user", "download:123456"]) end - describe "#tarball_path" do - subject { described_class.new(url, name, version, **specs).tarball_path } + describe "#cached_location" do + subject { described_class.new(url, name, version, **specs).cached_location } context "when URL ends with file" do it { is_expected.to eq(HOMEBREW_CACHE/"foo-.tar.gz") }