Skip to content

Commit d422c1c

Browse files
committed
Add cask install steps
- Expose structured steps for cask flight phases and API data. - Prefer steps over matching Ruby flight blocks with warnings. - Document cask usage and autocorrect simple file preparation.
1 parent e05e1d5 commit d422c1c

13 files changed

Lines changed: 379 additions & 5 deletions

File tree

Library/Homebrew/cask/artifact.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
require "cask/artifact/binary"
99
require "cask/artifact/colorpicker"
1010
require "cask/artifact/dictionary"
11+
require "cask/artifact/install_steps"
1112
require "cask/artifact/font"
1213
require "cask/artifact/input_method"
1314
require "cask/artifact/installer"

Library/Homebrew/cask/artifact/abstract_artifact.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,8 @@ def sort_order
6969
@sort_order ||= T.let(
7070
[
7171
PreflightBlock,
72+
PreflightSteps,
73+
UninstallPreflightSteps,
7274
# The `uninstall` stanza should be run first, as it may
7375
# depend on other artifacts still being installed.
7476
Uninstall,
@@ -99,6 +101,8 @@ def sort_order
99101
],
100102
Binary,
101103
Manpage,
104+
PostflightSteps,
105+
UninstallPostflightSteps,
102106
PostflightBlock,
103107
Zap,
104108
].each_with_index.flat_map { |classes, i| Array(classes).map { |c| [c, i] } }.to_h,
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
# typed: strict
2+
# frozen_string_literal: true
3+
4+
require "cask/artifact/abstract_artifact"
5+
require "install_steps"
6+
7+
module Cask
8+
module Artifact
9+
class AbstractInstallSteps < AbstractArtifact
10+
abstract!
11+
12+
sig { params(cask: Cask, steps: Homebrew::InstallSteps::Steps).void }
13+
def initialize(cask, steps)
14+
super
15+
@steps = T.let(Homebrew::InstallSteps::DSL.normalise_steps(steps), Homebrew::InstallSteps::Steps)
16+
end
17+
18+
sig { returns(Homebrew::InstallSteps::Steps) }
19+
attr_reader :steps
20+
21+
sig { override.returns(T::Array[T.anything]) }
22+
def to_args = [{ steps: }]
23+
24+
sig { override.returns(String) }
25+
def summarize
26+
::Utils.pluralize("install step", steps.length, include_count: true)
27+
end
28+
29+
private
30+
31+
sig { returns(Homebrew::InstallSteps::Runner) }
32+
def runner
33+
Homebrew::InstallSteps::Runner.new(context: cask)
34+
end
35+
end
36+
37+
class PreflightSteps < AbstractInstallSteps
38+
sig { params(_options: T.anything).void }
39+
def install_phase(**_options)
40+
runner.run(steps)
41+
end
42+
43+
sig { params(_options: T.anything).void }
44+
def uninstall_phase(**_options)
45+
runner.run(steps, phase: :uninstall)
46+
end
47+
end
48+
49+
class PostflightSteps < AbstractInstallSteps
50+
sig { params(_options: T.anything).void }
51+
def install_phase(**_options)
52+
runner.run(steps)
53+
end
54+
55+
sig { params(_options: T.anything).void }
56+
def uninstall_phase(**_options)
57+
runner.run(steps, phase: :uninstall)
58+
end
59+
end
60+
61+
class UninstallPreflightSteps < AbstractInstallSteps
62+
sig { params(_options: T.anything).void }
63+
def uninstall_phase(**_options)
64+
runner.run(steps)
65+
end
66+
end
67+
68+
class UninstallPostflightSteps < AbstractInstallSteps
69+
sig { params(_options: T.anything).void }
70+
def uninstall_phase(**_options)
71+
runner.run(steps)
72+
end
73+
end
74+
end
75+
end

Library/Homebrew/cask/dsl.rb

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,20 @@ class DSL
7676
Artifact::PostflightBlock,
7777
].freeze
7878

