From db8d488db418653d3a454dc2654f5a7d256ce379 Mon Sep 17 00:00:00 2001 From: apainintheneck Date: Fri, 17 Mar 2023 23:24:37 -0700 Subject: [PATCH 1/7] service: add info to API json and load from api As a part of serializing the hash, certain path placeholders are used for the HOMEBREW_PREFIX and $HOME directories. Beyond that certain elements need to be turned back into strings like cron and sockets and symbols need to be preserved as well. The run command accepts either an arg or kwargs so it has to be treated specially here. --- Library/Homebrew/formula.rb | 3 +- Library/Homebrew/formulary.rb | 15 +++++ Library/Homebrew/service.rb | 105 +++++++++++++++++++++++++++++++--- 3 files changed, 115 insertions(+), 8 deletions(-) diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb index bc3bb68d26..c71a854cc2 100644 --- a/Library/Homebrew/formula.rb +++ b/Library/Homebrew/formula.rb @@ -1043,7 +1043,7 @@ class Formula def service return unless service? - Homebrew::Service.new(self, &self.class.service) + @service ||= Homebrew::Service.new(self, &self.class.service) end # @private @@ -2132,6 +2132,7 @@ class Formula "disabled" => disabled?, "disable_date" => disable_date, "disable_reason" => disable_reason, + "service" => service&.serialize, "tap_git_head" => tap_git_head, "ruby_source_checksum" => {}, } diff --git a/Library/Homebrew/formulary.rb b/Library/Homebrew/formulary.rb index aa179fbcfa..0113ace173 100644 --- a/Library/Homebrew/formulary.rb +++ b/Library/Homebrew/formulary.rb @@ -263,6 +263,21 @@ module Formulary raise "Cannot build from source from abstract formula." end + if (service_hash = json_formula["service"]) + service_hash = Homebrew::Service.deserialize(service_hash) + run_params = service_hash.delete("run") + service do + if run_params.is_a?(Hash) + run(**run_params) + else + run run_params + end + service_hash.each do |key, arg| + public_send(key, arg) + end + end + end + @caveats_string = json_formula["caveats"] def caveats self.class.instance_variable_get(:@caveats_string) diff --git a/Library/Homebrew/service.rb b/Library/Homebrew/service.rb index 4725a8acb0..7f149b3a1f 100644 --- a/Library/Homebrew/service.rb +++ b/Library/Homebrew/service.rb @@ -45,6 +45,9 @@ module Homebrew ).returns(T.nilable(Array)) } def run(command = nil, macos: nil, linux: nil) + # Save parameters for serialization + @run_params ||= command || { macos: macos, linux: linux }.compact + command ||= on_system_conditional(macos: macos, linux: linux) case T.unsafe(command) when nil @@ -156,7 +159,7 @@ module Homebrew # @return [Boolean] sig { returns(T::Boolean) } def requires_root? - instance_eval(&@service_block) + eval_service_block @require_root.present? && @require_root == true end @@ -192,7 +195,7 @@ module Homebrew # @return [Boolean] sig { returns(T::Boolean) } def keep_alive? - instance_eval(&@service_block) + eval_service_block @keep_alive.present? && @keep_alive[:always] != false end @@ -320,7 +323,7 @@ module Homebrew parsed end - sig { params(variables: T::Hash[String, String]).returns(T.nilable(T::Hash[String, String])) } + sig { params(variables: T::Hash[Symbol, String]).returns(T.nilable(T::Hash[Symbol, String])) } def environment_variables(variables = {}) case T.unsafe(variables) when Hash @@ -351,7 +354,7 @@ module Homebrew sig { returns(T.nilable(T::Array[String])) } def command - instance_eval(&@service_block) + eval_service_block @run&.map(&:to_s) end @@ -359,7 +362,7 @@ module Homebrew # @return [String] sig { returns(String) } def manual_command - instance_eval(&@service_block) + eval_service_block vars = @environment_variables.except(:PATH) .map { |k, v| "#{k}=\"#{v}\"" } @@ -372,7 +375,7 @@ module Homebrew # @return [Boolean] sig { returns(T::Boolean) } def timed? - instance_eval(&@service_block) + eval_service_block @run_type == RUN_TYPE_CRON || @run_type == RUN_TYPE_INTERVAL end @@ -484,7 +487,7 @@ module Homebrew Unit=#{@formula.service_name} EOS - instance_eval(&@service_block) + eval_service_block options = [] options << "Persistent=true" if @run_type == RUN_TYPE_CRON options << "OnUnitActiveSec=#{@interval}" if @run_type == RUN_TYPE_INTERVAL @@ -497,5 +500,93 @@ module Homebrew timer + options.join("\n") end + + # Only evaluate the service block once. + sig { void } + def eval_service_block + return if @eval_service_block + + instance_eval(&@service_block) + @eval_service_block = true + end + + # Recursively prepare the service hash for inclusion in the formula API JSON. + # - Replace each Symbol with a String prefixed by ":" + # - Replace the local HOMEBREW_PREFIX with "$HOMEBREW_PREFIX" + # - Replace the local home directory with "$HOME" + def serialize(elem = to_h) + case elem + when String, Pathname + elem.to_s.gsub(HOMEBREW_PREFIX, HOMEBREW_PREFIX_PLACEHOLDER) + .gsub(Dir.home, "$HOME") + when Symbol + elem.inspect + when Array + elem.map { |value| serialize(value) } + when Hash + elem.to_h do |key, value| + key = key.inspect if key.is_a?(Symbol) + [key, serialize(value)] + end + else + elem + end + end + + # Recursively turn the service API hash values back into what is expected by the formula DSL. + # - Replace each String prefixed with ":" with a Symbol + # - Replace "$HOMEBREW_PREFIX" with the local HOMEBREW_PREFIX environment variable + # - Replace "$HOME" with the local home directory + def self.deserialize(elem) + case elem + when String + return T.must(elem[1..]).to_sym if elem.start_with?(":") + + elem.gsub(HOMEBREW_PREFIX_PLACEHOLDER, HOMEBREW_PREFIX) + .gsub("$HOME", Dir.home) + when Array + elem.map { |value| deserialize(value) } + when Hash + elem.to_h do |key, value| + key = key[1..].to_sym if key.start_with?(":") + [key, deserialize(value)] + end + else + elem + end + end + + sig { returns(Hash) } + def to_h + eval_service_block + + cron_string = if @cron.present? + [:Minute, :Hour, :Day, :Month, :Weekday] + .map { |key| @cron[key].to_s } + .join(" ") + end + + sockets_string = "#{@sockets[:type]}://#{@sockets[:host]}:#{@sockets[:port]}" if @sockets.present? + + { + "run" => @run_params, + "run_type" => @run_type, + "interval" => @interval, + "cron" => cron_string, + "keep_alive" => @keep_alive, + "launch_only_once" => @launch_only_once, + "require_root" => @require_root, + "environment_variables" => @environment_variables.presence, + "working_dir" => @working_dir, + "root_dir" => @root_dir, + "input_path" => @input_path, + "log_path" => @log_path, + "error_log_path" => @error_log_path, + "restart_delay" => @restart_delay, + "process_type" => @process_type, + "macos_legacy_timers" => @macos_legacy_timers, + "sockets" => sockets_string, + }.compact + end end end From 501730df5d9857f4d711f472725b22c433b14f12 Mon Sep 17 00:00:00 2001 From: apainintheneck Date: Sat, 18 Mar 2023 13:23:30 -0700 Subject: [PATCH 2/7] Add service serialization specs --- Library/Homebrew/test/service_spec.rb | 77 +++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/Library/Homebrew/test/service_spec.rb b/Library/Homebrew/test/service_spec.rb index 04934da624..f552e2c3f9 100644 --- a/Library/Homebrew/test/service_spec.rb +++ b/Library/Homebrew/test/service_spec.rb @@ -913,4 +913,81 @@ describe Homebrew::Service do expect(command).to eq(["#{HOMEBREW_PREFIX}/opt/#{name}/bin/beanstalkd", "test", "macos"]) end end + + describe "#serialize" do + context "with a single run command" do + let(:serialized_hash) do + { + "environment_variables" => { + ":PATH" => "$HOMEBREW_PREFIX/bin:$HOMEBREW_PREFIX/sbin:/usr/bin:/bin:/usr/sbin:/sbin", + }, + "run" => ["$HOMEBREW_PREFIX/opt/formula_name/bin/beanstalkd", "test"], + "run_type" => ":immediate", + "working_dir" => "$HOME", + } + end + + it "replaces local paths with placeholders" do + f = stub_formula do + service do + run [opt_bin/"beanstalkd", "test"] + environment_variables PATH: std_service_path_env + working_dir Dir.home + end + end + + expect(f.service.serialize).to eq(serialized_hash) + end + end + + context "with OS specific run commands" do + let(:serialized_hash) do + { + "run" => { + ":macos" => ["$HOMEBREW_PREFIX/opt/formula_name/bin/beanstalkd", "macos"], + ":linux" => ["$HOMEBREW_PREFIX/opt/formula_name/bin/beanstalkd", "linux"], + }, + "run_type" => ":immediate", + } + end + + it "replaces local paths with placeholders" do + f = stub_formula do + service do + run macos: [opt_bin/"beanstalkd", "macos"], linux: [opt_bin/"beanstalkd", "linux"] + end + end + + expect(f.service.serialize).to eq(serialized_hash) + end + end + end + + describe ".deserialize" do + let(:serialized_hash) do + { + "environment_variables" => { + ":PATH" => "$HOMEBREW_PREFIX/bin:$HOMEBREW_PREFIX/sbin:/usr/bin:/bin:/usr/sbin:/sbin", + }, + "run" => ["$HOMEBREW_PREFIX/opt/formula_name/bin/beanstalkd", "test"], + "run_type" => ":immediate", + "working_dir" => "$HOME", + } + end + + let(:deserialized_hash) do + { + "environment_variables" => { + PATH: "#{HOMEBREW_PREFIX}/bin:#{HOMEBREW_PREFIX}/sbin:/usr/bin:/bin:/usr/sbin:/sbin", + }, + "run" => ["#{HOMEBREW_PREFIX}/opt/formula_name/bin/beanstalkd", "test"], + "run_type" => :immediate, + "working_dir" => Dir.home, + } + end + + it "replaces placeholders with local paths" do + expect(described_class.deserialize(serialized_hash)).to eq(deserialized_hash) + end + end end From d42e9b40d5b5e03e7bd292c4cf977f7ed9a237b0 Mon Sep 17 00:00:00 2001 From: apainintheneck Date: Sat, 18 Mar 2023 15:18:29 -0700 Subject: [PATCH 3/7] formulary: add test for loading service blocks from API --- Library/Homebrew/test/formulary_spec.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Library/Homebrew/test/formulary_spec.rb b/Library/Homebrew/test/formulary_spec.rb index ef20f3229a..b20a00b237 100644 --- a/Library/Homebrew/test/formulary_spec.rb +++ b/Library/Homebrew/test/formulary_spec.rb @@ -272,6 +272,11 @@ describe Formulary do "conflicts_with_reasons" => ["it does"], "link_overwrite" => ["bin/abc"], "caveats" => "example caveat string", + "service" => { + "run" => ["$HOMEBREW_PREFIX/opt/formula_name/bin/beanstalkd", "test"], + "run_type" => :immediate, + "working_dir" => "$HOME", + }, }.merge(extra_items), } end @@ -354,6 +359,11 @@ describe Formulary do expect(formula.caveats).to eq "example caveat string" + expect(formula).to be_a_service + expect(formula.service.command).to eq(["#{HOMEBREW_PREFIX}/opt/formula_name/bin/beanstalkd", "test"]) + expect(formula.service.run_type).to eq(:immediate) + expect(formula.service.working_dir).to eq(Dir.home) + expect do formula.install end.to raise_error("Cannot build from source from abstract formula.") From 38146893c3966d0ad1a359f866df4dbf49b9e176 Mon Sep 17 00:00:00 2001 From: apainintheneck Date: Tue, 21 Mar 2023 19:05:41 -0700 Subject: [PATCH 4/7] api_hashable: Make API path subs generic This turns the ability to replace common paths with placeholders into a mixin that can be used with both Casks and Formulae. The idea here is to make formula hash generation more consistent. --- Library/Homebrew/cask/cask.rb | 36 ++----------------- Library/Homebrew/cask/cask_loader.rb | 4 +-- .../Homebrew/dev-cmd/generate-formula-api.rb | 1 + Library/Homebrew/extend/api_hashable.rb | 34 ++++++++++++++++++ Library/Homebrew/formula.rb | 2 ++ Library/Homebrew/global.rb | 2 ++ Library/Homebrew/test/cask/cask_spec.rb | 2 ++ 7 files changed, 45 insertions(+), 36 deletions(-) create mode 100644 Library/Homebrew/extend/api_hashable.rb diff --git a/Library/Homebrew/cask/cask.rb b/Library/Homebrew/cask/cask.rb index 04f68cda0c..9b497a4082 100644 --- a/Library/Homebrew/cask/cask.rb +++ b/Library/Homebrew/cask/cask.rb @@ -6,6 +6,7 @@ require "cask/config" require "cask/dsl" require "cask/metadata" require "utils/bottles" +require "extend/api_hashable" module Cask # An instance of a cask. @@ -16,11 +17,9 @@ module Cask extend Forwardable extend Predicable + extend APIHashable include Metadata - # Needs a leading slash to avoid `File.expand.path` complaining about non-absolute home. - HOME_PLACEHOLDER = "/$HOME" - HOMEBREW_PREFIX_PLACEHOLDER = "$HOMEBREW_PREFIX" APPDIR_PLACEHOLDER = "$APPDIR" # TODO: can be removed when API JSON is regenerated with HOMEBREW_PREFIX_PLACEHOLDER. @@ -31,37 +30,6 @@ module Cask attr_predicate :loaded_from_api? - class << self - def generating_hash! - return if generating_hash? - - # Apply monkeypatches for API generation - @old_homebrew_prefix = HOMEBREW_PREFIX - @old_home = Dir.home - Object.send(:remove_const, :HOMEBREW_PREFIX) - Object.const_set(:HOMEBREW_PREFIX, Pathname(HOMEBREW_PREFIX_PLACEHOLDER)) - ENV["HOME"] = HOME_PLACEHOLDER - - @generating_hash = true - end - - def generated_hash! - return unless generating_hash? - - # Revert monkeypatches for API generation - Object.send(:remove_const, :HOMEBREW_PREFIX) - Object.const_set(:HOMEBREW_PREFIX, @old_homebrew_prefix) - ENV["HOME"] = @old_home - - @generating_hash = false - end - - def generating_hash? - @generating_hash ||= false - @generating_hash == true - end - end - def self.all # TODO: ideally avoid using ARGV by moving to e.g. CLI::Parser if ARGV.exclude?("--eval-all") && !Homebrew::EnvConfig.eval_all? diff --git a/Library/Homebrew/cask/cask_loader.rb b/Library/Homebrew/cask/cask_loader.rb index 0fa41924df..3f884a630b 100644 --- a/Library/Homebrew/cask/cask_loader.rb +++ b/Library/Homebrew/cask/cask_loader.rb @@ -329,8 +329,8 @@ module Cask # TODO: HOMEBREW_OLD_PREFIX_PLACEHOLDER can be removed when API JSON is # regenerated with HOMEBREW_PREFIX_PLACEHOLDER. string.to_s - .gsub(Cask::HOME_PLACEHOLDER, Dir.home) - .gsub(Cask::HOMEBREW_PREFIX_PLACEHOLDER, HOMEBREW_PREFIX) + .gsub(HOMEBREW_HOME_PLACEHOLDER, Dir.home) + .gsub(HOMEBREW_PREFIX_PLACEHOLDER, HOMEBREW_PREFIX) .gsub(Cask::APPDIR_PLACEHOLDER, appdir) .gsub(Cask::HOMEBREW_OLD_PREFIX_PLACEHOLDER, HOMEBREW_PREFIX) end diff --git a/Library/Homebrew/dev-cmd/generate-formula-api.rb b/Library/Homebrew/dev-cmd/generate-formula-api.rb index c47c0f4571..fb05cb187c 100644 --- a/Library/Homebrew/dev-cmd/generate-formula-api.rb +++ b/Library/Homebrew/dev-cmd/generate-formula-api.rb @@ -50,6 +50,7 @@ module Homebrew FileUtils.mkdir_p directories Formulary.enable_factory_cache! + Formula.generating_hash! tap.formula_names.each do |name| formula = Formulary.factory(name) diff --git a/Library/Homebrew/extend/api_hashable.rb b/Library/Homebrew/extend/api_hashable.rb new file mode 100644 index 0000000000..0527a94938 --- /dev/null +++ b/Library/Homebrew/extend/api_hashable.rb @@ -0,0 +1,34 @@ +# typed: true +# frozen_string_literal: true + +# Used to substitute common paths with generic placeholders when generating JSON for the API. +module APIHashable + def generating_hash! + return if generating_hash? + + # Apply monkeypatches for API generation + @old_homebrew_prefix = HOMEBREW_PREFIX + @old_home = Dir.home + Object.send(:remove_const, :HOMEBREW_PREFIX) + Object.const_set(:HOMEBREW_PREFIX, Pathname.new(HOMEBREW_PREFIX_PLACEHOLDER)) + ENV["HOME"] = HOMEBREW_HOME_PLACEHOLDER + + @generating_hash = true + end + + def generated_hash! + return unless generating_hash? + + # Revert monkeypatches for API generation + Object.send(:remove_const, :HOMEBREW_PREFIX) + Object.const_set(:HOMEBREW_PREFIX, @old_homebrew_prefix) + ENV["HOME"] = @old_home + + @generating_hash = false + end + + def generating_hash? + @generating_hash ||= false + @generating_hash == true + end +end diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb index c71a854cc2..19f2bde8e6 100644 --- a/Library/Homebrew/formula.rb +++ b/Library/Homebrew/formula.rb @@ -30,6 +30,7 @@ require "find" require "utils/spdx" require "extend/on_system" require "api" +require "extend/api_hashable" # A formula provides instructions and metadata for Homebrew to install a piece # of software. Every Homebrew formula is a {Formula}. @@ -69,6 +70,7 @@ class Formula extend Forwardable extend Cachable extend Predicable + extend APIHashable # The name of this {Formula}. # e.g. `this-formula` diff --git a/Library/Homebrew/global.rb b/Library/Homebrew/global.rb index de6e01e5e4..1ed2ee7d21 100644 --- a/Library/Homebrew/global.rb +++ b/Library/Homebrew/global.rb @@ -58,6 +58,8 @@ HOMEBREW_MACOS_ARM_DEFAULT_REPOSITORY = ENV.fetch("HOMEBREW_MACOS_ARM_DEFAULT_RE HOMEBREW_LINUX_DEFAULT_PREFIX = ENV.fetch("HOMEBREW_LINUX_DEFAULT_PREFIX").freeze HOMEBREW_LINUX_DEFAULT_REPOSITORY = ENV.fetch("HOMEBREW_LINUX_DEFAULT_REPOSITORY").freeze HOMEBREW_PREFIX_PLACEHOLDER = "$HOMEBREW_PREFIX" +# Needs a leading slash to avoid `File.expand.path` complaining about non-absolute home. +HOMEBREW_HOME_PLACEHOLDER = "/$HOME" HOMEBREW_MACOS_NEWEST_UNSUPPORTED = ENV.fetch("HOMEBREW_MACOS_NEWEST_UNSUPPORTED").freeze HOMEBREW_MACOS_OLDEST_SUPPORTED = ENV.fetch("HOMEBREW_MACOS_OLDEST_SUPPORTED").freeze diff --git a/Library/Homebrew/test/cask/cask_spec.rb b/Library/Homebrew/test/cask/cask_spec.rb index ddc581c730..065d8fe6d0 100644 --- a/Library/Homebrew/test/cask/cask_spec.rb +++ b/Library/Homebrew/test/cask/cask_spec.rb @@ -337,6 +337,8 @@ describe Cask::Cask, :cask do expect(JSON.pretty_generate(h["variations"])).to eq expected_sha256_variations.strip end + # @note The calls to `APIHashable.generating_hash!` and `APIHashable.generated_hash!` + # are not idempotent so they can only be used in one test. it "returns the correct hash placeholders" do described_class.generating_hash! expect(described_class).to be_generating_hash From bc85857e1341eef2e105cbacd3e5f1e4eaaf47a0 Mon Sep 17 00:00:00 2001 From: apainintheneck Date: Tue, 21 Mar 2023 21:53:52 -0700 Subject: [PATCH 5/7] service: make serialization non-recursive This helps with type annotations and makes it easier to reason about the code as well. --- Library/Homebrew/formulary.rb | 2 +- Library/Homebrew/service.rb | 136 ++++++++++++++++++---------------- 2 files changed, 73 insertions(+), 65 deletions(-) diff --git a/Library/Homebrew/formulary.rb b/Library/Homebrew/formulary.rb index 0113ace173..b318dcf9d4 100644 --- a/Library/Homebrew/formulary.rb +++ b/Library/Homebrew/formulary.rb @@ -265,7 +265,7 @@ module Formulary if (service_hash = json_formula["service"]) service_hash = Homebrew::Service.deserialize(service_hash) - run_params = service_hash.delete("run") + run_params = service_hash.delete(:run) service do if run_params.is_a?(Hash) run(**run_params) diff --git a/Library/Homebrew/service.rb b/Library/Homebrew/service.rb index 7f149b3a1f..d676203d29 100644 --- a/Library/Homebrew/service.rb +++ b/Library/Homebrew/service.rb @@ -510,54 +510,9 @@ module Homebrew @eval_service_block = true end - # Recursively prepare the service hash for inclusion in the formula API JSON. - # - Replace each Symbol with a String prefixed by ":" - # - Replace the local HOMEBREW_PREFIX with "$HOMEBREW_PREFIX" - # - Replace the local home directory with "$HOME" - def serialize(elem = to_h) - case elem - when String, Pathname - elem.to_s.gsub(HOMEBREW_PREFIX, HOMEBREW_PREFIX_PLACEHOLDER) - .gsub(Dir.home, "$HOME") - when Symbol - elem.inspect - when Array - elem.map { |value| serialize(value) } - when Hash - elem.to_h do |key, value| - key = key.inspect if key.is_a?(Symbol) - [key, serialize(value)] - end - else - elem - end - end - - # Recursively turn the service API hash values back into what is expected by the formula DSL. - # - Replace each String prefixed with ":" with a Symbol - # - Replace "$HOMEBREW_PREFIX" with the local HOMEBREW_PREFIX environment variable - # - Replace "$HOME" with the local home directory - def self.deserialize(elem) - case elem - when String - return T.must(elem[1..]).to_sym if elem.start_with?(":") - - elem.gsub(HOMEBREW_PREFIX_PLACEHOLDER, HOMEBREW_PREFIX) - .gsub("$HOME", Dir.home) - when Array - elem.map { |value| deserialize(value) } - when Hash - elem.to_h do |key, value| - key = key[1..].to_sym if key.start_with?(":") - [key, deserialize(value)] - end - else - elem - end - end - + # Prepare the service hash for inclusion in the formula API JSON. sig { returns(Hash) } - def to_h + def serialize eval_service_block cron_string = if @cron.present? @@ -569,24 +524,77 @@ module Homebrew sockets_string = "#{@sockets[:type]}://#{@sockets[:host]}:#{@sockets[:port]}" if @sockets.present? { - "run" => @run_params, - "run_type" => @run_type, - "interval" => @interval, - "cron" => cron_string, - "keep_alive" => @keep_alive, - "launch_only_once" => @launch_only_once, - "require_root" => @require_root, - "environment_variables" => @environment_variables.presence, - "working_dir" => @working_dir, - "root_dir" => @root_dir, - "input_path" => @input_path, - "log_path" => @log_path, - "error_log_path" => @error_log_path, - "restart_delay" => @restart_delay, - "process_type" => @process_type, - "macos_legacy_timers" => @macos_legacy_timers, - "sockets" => sockets_string, + run: @run_params, + run_type: @run_type, + interval: @interval, + cron: cron_string, + keep_alive: @keep_alive, + launch_only_once: @launch_only_once, + require_root: @require_root, + environment_variables: @environment_variables.presence, + working_dir: @working_dir, + root_dir: @root_dir, + input_path: @input_path, + log_path: @log_path, + error_log_path: @error_log_path, + restart_delay: @restart_delay, + process_type: @process_type, + macos_legacy_timers: @macos_legacy_timers, + sockets: sockets_string, }.compact end + + # Turn the service API hash values back into what is expected by the formula DSL. + sig { params(api_hash: Hash).returns(Hash) } + def self.deserialize(api_hash) + hash = {} + hash[:run] = + case api_hash["run"] + when Hash + api_hash["run"].to_h do |key, array| + [ + key.to_sym, + array.map { |value| replace_placeholders(value) }, + ] + end + when Array + api_hash["run"].map { |value| replace_placeholders(value) } + end + + hash[:keep_alive] = api_hash["keep_alive"].transform_keys(&:to_sym) if api_hash.key?("keep_alive") + + if api_hash.key?("environment_variables") + hash[:environment_variables] = api_hash["environment_variables"].to_h do |key, value| + [key.to_sym, replace_placeholders(value)] + end + end + + %w[run_type process_type].each do |key| + next unless (value = api_hash[key]) + + hash[key.to_sym] = value.to_sym + end + + %w[working_dir root_dir input_path log_path error_log_path].each do |key| + next unless (value = api_hash[key]) + + hash[key.to_sym] = replace_placeholders(value) + end + + %w[interval cron launch_only_once require_root restart_delay macos_legacy_timers sockets].each do |key| + next if (value = api_hash[key]).nil? + + hash[key.to_sym] = value + end + + hash + end + + # Replace API path placeholders with local paths. + sig { params(string: String).returns(String) } + def self.replace_placeholders(string) + string.gsub(HOMEBREW_PREFIX_PLACEHOLDER, HOMEBREW_PREFIX) + .gsub(HOMEBREW_HOME_PLACEHOLDER, Dir.home) + end end end From e83a2562bba6a36b32403d2189654ea0fb60be5b Mon Sep 17 00:00:00 2001 From: apainintheneck Date: Tue, 21 Mar 2023 21:55:47 -0700 Subject: [PATCH 6/7] Update service and formulary specs --- Library/Homebrew/test/formulary_spec.rb | 4 +- Library/Homebrew/test/service_spec.rb | 80 ++++++++++--------------- 2 files changed, 34 insertions(+), 50 deletions(-) diff --git a/Library/Homebrew/test/formulary_spec.rb b/Library/Homebrew/test/formulary_spec.rb index b20a00b237..85cf0c6ae0 100644 --- a/Library/Homebrew/test/formulary_spec.rb +++ b/Library/Homebrew/test/formulary_spec.rb @@ -274,8 +274,8 @@ describe Formulary do "caveats" => "example caveat string", "service" => { "run" => ["$HOMEBREW_PREFIX/opt/formula_name/bin/beanstalkd", "test"], - "run_type" => :immediate, - "working_dir" => "$HOME", + "run_type" => "immediate", + "working_dir" => "/$HOME", }, }.merge(extra_items), } diff --git a/Library/Homebrew/test/service_spec.rb b/Library/Homebrew/test/service_spec.rb index f552e2c3f9..4f255d8aa8 100644 --- a/Library/Homebrew/test/service_spec.rb +++ b/Library/Homebrew/test/service_spec.rb @@ -915,51 +915,33 @@ describe Homebrew::Service do end describe "#serialize" do - context "with a single run command" do - let(:serialized_hash) do - { - "environment_variables" => { - ":PATH" => "$HOMEBREW_PREFIX/bin:$HOMEBREW_PREFIX/sbin:/usr/bin:/bin:/usr/sbin:/sbin", - }, - "run" => ["$HOMEBREW_PREFIX/opt/formula_name/bin/beanstalkd", "test"], - "run_type" => ":immediate", - "working_dir" => "$HOME", - } - end - - it "replaces local paths with placeholders" do - f = stub_formula do - service do - run [opt_bin/"beanstalkd", "test"] - environment_variables PATH: std_service_path_env - working_dir Dir.home - end - end - - expect(f.service.serialize).to eq(serialized_hash) - end + let(:serialized_hash) do + { + environment_variables: { + PATH: "$HOMEBREW_PREFIX/bin:$HOMEBREW_PREFIX/sbin:/usr/bin:/bin:/usr/sbin:/sbin", + }, + run: [Pathname("$HOMEBREW_PREFIX/opt/formula_name/bin/beanstalkd"), "test"], + run_type: :immediate, + working_dir: "/$HOME", + cron: "0 0 * * 0", + sockets: "tcp://0.0.0.0:80", + } end - context "with OS specific run commands" do - let(:serialized_hash) do - { - "run" => { - ":macos" => ["$HOMEBREW_PREFIX/opt/formula_name/bin/beanstalkd", "macos"], - ":linux" => ["$HOMEBREW_PREFIX/opt/formula_name/bin/beanstalkd", "linux"], - }, - "run_type" => ":immediate", - } - end - - it "replaces local paths with placeholders" do - f = stub_formula do - service do - run macos: [opt_bin/"beanstalkd", "macos"], linux: [opt_bin/"beanstalkd", "linux"] - end + it "replaces local paths with placeholders" do + f = stub_formula do + service do + run [opt_bin/"beanstalkd", "test"] + environment_variables PATH: std_service_path_env + working_dir Dir.home + cron "@weekly" + sockets "tcp://0.0.0.0:80" end - - expect(f.service.serialize).to eq(serialized_hash) end + + Formula.generating_hash! + expect(f.service.serialize).to eq(serialized_hash) + Formula.generated_hash! end end @@ -967,22 +949,24 @@ describe Homebrew::Service do let(:serialized_hash) do { "environment_variables" => { - ":PATH" => "$HOMEBREW_PREFIX/bin:$HOMEBREW_PREFIX/sbin:/usr/bin:/bin:/usr/sbin:/sbin", + "PATH" => "$HOMEBREW_PREFIX/bin:$HOMEBREW_PREFIX/sbin:/usr/bin:/bin:/usr/sbin:/sbin", }, "run" => ["$HOMEBREW_PREFIX/opt/formula_name/bin/beanstalkd", "test"], - "run_type" => ":immediate", - "working_dir" => "$HOME", + "run_type" => "immediate", + "working_dir" => HOMEBREW_HOME_PLACEHOLDER, + "keep_alive" => { "successful_exit" => false }, } end let(:deserialized_hash) do { - "environment_variables" => { + environment_variables: { PATH: "#{HOMEBREW_PREFIX}/bin:#{HOMEBREW_PREFIX}/sbin:/usr/bin:/bin:/usr/sbin:/sbin", }, - "run" => ["#{HOMEBREW_PREFIX}/opt/formula_name/bin/beanstalkd", "test"], - "run_type" => :immediate, - "working_dir" => Dir.home, + run: ["#{HOMEBREW_PREFIX}/opt/formula_name/bin/beanstalkd", "test"], + run_type: :immediate, + working_dir: Dir.home, + keep_alive: { successful_exit: false }, } end From 801ee5e4744ba4f046e01d442ffd749af2d163a5 Mon Sep 17 00:00:00 2001 From: apainintheneck Date: Wed, 22 Mar 2023 19:43:49 -0700 Subject: [PATCH 7/7] Address feedback - style nits - better comments for tests that are not idempotent - moved appdir placeholder constant to global.rb --- Library/Homebrew/cask/cask.rb | 2 -- Library/Homebrew/cask/cask_loader.rb | 2 +- Library/Homebrew/cask/dsl.rb | 2 +- Library/Homebrew/global.rb | 1 + Library/Homebrew/service.rb | 7 ++++--- Library/Homebrew/test/cask/cask_spec.rb | 2 +- Library/Homebrew/test/service_spec.rb | 2 ++ 7 files changed, 10 insertions(+), 8 deletions(-) diff --git a/Library/Homebrew/cask/cask.rb b/Library/Homebrew/cask/cask.rb index 9b497a4082..e654f3260d 100644 --- a/Library/Homebrew/cask/cask.rb +++ b/Library/Homebrew/cask/cask.rb @@ -20,8 +20,6 @@ module Cask extend APIHashable include Metadata - APPDIR_PLACEHOLDER = "$APPDIR" - # TODO: can be removed when API JSON is regenerated with HOMEBREW_PREFIX_PLACEHOLDER. HOMEBREW_OLD_PREFIX_PLACEHOLDER = "$(brew --prefix)" diff --git a/Library/Homebrew/cask/cask_loader.rb b/Library/Homebrew/cask/cask_loader.rb index 3f884a630b..a47a6464a3 100644 --- a/Library/Homebrew/cask/cask_loader.rb +++ b/Library/Homebrew/cask/cask_loader.rb @@ -331,7 +331,7 @@ module Cask string.to_s .gsub(HOMEBREW_HOME_PLACEHOLDER, Dir.home) .gsub(HOMEBREW_PREFIX_PLACEHOLDER, HOMEBREW_PREFIX) - .gsub(Cask::APPDIR_PLACEHOLDER, appdir) + .gsub(HOMEBREW_CASK_APPDIR_PLACEHOLDER, appdir) .gsub(Cask::HOMEBREW_OLD_PREFIX_PLACEHOLDER, HOMEBREW_PREFIX) end diff --git a/Library/Homebrew/cask/dsl.rb b/Library/Homebrew/cask/dsl.rb index 461b87af5b..7976100525 100644 --- a/Library/Homebrew/cask/dsl.rb +++ b/Library/Homebrew/cask/dsl.rb @@ -375,7 +375,7 @@ module Cask # @api public def appdir - return Cask::APPDIR_PLACEHOLDER if Cask.generating_hash? + return HOMEBREW_CASK_APPDIR_PLACEHOLDER if Cask.generating_hash? cask.config.appdir end diff --git a/Library/Homebrew/global.rb b/Library/Homebrew/global.rb index 1ed2ee7d21..3c3cecdb2d 100644 --- a/Library/Homebrew/global.rb +++ b/Library/Homebrew/global.rb @@ -60,6 +60,7 @@ HOMEBREW_LINUX_DEFAULT_REPOSITORY = ENV.fetch("HOMEBREW_LINUX_DEFAULT_REPOSITORY HOMEBREW_PREFIX_PLACEHOLDER = "$HOMEBREW_PREFIX" # Needs a leading slash to avoid `File.expand.path` complaining about non-absolute home. HOMEBREW_HOME_PLACEHOLDER = "/$HOME" +HOMEBREW_CASK_APPDIR_PLACEHOLDER = "$APPDIR" HOMEBREW_MACOS_NEWEST_UNSUPPORTED = ENV.fetch("HOMEBREW_MACOS_NEWEST_UNSUPPORTED").freeze HOMEBREW_MACOS_OLDEST_SUPPORTED = ENV.fetch("HOMEBREW_MACOS_OLDEST_SUPPORTED").freeze diff --git a/Library/Homebrew/service.rb b/Library/Homebrew/service.rb index d676203d29..2e4aefe46b 100644 --- a/Library/Homebrew/service.rb +++ b/Library/Homebrew/service.rb @@ -46,7 +46,8 @@ module Homebrew } def run(command = nil, macos: nil, linux: nil) # Save parameters for serialization - @run_params ||= command || { macos: macos, linux: linux }.compact + @run_params ||= command + @run_params ||= { macos: macos, linux: linux }.compact command ||= on_system_conditional(macos: macos, linux: linux) case T.unsafe(command) @@ -554,11 +555,11 @@ module Homebrew api_hash["run"].to_h do |key, array| [ key.to_sym, - array.map { |value| replace_placeholders(value) }, + array.map(&method(:replace_placeholders)), ] end when Array - api_hash["run"].map { |value| replace_placeholders(value) } + api_hash["run"].map(&method(:replace_placeholders)) end hash[:keep_alive] = api_hash["keep_alive"].transform_keys(&:to_sym) if api_hash.key?("keep_alive") diff --git a/Library/Homebrew/test/cask/cask_spec.rb b/Library/Homebrew/test/cask/cask_spec.rb index 065d8fe6d0..f9dea17485 100644 --- a/Library/Homebrew/test/cask/cask_spec.rb +++ b/Library/Homebrew/test/cask/cask_spec.rb @@ -337,7 +337,7 @@ describe Cask::Cask, :cask do expect(JSON.pretty_generate(h["variations"])).to eq expected_sha256_variations.strip end - # @note The calls to `APIHashable.generating_hash!` and `APIHashable.generated_hash!` + # @note The calls to `Cask.generating_hash!` and `Cask.generated_hash!` # are not idempotent so they can only be used in one test. it "returns the correct hash placeholders" do described_class.generating_hash! diff --git a/Library/Homebrew/test/service_spec.rb b/Library/Homebrew/test/service_spec.rb index 4f255d8aa8..f9089f012f 100644 --- a/Library/Homebrew/test/service_spec.rb +++ b/Library/Homebrew/test/service_spec.rb @@ -928,6 +928,8 @@ describe Homebrew::Service do } end + # @note The calls to `Formula.generating_hash!` and `Formula.generated_hash!` + # are not idempotent so they can only be used in one test. it "replaces local paths with placeholders" do f = stub_formula do service do