diff --git a/Library/Homebrew/dependency_collector.rb b/Library/Homebrew/dependency_collector.rb index eddc0f53e10b2ab85f6d942e3710f14bb30ad637..7420c2003bba0cd75c43bef5f798e1470e8d3aa1 100644 --- a/Library/Homebrew/dependency_collector.rb +++ b/Library/Homebrew/dependency_collector.rb @@ -60,7 +60,7 @@ class DependencyCollector end def subversion_dep_if_needed(tags) - return if Utils.svn_available? + return if Utils::Svn.available? Dependency.new("subversion", tags) end diff --git a/Library/Homebrew/dev-cmd/audit.rb b/Library/Homebrew/dev-cmd/audit.rb index 80b80f09e75962b9c8b87b40e34d3d7654d1e8fd..5a117ff5dafb29d8d879e1f871f2e462d5d16c42 100644 --- a/Library/Homebrew/dev-cmd/audit.rb +++ b/Library/Homebrew/dev-cmd/audit.rb @@ -1088,9 +1088,9 @@ module Homebrew problem "The URL #{url} is not a valid git URL" unless Utils::Git.remote_exists? url elsif strategy <= SubversionDownloadStrategy next unless DevelopmentTools.subversion_handles_most_https_certificates? - next unless Utils.svn_available? + next unless Utils::Svn.available? - problem "The URL #{url} is not a valid svn URL" unless Utils.svn_remote_exists? url + problem "The URL #{url} is not a valid svn URL" unless Utils::Svn.remote_exists? url end end end diff --git a/Library/Homebrew/download_strategy.rb b/Library/Homebrew/download_strategy.rb index 4ff56f290dbad83bc0304d4b09483238ded5a7f8..739b9f5c787fe38404a677099c1cf998538422f0 100644 --- a/Library/Homebrew/download_strategy.rb +++ b/Library/Homebrew/download_strategy.rb @@ -523,7 +523,7 @@ class SubversionDownloadStrategy < VCSDownloadStrategy end def source_modified_time - time = if Version.create(Utils.svn_version) >= Version.create("1.9") + time = if Version.create(Utils::Svn.version) >= Version.create("1.9") out, = system_command("svn", args: ["info", "--show-item", "last-changed-date"], chdir: cached_location) out else diff --git a/Library/Homebrew/test/utils/svn_spec.rb b/Library/Homebrew/test/utils/svn_spec.rb index 92fb2b00660f4db119bc9aa717848075bc9faaeb..da6ec91ec7749f604690f7118446a12efcadc9a3 100644 --- a/Library/Homebrew/test/utils/svn_spec.rb +++ b/Library/Homebrew/test/utils/svn_spec.rb @@ -2,49 +2,48 @@ require "utils/svn" -describe Utils do - describe "#self.svn_available?" do - before do - described_class.clear_svn_version_cache - end +describe Utils::Svn do + before do + described_class.clear_version_cache + end + describe "::available?" do it "returns svn version if svn available" do if quiet_system "#{HOMEBREW_SHIMS_PATH}/scm/svn", "--version" - expect(described_class).to be_svn_available + expect(described_class).to be_available else - expect(described_class).not_to be_svn_available + expect(described_class).not_to be_available end end end - describe "#self.svn_version" do - before do - described_class.clear_svn_version_cache - end - - it "returns nil when svn is not available" do - allow(described_class).to receive(:svn_available?).and_return(false) - expect(described_class.svn_version).to eq(nil) + describe "::version" do + it "returns svn version if svn available" do + if quiet_system "#{HOMEBREW_SHIMS_PATH}/scm/svn", "--version" + expect(described_class.version).to match(/^\d+\.\d+\.\d+$/) + else + expect(described_class.version).to be_nil + end end it "returns version of svn when svn is available", :needs_svn do - expect(described_class.svn_version).not_to be_nil + expect(described_class.version).not_to be_nil end end - describe "#self.svn_remote_exists?" do + describe "::remote_exists?" do it "returns true when svn is not available" do - allow(described_class).to receive(:svn_available?).and_return(false) - expect(described_class).to be_svn_remote_exists("blah") + allow(described_class).to receive(:available?).and_return(false) + expect(described_class).to be_remote_exists("blah") end context "when svn is available" do before do - allow(described_class).to receive(:svn_available?).and_return(true) + allow(described_class).to receive(:available?).and_return(true) end it "returns false when remote does not exist" do - expect(described_class).not_to be_svn_remote_exists(HOMEBREW_CACHE/"install") + expect(described_class).not_to be_remote_exists(HOMEBREW_CACHE/"install") end it "returns true when remote exists", :needs_network, :needs_svn do @@ -54,7 +53,7 @@ describe Utils do "https://github.com/Homebrew/install" end - expect(described_class).to be_svn_remote_exists(HOMEBREW_CACHE/"install") + expect(described_class).to be_remote_exists(HOMEBREW_CACHE/"install") end end end diff --git a/Library/Homebrew/utils/svn.rb b/Library/Homebrew/utils/svn.rb index aa84cc0b6120ff34b9e1af1cd376a47875d8b21e..1492b083198391655a9a7e8783c59a15a0b168e1 100644 --- a/Library/Homebrew/utils/svn.rb +++ b/Library/Homebrew/utils/svn.rb @@ -1,32 +1,36 @@ # frozen_string_literal: true +require "system_command" + module Utils - def self.clear_svn_version_cache - remove_instance_variable(:@svn_available) if defined?(@svn_available) - remove_instance_variable(:@svn_version) if defined?(@svn_version) - end + # Helper functions for querying SVN information. + # + # @api private + module Svn + module_function - def self.svn_available? - return @svn_available if defined?(@svn_available) + def available? + version.present? + end - @svn_available = quiet_system HOMEBREW_SHIMS_PATH/"scm/svn", "--version" - end + def version + return @version if defined?(@version) - def self.svn_version - return unless svn_available? - return @svn_version if defined?(@svn_version) + stdout, _, status = system_command(HOMEBREW_SHIMS_PATH/"scm/svn", args: ["--version"], print_stderr: false) + @version = status.success? ? stdout.chomp[/svn, version (\d+(?:\.\d+)*)/, 1] : nil + end - @svn_version = Utils.popen_read( - HOMEBREW_SHIMS_PATH/"scm/svn", "--version" - ).chomp[/svn, version (\d+(?:\.\d+)*)/, 1] - end + def remote_exists?(url) + return true unless available? - def self.svn_remote_exists?(url) - return true unless svn_available? + # OK to unconditionally trust here because we're just checking if + # a URL exists. + quiet_system "svn", "ls", url, "--depth", "empty", + "--non-interactive", "--trust-server-cert" + end - # OK to unconditionally trust here because we're just checking if - # a URL exists. - quiet_system "svn", "ls", url, "--depth", "empty", - "--non-interactive", "--trust-server-cert" + def clear_version_cache + remove_instance_variable(:@version) if defined?(@version) + end end end