Fix cask sharding issues

- Fix cask info output being incorrect
- Improve some code referring to casks as formulae
- Move livecheck cask fixtures to not shadow existing names
- Adjust the cask tap symlinking logic to make handling outdated
  shadowed casks significantly easier
- Fix various flaky tests caused by casks sharding logic
- Prefer longer paths when there's multiple formulae or casks in a tap
  with the same name rather than always using the first
This commit is contained in:
Mike McQuaid 2023-08-10 16:08:47 +01:00
parent 1a8f1be34b
commit 22553cd34a
No known key found for this signature in database
GPG Key ID: 3338A31AFDB1D829
16 changed files with 70 additions and 49 deletions

View File

@ -78,7 +78,7 @@ module Cask
url = if cask.tap.custom_remote? && !cask.tap.remote.nil? url = if cask.tap.custom_remote? && !cask.tap.remote.nil?
cask.tap.remote cask.tap.remote
else else
"#{cask.tap.default_remote}/blob/HEAD/Casks/#{cask.token}.rb" "#{cask.tap.default_remote}/blob/HEAD/#{cask.tap.relative_cask_path(cask.token)}"
end end
"From: #{Formatter.url(url)}" "From: #{Formatter.url(url)}"

View File

@ -241,18 +241,23 @@ module Homebrew
end end
end end
def github_info(formula) def github_info(formula_or_cask)
return formula.path if formula.tap.blank? || formula.tap.remote.blank? return formula_or_cask.path if formula_or_cask.tap.blank? || formula_or_cask.tap.remote.blank?
path = case formula path = case formula_or_cask
when Formula when Formula
formula = formula_or_cask
formula.path.relative_path_from(T.must(formula.tap).path) formula.path.relative_path_from(T.must(formula.tap).path)
when Cask::Cask when Cask::Cask
return "#{formula.tap.default_remote}/blob/HEAD/Casks/#{formula.token}.rb" if formula.sourcefile_path.blank? cask = formula_or_cask
if cask.sourcefile_path.blank?
formula.sourcefile_path.relative_path_from(formula.tap.path) return "#{cask.tap.default_remote}/blob/HEAD/#{cask.tap.relative_cask_path(cask.token)}"
end end
github_remote_path(formula.tap.remote, path)
cask.sourcefile_path.relative_path_from(cask.tap.path)
end
github_remote_path(formula_or_cask.tap.remote, path)
end end
def info_formula(formula, args:) def info_formula(formula, args:)

View File

