Merge pull request #8056 from reitermarkus/audit

Refactor `brew cask audit`.
This commit is contained in:
Markus Reiter 2020-07-23 00:39:25 +02:00 committed by GitHub
commit fda2183d5e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 160 additions and 106 deletions

View File

@ -17,12 +17,12 @@ module Cask
attr_predicate :appcast? attr_predicate :appcast?
def initialize(cask, appcast: false, download: false, def initialize(cask, appcast: false, download: false, quarantine: nil,
token_conflicts: false, online: false, strict: false, token_conflicts: false, online: false, strict: false,
new_cask: false, commit_range: nil, command: SystemCommand) new_cask: false, commit_range: nil, command: SystemCommand)
@cask = cask @cask = cask
@appcast = appcast @appcast = appcast
@download = download @download = Download.new(cask, quarantine: quarantine) if download
@online = online @online = online
@strict = strict @strict = strict
@new_cask = new_cask @new_cask = new_cask

View File

@ -1,7 +1,5 @@
# frozen_string_literal: true # frozen_string_literal: true
require "cask/download"
module Cask module Cask
class Auditor class Auditor
include Checkable include Checkable
@ -51,28 +49,26 @@ module Cask
private private
def audit_all_languages def audit_all_languages
saved_languages = MacOS.instance_variable_get(:@languages) language_blocks.keys.all?(&method(:audit_languages))
begin
language_blocks.keys.all?(&method(:audit_languages))
ensure
MacOS.instance_variable_set(:@languages, saved_languages)
end
end end
def audit_languages(languages) def audit_languages(languages)
ohai "Auditing language: #{languages.map { |lang| "'#{lang}'" }.to_sentence}" ohai "Auditing language: #{languages.map { |lang| "'#{lang}'" }.to_sentence}"
MacOS.instance_variable_set(:@languages, languages) localized_cask = CaskLoader.load(cask.sourcefile_path)
audit_cask_instance(CaskLoader.load(cask.sourcefile_path)) config = localized_cask.config
config.languages = languages
localized_cask.config = config
audit_cask_instance(localized_cask)
end end
def audit_cask_instance(cask) def audit_cask_instance(cask)
download = audit_download? && Download.new(cask, quarantine: quarantine?)
audit = Audit.new(cask, appcast: audit_appcast?, audit = Audit.new(cask, appcast: audit_appcast?,
online: audit_online?, online: audit_online?,
strict: audit_strict?, strict: audit_strict?,
new_cask: audit_new_cask?, new_cask: audit_new_cask?,
token_conflicts: audit_token_conflicts?, token_conflicts: audit_token_conflicts?,
download: download, download: audit_download?,
quarantine: quarantine?,
commit_range: commit_range) commit_range: commit_range)
audit.run! audit.run!
puts audit.summary puts audit.summary

View File

@ -65,8 +65,7 @@ module Cask
option "--help", :help, false option "--help", :help, false
# handled in OS::Mac option "--language=a,b,c", ->(value) { Config.global.languages = value }
option "--language a,b,c", ->(*) {}
# override default handling of --version # override default handling of --version
option "--version", ->(*) { raise OptionParser::InvalidOption } option "--version", ->(*) { raise OptionParser::InvalidOption }
@ -180,8 +179,6 @@ module Cask
end end
def process_options(*args) def process_options(*args)
exclude_regex = /^--#{Regexp.union(*Config::DEFAULT_DIRS.keys.map(&Regexp.public_method(:escape)))}=/
non_options = [] non_options = []
if idx = args.index("--") if idx = args.index("--")
@ -189,15 +186,31 @@ module Cask
args = args.first(idx) args = args.first(idx)
end end
exclude_regex = /^--#{Regexp.union(*Config::DEFAULT_DIRS.keys.map(&Regexp.public_method(:escape)))}=/
cask_opts = Shellwords.shellsplit(ENV.fetch("HOMEBREW_CASK_OPTS", "")) cask_opts = Shellwords.shellsplit(ENV.fetch("HOMEBREW_CASK_OPTS", ""))
.reject { |arg| arg.match?(exclude_regex) } .reject { |arg| arg.match?(exclude_regex) }
all_args = cask_opts + args all_args = cask_opts + args
remaining = all_args.select do |arg| i = 0
!process_arguments([arg]).empty? remaining = []
rescue OptionParser::InvalidOption, OptionParser::MissingArgument, OptionParser::AmbiguousOption
true while i < all_args.count
begin
arg = all_args[i]
remaining << arg unless process_arguments([arg]).empty?
rescue OptionParser::MissingArgument
raise if i + 1 >= all_args.count
args = all_args[i..(i + 1)]
process_arguments(args)
i += 1
rescue OptionParser::InvalidOption
remaining << arg
end
i += 1
end end
remaining + non_options remaining + non_options

