diff --git a/Library/Homebrew/cli_parser.rb b/Library/Homebrew/cli_parser.rb index 160071f87f..f35328a574 100644 --- a/Library/Homebrew/cli_parser.rb +++ b/Library/Homebrew/cli_parser.rb @@ -27,6 +27,7 @@ module Homebrew Homebrew.args.instance_eval { undef tap } @constraints = [] @conflicts = [] + @switch_sources = {} @processed_options = [] @desc_line_length = 43 @hide_from_man_page = false @@ -51,14 +52,14 @@ module Homebrew end process_option(*names, description) @parser.on(*names, *wrap_option_desc(description)) do - enable_switch(*names) + enable_switch(*names, from: :args) end names.each do |name| set_constraints(name, required_for: required_for, depends_on: depends_on) end - enable_switch(*names) if !env.nil? && !ENV["HOMEBREW_#{env.to_s.upcase}"].nil? + enable_switch(*names, from: :env) if !env.nil? && !ENV["HOMEBREW_#{env.to_s.upcase}"].nil? end alias switch_option switch @@ -172,12 +173,19 @@ module Homebrew private - def enable_switch(*names) + def enable_switch(*names, from:) names.each do |name| + @switch_sources[option_to_name(name)] = from Homebrew.args["#{option_to_name(name)}?"] = true end end + def disable_switch(*names) + names.each do |name| + Homebrew.args.delete_field("#{option_to_name(name)}?") + end + end + # These are common/global switches accessible throughout Homebrew def common_switch(name) Homebrew::CLI::Parser.global_options.fetch(name, name) @@ -225,7 +233,13 @@ module Homebrew next if violations.count < 2 - raise OptionConflictError, violations.map(&method(:name_to_option)) + env_var_options = violations.select do |option| + @switch_sources[option_to_name(option)] == :env_var + end + + select_cli_arg = violations.count - env_var_options.count == 1 + raise OptionConflictError, violations.map(&method(:name_to_option)) unless select_cli_arg + env_var_options.each(&method(:disable_switch)) end end diff --git a/Library/Homebrew/test/cli_parser_spec.rb b/Library/Homebrew/test/cli_parser_spec.rb index 975f5ed86b..c5c1f25764 100644 --- a/Library/Homebrew/test/cli_parser_spec.rb +++ b/Library/Homebrew/test/cli_parser_spec.rb @@ -145,8 +145,8 @@ describe Homebrew::CLI::Parser do describe "test constraints for switch options" do subject(:parser) { described_class.new do - switch "-a", "--switch-a" - switch "-b", "--switch-b" + switch "-a", "--switch-a", env: "switch_a" + switch "-b", "--switch-b", env: "switch_b" switch "--switch-c", required_for: "--switch-a" switch "--switch-d", depends_on: "--switch-b" @@ -177,6 +177,22 @@ describe Homebrew::CLI::Parser do parser.parse(["--switch-b"]) expect(Homebrew.args.switch_b?).to be true end + + it "prioritizes cli arguments over env vars when they conflict" do + allow(ENV).to receive(:[]).with("HOMEBREW_SWITCH_A").and_return("1") + allow(ENV).to receive(:[]).with("HOMEBREW_SWITCH_B").and_return("0") + allow(ENV).to receive(:[]) + parser.parse(["--switch-b"]) + expect(Homebrew.args.switch_a).to be_falsy + expect(Homebrew.args).to be_switch_b + end + + it "raises an exception on constraint violation when both are env vars" do + allow(ENV).to receive(:[]).with("HOMEBREW_SWITCH_A").and_return("1") + allow(ENV).to receive(:[]).with("HOMEBREW_SWITCH_B").and_return("1") + allow(ENV).to receive(:[]) + expect { parser.parse(["--switch-a", "--switch-b"]) }.to raise_error(Homebrew::CLI::OptionConflictError) + end end describe "test immutability of args" do