From 287dfee35f830cbe3ac94d79bf1fd97011079973 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Sun, 13 Dec 2020 03:12:30 +0100 Subject: [PATCH] Properly handle `~` artifact targets. --- Library/Homebrew/cask/artifact/artifact.rb | 21 ++++++++++--------- Library/Homebrew/cask/artifact/relocated.rb | 15 +++++++++++-- Library/Homebrew/cask/cask.rbi | 2 ++ .../cask/artifact/generic_artifact_spec.rb | 18 +++++++++++++++- Library/Homebrew/test/cask/audit_spec.rb | 10 +++++++-- .../generic-artifact-user-relative-target.rb | 11 ++++++++++ 6 files changed, 62 insertions(+), 15 deletions(-) create mode 100644 Library/Homebrew/test/support/fixtures/cask/Casks/generic-artifact-user-relative-target.rb diff --git a/Library/Homebrew/cask/artifact/artifact.rb b/Library/Homebrew/cask/artifact/artifact.rb index 8d083a1bd5..5bfe30d8bd 100644 --- a/Library/Homebrew/cask/artifact/artifact.rb +++ b/Library/Homebrew/cask/artifact/artifact.rb @@ -1,4 +1,4 @@ -# typed: false +# typed: true # frozen_string_literal: true require "cask/artifact/moved" @@ -19,25 +19,26 @@ module Cask "Generic Artifact" end + sig { params(cask: Cask, args: T.untyped).returns(T.attached_class) } def self.from_args(cask, *args) - source_string, target_hash = args + source, options = args - raise CaskInvalidError.new(cask.token, "no source given for #{english_name}") if source_string.nil? + raise CaskInvalidError.new(cask.token, "No source provided for #{english_name}.") if source.blank? - unless target_hash.is_a?(Hash) - raise CaskInvalidError.new(cask.token, "target required for #{english_name} '#{source_string}'") + unless options.try(:key?, :target) + raise CaskInvalidError.new(cask.token, "#{english_name} '#{source}' requires a target.") end - target_hash.assert_valid_keys!(:target) - - new(cask, source_string, **target_hash) + new(cask, source, **options) end + sig { params(target: T.any(String, Pathname)).returns(Pathname) } def resolve_target(target) - Pathname(target) + super(target, base_dir: nil) end - def initialize(cask, source, target: nil) + sig { params(cask: Cask, source: T.any(String, Pathname), target: T.any(String, Pathname)).void } + def initialize(cask, source, target:) super(cask, source, target: target) end end diff --git a/Library/Homebrew/cask/artifact/relocated.rb b/Library/Homebrew/cask/artifact/relocated.rb index f5e61e9677..6943f233f0 100644 --- a/Library/Homebrew/cask/artifact/relocated.rb +++ b/Library/Homebrew/cask/artifact/relocated.rb @@ -28,12 +28,23 @@ module Cask new(cask, source_string, **target_hash) end - def resolve_target(target) - config.public_send(self.class.dirmethod).join(target) + def resolve_target(target, base_dir: config.public_send(self.class.dirmethod)) + target = Pathname(target) + + if target.relative? + return target.expand_path if target.descend.first.to_s == "~" + return base_dir/target if base_dir + end + + target end attr_reader :source, :target + sig do + params(cask: Cask, source: T.nilable(T.any(String, Pathname)), target: T.nilable(T.any(String, Pathname))) + .void + end def initialize(cask, source, target: nil) super(cask) diff --git a/Library/Homebrew/cask/cask.rbi b/Library/Homebrew/cask/cask.rbi index f15294c79a..4a7570c774 100644 --- a/Library/Homebrew/cask/cask.rbi +++ b/Library/Homebrew/cask/cask.rbi @@ -5,5 +5,7 @@ module Cask def artifacts; end def homepage; end + + def staged_path; end end end diff --git a/Library/Homebrew/test/cask/artifact/generic_artifact_spec.rb b/Library/Homebrew/test/cask/artifact/generic_artifact_spec.rb index 67420059c3..d806947838 100644 --- a/Library/Homebrew/test/cask/artifact/generic_artifact_spec.rb +++ b/Library/Homebrew/test/cask/artifact/generic_artifact_spec.rb @@ -23,7 +23,23 @@ describe Cask::Artifact::Artifact, :cask do it "fails to load" do expect { Cask::CaskLoader.load(cask_path("invalid/invalid-generic-artifact-no-target")) - }.to raise_error(Cask::CaskInvalidError, /target required for Generic Artifact/) + }.to raise_error(Cask::CaskInvalidError, /Generic Artifact.*requires.*target/) + end + end + + context "with relative target" do + it "does not fail to load" do + expect { + Cask::CaskLoader.load(cask_path("generic-artifact-relative-target")) + }.not_to raise_error + end + end + + context "with user-relative target" do + it "does not fail to load" do + expect { + Cask::CaskLoader.load(cask_path("generic-artifact-user-relative-target")) + }.not_to raise_error end end diff --git a/Library/Homebrew/test/cask/audit_spec.rb b/Library/Homebrew/test/cask/audit_spec.rb index 9477a16f2b..38a3630e32 100644 --- a/Library/Homebrew/test/cask/audit_spec.rb +++ b/Library/Homebrew/test/cask/audit_spec.rb @@ -756,13 +756,19 @@ describe Cask::Audit, :cask do context "with relative target" do let(:cask_token) { "generic-artifact-relative-target" } - it { is_expected.to fail_with(/target must be absolute path for Generic Artifact/) } + it { is_expected.to fail_with(/target must be.*absolute/) } + end + + context "with user-relative target" do + let(:cask_token) { "generic-artifact-user-relative-target" } + + it { is_expected.not_to fail_with(/target must be.*absolute/) } end context "with absolute target" do let(:cask_token) { "generic-artifact-absolute-target" } - it { is_expected.not_to fail_with(/target required for Generic Artifact/) } + it { is_expected.not_to fail_with(/target must be.*absolute/) } end end diff --git a/Library/Homebrew/test/support/fixtures/cask/Casks/generic-artifact-user-relative-target.rb b/Library/Homebrew/test/support/fixtures/cask/Casks/generic-artifact-user-relative-target.rb new file mode 100644 index 0000000000..4cfb484fc3 --- /dev/null +++ b/Library/Homebrew/test/support/fixtures/cask/Casks/generic-artifact-user-relative-target.rb @@ -0,0 +1,11 @@ +cask "generic-artifact-user-relative-target" do + version "1.2.3" + sha256 "d5b2dfbef7ea28c25f7a77cd7fa14d013d82b626db1d82e00e25822464ba19e2" + + url "file://#{TEST_FIXTURE_DIR}/cask/AppWithBinary.zip" + name "With Binary" + desc "Cask with a binary stanza" + homepage "https://brew.sh/with-binary" + + artifact "Caffeine.app", target: "~/Desktop/Caffeine.app" +end