View File

@ -2,6 +2,9 @@
require "json" require "json"
require "lazy_object"
require "locale"
require "extend/hash_validator" require "extend/hash_validator"
using HashValidator using HashValidator
@ -24,6 +27,12 @@ module Cask
screen_saverdir: "~/Library/Screen Savers", screen_saverdir: "~/Library/Screen Savers",
}.freeze }.freeze
def self.defaults
{
languages: LazyObject.new { MacOS.languages },
}.merge(DEFAULT_DIRS).freeze
end
def self.global def self.global
@global ||= new @global ||= new
end end
@ -69,16 +78,16 @@ module Cask
attr_accessor :explicit attr_accessor :explicit
def initialize(default: nil, env: nil, explicit: {}) def initialize(default: nil, env: nil, explicit: {})
@default = self.class.canonicalize(DEFAULT_DIRS.merge(default)) if default @default = self.class.canonicalize(self.class.defaults.merge(default)) if default
@env = self.class.canonicalize(env) if env @env = self.class.canonicalize(env) if env
@explicit = self.class.canonicalize(explicit) @explicit = self.class.canonicalize(explicit)
@env&.assert_valid_keys!(*DEFAULT_DIRS.keys) @env&.assert_valid_keys!(*self.class.defaults.keys)
@explicit.assert_valid_keys!(*DEFAULT_DIRS.keys) @explicit.assert_valid_keys!(*self.class.defaults.keys)
end end
def default def default
@default ||= self.class.canonicalize(DEFAULT_DIRS) @default ||= self.class.canonicalize(self.class.defaults)
end end
def env def env
@ -86,7 +95,16 @@ module Cask
Shellwords.shellsplit(ENV.fetch("HOMEBREW_CASK_OPTS", "")) Shellwords.shellsplit(ENV.fetch("HOMEBREW_CASK_OPTS", ""))
.select { |arg| arg.include?("=") } .select { |arg| arg.include?("=") }
.map { |arg| arg.split("=", 2) } .map { |arg| arg.split("=", 2) }
.map { |(flag, value)| [flag.sub(/^--/, ""), value] }, .map do |(flag, value)|
key = flag.sub(/^--/, "")
if key == "language"
key = "languages"
value = value.split(",")
end
[key, value]
end,
) )
end end
@ -98,6 +116,24 @@ module Cask
@manpagedir ||= HOMEBREW_PREFIX/"share/man" @manpagedir ||= HOMEBREW_PREFIX/"share/man"
end end
def languages
[
*explicit[:languages],
*env[:languages],
*default[:languages],
].uniq.select do |lang|
# Ensure all languages are valid.
Locale.parse(lang)
true
rescue Locale::ParserError
false
end
end
def languages=(languages)
explicit[:languages] = languages
end
DEFAULT_DIRS.each_key do |dir| DEFAULT_DIRS.each_key do |dir|
define_method(dir) do define_method(dir) do
explicit.fetch(dir, env.fetch(dir, default.fetch(dir))) explicit.fetch(dir, env.fetch(dir, default.fetch(dir)))

View File

@ -137,13 +137,13 @@ module Cask
raise CaskInvalidError.new(cask, "No default language specified.") if @language_blocks.default.nil? raise CaskInvalidError.new(cask, "No default language specified.") if @language_blocks.default.nil?
locales = MacOS.languages locales = cask.config.languages
.map do |language| .map do |language|
Locale.parse(language) Locale.parse(language)
rescue Locale::ParserError rescue Locale::ParserError
nil nil
end end
.compact .compact
locales.each do |locale| locales.each do |locale|
key = locale.detect(@language_blocks.keys) key = locale.detect(@language_blocks.keys)
@ -225,7 +225,7 @@ module Cask
end end
def caskroom_path def caskroom_path
@cask.caskroom_path cask.caskroom_path
end end
def staged_path def staged_path

View File

@ -68,21 +68,7 @@ module OS
end end
os_langs = os_langs.scan(/[^ \n"(),]+/) os_langs = os_langs.scan(/[^ \n"(),]+/)
@languages = [ @languages = os_langs
*Homebrew.args.value("language")&.split(","),
*ENV["HOMEBREW_LANGUAGES"]&.split(","),
*os_langs,
].uniq
# Ensure all languages are valid
@languages.select! do |lang|
Locale.parse(lang)
true
rescue Locale::ParserError
false
end
@languages
end end
def language def language

