From f012b5acf875a265d34721577ccb01514d9560df Mon Sep 17 00:00:00 2001 From: commitay Date: Tue, 5 Jun 2018 16:42:15 +1000 Subject: [PATCH 1/3] cask audit: check for sourceforge appcast --- Library/Homebrew/cask/lib/hbc/audit.rb | 16 ++++++++++++-- Library/Homebrew/test/cask/audit_spec.rb | 22 +++++++++++++++++++ .../cask/Casks/sourceforge-with-appcast.rb | 8 +++++++ 3 files changed, 44 insertions(+), 2 deletions(-) create mode 100644 Library/Homebrew/test/support/fixtures/cask/Casks/sourceforge-with-appcast.rb diff --git a/Library/Homebrew/cask/lib/hbc/audit.rb b/Library/Homebrew/cask/lib/hbc/audit.rb index 45c6f3ec3a..cddfc5558e 100644 --- a/Library/Homebrew/cask/lib/hbc/audit.rb +++ b/Library/Homebrew/cask/lib/hbc/audit.rb @@ -34,7 +34,7 @@ module Hbc check_single_pre_postflight check_single_uninstall_zap check_untrusted_pkg - check_github_releases_appcast + check_hosting_with_appcast check_latest_with_appcast check_stanza_requires_uninstall self @@ -246,13 +246,25 @@ module Hbc add_warning "Casks with an appcast should not use version :latest" end - def check_github_releases_appcast + def check_hosting_with_appcast return if cask.appcast + check_github_releases_appcast + check_sourceforge_appcast + end + + def check_github_releases_appcast return unless cask.url.to_s =~ %r{github.com/([^/]+)/([^/]+)/releases/download/(\S+)} add_warning "Cask uses GitHub releases, please add an appcast. See https://github.com/Homebrew/homebrew-cask/blob/master/doc/cask_language_reference/stanzas/appcast.md" end + def check_sourceforge_appcast + return if cask.version.latest? + return unless cask.url.to_s =~ %r{sourceforge.net/(\S+)} + + add_warning "Cask hosted on SourceForge, please add an appcast. See https://github.com/Homebrew/homebrew-cask/blob/master/doc/cask_language_reference/stanzas/appcast.md" + end + def check_url return unless cask.url check_download_url_format diff --git a/Library/Homebrew/test/cask/audit_spec.rb b/Library/Homebrew/test/cask/audit_spec.rb index 7ec1675a97..e6cef82072 100644 --- a/Library/Homebrew/test/cask/audit_spec.rb +++ b/Library/Homebrew/test/cask/audit_spec.rb @@ -460,6 +460,28 @@ describe Hbc::Audit, :cask do end end + describe "SourceForge appcast check" do + let(:error_msg) { /Cask hosted on SourceForge/ } + + context "when the Cask is not hosted on SourceForge" do + let(:cask_token) { "basic-cask" } + + it { is_expected.not_to warn_with(error_msg) } + end + + context "when the Cask is hosted on SourceForge and has an appcast" do + let(:cask_token) { "sourceforge-with-appcast" } + + it { is_expected.not_to warn_with(error_msg) } + end + + context "when the Cask is hosted on SourceForge and does not have an appcast" do + let(:cask_token) { "sourceforge-correct-url-format" } + + it { is_expected.to warn_with(error_msg) } + end + end + describe "latest with appcast checks" do let(:error_msg) { "Casks with an appcast should not use version :latest" } diff --git a/Library/Homebrew/test/support/fixtures/cask/Casks/sourceforge-with-appcast.rb b/Library/Homebrew/test/support/fixtures/cask/Casks/sourceforge-with-appcast.rb new file mode 100644 index 0000000000..fd4a388bcb --- /dev/null +++ b/Library/Homebrew/test/support/fixtures/cask/Casks/sourceforge-with-appcast.rb @@ -0,0 +1,8 @@ +cask 'sourceforge-with-appcast' do + version '1.2.3' + + url 'https://downloads.sourceforge.net/something/Something-1.2.3.dmg' + appcast 'https://sourceforge.net/projects/something/rss', + checkpoint: '407fb59baa4b9eb7651d9243b89c30b7481590947ef78bd5a4c24f5810f56531' + homepage 'https://sourceforge.net/projects/something/' +end From c354d76e479bf52e966d4c4b4e1136b25c8ca848 Mon Sep 17 00:00:00 2001 From: commitay Date: Tue, 5 Jun 2018 19:09:14 +1000 Subject: [PATCH 2/3] cask audit: various --- Library/Homebrew/test/cask/audit_spec.rb | 76 ++++++++++++------------ 1 file changed, 38 insertions(+), 38 deletions(-) diff --git a/Library/Homebrew/test/cask/audit_spec.rb b/Library/Homebrew/test/cask/audit_spec.rb index e6cef82072..8ee7eb95e1 100644 --- a/Library/Homebrew/test/cask/audit_spec.rb +++ b/Library/Homebrew/test/cask/audit_spec.rb @@ -94,190 +94,190 @@ describe Hbc::Audit, :cask do end describe "pkg allow_untrusted checks" do - let(:error_msg) { "allow_untrusted is not permitted in official Homebrew-Cask taps" } + let(:warning_msg) { "allow_untrusted is not permitted in official Homebrew-Cask taps" } context "when the Cask has no pkg stanza" do let(:cask_token) { "basic-cask" } - it { is_expected.not_to warn_with(error_msg) } + it { is_expected.not_to warn_with(warning_msg) } end context "when the Cask does not have allow_untrusted" do let(:cask_token) { "with-uninstall-pkgutil" } - it { is_expected.not_to warn_with(error_msg) } + it { is_expected.not_to warn_with(warning_msg) } end context "when the Cask has allow_untrusted" do let(:cask_token) { "with-allow-untrusted" } - it { is_expected.to warn_with(error_msg) } + it { is_expected.to warn_with(warning_msg) } end end describe "when the Cask stanza requires uninstall" do - let(:error_msg) { "installer and pkg stanzas require an uninstall stanza" } + let(:warning_msg) { "installer and pkg stanzas require an uninstall stanza" } context "when the Cask does not require an uninstall" do let(:cask_token) { "basic-cask" } - it { is_expected.not_to warn_with(error_msg) } + it { is_expected.not_to warn_with(warning_msg) } end context "when the pkg Cask has an uninstall" do let(:cask_token) { "with-uninstall-pkgutil" } - it { is_expected.not_to warn_with(error_msg) } + it { is_expected.not_to warn_with(warning_msg) } end context "when the installer Cask has an uninstall" do let(:cask_token) { "installer-with-uninstall" } - it { is_expected.not_to warn_with(error_msg) } + it { is_expected.not_to warn_with(warning_msg) } end context "when the installer Cask does not have an uninstall" do let(:cask_token) { "with-installer-manual" } - it { is_expected.to warn_with(error_msg) } + it { is_expected.to warn_with(warning_msg) } end context "when the pkg Cask does not have an uninstall" do let(:cask_token) { "pkg-without-uninstall" } - it { is_expected.to warn_with(error_msg) } + it { is_expected.to warn_with(warning_msg) } end end describe "preflight stanza checks" do - let(:error_msg) { "only a single preflight stanza is allowed" } + let(:warning_msg) { "only a single preflight stanza is allowed" } context "when the Cask has no preflight stanza" do let(:cask_token) { "with-zap-rmdir" } - it { is_expected.not_to warn_with(error_msg) } + it { is_expected.not_to warn_with(warning_msg) } end context "when the Cask has only one preflight stanza" do let(:cask_token) { "with-preflight" } - it { is_expected.not_to warn_with(error_msg) } + it { is_expected.not_to warn_with(warning_msg) } end context "when the Cask has multiple preflight stanzas" do let(:cask_token) { "with-preflight-multi" } - it { is_expected.to warn_with(error_msg) } + it { is_expected.to warn_with(warning_msg) } end end describe "uninstall_postflight stanza checks" do - let(:error_msg) { "only a single postflight stanza is allowed" } + let(:warning_msg) { "only a single postflight stanza is allowed" } context "when the Cask has no postflight stanza" do let(:cask_token) { "with-zap-rmdir" } - it { is_expected.not_to warn_with(error_msg) } + it { is_expected.not_to warn_with(warning_msg) } end context "when the Cask has only one postflight stanza" do let(:cask_token) { "with-postflight" } - it { is_expected.not_to warn_with(error_msg) } + it { is_expected.not_to warn_with(warning_msg) } end context "when the Cask has multiple postflight stanzas" do let(:cask_token) { "with-postflight-multi" } - it { is_expected.to warn_with(error_msg) } + it { is_expected.to warn_with(warning_msg) } end end describe "uninstall stanza checks" do - let(:error_msg) { "only a single uninstall stanza is allowed" } + let(:warning_msg) { "only a single uninstall stanza is allowed" } context "when the Cask has no uninstall stanza" do let(:cask_token) { "with-zap-rmdir" } - it { is_expected.not_to warn_with(error_msg) } + it { is_expected.not_to warn_with(warning_msg) } end context "when the Cask has only one uninstall stanza" do let(:cask_token) { "with-uninstall-rmdir" } - it { is_expected.not_to warn_with(error_msg) } + it { is_expected.not_to warn_with(warning_msg) } end context "when the Cask has multiple uninstall stanzas" do let(:cask_token) { "with-uninstall-multi" } - it { is_expected.to warn_with(error_msg) } + it { is_expected.to warn_with(warning_msg) } end end describe "uninstall_preflight stanza checks" do - let(:error_msg) { "only a single uninstall_preflight stanza is allowed" } + let(:warning_msg) { "only a single uninstall_preflight stanza is allowed" } context "when the Cask has no uninstall_preflight stanza" do let(:cask_token) { "with-zap-rmdir" } - it { is_expected.not_to warn_with(error_msg) } + it { is_expected.not_to warn_with(warning_msg) } end context "when the Cask has only one uninstall_preflight stanza" do let(:cask_token) { "with-uninstall-preflight" } - it { is_expected.not_to warn_with(error_msg) } + it { is_expected.not_to warn_with(warning_msg) } end context "when the Cask has multiple uninstall_preflight stanzas" do let(:cask_token) { "with-uninstall-preflight-multi" } - it { is_expected.to warn_with(error_msg) } + it { is_expected.to warn_with(warning_msg) } end end describe "uninstall_postflight stanza checks" do - let(:error_msg) { "only a single uninstall_postflight stanza is allowed" } + let(:warning_msg) { "only a single uninstall_postflight stanza is allowed" } context "when the Cask has no uninstall_postflight stanza" do let(:cask_token) { "with-zap-rmdir" } - it { is_expected.not_to warn_with(error_msg) } + it { is_expected.not_to warn_with(warning_msg) } end context "when the Cask has only one uninstall_postflight stanza" do let(:cask_token) { "with-uninstall-postflight" } - it { is_expected.not_to warn_with(error_msg) } + it { is_expected.not_to warn_with(warning_msg) } end context "when the Cask has multiple uninstall_postflight stanzas" do let(:cask_token) { "with-uninstall-postflight-multi" } - it { is_expected.to warn_with(error_msg) } + it { is_expected.to warn_with(warning_msg) } end end describe "zap stanza checks" do - let(:error_msg) { "only a single zap stanza is allowed" } + let(:warning_msg) { "only a single zap stanza is allowed" } context "when the Cask has no zap stanza" do let(:cask_token) { "with-uninstall-rmdir" } - it { is_expected.not_to warn_with(error_msg) } + it { is_expected.not_to warn_with(warning_msg) } end context "when the Cask has only one zap stanza" do let(:cask_token) { "with-zap-rmdir" } - it { is_expected.not_to warn_with(error_msg) } + it { is_expected.not_to warn_with(warning_msg) } end context "when the Cask has multiple zap stanzas" do let(:cask_token) { "with-zap-multi" } - it { is_expected.to warn_with(error_msg) } + it { is_expected.to warn_with(warning_msg) } end end @@ -483,24 +483,24 @@ describe Hbc::Audit, :cask do end describe "latest with appcast checks" do - let(:error_msg) { "Casks with an appcast should not use version :latest" } + let(:warning_msg) { "Casks with an appcast should not use version :latest" } context "when the Cask is :latest and does not have an appcast" do let(:cask_token) { "version-latest" } - it { is_expected.not_to warn_with(error_msg) } + it { is_expected.not_to warn_with(warning_msg) } end context "when the Cask is versioned and has an appcast" do let(:cask_token) { "with-appcast" } - it { is_expected.not_to warn_with(error_msg) } + it { is_expected.not_to warn_with(warning_msg) } end context "when the Cask is :latest and has an appcast" do let(:cask_token) { "latest-with-appcast" } - it { is_expected.to warn_with(error_msg) } + it { is_expected.to warn_with(warning_msg) } end end From b9b3952494042256a7c836aa1d6df4b0b82d4f5c Mon Sep 17 00:00:00 2001 From: commitay Date: Tue, 5 Jun 2018 19:20:38 +1000 Subject: [PATCH 3/3] cask audit: review changes --- Library/Homebrew/cask/lib/hbc/audit.rb | 4 ++-- Library/Homebrew/test/cask/audit_spec.rb | 28 ++++++++++++------------ 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/Library/Homebrew/cask/lib/hbc/audit.rb b/Library/Homebrew/cask/lib/hbc/audit.rb index cddfc5558e..1707598b1c 100644 --- a/Library/Homebrew/cask/lib/hbc/audit.rb +++ b/Library/Homebrew/cask/lib/hbc/audit.rb @@ -255,14 +255,14 @@ module Hbc def check_github_releases_appcast return unless cask.url.to_s =~ %r{github.com/([^/]+)/([^/]+)/releases/download/(\S+)} - add_warning "Cask uses GitHub releases, please add an appcast. See https://github.com/Homebrew/homebrew-cask/blob/master/doc/cask_language_reference/stanzas/appcast.md" + add_warning "Download uses GitHub releases, please add an appcast. See https://github.com/Homebrew/homebrew-cask/blob/master/doc/cask_language_reference/stanzas/appcast.md" end def check_sourceforge_appcast return if cask.version.latest? return unless cask.url.to_s =~ %r{sourceforge.net/(\S+)} - add_warning "Cask hosted on SourceForge, please add an appcast. See https://github.com/Homebrew/homebrew-cask/blob/master/doc/cask_language_reference/stanzas/appcast.md" + add_warning "Download is hosted on SourceForge, please add an appcast. See https://github.com/Homebrew/homebrew-cask/blob/master/doc/cask_language_reference/stanzas/appcast.md" end def check_url diff --git a/Library/Homebrew/test/cask/audit_spec.rb b/Library/Homebrew/test/cask/audit_spec.rb index 8ee7eb95e1..f287d399a7 100644 --- a/Library/Homebrew/test/cask/audit_spec.rb +++ b/Library/Homebrew/test/cask/audit_spec.rb @@ -439,46 +439,46 @@ describe Hbc::Audit, :cask do end describe "GitHub releases appcast check" do - let(:error_msg) { /Cask uses GitHub releases/ } + let(:appcast_warning) { /Download uses GitHub releases/ } - context "when the Cask does not use GitHub releases" do + context "when the download does not use GitHub releases" do let(:cask_token) { "basic-cask" } - it { is_expected.not_to warn_with(error_msg) } + it { is_expected.not_to warn_with(appcast_warning) } end - context "when the Cask uses GitHub releases and has an appcast" do + context "when the download uses GitHub releases and has an appcast" do let(:cask_token) { "github-with-appcast" } - it { is_expected.not_to warn_with(error_msg) } + it { is_expected.not_to warn_with(appcast_warning) } end - context "when the Cask uses GitHub releases and does not have an appcast" do + context "when the download uses GitHub releases and does not have an appcast" do let(:cask_token) { "github-without-appcast" } - it { is_expected.to warn_with(error_msg) } + it { is_expected.to warn_with(appcast_warning) } end end describe "SourceForge appcast check" do - let(:error_msg) { /Cask hosted on SourceForge/ } + let(:appcast_warning) { /Download is hosted on SourceForge/ } - context "when the Cask is not hosted on SourceForge" do + context "when the download is not hosted on SourceForge" do let(:cask_token) { "basic-cask" } - it { is_expected.not_to warn_with(error_msg) } + it { is_expected.not_to warn_with(appcast_warning) } end - context "when the Cask is hosted on SourceForge and has an appcast" do + context "when the download is hosted on SourceForge and has an appcast" do let(:cask_token) { "sourceforge-with-appcast" } - it { is_expected.not_to warn_with(error_msg) } + it { is_expected.not_to warn_with(appcast_warning) } end - context "when the Cask is hosted on SourceForge and does not have an appcast" do + context "when the download is hosted on SourceForge and does not have an appcast" do let(:cask_token) { "sourceforge-correct-url-format" } - it { is_expected.to warn_with(error_msg) } + it { is_expected.to warn_with(appcast_warning) } end end