From 3ef5e06943404f09f2674ef349c91c41bb2eb120 Mon Sep 17 00:00:00 2001 From: Tim Masliuchenko Date: Tue, 22 Oct 2019 15:19:40 +0300 Subject: [PATCH 1/5] Add Manpage artifact --- Library/Homebrew/cask/artifact.rb | 1 + Library/Homebrew/cask/artifact/manpage.rb | 55 +++++++++++++++++ Library/Homebrew/cask/config.rb | 4 ++ Library/Homebrew/cask/dsl.rb | 1 + .../rubocops/cask/constants/stanza.rb | 1 + .../test/cask/artifact/manpage_spec.rb | 56 ++++++++++++++++++ .../support/fixtures/cask/AppWithManpage.zip | Bin 0 -> 354 bytes .../invalid/invalid-manpage-no-section.rb | 9 +++ .../with-autodetected-manpage-section.rb | 9 +++ .../Casks/with-explicit-manpage-section.rb | 9 +++ 10 files changed, 145 insertions(+) create mode 100644 Library/Homebrew/cask/artifact/manpage.rb create mode 100644 Library/Homebrew/test/cask/artifact/manpage_spec.rb create mode 100644 Library/Homebrew/test/support/fixtures/cask/AppWithManpage.zip create mode 100644 Library/Homebrew/test/support/fixtures/cask/Casks/invalid/invalid-manpage-no-section.rb create mode 100644 Library/Homebrew/test/support/fixtures/cask/Casks/with-autodetected-manpage-section.rb create mode 100644 Library/Homebrew/test/support/fixtures/cask/Casks/with-explicit-manpage-section.rb diff --git a/Library/Homebrew/cask/artifact.rb b/Library/Homebrew/cask/artifact.rb index 98ccf9d66c..278d0c4442 100644 --- a/Library/Homebrew/cask/artifact.rb +++ b/Library/Homebrew/cask/artifact.rb @@ -23,6 +23,7 @@ require "cask/artifact/stage_only" require "cask/artifact/suite" require "cask/artifact/uninstall" require "cask/artifact/zap" +require "cask/artifact/manpage" module Cask module Artifact diff --git a/Library/Homebrew/cask/artifact/manpage.rb b/Library/Homebrew/cask/artifact/manpage.rb new file mode 100644 index 0000000000..d7957c625d --- /dev/null +++ b/Library/Homebrew/cask/artifact/manpage.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require "cask/artifact/moved" + +require "extend/hash_validator" +using HashValidator + +module Cask + module Artifact + class Manpage < Moved + def self.from_args(cask, *args) + source_string, section_hash = args + section = nil + + if section_hash + raise CaskInvalidError unless section_hash.respond_to?(:keys) + + section_hash.assert_valid_keys!(:section) + + section = section_hash[:section] + else + section = source_string.split(".").last + end + + raise CaskInvalidError, "section should be a positive number" unless section.to_i.positive? + + section_hash ||= {} + + new(cask, source_string, **section_hash) + end + + def resolve_target(_target) + config.manpagedir.join("man#{section}", target_name) + end + + def initialize(cask, source, section: nil) + @source_section = section + + super(cask, source) + end + + def extension + @source.extname.downcase[1..-1].to_s + end + + def section + (@source_section || extension).to_i + end + + def target_name + "#{@source.basename(@source.extname)}.#{section}" + end + end + end +end diff --git a/Library/Homebrew/cask/config.rb b/Library/Homebrew/cask/config.rb index 8c2aa3da68..41fba64514 100644 --- a/Library/Homebrew/cask/config.rb +++ b/Library/Homebrew/cask/config.rb @@ -93,6 +93,10 @@ module Cask @binarydir ||= HOMEBREW_PREFIX/"bin" end + def manpagedir + @manpagedir ||= HOMEBREW_PREFIX/"share/man" + end + DEFAULT_DIRS.keys.each do |dir| define_method(dir) do explicit.fetch(dir, env.fetch(dir, default.fetch(dir))) diff --git a/Library/Homebrew/cask/dsl.rb b/Library/Homebrew/cask/dsl.rb index 9ed09b4bb7..9e6c2a29eb 100644 --- a/Library/Homebrew/cask/dsl.rb +++ b/Library/Homebrew/cask/dsl.rb @@ -46,6 +46,7 @@ module Cask Artifact::Vst3Plugin, Artifact::Uninstall, Artifact::Zap, + Artifact::Manpage, ].freeze ACTIVATABLE_ARTIFACT_CLASSES = (ORDINARY_ARTIFACT_CLASSES - [Artifact::StageOnly]).freeze diff --git a/Library/Homebrew/rubocops/cask/constants/stanza.rb b/Library/Homebrew/rubocops/cask/constants/stanza.rb index ad7b41ce46..3684eef095 100644 --- a/Library/Homebrew/rubocops/cask/constants/stanza.rb +++ b/Library/Homebrew/rubocops/cask/constants/stanza.rb @@ -32,6 +32,7 @@ module RuboCop :vst_plugin, :artifact, :stage_only, + :manpage, ], [:preflight], [:postflight], diff --git a/Library/Homebrew/test/cask/artifact/manpage_spec.rb b/Library/Homebrew/test/cask/artifact/manpage_spec.rb new file mode 100644 index 0000000000..e4e0a8d04e --- /dev/null +++ b/Library/Homebrew/test/cask/artifact/manpage_spec.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +describe Cask::Artifact::Manpage, :cask do + let(:cask_token) { "" } + let(:cask) { Cask::CaskLoader.load(cask_path(cask_token)) } + + context "without section" do + let(:cask_token) { "invalid/invalid-manpage-no-section" } + + it "fails to load a cask without section" do + expect { cask }.to raise_error(Cask::CaskInvalidError, /section should be a positive number/) + end + end + + context "with install" do + let(:install_phase) { + lambda do + cask.artifacts.select { |a| a.is_a?(described_class) }.each do |artifact| + artifact.install_phase(command: NeverSudoSystemCommand, force: false) + end + end + } + + let(:source_path) { cask.staged_path.join("manpage.1") } + let(:expected_section) { "" } + let(:target_path) { cask.config.manpagedir.join("man#{expected_section}/manpage.#{expected_section}") } + + before do + InstallHelper.install_without_artifacts(cask) + end + + context "with autodetected section" do + let(:cask_token) { "with-autodetected-manpage-section" } + let(:expected_section) { 1 } + + it "moves the manpage to the proper directory" do + install_phase.call + + expect(target_path).to exist + expect(source_path).not_to exist + end + end + + context "with explicit section" do + let(:cask_token) { "with-explicit-manpage-section" } + let(:expected_section) { 3 } + + it "moves the manpage to the proper directory" do + install_phase.call + + expect(target_path).to exist + expect(source_path).not_to exist + end + end + end +end diff --git a/Library/Homebrew/test/support/fixtures/cask/AppWithManpage.zip b/Library/Homebrew/test/support/fixtures/cask/AppWithManpage.zip new file mode 100644 index 0000000000000000000000000000000000000000..938933a1cae691c0e8cad55eac5521f21e1369ae GIT binary patch literal 354 zcmWIWW@Zs#0D;bGB^NLQN^meJFgO+z=p_~u=!b^zGO!nC9t~;*;+Bu=VoNKy85mi< zGBPlLbq9b=;b7ok=tnb!6KG0qVqQUFda9lw*qAz?5g^=+X$&(%fHyk_$bKfMBLch` znM9azI}oaj;jJTxMZ9w%rXo8SW+TYKFz~j~4ag+g`2pUnY#^;nK)4o2&j)cB0G!@H A0ssI2 literal 0 HcmV?d00001 diff --git a/Library/Homebrew/test/support/fixtures/cask/Casks/invalid/invalid-manpage-no-section.rb b/Library/Homebrew/test/support/fixtures/cask/Casks/invalid/invalid-manpage-no-section.rb new file mode 100644 index 0000000000..6dd3148756 --- /dev/null +++ b/Library/Homebrew/test/support/fixtures/cask/Casks/invalid/invalid-manpage-no-section.rb @@ -0,0 +1,9 @@ +cask 'invalid-manpage-no-section' do + version '1.2.3' + sha256 '67cdb8a02803ef37fdbf7e0be205863172e41a561ca446cd84f0d7ab35a99d94' + + url "file://#{TEST_FIXTURE_DIR}/cask/AppWithManpage.zip" + homepage 'https://brew.sh/with-generic-artifact' + + manpage 'manpage' +end diff --git a/Library/Homebrew/test/support/fixtures/cask/Casks/with-autodetected-manpage-section.rb b/Library/Homebrew/test/support/fixtures/cask/Casks/with-autodetected-manpage-section.rb new file mode 100644 index 0000000000..a92b9be233 --- /dev/null +++ b/Library/Homebrew/test/support/fixtures/cask/Casks/with-autodetected-manpage-section.rb @@ -0,0 +1,9 @@ +cask 'with-autodetected-manpage-section' do + version '1.2.3' + sha256 '67cdb8a02803ef37fdbf7e0be205863172e41a561ca446cd84f0d7ab35a99d94' + + url "file://#{TEST_FIXTURE_DIR}/cask/AppWithManpage.zip" + homepage 'https://brew.sh/with-autodetected-manpage-section' + + manpage 'manpage.1' +end diff --git a/Library/Homebrew/test/support/fixtures/cask/Casks/with-explicit-manpage-section.rb b/Library/Homebrew/test/support/fixtures/cask/Casks/with-explicit-manpage-section.rb new file mode 100644 index 0000000000..ce01e051a5 --- /dev/null +++ b/Library/Homebrew/test/support/fixtures/cask/Casks/with-explicit-manpage-section.rb @@ -0,0 +1,9 @@ +cask 'with-explicit-manpage-section' do + version '1.2.3' + sha256 '67cdb8a02803ef37fdbf7e0be205863172e41a561ca446cd84f0d7ab35a99d94' + + url "file://#{TEST_FIXTURE_DIR}/cask/AppWithManpage.zip" + homepage 'https://brew.sh/with-explicit-manpage-section' + + manpage 'manpage.1', section: 3 +end From 22d821323cff738fd8fc331c49c0408dd22c9e28 Mon Sep 17 00:00:00 2001 From: Tim Masliuchenko Date: Wed, 23 Oct 2019 16:28:00 +0300 Subject: [PATCH 2/5] Feedbacks --- Library/Homebrew/cask/artifact.rb | 4 +- Library/Homebrew/cask/artifact/manpage.rb | 40 +++++-------------- Library/Homebrew/cask/dsl.rb | 2 +- .../rubocops/cask/constants/stanza.rb | 2 +- .../test/cask/artifact/manpage_spec.rb | 17 +------- .../Casks/with-explicit-manpage-section.rb | 9 ----- 6 files changed, 15 insertions(+), 59 deletions(-) delete mode 100644 Library/Homebrew/test/support/fixtures/cask/Casks/with-explicit-manpage-section.rb diff --git a/Library/Homebrew/cask/artifact.rb b/Library/Homebrew/cask/artifact.rb index 278d0c4442..07d8925155 100644 --- a/Library/Homebrew/cask/artifact.rb +++ b/Library/Homebrew/cask/artifact.rb @@ -2,6 +2,7 @@ require "cask/artifact/app" require "cask/artifact/artifact" # generic 'artifact' stanza +require "cask/artifact/audio_unit_plugin" require "cask/artifact/binary" require "cask/artifact/colorpicker" require "cask/artifact/dictionary" @@ -9,7 +10,7 @@ require "cask/artifact/font" require "cask/artifact/input_method" require "cask/artifact/installer" require "cask/artifact/internet_plugin" -require "cask/artifact/audio_unit_plugin" +require "cask/artifact/manpage" require "cask/artifact/vst_plugin" require "cask/artifact/vst3_plugin" require "cask/artifact/pkg" @@ -23,7 +24,6 @@ require "cask/artifact/stage_only" require "cask/artifact/suite" require "cask/artifact/uninstall" require "cask/artifact/zap" -require "cask/artifact/manpage" module Cask module Artifact diff --git a/Library/Homebrew/cask/artifact/manpage.rb b/Library/Homebrew/cask/artifact/manpage.rb index d7957c625d..6938f6035d 100644 --- a/Library/Homebrew/cask/artifact/manpage.rb +++ b/Library/Homebrew/cask/artifact/manpage.rb @@ -1,50 +1,28 @@ # frozen_string_literal: true -require "cask/artifact/moved" - -require "extend/hash_validator" -using HashValidator +require "cask/artifact/symlinked" module Cask module Artifact - class Manpage < Moved - def self.from_args(cask, *args) - source_string, section_hash = args - section = nil - - if section_hash - raise CaskInvalidError unless section_hash.respond_to?(:keys) - - section_hash.assert_valid_keys!(:section) - - section = section_hash[:section] - else - section = source_string.split(".").last - end + class Manpage < Symlinked + def self.from_args(cask, source) + section = source.split(".").last raise CaskInvalidError, "section should be a positive number" unless section.to_i.positive? - section_hash ||= {} + new(cask, source) + end - new(cask, source_string, **section_hash) + def initialize(cask, source) + super end def resolve_target(_target) config.manpagedir.join("man#{section}", target_name) end - def initialize(cask, source, section: nil) - @source_section = section - - super(cask, source) - end - - def extension - @source.extname.downcase[1..-1].to_s - end - def section - (@source_section || extension).to_i + @source.extname.downcase[1..-1].to_s.to_i end def target_name diff --git a/Library/Homebrew/cask/dsl.rb b/Library/Homebrew/cask/dsl.rb index 9e6c2a29eb..5090744e73 100644 --- a/Library/Homebrew/cask/dsl.rb +++ b/Library/Homebrew/cask/dsl.rb @@ -35,6 +35,7 @@ module Cask Artifact::Font, Artifact::InputMethod, Artifact::InternetPlugin, + Artifact::Manpage, Artifact::Pkg, Artifact::Prefpane, Artifact::Qlplugin, @@ -46,7 +47,6 @@ module Cask Artifact::Vst3Plugin, Artifact::Uninstall, Artifact::Zap, - Artifact::Manpage, ].freeze ACTIVATABLE_ARTIFACT_CLASSES = (ORDINARY_ARTIFACT_CLASSES - [Artifact::StageOnly]).freeze diff --git a/Library/Homebrew/rubocops/cask/constants/stanza.rb b/Library/Homebrew/rubocops/cask/constants/stanza.rb index 3684eef095..33bdb1f034 100644 --- a/Library/Homebrew/rubocops/cask/constants/stanza.rb +++ b/Library/Homebrew/rubocops/cask/constants/stanza.rb @@ -19,6 +19,7 @@ module RuboCop :pkg, :installer, :binary, + :manpage, :colorpicker, :dictionary, :font, @@ -32,7 +33,6 @@ module RuboCop :vst_plugin, :artifact, :stage_only, - :manpage, ], [:preflight], [:postflight], diff --git a/Library/Homebrew/test/cask/artifact/manpage_spec.rb b/Library/Homebrew/test/cask/artifact/manpage_spec.rb index e4e0a8d04e..b1813dfa7f 100644 --- a/Library/Homebrew/test/cask/artifact/manpage_spec.rb +++ b/Library/Homebrew/test/cask/artifact/manpage_spec.rb @@ -33,23 +33,10 @@ describe Cask::Artifact::Manpage, :cask do let(:cask_token) { "with-autodetected-manpage-section" } let(:expected_section) { 1 } - it "moves the manpage to the proper directory" do + it "links the manpage to the proper directory" do install_phase.call - expect(target_path).to exist - expect(source_path).not_to exist - end - end - - context "with explicit section" do - let(:cask_token) { "with-explicit-manpage-section" } - let(:expected_section) { 3 } - - it "moves the manpage to the proper directory" do - install_phase.call - - expect(target_path).to exist - expect(source_path).not_to exist + expect(File).to be_identical target_path, source_path end end end diff --git a/Library/Homebrew/test/support/fixtures/cask/Casks/with-explicit-manpage-section.rb b/Library/Homebrew/test/support/fixtures/cask/Casks/with-explicit-manpage-section.rb deleted file mode 100644 index ce01e051a5..0000000000 --- a/Library/Homebrew/test/support/fixtures/cask/Casks/with-explicit-manpage-section.rb +++ /dev/null @@ -1,9 +0,0 @@ -cask 'with-explicit-manpage-section' do - version '1.2.3' - sha256 '67cdb8a02803ef37fdbf7e0be205863172e41a561ca446cd84f0d7ab35a99d94' - - url "file://#{TEST_FIXTURE_DIR}/cask/AppWithManpage.zip" - homepage 'https://brew.sh/with-explicit-manpage-section' - - manpage 'manpage.1', section: 3 -end From 87f29857f0d5a8c862ed90b1740e192d5312c0db Mon Sep 17 00:00:00 2001 From: Tim Masliuchenko Date: Wed, 23 Oct 2019 18:34:16 +0300 Subject: [PATCH 3/5] Pass section to the constructor --- Library/Homebrew/cask/artifact/manpage.rb | 18 +++++++++--------- .../test/cask/artifact/manpage_spec.rb | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/Library/Homebrew/cask/artifact/manpage.rb b/Library/Homebrew/cask/artifact/manpage.rb index 6938f6035d..9f6f57aebd 100644 --- a/Library/Homebrew/cask/artifact/manpage.rb +++ b/Library/Homebrew/cask/artifact/manpage.rb @@ -5,26 +5,26 @@ require "cask/artifact/symlinked" module Cask module Artifact class Manpage < Symlinked + attr_reader :section + def self.from_args(cask, source) - section = source.split(".").last + section = source.to_s[/\.([1-8]|n|l)$/, 1] - raise CaskInvalidError, "section should be a positive number" unless section.to_i.positive? + raise CaskInvalidError, "'#{source}' is not a valid man page name" unless section - new(cask, source) + new(cask, source, section) end - def initialize(cask, source) - super + def initialize(cask, source, section) + @section = section + + super(cask, source) end def resolve_target(_target) config.manpagedir.join("man#{section}", target_name) end - def section - @source.extname.downcase[1..-1].to_s.to_i - end - def target_name "#{@source.basename(@source.extname)}.#{section}" end diff --git a/Library/Homebrew/test/cask/artifact/manpage_spec.rb b/Library/Homebrew/test/cask/artifact/manpage_spec.rb index b1813dfa7f..a105a8fd51 100644 --- a/Library/Homebrew/test/cask/artifact/manpage_spec.rb +++ b/Library/Homebrew/test/cask/artifact/manpage_spec.rb @@ -8,7 +8,7 @@ describe Cask::Artifact::Manpage, :cask do let(:cask_token) { "invalid/invalid-manpage-no-section" } it "fails to load a cask without section" do - expect { cask }.to raise_error(Cask::CaskInvalidError, /section should be a positive number/) + expect { cask }.to raise_error(Cask::CaskInvalidError, /is not a valid man page name/) end end From 6d1fafae13d9735cb7edc5e282db4944a6884650 Mon Sep 17 00:00:00 2001 From: Tim Masliuchenko Date: Thu, 24 Oct 2019 09:43:59 +0300 Subject: [PATCH 4/5] Final tweaks --- Library/Homebrew/cask/artifact/abstract_artifact.rb | 1 + Library/Homebrew/cask/artifact/manpage.rb | 8 ++------ 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/Library/Homebrew/cask/artifact/abstract_artifact.rb b/Library/Homebrew/cask/artifact/abstract_artifact.rb index d75748d997..fac91f0b49 100644 --- a/Library/Homebrew/cask/artifact/abstract_artifact.rb +++ b/Library/Homebrew/cask/artifact/abstract_artifact.rb @@ -71,6 +71,7 @@ module Cask # targets are created prior to linking. Pkg, Binary, + Manpage, PostflightBlock, Zap, ].each_with_index.flat_map { |classes, i| [*classes].map { |c| [c, i] } }.to_h diff --git a/Library/Homebrew/cask/artifact/manpage.rb b/Library/Homebrew/cask/artifact/manpage.rb index 9f6f57aebd..942a83c438 100644 --- a/Library/Homebrew/cask/artifact/manpage.rb +++ b/Library/Homebrew/cask/artifact/manpage.rb @@ -21,12 +21,8 @@ module Cask super(cask, source) end - def resolve_target(_target) - config.manpagedir.join("man#{section}", target_name) - end - - def target_name - "#{@source.basename(@source.extname)}.#{section}" + def resolve_target(target) + config.manpagedir.join("man#{section}", target) end end end From 91ddb6e53d6cd42d06c4f28375efe353d7058a65 Mon Sep 17 00:00:00 2001 From: Tim Masliuchenko Date: Thu, 24 Oct 2019 14:22:13 +0300 Subject: [PATCH 5/5] Cleanup specs --- Library/Homebrew/test/cask/artifact/manpage_spec.rb | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Library/Homebrew/test/cask/artifact/manpage_spec.rb b/Library/Homebrew/test/cask/artifact/manpage_spec.rb index a105a8fd51..e94e9a8ce1 100644 --- a/Library/Homebrew/test/cask/artifact/manpage_spec.rb +++ b/Library/Homebrew/test/cask/artifact/manpage_spec.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true describe Cask::Artifact::Manpage, :cask do - let(:cask_token) { "" } let(:cask) { Cask::CaskLoader.load(cask_path(cask_token)) } context "without section" do @@ -22,8 +21,7 @@ describe Cask::Artifact::Manpage, :cask do } let(:source_path) { cask.staged_path.join("manpage.1") } - let(:expected_section) { "" } - let(:target_path) { cask.config.manpagedir.join("man#{expected_section}/manpage.#{expected_section}") } + let(:target_path) { cask.config.manpagedir.join("man1/manpage.1") } before do InstallHelper.install_without_artifacts(cask) @@ -31,7 +29,6 @@ describe Cask::Artifact::Manpage, :cask do context "with autodetected section" do let(:cask_token) { "with-autodetected-manpage-section" } - let(:expected_section) { 1 } it "links the manpage to the proper directory" do install_phase.call