From 8e98ce69f39cf014f63195ed880d3f445c905757 Mon Sep 17 00:00:00 2001 From: Bo Anderson Date: Thu, 18 Mar 2021 14:46:48 +0000 Subject: [PATCH] Stricter handling of CLI args --- Library/Homebrew/build.rb | 2 +- Library/Homebrew/cask/cmd/audit.rb | 8 +- Library/Homebrew/cli/args.rb | 30 +++- Library/Homebrew/cli/args.rbi | 102 ++++++------ Library/Homebrew/cli/named_args.rb | 6 +- Library/Homebrew/cli/parser.rb | 19 ++- Library/Homebrew/cmd/--env.rb | 4 +- Library/Homebrew/cmd/install.rb | 14 +- Library/Homebrew/cmd/outdated.rb | 4 +- Library/Homebrew/cmd/reinstall.rb | 38 ++++- Library/Homebrew/cmd/upgrade.rb | 29 +++- Library/Homebrew/dev-cmd/audit.rb | 1 - Library/Homebrew/dev-cmd/test.rb | 2 + Library/Homebrew/formula_auditor.rb | 1 - Library/Homebrew/reinstall.rb | 54 +++--- Library/Homebrew/test/cask/cmd/audit_spec.rb | 18 +- Library/Homebrew/test/cli/named_args_spec.rb | 1 + Library/Homebrew/test/cli/parser_spec.rb | 18 +- Library/Homebrew/upgrade.rb | 165 +++++++++++++------ 19 files changed, 349 insertions(+), 167 deletions(-) diff --git a/Library/Homebrew/build.rb b/Library/Homebrew/build.rb index 62c566f922..7429e77423 100644 --- a/Library/Homebrew/build.rb +++ b/Library/Homebrew/build.rb @@ -27,7 +27,7 @@ class Build @formula.build = BuildOptions.new(options, formula.options) @args = args - if args.ignore_deps? + if args.ignore_dependencies? @deps = [] @reqs = [] else diff --git a/Library/Homebrew/cask/cmd/audit.rb b/Library/Homebrew/cask/cmd/audit.rb index c614241353..2afdd73428 100644 --- a/Library/Homebrew/cask/cmd/audit.rb +++ b/Library/Homebrew/cask/cmd/audit.rb @@ -13,15 +13,15 @@ module Cask def self.parser super do - switch "--download", + switch "--[no-]download", description: "Audit the downloaded file" switch "--[no-]appcast", description: "Audit the appcast" - switch "--token-conflicts", + switch "--[no-]token-conflicts", description: "Audit for token conflicts" - switch "--strict", + switch "--[no-]strict", description: "Run additional, stricter style checks" - switch "--online", + switch "--[no-]online", description: "Run additional, slower style checks that require a network connection" switch "--new-cask", description: "Run various additional style checks to determine if a new cask is eligible " \ diff --git a/Library/Homebrew/cli/args.rb b/Library/Homebrew/cli/args.rb index 2be2b3683b..ba341a36f1 100644 --- a/Library/Homebrew/cli/args.rb +++ b/Library/Homebrew/cli/args.rb @@ -22,6 +22,7 @@ module Homebrew @processed_options = [] @options_only = [] @flags_only = [] + @cask_options = false # Can set these because they will be overwritten by freeze_named_args! # (whereas other values below will only be overwritten if passed). @@ -33,12 +34,13 @@ module Homebrew self[:remaining] = remaining_args.freeze end - def freeze_named_args!(named_args) + def freeze_named_args!(named_args, cask_options:) self[:named] = NamedArgs.new( *named_args.freeze, override_spec: spec(nil), - force_bottle: force_bottle?, + force_bottle: self[:force_bottle?], flags: flags_only, + cask_options: cask_options, parent: self, ) end @@ -64,12 +66,8 @@ module Homebrew named.blank? end - def build_stable? - !HEAD? - end - def build_from_source_formulae - if build_from_source? || HEAD? || build_bottle? + if build_from_source? || self[:HEAD?] || self[:build_bottle?] named.to_formulae_and_casks.select { |f| f.is_a?(Formula) }.map(&:full_name) else [] @@ -129,12 +127,28 @@ module Homebrew end def spec(default = :stable) - if HEAD? + if self[:HEAD?] :head else default end end + + def respond_to_missing?(*) + !frozen? + end + + def method_missing(method_name, *args) + return_value = super + + # Once we are frozen, verify any arg method calls are already defined in the table. + # The default OpenStruct behaviour is to return nil for anything unknown. + if frozen? && args.empty? && !@table.key?(method_name) + raise NoMethodError, "CLI arg for `#{method_name}` is not declared for this command" + end + + return_value + end end end end diff --git a/Library/Homebrew/cli/args.rbi b/Library/Homebrew/cli/args.rbi index 50ece18acd..3ef5dcf0c5 100644 --- a/Library/Homebrew/cli/args.rbi +++ b/Library/Homebrew/cli/args.rbi @@ -3,145 +3,145 @@ module Homebrew module CLI class Args < OpenStruct - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def HEAD?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def include_test?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def build_bottle?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def build_universal?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def build_from_source?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def force_bottle?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def newer_only?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def full_name?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def json?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def debug?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def quiet?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def verbose?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def fetch_HEAD?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def cask?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def dry_run?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def skip_cask_deps?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def greedy?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def force?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def ignore_pinned?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def display_times?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def formula?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def zap?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def ignore_dependencies?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def aliases?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def fix?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def keep_tmp?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def overwrite?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def silent?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def repair?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def prune_prefix?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def upload?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def total?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def dependents?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def installed?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def all?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def full?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def list_pinned?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def display_cop_names?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def syntax?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def ignore_non_pypi_packages?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def test?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def reverse?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def print_only?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def markdown?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def reset_cache?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def major?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def minor?; end sig { returns(T.nilable(String)) } @@ -162,13 +162,13 @@ module Homebrew sig { returns(T.nilable(String)) } def name; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def no_publish?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def shallow?; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def fail_if_not_changed?; end sig { returns(T.nilable(String)) } @@ -230,7 +230,7 @@ module Homebrew sig { returns(T.nilable(T::Array[String])) } def update; end - sig { returns(T.nilable(T::Boolean)) } + sig { returns(T::Boolean) } def s?; end sig { returns(T.nilable(String)) } diff --git a/Library/Homebrew/cli/named_args.rb b/Library/Homebrew/cli/named_args.rb index f8dd68b917..89205c0297 100644 --- a/Library/Homebrew/cli/named_args.rb +++ b/Library/Homebrew/cli/named_args.rb @@ -13,7 +13,7 @@ module Homebrew class NamedArgs < Array extend T::Sig - def initialize(*args, parent: Args.new, override_spec: nil, force_bottle: false, flags: []) + def initialize(*args, parent: Args.new, override_spec: nil, force_bottle: false, flags: [], cask_options: false) require "cask/cask" require "cask/cask_loader" require "formulary" @@ -24,6 +24,7 @@ module Homebrew @override_spec = override_spec @force_bottle = force_bottle @flags = flags + @cask_options = cask_options @parent = parent super(@args) @@ -110,7 +111,8 @@ module Homebrew if only != :formula begin - cask = Cask::CaskLoader.load(name, config: Cask::Config.from_args(@parent)) + config = Cask::Config.from_args(@parent) if @cask_options + cask = Cask::CaskLoader.load(name, config: config) if unreadable_error.present? onoe <<~EOS diff --git a/Library/Homebrew/cli/parser.rb b/Library/Homebrew/cli/parser.rb index 1ebd604576..8e38d33ba3 100644 --- a/Library/Homebrew/cli/parser.rb +++ b/Library/Homebrew/cli/parser.rb @@ -136,6 +136,7 @@ module Homebrew @usage_banner = nil @hide_from_man_page = false @formula_options = false + @cask_options = false self.class.global_options.each do |short, long, desc| switch short, long, description: desc, env: option_to_name(long), method: :on_tail @@ -328,9 +329,10 @@ module Homebrew check_named_args(named_args) end - @args.freeze_named_args!(named_args) + @args.freeze_named_args!(named_args, cask_options: @cask_options) @args.freeze_remaining_args!(non_options.empty? ? remaining : [*remaining, "--", non_options]) @args.freeze_processed_options!(@processed_options) + @args.freeze @args_parsed = true @@ -357,6 +359,7 @@ module Homebrew send(method, *args, **options) conflicts "--formula", args.last end + @cask_options = true end sig { void } @@ -518,7 +521,11 @@ module Homebrew def disable_switch(*names) names.each do |name| - @args.delete_field("#{option_to_name(name)}?") + @args["#{option_to_name(name)}?"] = if name.start_with?("--[no-]") + nil + else + false + end end end @@ -617,6 +624,14 @@ module Homebrew @processed_options.reject! { |existing| existing.second == option.long.first } if option.long.first.present? @processed_options << [option.short.first, option.long.first, option.arg, option.desc.first] + if type == :switch + disable_switch(*args) + else + args.each do |name| + @args[option_to_name(name)] = nil + end + end + return if self.class.global_options.include? [option.short.first, option.long.first, option.desc.first] @non_global_processed_options << [option.long.first || option.short.first, type] diff --git a/Library/Homebrew/cmd/--env.rb b/Library/Homebrew/cmd/--env.rb index d28330029a..f9c12119d9 100644 --- a/Library/Homebrew/cmd/--env.rb +++ b/Library/Homebrew/cmd/--env.rb @@ -34,8 +34,8 @@ module Homebrew def __env args = __env_args.parse - ENV.activate_extensions!(env: args.env) - ENV.deps = args.named.to_formulae if superenv?(args.env) + ENV.activate_extensions! + ENV.deps = args.named.to_formulae if superenv?(nil) ENV.setup_build_environment shell = if args.plain? diff --git a/Library/Homebrew/cmd/install.rb b/Library/Homebrew/cmd/install.rb index b02fca771b..6fee8a103d 100644 --- a/Library/Homebrew/cmd/install.rb +++ b/Library/Homebrew/cmd/install.rb @@ -316,7 +316,19 @@ module Homebrew Cleanup.install_formula_clean!(f) end - Upgrade.check_installed_dependents(installed_formulae, args: args) + Upgrade.check_installed_dependents( + installed_formulae, + flags: args.flags_only, + installed_on_request: args.named.present?, + force_bottle: args.force_bottle?, + build_from_source_formulae: args.build_from_source_formulae, + interactive: args.interactive?, + keep_tmp: args.keep_tmp?, + force: args.force?, + debug: args.debug?, + quiet: args.quiet?, + verbose: args.verbose?, + ) Homebrew.messages.display_messages(display_times: args.display_times?) rescue FormulaUnreadableError, FormulaClassUnavailableError, diff --git a/Library/Homebrew/cmd/outdated.rb b/Library/Homebrew/cmd/outdated.rb index a7a6da48f1..2b55ae852c 100644 --- a/Library/Homebrew/cmd/outdated.rb +++ b/Library/Homebrew/cmd/outdated.rb @@ -171,7 +171,7 @@ module Homebrew if args.named.present? select_outdated(args.named.to_casks, args: args) else - select_outdated(Cask::Caskroom.casks(config: Cask::Config.from_args(args)), args: args) + select_outdated(Cask::Caskroom.casks, args: args) end end @@ -180,7 +180,7 @@ module Homebrew if formulae.blank? && casks.blank? formulae = Formula.installed - casks = Cask::Caskroom.casks(config: Cask::Config.from_args(args)) + casks = Cask::Caskroom.casks end [select_outdated(formulae, args: args).sort, select_outdated(casks, args: args)] diff --git a/Library/Homebrew/cmd/reinstall.rb b/Library/Homebrew/cmd/reinstall.rb index 86657a3002..ef3b9b5276 100644 --- a/Library/Homebrew/cmd/reinstall.rb +++ b/Library/Homebrew/cmd/reinstall.rb @@ -88,17 +88,41 @@ module Homebrew formulae, casks = args.named.to_formulae_and_casks(method: :resolve) .partition { |o| o.is_a?(Formula) } - formulae.each do |f| - if f.pinned? - onoe "#{f.full_name} is pinned. You must unpin it to reinstall." + formulae.each do |formula| + if formula.pinned? + onoe "#{formula.full_name} is pinned. You must unpin it to reinstall." next end - Migrator.migrate_if_needed(f, force: args.force?) - reinstall_formula(f, args: args) - Cleanup.install_formula_clean!(f) + Migrator.migrate_if_needed(formula, force: args.force?) + reinstall_formula( + formula, + flags: args.flags_only, + installed_on_request: args.named.present?, + force_bottle: args.force_bottle?, + build_from_source_formulae: args.build_from_source_formulae, + interactive: args.interactive?, + keep_tmp: args.keep_tmp?, + force: args.force?, + debug: args.debug?, + quiet: args.quiet?, + verbose: args.verbose?, + ) + Cleanup.install_formula_clean!(formula) end - Upgrade.check_installed_dependents(formulae, args: args) + Upgrade.check_installed_dependents( + formulae, + flags: args.flags_only, + installed_on_request: args.named.present?, + force_bottle: args.force_bottle?, + build_from_source_formulae: args.build_from_source_formulae, + interactive: args.interactive?, + keep_tmp: args.keep_tmp?, + force: args.force?, + debug: args.debug?, + quiet: args.quiet?, + verbose: args.verbose?, + ) if casks.any? Cask::Cmd::Reinstall.reinstall_casks( diff --git a/Library/Homebrew/cmd/upgrade.rb b/Library/Homebrew/cmd/upgrade.rb index 56155ffa53..ba111c25b5 100644 --- a/Library/Homebrew/cmd/upgrade.rb +++ b/Library/Homebrew/cmd/upgrade.rb @@ -172,9 +172,34 @@ module Homebrew puts formulae_upgrades.join("\n") end - Upgrade.upgrade_formulae(formulae_to_install, args: args) + Upgrade.upgrade_formulae( + formulae_to_install, + flags: args.flags_only, + installed_on_request: args.named.present?, + force_bottle: args.force_bottle?, + build_from_source_formulae: args.build_from_source_formulae, + interactive: args.interactive?, + keep_tmp: args.keep_tmp?, + force: args.force?, + debug: args.debug?, + quiet: args.quiet?, + verbose: args.verbose?, + ) - Upgrade.check_installed_dependents(formulae_to_install, args: args) + Upgrade.check_installed_dependents( + formulae_to_install, + flags: args.flags_only, + dry_run: args.dry_run?, + installed_on_request: args.named.present?, + force_bottle: args.force_bottle?, + build_from_source_formulae: args.build_from_source_formulae, + interactive: args.interactive?, + keep_tmp: args.keep_tmp?, + force: args.force?, + debug: args.debug?, + quiet: args.quiet?, + verbose: args.verbose?, + ) true end diff --git a/Library/Homebrew/dev-cmd/audit.rb b/Library/Homebrew/dev-cmd/audit.rb index 0a04c826f0..91c56b000f 100644 --- a/Library/Homebrew/dev-cmd/audit.rb +++ b/Library/Homebrew/dev-cmd/audit.rb @@ -179,7 +179,6 @@ module Homebrew tap_audit_exceptions: f.tap&.audit_exceptions, style_offenses: style_offenses ? style_offenses.for_path(f.path) : nil, display_cop_names: args.display_cop_names?, - build_stable: args.build_stable?, }.compact fa = FormulaAuditor.new(f, **options) diff --git a/Library/Homebrew/dev-cmd/test.rb b/Library/Homebrew/dev-cmd/test.rb index ccf5922536..629ada5ecd 100644 --- a/Library/Homebrew/dev-cmd/test.rb +++ b/Library/Homebrew/dev-cmd/test.rb @@ -21,6 +21,8 @@ module Homebrew *Example:* `brew install jruby && brew test jruby` EOS + switch "-f", "--force", + description: "Test formulae even if they are unlinked." switch "--HEAD", description: "Test the head version of a formula." switch "--keep-tmp", diff --git a/Library/Homebrew/formula_auditor.rb b/Library/Homebrew/formula_auditor.rb index f044ec817f..1b9e1405ad 100644 --- a/Library/Homebrew/formula_auditor.rb +++ b/Library/Homebrew/formula_auditor.rb @@ -21,7 +21,6 @@ module Homebrew @new_formula = options[:new_formula] && !@versioned_formula @strict = options[:strict] @online = options[:online] - @build_stable = options[:build_stable] @git = options[:git] @display_cop_names = options[:display_cop_names] @only = options[:only] diff --git a/Library/Homebrew/reinstall.rb b/Library/Homebrew/reinstall.rb index f75867ed7e..ae9a5e0f10 100644 --- a/Library/Homebrew/reinstall.rb +++ b/Library/Homebrew/reinstall.rb @@ -8,55 +8,61 @@ require "messages" module Homebrew module_function - def reinstall_formula(f, args:, build_from_source: false) - return if args.dry_run? - - if f.opt_prefix.directory? - keg = Keg.new(f.opt_prefix.resolved_path) + def reinstall_formula( + formula, + flags:, + installed_on_request: false, + force_bottle: false, + build_from_source_formulae: [], + interactive: false, + keep_tmp: false, + force: false, + debug: false, + quiet: false, + verbose: false + ) + if formula.opt_prefix.directory? + keg = Keg.new(formula.opt_prefix.resolved_path) tab = Tab.for_keg(keg) keg_had_linked_opt = true keg_was_linked = keg.linked? backup keg end - build_options = BuildOptions.new(Options.create(args.flags_only), f.options) + build_options = BuildOptions.new(Options.create(flags), formula.options) options = build_options.used_options - options |= f.build.used_options - options &= f.options - - build_from_source_formulae = args.build_from_source_formulae - build_from_source_formulae << f.full_name if build_from_source + options |= formula.build.used_options + options &= formula.options fi = FormulaInstaller.new( - f, + formula, **{ options: options, link_keg: keg_had_linked_opt ? keg_was_linked : nil, installed_as_dependency: tab&.installed_as_dependency, - installed_on_request: tab&.installed_on_request, - build_bottle: args.build_bottle? || tab&.built_bottle?, - force_bottle: args.force_bottle?, + installed_on_request: installed_on_request || tab&.installed_on_request, + build_bottle: tab&.built_bottle?, + force_bottle: force_bottle, build_from_source_formulae: build_from_source_formulae, - git: args.git?, - interactive: args.interactive?, - keep_tmp: args.keep_tmp?, - force: args.force?, - debug: args.debug?, - quiet: args.quiet?, - verbose: args.verbose?, + interactive: interactive, + keep_tmp: keep_tmp, + force: force, + debug: debug, + quiet: quiet, + verbose: verbose, }.compact, ) fi.prelude fi.fetch - oh1 "Reinstalling #{Formatter.identifier(f.full_name)} #{options.to_a.join " "}" + oh1 "Reinstalling #{Formatter.identifier(formula.full_name)} #{options.to_a.join " "}" fi.install fi.finish rescue FormulaInstallationAlreadyAttemptedError nil rescue Exception # rubocop:disable Lint/RescueException - ignore_interrupts { restore_backup(keg, keg_was_linked, verbose: args.verbose?) } + ignore_interrupts { restore_backup(keg, keg_was_linked, verbose: verbose) } raise else begin diff --git a/Library/Homebrew/test/cask/cmd/audit_spec.rb b/Library/Homebrew/test/cask/cmd/audit_spec.rb index 8faae5b8fc..2f7bd3d68e 100644 --- a/Library/Homebrew/test/cask/cmd/audit_spec.rb +++ b/Library/Homebrew/test/cask/cmd/audit_spec.rb @@ -21,7 +21,7 @@ describe Cask::Cmd::Audit, :cask do expect(Cask::CaskLoader).to receive(:load).with(cask_token, any_args).and_return(cask) expect(Cask::Auditor).to receive(:audit) - .with(cask, quarantine: true, any_named_args: true) + .with(cask, audit_new_cask: false, quarantine: true, any_named_args: true) .and_return(result) described_class.run(cask_token) @@ -31,7 +31,7 @@ describe Cask::Cmd::Audit, :cask do it "does not pass anything if no flags are specified" do allow(Cask::CaskLoader).to receive(:load).and_return(cask) expect(Cask::Auditor).to receive(:audit) - .with(cask, quarantine: true, any_named_args: true) + .with(cask, audit_new_cask: false, quarantine: true, any_named_args: true) .and_return(result) described_class.run("casktoken") @@ -40,7 +40,7 @@ describe Cask::Cmd::Audit, :cask do it "passes `audit_download` if the `--download` flag is specified" do allow(Cask::CaskLoader).to receive(:load).and_return(cask) expect(Cask::Auditor).to receive(:audit) - .with(cask, audit_download: true, quarantine: true, any_named_args: true) + .with(cask, audit_download: true, audit_new_cask: false, quarantine: true, any_named_args: true) .and_return(result) described_class.run("casktoken", "--download") @@ -49,7 +49,7 @@ describe Cask::Cmd::Audit, :cask do it "passes `audit_token_conflicts` if the `--token-conflicts` flag is specified" do allow(Cask::CaskLoader).to receive(:load).and_return(cask) expect(Cask::Auditor).to receive(:audit) - .with(cask, audit_token_conflicts: true, quarantine: true, any_named_args: true) + .with(cask, audit_token_conflicts: true, audit_new_cask: false, quarantine: true, any_named_args: true) .and_return(result) described_class.run("casktoken", "--token-conflicts") @@ -58,7 +58,7 @@ describe Cask::Cmd::Audit, :cask do it "passes `audit_strict` if the `--strict` flag is specified" do allow(Cask::CaskLoader).to receive(:load).and_return(cask) expect(Cask::Auditor).to receive(:audit) - .with(cask, audit_strict: true, quarantine: true, any_named_args: true) + .with(cask, audit_strict: true, audit_new_cask: false, quarantine: true, any_named_args: true) .and_return(result) described_class.run("casktoken", "--strict") @@ -67,7 +67,7 @@ describe Cask::Cmd::Audit, :cask do it "passes `audit_online` if the `--online` flag is specified" do allow(Cask::CaskLoader).to receive(:load).and_return(cask) expect(Cask::Auditor).to receive(:audit) - .with(cask, audit_online: true, quarantine: true, any_named_args: true) + .with(cask, audit_online: true, audit_new_cask: false, quarantine: true, any_named_args: true) .and_return(result) described_class.run("casktoken", "--online") @@ -85,7 +85,7 @@ describe Cask::Cmd::Audit, :cask do it "passes `language` if the `--language` flag is specified" do allow(Cask::CaskLoader).to receive(:load).and_return(cask) expect(Cask::Auditor).to receive(:audit) - .with(cask, quarantine: true, language: ["de-AT"], any_named_args: true) + .with(cask, audit_new_cask: false, quarantine: true, language: ["de-AT"], any_named_args: true) .and_return(result) described_class.run("casktoken", "--language=de-AT") @@ -94,7 +94,7 @@ describe Cask::Cmd::Audit, :cask do it "passes `quarantine` if the `--no-quarantine` flag is specified" do allow(Cask::CaskLoader).to receive(:load).and_return(cask) expect(Cask::Auditor).to receive(:audit) - .with(cask, quarantine: false, any_named_args: true) + .with(cask, audit_new_cask: false, quarantine: false, any_named_args: true) .and_return(result) described_class.run("casktoken", "--no-quarantine") @@ -105,7 +105,7 @@ describe Cask::Cmd::Audit, :cask do allow(Cask::CaskLoader).to receive(:load).and_return(cask) expect(Cask::Auditor).to receive(:audit) - .with(cask, quarantine: false, any_named_args: true) + .with(cask, audit_new_cask: false, quarantine: false, any_named_args: true) .and_return(result) described_class.run("casktoken") diff --git a/Library/Homebrew/test/cli/named_args_spec.rb b/Library/Homebrew/test/cli/named_args_spec.rb index 120d8e36f4..463c93986b 100644 --- a/Library/Homebrew/test/cli/named_args_spec.rb +++ b/Library/Homebrew/test/cli/named_args_spec.rb @@ -11,6 +11,7 @@ end def setup_unredable_cask(name) error = Cask::CaskUnreadableError.new(name, "testing") allow(Cask::CaskLoader).to receive(:load).with(name).and_raise(error) + allow(Cask::CaskLoader).to receive(:load).with(name, config: nil).and_raise(error) config = instance_double(Cask::Config) allow(Cask::Config).to receive(:from_args).and_return(config) diff --git a/Library/Homebrew/test/cli/parser_spec.rb b/Library/Homebrew/test/cli/parser_spec.rb index 042c843dc6..0310e3e469 100644 --- a/Library/Homebrew/test/cli/parser_spec.rb +++ b/Library/Homebrew/test/cli/parser_spec.rb @@ -23,6 +23,11 @@ describe Homebrew::CLI::Parser do end } + it "does not create no_positive?" do + args = parser.parse(["--no-positive"]) + expect { args.no_positive? }.to raise_error(NoMethodError) + end + it "sets the positive name to false if the negative flag is passed" do args = parser.parse(["--no-positive"]) expect(args).not_to be_positive @@ -32,6 +37,11 @@ describe Homebrew::CLI::Parser do args = parser.parse(["--positive"]) expect(args).to be_positive end + + it "does not set the positive name if the positive flag is not passed" do + args = parser.parse([]) + expect(args.positive?).to be nil + end end context "when using negative options" do @@ -43,7 +53,7 @@ describe Homebrew::CLI::Parser do it "does not set the positive name" do args = parser.parse(["--no-positive"]) - expect(args.positive?).to be nil + expect { args.positive? }.to raise_error(NoMethodError) end it "fails when using the positive name" do @@ -82,10 +92,10 @@ describe Homebrew::CLI::Parser do expect(args.named).to eq %w[unnamed args] end - it "parses a single option and checks other options to be nil" do + it "parses a single option and checks other options to be false" do args = parser.parse(["--verbose"]) expect(args).to be_verbose - expect(args.more_verbose?).to be nil + expect(args.more_verbose?).to be false end it "raises an exception and outputs help text when an invalid option is passed" do @@ -227,7 +237,7 @@ describe Homebrew::CLI::Parser do allow(Homebrew::EnvConfig).to receive(:switch_a?).and_return(true) allow(Homebrew::EnvConfig).to receive(:switch_b?).and_return(false) args = parser.parse(["--switch-b"]) - expect(args.switch_a).to be_falsy + expect(args.switch_a?).to be false expect(args).to be_switch_b end diff --git a/Library/Homebrew/upgrade.rb b/Library/Homebrew/upgrade.rb index c74cff3ce4..f4f83df009 100644 --- a/Library/Homebrew/upgrade.rb +++ b/Library/Homebrew/upgrade.rb @@ -14,9 +14,20 @@ module Homebrew module Upgrade module_function - def upgrade_formulae(formulae_to_install, args:) + def upgrade_formulae( + formulae_to_install, + flags:, + installed_on_request: false, + force_bottle: false, + build_from_source_formulae: [], + interactive: false, + keep_tmp: false, + force: false, + debug: false, + quiet: false, + verbose: false + ) return if formulae_to_install.empty? - return if args.dry_run? # Sort keg-only before non-keg-only formulae to avoid any needless conflicts # with outdated, non-keg-only versions of formulae being upgraded. @@ -30,66 +41,90 @@ module Homebrew end end - formulae_to_install.each do |f| - Migrator.migrate_if_needed(f, force: args.force?) + formulae_to_install.each do |formula| + Migrator.migrate_if_needed(formula, force: force) begin - upgrade_formula(f, args: args) - Cleanup.install_formula_clean!(f) + upgrade_formula( + formula, + flags: flags, + installed_on_request: installed_on_request, + force_bottle: force_bottle, + build_from_source_formulae: build_from_source_formulae, + interactive: interactive, + keep_tmp: keep_tmp, + force: force, + debug: debug, + quiet: quiet, + verbose: verbose, + ) + Cleanup.install_formula_clean!(formula) rescue UnsatisfiedRequirements => e - ofail "#{f}: #{e}" + ofail "#{formula}: #{e}" end end end - def upgrade_formula(f, args:) - return if args.dry_run? - - if f.opt_prefix.directory? - keg = Keg.new(f.opt_prefix.resolved_path) + def upgrade_formula( + formula, + flags:, + installed_on_request: false, + force_bottle: false, + build_from_source_formulae: [], + interactive: false, + keep_tmp: false, + force: false, + debug: false, + quiet: false, + verbose: false + ) + if formula.opt_prefix.directory? + keg = Keg.new(formula.opt_prefix.resolved_path) keg_had_linked_opt = true keg_was_linked = keg.linked? end - formulae_maybe_with_kegs = [f] + f.old_installed_formulae + formulae_maybe_with_kegs = [formula] + formula.old_installed_formulae outdated_kegs = formulae_maybe_with_kegs.map(&:linked_keg) .select(&:directory?) .map { |k| Keg.new(k.resolved_path) } linked_kegs = outdated_kegs.select(&:linked?) - if f.opt_prefix.directory? - keg = Keg.new(f.opt_prefix.resolved_path) + if formula.opt_prefix.directory? + keg = Keg.new(formula.opt_prefix.resolved_path) tab = Tab.for_keg(keg) end - build_options = BuildOptions.new(Options.create(args.flags_only), f.options) + build_options = BuildOptions.new(Options.create(flags), formula.options) options = build_options.used_options - options |= f.build.used_options - options &= f.options + options |= formula.build.used_options + options &= formula.options fi = FormulaInstaller.new( - f, + formula, **{ options: options, link_keg: keg_had_linked_opt ? keg_was_linked : nil, installed_as_dependency: tab&.installed_as_dependency, - installed_on_request: args.named.present? || tab&.installed_on_request, - build_bottle: args.build_bottle? || tab&.built_bottle?, - force_bottle: args.force_bottle?, - build_from_source_formulae: args.build_from_source_formulae, - keep_tmp: args.keep_tmp?, - force: args.force?, - debug: args.debug?, - quiet: args.quiet?, - verbose: args.verbose?, + installed_on_request: installed_on_request || tab&.installed_on_request, + build_bottle: tab&.built_bottle?, + force_bottle: force_bottle, + build_from_source_formulae: build_from_source_formulae, + interactive: interactive, + keep_tmp: keep_tmp, + force: force, + debug: debug, + quiet: quiet, + verbose: verbose, }.compact, ) - upgrade_version = if f.optlinked? - "#{Keg.new(f.opt_prefix).version} -> #{f.pkg_version}" + upgrade_version = if formula.optlinked? + "#{Keg.new(formula.opt_prefix).version} -> #{formula.pkg_version}" else - "-> #{f.pkg_version}" + "-> #{formula.pkg_version}" end - oh1 "Upgrading #{Formatter.identifier(f.full_specified_name)} #{upgrade_version} #{fi.options.to_a.join(" ")}" + oh1 "Upgrading #{Formatter.identifier(formula.full_specified_name)} " \ + "#{upgrade_version} #{fi.options.to_a.join(" ")}" fi.prelude fi.fetch @@ -108,13 +143,13 @@ module Homebrew rescue CannotInstallFormulaError, DownloadError => e ofail e rescue BuildError => e - e.dump(verbose: args.verbose?) + e.dump(verbose: verbose) puts Homebrew.failed = true ensure # restore previous installation state if build failed begin - linked_kegs.each(&:link) unless f.latest_version_installed? + linked_kegs.each(&:link) unless formula.latest_version_installed? rescue nil end @@ -136,10 +171,23 @@ module Homebrew end end - def check_installed_dependents(formulae, args:) + def check_installed_dependents( + formulae, + flags:, + dry_run: false, + installed_on_request: false, + force_bottle: false, + build_from_source_formulae: [], + interactive: false, + keep_tmp: false, + force: false, + debug: false, + quiet: false, + verbose: false + ) return if Homebrew::EnvConfig.no_installed_dependents_check? - installed_formulae = args.dry_run? ? formulae : FormulaInstaller.installed.to_a + installed_formulae = dry_run ? formulae : FormulaInstaller.installed.to_a return if installed_formulae.empty? already_broken_dependents = check_broken_dependents(installed_formulae) @@ -150,7 +198,7 @@ module Homebrew .select(&:outdated?) return if outdated_dependents.blank? && already_broken_dependents.blank? - outdated_dependents -= installed_formulae if args.dry_run? + outdated_dependents -= installed_formulae if dry_run upgradeable_dependents = outdated_dependents.reject(&:pinned?) @@ -169,10 +217,10 @@ module Homebrew # Print the upgradable dependents. if upgradeable_dependents.blank? - ohai "No outdated dependents to upgrade!" unless args.dry_run? + ohai "No outdated dependents to upgrade!" unless dry_run else plural = "dependent".pluralize(upgradeable_dependents.count) - verb = args.dry_run? ? "Would upgrade" : "Upgrading" + verb = dry_run ? "Would upgrade" : "Upgrading" ohai "#{verb} #{upgradeable_dependents.count} #{plural}:" formulae_upgrades = upgradeable_dependents.map do |f| name = f.full_specified_name @@ -185,16 +233,30 @@ module Homebrew puts formulae_upgrades.join(", ") end - upgrade_formulae(upgradeable_dependents, args: args) + unless dry_run + upgrade_formulae( + upgradeable_dependents, + flags: flags, + installed_on_request: installed_on_request, + force_bottle: force_bottle, + build_from_source_formulae: build_from_source_formulae, + interactive: interactive, + keep_tmp: keep_tmp, + force: force, + debug: debug, + quiet: quiet, + verbose: verbose, + ) + end # Update installed formulae after upgrading installed_formulae = FormulaInstaller.installed.to_a # Assess the dependents tree again now we've upgraded. - oh1 "Checking for dependents of upgraded formulae..." unless args.dry_run? + oh1 "Checking for dependents of upgraded formulae..." unless dry_run broken_dependents = check_broken_dependents(installed_formulae) if broken_dependents.blank? - if args.dry_run? + if dry_run ohai "No currently broken dependents found!" opoo "If they are broken by the upgrade they will also be upgraded or reinstalled." else @@ -233,10 +295,21 @@ module Homebrew .join(", ") end - return if args.dry_run? + return if dry_run - reinstallable_broken_dependents.each do |f| - Homebrew.reinstall_formula(f, build_from_source: true, args: args) + reinstallable_broken_dependents.each do |formula| + Homebrew.reinstall_formula( + formula, + flags: flags, + force_bottle: force_bottle, + build_from_source_formulae: build_from_source_formulae + [formula], + interactive: interactive, + keep_tmp: keep_tmp, + force: force, + debug: debug, + quiet: quiet, + verbose: verbose, + ) rescue FormulaInstallationAlreadyAttemptedError # We already attempted to reinstall f as part of the dependency tree of # another formula. In that case, don't generate an error, just move on. @@ -244,7 +317,7 @@ module Homebrew rescue CannotInstallFormulaError, DownloadError => e ofail e rescue BuildError => e - e.dump(verbose: args.verbose?) + e.dump(verbose: verbose) puts Homebrew.failed = true end