79+
INSTALL_STEP_ARTIFACT_CLASSES = [
80+
Artifact::PreflightSteps,
81+
Artifact::PostflightSteps,
82+
Artifact::UninstallPreflightSteps,
83+
Artifact::UninstallPostflightSteps,
84+
].freeze
85+
86+
INSTALL_STEP_FLIGHT_BLOCK_CLASSES = T.let({
87+
Artifact::PreflightSteps => [Artifact::PreflightBlock, :preflight],
88+
Artifact::PostflightSteps => [Artifact::PostflightBlock, :postflight],
89+
Artifact::UninstallPreflightSteps => [Artifact::PreflightBlock, :uninstall_preflight],
90+
Artifact::UninstallPostflightSteps => [Artifact::PostflightBlock, :uninstall_postflight],
91+
}.freeze, T::Hash[T.untyped, T.untyped])
92+
7993
DSL_METHODS = T.let(Set.new([
8094
:arch,
8195
:artifacts,
@@ -121,6 +135,7 @@ class DSL
121135
*ORDINARY_ARTIFACT_CLASSES.map(&:dsl_key),
122136
*ACTIVATABLE_ARTIFACT_CLASSES.map(&:dsl_key),
123137
*ARTIFACT_BLOCK_CLASSES.flat_map { |klass| [klass.dsl_key, klass.uninstall_dsl_key] },
138+
*INSTALL_STEP_ARTIFACT_CLASSES.map(&:dsl_key),
124139
]).freeze, T::Set[Symbol])
125140

126141
include OnSystem::MacOSAndLinux
@@ -798,11 +813,61 @@ def disable!(date:, because:, replacement: nil, replacement_formula: nil, replac
798813
[klass.dsl_key, klass.uninstall_dsl_key].each do |dsl_key|
799814
define_method(dsl_key) do |&block|
800815
T.bind(self, DSL)
801-
artifacts.add(klass.new(cask, dsl_key => block))
816+
if install_step_artifact_defined?(dsl_key)
817+
warn_on_install_step_conflict(dsl_key, T.must(install_step_artifact_class(dsl_key)).dsl_key)
818+
else
819+
artifacts.add(klass.new(cask, dsl_key => block))
820+
end
802821
end
803822
end
804823
end
805824

825+
INSTALL_STEP_ARTIFACT_CLASSES.each do |klass|
826+
define_method(klass.dsl_key) do |steps = nil, **kwargs, &block|
827+
T.bind(self, DSL)
828+
steps = if block
829+
Homebrew::InstallSteps::DSL.build(default_base: :staged_path, default_source_base: :staged_path,
830+
default_target_base: :staged_path, &block)
831+
else
832+
Homebrew::InstallSteps::DSL.normalise_steps([kwargs[:steps] || steps].flatten.compact)
833+
end
834+
remove_conflicting_flight_blocks(klass)
835+
artifacts.add(klass.new(cask, steps))
836+
end
837+
end
838+
839+
sig { params(dsl_key: Symbol).returns(T::Boolean) }
840+
def install_step_artifact_defined?(dsl_key)
841+
return false unless (klass = install_step_artifact_class(dsl_key))
842+
843+
artifacts.any?(klass)
844+
end
845+
846+
sig { params(dsl_key: Symbol).returns(T.nilable(T.class_of(Artifact::AbstractInstallSteps))) }
847+
def install_step_artifact_class(dsl_key)
848+
INSTALL_STEP_FLIGHT_BLOCK_CLASSES.find do |_step_class, (_block_class, block_dsl_key)|
849+
block_dsl_key == dsl_key
850+
end&.first
851+
end
852+
853+
sig { params(klass: T.class_of(Artifact::AbstractInstallSteps)).void }
854+
def remove_conflicting_flight_blocks(klass)
855+
flight_block_class, dsl_key = INSTALL_STEP_FLIGHT_BLOCK_CLASSES.fetch(klass)
856+
conflicting_flight_blocks = artifacts.select do |artifact|
857+
artifact.is_a?(flight_block_class) && T.unsafe(artifact).directives.key?(dsl_key)
858+
end
859+
860+
conflicting_flight_blocks.each do |artifact|
861+
warn_on_install_step_conflict(dsl_key, klass.dsl_key)
862+
artifacts.delete(artifact)
863+
end
864+
end
865+
866+
sig { params(dsl_key: Symbol, steps_key: Symbol).void }
867+
def warn_on_install_step_conflict(dsl_key, steps_key)
868+
opoo "#{token}: `#{dsl_key}` is ignored because `#{steps_key}` is defined!"
869+
end
870+
806871
sig { override.params(method: Symbol, _args: T.anything).returns(T.noreturn) }
807872
def method_missing(method, *_args)
808873
raise NoMethodError, "undefined method '#{method}' for Cask '#{token}'"

Library/Homebrew/cask/installer.rb

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,8 @@ def install_artifacts(predecessor: nil)
331331
Artifact::KeyboardLayout,
332332
Artifact::Mdimporter,
333333
Artifact::Moved,
334+
Artifact::PostflightSteps,
335+
Artifact::PreflightSteps,
334336
Artifact::Pkg,
335337
Artifact::Qlplugin,
336338
Artifact::Symlinked,
@@ -652,9 +654,13 @@ def uninstall_artifacts(clear: false, successor: nil, quit: true)
652654
Artifact::GeneratedCompletion,
653655
Artifact::KeyboardLayout,
654656
Artifact::Moved,
657+
Artifact::PostflightSteps,
658+
Artifact::PreflightSteps,
655659
Artifact::Qlplugin,
656660
Artifact::Symlinked,
657661
Artifact::Uninstall,
662+
Artifact::UninstallPostflightSteps,
663+
Artifact::UninstallPreflightSteps,
658664
),
659665
)
660666