@ -516,6 +516,12 @@ class Tap
cask_dir/"#{token.downcase}.rb" cask_dir/"#{token.downcase}.rb"
end end
sig { params(token: String).returns(String) }
def relative_cask_path(token)
new_cask_path(token).to_s
.delete_prefix("#{path}/")
end
def contents def contents
contents = [] contents = []
@ -563,9 +569,10 @@ class Tap
sig { returns(T::Hash[String, Pathname]) } sig { returns(T::Hash[String, Pathname]) }
def formula_files_by_name def formula_files_by_name
formula_files.each_with_object({}) do |file, hash| formula_files.each_with_object({}) do |file, hash|
# If there's more than one file with the same basename: intentionally # If there's more than one file with the same basename: use the longer one to prioritise more specifc results.
# ignore the later ones here. basename = file.basename(".rb").to_s
hash[file.basename(".rb").to_s] ||= file existing_file = hash[basename]
hash[basename] = file if existing_file.nil? || existing_file.to_s.length < file.to_s.length
end end
end end
@ -592,9 +599,10 @@ class Tap
sig { returns(T::Hash[String, Pathname]) } sig { returns(T::Hash[String, Pathname]) }
def cask_files_by_name def cask_files_by_name
cask_files.each_with_object({}) do |file, hash| cask_files.each_with_object({}) do |file, hash|
# If there's more than one file with the same basename: intentionally # If there's more than one file with the same basename: use the longer one to prioritise more specifc results.
# ignore the later ones here. basename = file.basename(".rb").to_s
hash[file.basename(".rb").to_s] ||= file existing_file = hash[basename]
hash[basename] = file if existing_file.nil? || existing_file.to_s.length < file.to_s.length
end end
end end
@ -1110,9 +1118,10 @@ class CoreTap < AbstractCoreTap
Homebrew::API::Formula.all_formulae.each_with_object({}) do |item, hash| Homebrew::API::Formula.all_formulae.each_with_object({}) do |item, hash|
name, formula_hash = item name, formula_hash = item
# If there's more than one file with the same basename: intentionally # If there's more than one item with the same path: use the longer one to prioritise more specific results.
# ignore the later ones here. existing_path = hash[name]
hash[name] ||= path/formula_hash["ruby_source_path"] new_path = path/formula_hash["ruby_source_path"]
hash[name] = new_path if existing_path.nil? || existing_path.to_s.length < new_path.to_s.length
end end
end end
end end
@ -1158,9 +1167,10 @@ class CoreCaskTap < AbstractCoreTap
Homebrew::API::Cask.all_casks.each_with_object({}) do |item, hash| Homebrew::API::Cask.all_casks.each_with_object({}) do |item, hash|
name, cask_hash = item name, cask_hash = item
# If there's more than one file with the same basename: intentionally # If there's more than one item with the same path: use the longer one to prioritise more specific results.
# ignore the later ones here. existing_path = hash[name]
hash[name] ||= path/cask_hash["ruby_source_path"] new_path = path/cask_hash["ruby_source_path"]
hash[name] = new_path if existing_path.nil? || existing_path.to_s.length < new_path.to_s.length
end end
end end

View File

@ -531,37 +531,37 @@ describe Cask::Audit, :cask do
end end
context "when the Cask is discontinued" do context "when the Cask is discontinued" do
let(:cask_token) { "livecheck/discontinued" } let(:cask_token) { "livecheck/livecheck-discontinued" }
it { is_expected.not_to error_with(message) } it { is_expected.not_to error_with(message) }
end end
context "when the Cask has a livecheck block referencing a discontinued Cask" do context "when the Cask has a livecheck block referencing a discontinued Cask" do
let(:cask_token) { "livecheck/discontinued-reference" } let(:cask_token) { "livecheck/livecheck-discontinued-reference" }
it { is_expected.not_to error_with(message) } it { is_expected.not_to error_with(message) }
end end
context "when version is :latest" do context "when version is :latest" do
let(:cask_token) { "livecheck/version-latest" } let(:cask_token) { "livecheck/livecheck-version-latest" }
it { is_expected.not_to error_with(message) } it { is_expected.not_to error_with(message) }
end end
context "when the Cask has a livecheck block referencing a Cask where version is :latest" do context "when the Cask has a livecheck block referencing a Cask where version is :latest" do
let(:cask_token) { "livecheck/version-latest-reference" } let(:cask_token) { "livecheck/livecheck-version-latest-reference" }
it { is_expected.not_to error_with(message) } it { is_expected.not_to error_with(message) }
end end
context "when url is unversioned" do context "when url is unversioned" do
let(:cask_token) { "livecheck/url-unversioned" } let(:cask_token) { "livecheck/livecheck-url-unversioned" }
it { is_expected.not_to error_with(message) } it { is_expected.not_to error_with(message) }
end end
context "when the Cask has a livecheck block referencing a Cask with an unversioned url" do context "when the Cask has a livecheck block referencing a Cask with an unversioned url" do
let(:cask_token) { "livecheck/url-unversioned-reference" } let(:cask_token) { "livecheck/livecheck-url-unversioned-reference" }
it { is_expected.not_to error_with(message) } it { is_expected.not_to error_with(message) }
end end

View File