View File

@ -15,16 +15,6 @@ describe Cask::Audit, :cask do
end end
end end
matcher :fail do
match(&:errors?)
end
matcher :warn do
match do |audit|
audit.warnings? && !audit.errors?
end
end
matcher :fail_with do |error_msg| matcher :fail_with do |error_msg|
match do |audit| match do |audit|
include_msg?(audit.errors, error_msg) include_msg?(audit.errors, error_msg)
@ -764,23 +754,27 @@ describe Cask::Audit, :cask do
describe "audit of downloads" do describe "audit of downloads" do
let(:cask_token) { "with-binary" } let(:cask_token) { "with-binary" }
let(:cask) { Cask::CaskLoader.load(cask_token) } let(:cask) { Cask::CaskLoader.load(cask_token) }
let(:download) { instance_double(Cask::Download) } let(:download_double) { instance_double(Cask::Download) }
let(:verify) { class_double(Cask::Verify).as_stubbed_const } let(:verify) { class_double(Cask::Verify).as_stubbed_const }
let(:error_msg) { "Download Failed" } let(:error_msg) { "Download Failed" }
before do
allow(audit).to receive(:download).and_return(download_double)
end
it "when download and verification succeed it does not fail" do it "when download and verification succeed it does not fail" do
expect(download).to receive(:perform) expect(download_double).to receive(:perform)
expect(verify).to receive(:all) expect(verify).to receive(:all)
expect(subject).not_to fail_with(/#{error_msg}/) expect(subject).not_to fail_with(/#{error_msg}/)
end end
it "when download fails it does not fail" do it "when download fails it does not fail" do
expect(download).to receive(:perform).and_raise(StandardError.new(error_msg)) expect(download_double).to receive(:perform).and_raise(StandardError.new(error_msg))
expect(subject).to fail_with(/#{error_msg}/) expect(subject).to fail_with(/#{error_msg}/)
end end
it "when verification fails it does not fail" do it "when verification fails it does not fail" do
expect(download).to receive(:perform) expect(download_double).to receive(:perform)
expect(verify).to receive(:all).and_raise(StandardError.new(error_msg)) expect(verify).to receive(:all).and_raise(StandardError.new(error_msg))
expect(subject).to fail_with(/#{error_msg}/) expect(subject).to fail_with(/#{error_msg}/)
end end

View File

@ -15,12 +15,6 @@ describe Cask::Cmd, :cask do
]) ])
end end
it "ignores the `--language` option, which is handled in `OS::Mac`" do
cli = described_class.new("--language=en")
expect(cli).to receive(:detect_internal_command).with(no_args)
cli.run
end
context "when given no arguments" do context "when given no arguments" do
it "exits successfully" do it "exits successfully" do
expect(subject).not_to receive(:exit).with(be_nonzero) expect(subject).not_to receive(:exit).with(be_nonzero)

View File

