Skip to content

Commit

Permalink
Improve cask audit
Browse files Browse the repository at this point in the history
- check for cask.url in audit steps
- check for cask.version in audit steps
- check for cask.sha256 in fetch command
- stop omitting casks based on nil url in audit command

It would be nice to be able to omit casks from the audit
if the os is not supported but there is not easy way to
do that without updating the SimulateSystem code or
refactoring how MacOSRequirement's are defined in the DSL.
  • Loading branch information
apainintheneck committed Sep 8, 2023
1 parent 43295b2 commit 1dc9274
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 12 deletions.
13 changes: 10 additions & 3 deletions Library/Homebrew/cask/audit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ def audit_no_string_version_latest
sig { void }
def audit_sha256_no_check_if_latest
return unless cask.sha256
return unless cask.version

odebug "Auditing sha256 :no_check with version :latest"
return unless cask.version.latest?
Expand Down Expand Up @@ -268,7 +269,7 @@ def audit_sha256_invalid

sig { void }
def audit_latest_with_livecheck
return unless cask.version.latest?
return unless cask.version&.latest?
return unless cask.livecheckable?
return if cask.livecheck.skip?

Expand All @@ -277,7 +278,7 @@ def audit_latest_with_livecheck

sig { void }
def audit_latest_with_auto_updates
return unless cask.version.latest?
return unless cask.version&.latest?
return unless cask.auto_updates

add_error "Casks with `version :latest` should not use `auto_updates`."
Expand All @@ -288,7 +289,8 @@ def audit_latest_with_auto_updates
sig { params(livecheck_result: T.any(NilClass, T::Boolean, Symbol)).void }
def audit_hosting_with_livecheck(livecheck_result: audit_livecheck_version)
return if cask.discontinued?
return if cask.version.latest?
return if cask.version&.latest?
return unless cask.url
return if block_url_offline?
return if cask.livecheckable?
return if livecheck_result == :auto_detected
Expand Down Expand Up @@ -328,6 +330,7 @@ def audit_download_url_format

sig { void }
def audit_unnecessary_verified
return unless cask.url
return if block_url_offline?
return unless verified_present?
return unless url_match_homepage?
Expand All @@ -340,6 +343,7 @@ def audit_unnecessary_verified

sig { void }
def audit_missing_verified
return unless cask.url
return if block_url_offline?
return if file_url?
return if url_match_homepage?
Expand All @@ -352,6 +356,7 @@ def audit_missing_verified

sig { void }
def audit_no_match
return unless cask.url
return if block_url_offline?
return unless verified_present?
return if verified_matches_url?
Expand Down Expand Up @@ -453,6 +458,7 @@ def audit_download

sig { void }
def audit_livecheck_unneeded_long_version
return if cask.version.nil? || cask.url.nil?
return if cask.livecheck.strategy != :sparkle
return unless cask.version.csv.second
return if cask.url.to_s.include? cask.version.csv.second
Expand Down Expand Up @@ -506,6 +512,7 @@ def audit_signing
sig { returns(T.any(NilClass, T::Boolean, Symbol)) }
def audit_livecheck_version
return unless online?
return unless cask.version

referenced_cask, = Homebrew::Livecheck.resolve_livecheck_reference(cask)

Expand Down
2 changes: 2 additions & 0 deletions Library/Homebrew/cask/download.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ def checksum

sig { override.returns(T.nilable(Version)) }
def version
return if cask.version.nil?

@version ||= Version.new(cask.version)
end

Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/cmd/fetch.rb
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ def self.fetch
SimulateSystem.with os: os, arch: arch do
cask = Cask::CaskLoader.load(ref)

if cask.url.nil?
if cask.url.nil? || cask.sha256.nil?
opoo "Cask #{cask} is not supported on os #{os} and arch #{arch}"
next
end
Expand Down
9 changes: 1 addition & 8 deletions Library/Homebrew/dev-cmd/audit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -242,17 +242,10 @@ def self.audit
next [] if os == :linux

SimulateSystem.with os: os, arch: arch do
simulated_cask = Cask::CaskLoader.load(path)

if simulated_cask.url.nil?
opoo "Cask #{cask} is not supported on os #{os} and arch #{arch}"
next []
end

odebug "Auditing Cask #{cask} on os #{os} and arch #{arch}"

Cask::Auditor.audit(
simulated_cask,
Cask::CaskLoader.load(path),
# For switches, we add `|| nil` so that `nil` will be passed
# instead of `false` if they aren't set.
# This way, we can distinguish between "not set" and "set to false".
Expand Down

0 comments on commit 1dc9274

Please sign in to comment.