@ -41,7 +41,7 @@ describe Cask::Cask, :cask do
end end
it "returns an instance of the Cask from a JSON file" do it "returns an instance of the Cask from a JSON file" do
c = Cask::CaskLoader.load("#{tap_path}/caffeine.json") c = Cask::CaskLoader.load("#{TEST_FIXTURE_DIR}/cask/caffeine.json")
expect(c).to be_a(described_class) expect(c).to be_a(described_class)
expect(c.token).to eq("caffeine") expect(c.token).to eq("caffeine")
end end

View File

@ -10,7 +10,7 @@ describe Cask::Info, :cask do
==> local-transmission: 2.61 ==> local-transmission: 2.61
https://transmissionbt.com/ https://transmissionbt.com/
Not installed Not installed
From: https://github.com/Homebrew/homebrew-cask/blob/HEAD/Casks/local-transmission.rb From: https://github.com/Homebrew/homebrew-cask/blob/HEAD/Casks/l/local-transmission.rb
==> Name ==> Name
Transmission Transmission
==> Description ==> Description
@ -27,7 +27,7 @@ describe Cask::Info, :cask do
==> with-auto-updates: 1.0 (auto_updates) ==> with-auto-updates: 1.0 (auto_updates)
https://brew.sh/autoupdates https://brew.sh/autoupdates
Not installed Not installed
From: https://github.com/Homebrew/homebrew-cask/blob/HEAD/Casks/with-auto-updates.rb From: https://github.com/Homebrew/homebrew-cask/blob/HEAD/Casks/w/with-auto-updates.rb
==> Name ==> Name
AutoUpdates AutoUpdates
==> Description ==> Description
@ -44,7 +44,7 @@ describe Cask::Info, :cask do
==> with-caveats: 1.2.3 ==> with-caveats: 1.2.3
https://brew.sh/ https://brew.sh/
Not installed Not installed
From: https://github.com/Homebrew/homebrew-cask/blob/HEAD/Casks/with-caveats.rb From: https://github.com/Homebrew/homebrew-cask/blob/HEAD/Casks/w/with-caveats.rb
==> Name ==> Name
None None
==> Description ==> Description
@ -71,7 +71,7 @@ describe Cask::Info, :cask do
==> with-conditional-caveats: 1.2.3 ==> with-conditional-caveats: 1.2.3
https://brew.sh/ https://brew.sh/
Not installed Not installed
From: https://github.com/Homebrew/homebrew-cask/blob/HEAD/Casks/with-conditional-caveats.rb From: https://github.com/Homebrew/homebrew-cask/blob/HEAD/Casks/w/with-conditional-caveats.rb
==> Name ==> Name
None None
==> Description ==> Description
@ -88,7 +88,7 @@ describe Cask::Info, :cask do
==> with-languages: 1.2.3 ==> with-languages: 1.2.3
https://brew.sh/ https://brew.sh/
Not installed Not installed
From: https://github.com/Homebrew/homebrew-cask/blob/HEAD/Casks/with-languages.rb From: https://github.com/Homebrew/homebrew-cask/blob/HEAD/Casks/w/with-languages.rb
==> Name ==> Name
None None
==> Description ==> Description
@ -107,7 +107,7 @@ describe Cask::Info, :cask do
==> without-languages: 1.2.3 ==> without-languages: 1.2.3
https://brew.sh/ https://brew.sh/
Not installed Not installed
From: https://github.com/Homebrew/homebrew-cask/blob/HEAD/Casks/without-languages.rb From: https://github.com/Homebrew/homebrew-cask/blob/HEAD/Casks/w/without-languages.rb
==> Name ==> Name
None None
==> Description ==> Description

View File