@ -118,8 +118,8 @@ describe Cask::DSL, :cask do
end end
describe "language stanza" do describe "language stanza" do
it "allows multilingual casks" do context "when language is set explicitly" do
cask = lambda do subject(:cask) {
Cask::Cask.new("cask-with-apps") do Cask::Cask.new("cask-with-apps") do
language "zh" do language "zh" do
sha256 "abc123" sha256 "abc123"
@ -133,37 +133,65 @@ describe Cask::DSL, :cask do
url "https://example.org/#{language}.zip" url "https://example.org/#{language}.zip"
end end
}
matcher :be_the_chinese_version do
match do |cask|
expect(cask.language).to eq("zh-CN")
expect(cask.sha256).to eq("abc123")
expect(cask.url.to_s).to eq("https://example.org/zh-CN.zip")
end
end end
allow(MacOS).to receive(:languages).and_return(["zh"]) matcher :be_the_english_version do
expect(cask.call.language).to eq("zh-CN") match do |cask|
expect(cask.call.sha256).to eq("abc123") expect(cask.language).to eq("en-US")
expect(cask.call.url.to_s).to eq("https://example.org/zh-CN.zip") expect(cask.sha256).to eq("xyz789")
expect(cask.url.to_s).to eq("https://example.org/en-US.zip")
end
end
allow(MacOS).to receive(:languages).and_return(["zh-XX"]) before do
expect(cask.call.language).to eq("zh-CN") config = cask.config
expect(cask.call.sha256).to eq("abc123") config.languages = languages
expect(cask.call.url.to_s).to eq("https://example.org/zh-CN.zip") cask.config = config
end
allow(MacOS).to receive(:languages).and_return(["en"]) context "to 'zh'" do
expect(cask.call.language).to eq("en-US") let(:languages) { ["zh"] }
expect(cask.call.sha256).to eq("xyz789")
expect(cask.call.url.to_s).to eq("https://example.org/en-US.zip")
allow(MacOS).to receive(:languages).and_return(["xx-XX"]) it { is_expected.to be_the_chinese_version }
expect(cask.call.language).to eq("en-US") end
expect(cask.call.sha256).to eq("xyz789")
expect(cask.call.url.to_s).to eq("https://example.org/en-US.zip")
allow(MacOS).to receive(:languages).and_return(["xx-XX", "zh", "en"]) context "to 'zh-XX'" do
expect(cask.call.language).to eq("zh-CN") let(:languages) { ["zh-XX"] }
expect(cask.call.sha256).to eq("abc123")
expect(cask.call.url.to_s).to eq("https://example.org/zh-CN.zip")
allow(MacOS).to receive(:languages).and_return(["xx-XX", "en-US", "zh"]) it { is_expected.to be_the_chinese_version }
expect(cask.call.language).to eq("en-US") end
expect(cask.call.sha256).to eq("xyz789")
expect(cask.call.url.to_s).to eq("https://example.org/en-US.zip") context "to 'en'" do
let(:languages) { ["en"] }
it { is_expected.to be_the_english_version }
end
context "to 'xx-XX'" do
let(:languages) { ["xx-XX"] }
it { is_expected.to be_the_english_version }
end
context "to 'xx-XX,zh,en'" do
let(:languages) { ["xx-XX", "zh", "en"] }
it { is_expected.to be_the_chinese_version }
end
context "to 'xx-XX,en-US,zh'" do
let(:languages) { ["xx-XX", "en-US", "zh"] }
it { is_expected.to be_the_english_version }
end
end end
it "returns an empty array if no languages are specified" do it "returns an empty array if no languages are specified" do

View File

@ -6,7 +6,7 @@ describe "Homebrew.readall_args" do
it_behaves_like "parseable arguments" it_behaves_like "parseable arguments"
end end
describe "brew readall", :integration_test do describe "brew readall", :integration_test, timeout: 180 do
it "imports all Formulae for a given Tap" do it "imports all Formulae for a given Tap" do
formula_file = setup_test_formula "testball" formula_file = setup_test_formula "testball"

View File

@ -5,10 +5,8 @@ require "os/mac"
describe OS::Mac do describe OS::Mac do
describe "::languages" do describe "::languages" do
specify "all languages can be parsed by Locale::parse" do it "returns a list of all languages" do
subject.languages.each do |language| expect(subject.languages).not_to be_empty
expect { Locale.parse(language) }.not_to raise_error
end
end end
end end
@ -16,10 +14,6 @@ describe OS::Mac do
it "returns the first item from #languages" do it "returns the first item from #languages" do
expect(subject.language).to eq(subject.languages.first) expect(subject.language).to eq(subject.languages.first)
end end
it "can be parsed by Locale::parse" do
expect { Locale.parse(subject.language) }.not_to raise_error
end
end end
describe "::sdk_path_if_needed" do describe "::sdk_path_if_needed" do

View File

@ -29,6 +29,7 @@ require "rubocop"
require "rubocop/rspec/support" require "rubocop/rspec/support"
require "find" require "find"
require "byebug" require "byebug"
require "timeout"
$LOAD_PATH.push(File.expand_path("#{ENV["HOMEBREW_LIBRARY"]}/Homebrew/test/support/lib")) $LOAD_PATH.push(File.expand_path("#{ENV["HOMEBREW_LIBRARY"]}/Homebrew/test/support/lib"))
@ -181,7 +182,19 @@ RSpec.configure do |config|
$stderr.reopen(File::NULL) $stderr.reopen(File::NULL)
end end
example.run begin
timeout = example.metadata.fetch(:timeout, 60)
inner_timeout = nil
Timeout.timeout(timeout) do
example.run
rescue Timeout::Error => e
inner_timeout = e
end
rescue Timeout::Error
raise "Example exceeded maximum runtime of #{timeout} seconds."
end
raise inner_timeout if inner_timeout
rescue SystemExit => e rescue SystemExit => e
raise "Unexpected exit with status #{e.status}." raise "Unexpected exit with status #{e.status}."
ensure ensure