Library/Homebrew/json_api_postinstall_preflight_postflight_plan.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ Local scan source: `homebrew/cask` at `4ed4e04eaa5`.
113113
paths to `prefix`; expose the ordered array through `FormulaStruct`; make
114114
`post_install_steps` take precedence over `post_install`; document that the
115115
two forms must not be mixed.
116-
- [ ] PR 3, cask flight steps.
116+
- [x] PR 3, cask flight steps.
117117
Commit: `Add cask install steps`.
118118
Scope: cask artifacts for `preflight_steps`, `postflight_steps`,
119119
`uninstall_preflight_steps` and `uninstall_postflight_steps`, cask API

Library/Homebrew/rubocops/cask/install_steps.rb

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ module Cop
88
module Cask
99
# This cop checks declarative install step usage.
1010
class InstallSteps < Base
11+
extend AutoCorrector
1112
include CaskHelp
1213
include ::RuboCop::Cop::InstallStepsHelper
1314

@@ -26,10 +27,15 @@ def on_cask(cask_block)
2627
stanzas = cask_block.stanzas
2728
INSTALL_STEP_PAIRS.each do |flight_block, steps_block|
2829
next unless (flight_stanza = stanzas.find { |stanza| stanza.stanza_name == flight_block })
29-
next unless (steps_stanza = stanzas.find { |stanza| stanza.stanza_name == steps_block })
3030

31-
add_offense(steps_stanza.source_range,
32-
message: "`#{flight_stanza.stanza_name}` and `#{steps_block}` cannot both be used.")
31+
steps_stanza = stanzas.find { |stanza| stanza.stanza_name == steps_block }
32+
33+
if steps_stanza
34+
add_offense(steps_stanza.source_range,
35+
message: "`#{flight_stanza.stanza_name}` and `#{steps_block}` cannot both be used.")
36+
else
37+
audit_flight_block(flight_stanza, steps_block)
38+
end
3339
end
3440