@ -34,6 +34,7 @@ describe Cask::Upgrade, :cask do
before do before do
installed.each { |cask| Cask::Installer.new(Cask::CaskLoader.load(cask_path(cask))).install } installed.each { |cask| Cask::Installer.new(Cask::CaskLoader.load(cask_path(cask))).install }
FileUtils.rm_rf CoreCaskTap.instance.cask_dir/"outdated"
allow_any_instance_of(described_class).to receive(:verbose?).and_return(true) allow_any_instance_of(described_class).to receive(:verbose?).and_return(true)
end end
@ -227,6 +228,7 @@ describe Cask::Upgrade, :cask do
before do before do
installed.each { |cask| Cask::Installer.new(Cask::CaskLoader.load(cask_path(cask))).install } installed.each { |cask| Cask::Installer.new(Cask::CaskLoader.load(cask_path(cask))).install }
FileUtils.rm_rf CoreCaskTap.instance.cask_dir/"outdated"
allow_any_instance_of(described_class).to receive(:verbose?).and_return(true) allow_any_instance_of(described_class).to receive(:verbose?).and_return(true)
end end
@ -412,6 +414,7 @@ describe Cask::Upgrade, :cask do
before do before do
installed.each { |cask| Cask::Installer.new(Cask::CaskLoader.load(cask_path(cask))).install } installed.each { |cask| Cask::Installer.new(Cask::CaskLoader.load(cask_path(cask))).install }
FileUtils.rm_rf CoreCaskTap.instance.cask_dir/"outdated"
allow_any_instance_of(described_class).to receive(:verbose?).and_return(true) allow_any_instance_of(described_class).to receive(:verbose?).and_return(true)
end end
@ -468,6 +471,7 @@ describe Cask::Upgrade, :cask do
before do before do
installed.each { |cask| Cask::Installer.new(Cask::CaskLoader.load(cask_path(cask))).install } installed.each { |cask| Cask::Installer.new(Cask::CaskLoader.load(cask_path(cask))).install }
FileUtils.rm_rf CoreCaskTap.instance.cask_dir/"outdated"
allow_any_instance_of(described_class).to receive(:verbose?).and_return(true) allow_any_instance_of(described_class).to receive(:verbose?).and_return(true)
end end

View File

@ -108,7 +108,7 @@ describe Homebrew::Livecheck::Strategy::ExtractPlist do
describe "::find_versions" do describe "::find_versions" do
it "returns a for an installer artifact" do it "returns a for an installer artifact" do
cask = Cask::CaskLoader.load(cask_path("livecheck/installer-manual-livecheck")) cask = Cask::CaskLoader.load(cask_path("livecheck/livecheck-installer-manual"))
installer_artifact = cask.artifacts.first installer_artifact = cask.artifacts.first
expect(installer_artifact).to be_a(Cask::Artifact::Installer) expect(installer_artifact).to be_a(Cask::Artifact::Installer)

View File

@ -1,4 +1,4 @@
cask "discontinued-reference" do cask "livecheck-discontinued-reference" do
version "1.2.3" version "1.2.3"
sha256 "8c62a2b791cf5f0da6066a0a4b6e85f62949cd60975da062df44adf887f4370b" sha256 "8c62a2b791cf5f0da6066a0a4b6e85f62949cd60975da062df44adf887f4370b"
@ -11,7 +11,7 @@ cask "discontinued-reference" do
homepage "http://localhost/homebrew/test/cask/audit/livecheck/discontinued" homepage "http://localhost/homebrew/test/cask/audit/livecheck/discontinued"
livecheck do livecheck do
cask "livecheck/discontinued" cask "livecheck/livecheck-discontinued"
end end
app "TestCask.app" app "TestCask.app"

View File

@ -1,4 +1,4 @@
cask "discontinued" do cask "livecheck-discontinued" do
version "1.2.3" version "1.2.3"
sha256 "8c62a2b791cf5f0da6066a0a4b6e85f62949cd60975da062df44adf887f4370b" sha256 "8c62a2b791cf5f0da6066a0a4b6e85f62949cd60975da062df44adf887f4370b"

View File

@ -1,4 +1,4 @@
cask "installer-manual-livecheck" do cask "livecheck-installer-manual" do
version "1.2.3" version "1.2.3"
sha256 "78c670559a609f5d89a5d75eee49e2a2dab48aa3ea36906d14d5f7104e483bb9" sha256 "78c670559a609f5d89a5d75eee49e2a2dab48aa3ea36906d14d5f7104e483bb9"