3541
stanzas.each do |stanza|
@@ -41,6 +47,28 @@ def on_cask(cask_block)
4147
add_offense(offense_node, message: STEP_BLOCK_MSG)
4248
end
4349
end
50+
51+
private
52+
53+
sig { params(flight_stanza: RuboCop::Cask::AST::Stanza, steps_block: Symbol).void }
54+
def audit_flight_block(flight_stanza, steps_block)
55+
return unless flight_stanza.method_node.block_type?
56+
57+
block_node = T.cast(flight_stanza.method_node, RuboCop::AST::BlockNode)
58+
step_lines = simple_install_step_lines(block_node.body,
59+
default_base: :staged_path,
60+
default_source_base: :staged_path,
61+
default_target_base: :staged_path)
62+
return if step_lines.blank?
63+
64+
add_offense(block_node.source_range,
65+
message: format(SIMPLE_STEP_CONVERSION_MSG, steps_block:)) do |corrector|
66+
corrector.replace(
67+
block_node.source_range,
68+
install_steps_block_source(steps_block, step_lines, block_node.source_range.column),
69+
)
70+
end
71+
end
4472
end
4573
end
4674
end

Library/Homebrew/sorbet/rbi/dsl/cask/cask.rbi

Lines changed: 12 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
# typed: false
2+
# frozen_string_literal: true
3+
4+
RSpec.describe Cask::Artifact::PreflightSteps, :cask do
5+
let(:cask) do
6+
Cask::Cask.new("with-install-steps") do
7+
version "1.2.3"
8+
sha256 :no_check
9+
url "file://#{TEST_FIXTURE_DIR}/cask/container.zip"
10+
11+
preflight_steps do
12+
mkdir_p "Prepared"
13+
touch "Prepared/touched"
14+
end
15+
16+
postflight_steps do
17+
mv "move-source", "Prepared/moved"
18+
ln_s "Prepared/moved", "PreparedLink", source_base: :relative, uninstall: true
19+
end
20+
21+
uninstall_preflight_steps do
22+
mkdir "UninstallPrepared"
23+
touch "UninstallPrepared/touched"
24+
end
25+
26+
uninstall_postflight_steps do
27+
move_children "UninstallPrepared", "UninstallMoved"
28+
end
29+
end
30+
end
31+
32+
it "runs structured steps through installer artifact phases" do
33+
cask.staged_path.mkpath
34+
cask.config_path.dirname.mkpath
35+
(cask.staged_path/"move-source").write "moved"
36+
37+
installer = Cask::Installer.new(cask, command: NeverSudoSystemCommand)
38+
installer.install_artifacts
39+
40+
expect(cask.staged_path/"Prepared").to be_a_directory
41+
expect(cask.staged_path/"Prepared/touched").to exist
42+
expect(cask.staged_path/"Prepared/moved").to exist
43+
expect(cask.staged_path/"PreparedLink").to be_a_symlink
44+
45+
installer.uninstall_artifacts
46+
47+
expect(cask.staged_path/"PreparedLink").not_to exist
48+
expect(cask.staged_path/"UninstallMoved/touched").to exist
49+
end
50+
51+
it "ignores a flight block when matching steps are defined" do
52+
cask = nil
53+
expect do
54+
cask = Cask::Cask.new("with-install-steps-conflict") do
55+
version "1.2.3"
56+
sha256 :no_check
57+
url "file://#{TEST_FIXTURE_DIR}/cask/container.zip"
58+
59+
preflight do
60+
touch "ruby-block-ran"
61+
end
62+
63+
preflight_steps do
64+
touch "steps-ran"
65+
end
66+
end
67+
end.to output(/`preflight` is ignored because `preflight_steps` is defined/).to_stderr
68+
69+
cask = T.must(cask)
70+
cask.staged_path.mkpath
71+
cask.config_path.dirname.mkpath
72+
73+
Cask::Installer.new(cask, command: NeverSudoSystemCommand).install_artifacts
74+
75+
expect(cask.staged_path/"ruby-block-ran").not_to exist
76+
expect(cask.staged_path/"steps-ran").to exist
77+
end
78+
end

0 commit comments

Comments
 (0)