View File

@ -1,4 +1,4 @@
cask "url-unversioned-reference" do cask "livecheck-url-unversioned-reference" do
version "1.2.3" version "1.2.3"
sha256 "8c62a2b791cf5f0da6066a0a4b6e85f62949cd60975da062df44adf887f4370b" sha256 "8c62a2b791cf5f0da6066a0a4b6e85f62949cd60975da062df44adf887f4370b"
@ -11,7 +11,7 @@ cask "url-unversioned-reference" do
homepage "http://localhost/homebrew/test/cask/audit/livecheck/url-unversioned" homepage "http://localhost/homebrew/test/cask/audit/livecheck/url-unversioned"
livecheck do livecheck do
cask "livecheck/url-unversioned" cask "livecheck/livecheck-url-unversioned"
end end
app "TestCask.app" app "TestCask.app"

View File

@ -1,4 +1,4 @@
cask "url-unversioned" do cask "livecheck-url-unversioned" do
version "1.2.3" version "1.2.3"
sha256 "8c62a2b791cf5f0da6066a0a4b6e85f62949cd60975da062df44adf887f4370b" sha256 "8c62a2b791cf5f0da6066a0a4b6e85f62949cd60975da062df44adf887f4370b"

View File

@ -1,4 +1,4 @@
cask "version-latest-reference" do cask "livecheck-version-latest-reference" do
version :latest version :latest
sha256 :no_check sha256 :no_check
@ -11,7 +11,7 @@ cask "version-latest-reference" do
homepage "http://localhost/homebrew/test/cask/audit/livecheck/version-latest" homepage "http://localhost/homebrew/test/cask/audit/livecheck/version-latest"
livecheck do livecheck do
cask "livecheck/version-latest" cask "livecheck/livecheck-version-latest"
end end
app "TestCask.app" app "TestCask.app"

View File

@ -1,4 +1,4 @@
cask "version-latest" do cask "livecheck-version-latest" do
version :latest version :latest
sha256 :no_check sha256 :no_check

View File

@ -39,21 +39,23 @@ RSpec.shared_context "Homebrew Cask", :needs_macos do # rubocop:disable RSpec/Co
Cask::Config::DEFAULT_DIRS_PATHNAMES.each_value(&:mkpath) Cask::Config::DEFAULT_DIRS_PATHNAMES.each_value(&:mkpath)
CoreCaskTap.instance.tap do |tap| CoreCaskTap.instance.tap do |tap|
FileUtils.mkdir_p tap.path.dirname tap.cask_dir.mkpath
FileUtils.ln_sf TEST_FIXTURE_DIR.join("cask"), tap.path (TEST_FIXTURE_DIR/"cask/Casks").children.each do |casks_path|
FileUtils.ln_sf casks_path, tap.cask_dir
end
end end
third_party_tap.tap do |tap| third_party_tap.tap do |tap|
FileUtils.mkdir_p tap.path.dirname tap.path.parent.mkpath
FileUtils.ln_sf TEST_FIXTURE_DIR.join("third-party"), tap.path FileUtils.ln_sf TEST_FIXTURE_DIR/"third-party", tap.path
end end
example.run example.run
ensure ensure
FileUtils.rm_rf Cask::Config::DEFAULT_DIRS_PATHNAMES.values FileUtils.rm_rf Cask::Config::DEFAULT_DIRS_PATHNAMES.values
FileUtils.rm_rf [Cask::Config.new.binarydir, Cask::Caskroom.path, Cask::Cache.path] FileUtils.rm_rf [Cask::Config.new.binarydir, Cask::Caskroom.path, Cask::Cache.path]
CoreCaskTap.instance.path.unlink FileUtils.rm_rf CoreCaskTap.instance.path
third_party_tap.path.unlink FileUtils.rm_rf third_party_tap.path
FileUtils.rm_rf third_party_tap.path.parent FileUtils.rm_rf third_party_tap.path.parent
end end
end end