From f8d84249aaf748ba4e5699b2327623b69083737d Mon Sep 17 00:00:00 2001 From: Ben Muschol Date: Fri, 15 Feb 2019 00:31:13 -0500 Subject: [PATCH] Prioritize CLI arguments over env vars when they conflict --- Library/Homebrew/cli_parser.rb | 24 +++++++++++++++++++++--- Library/Homebrew/test/cli_parser_spec.rb | 19 +++++++++++++++++-- 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/Library/Homebrew/cli_parser.rb b/Library/Homebrew/cli_parser.rb index 160071f87f..d18811ef94 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 @@ -58,7 +59,7 @@ module Homebrew 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, source: :env_var) 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, source: :cli_arg) names.each do |name| + @switch_sources[option_to_name(name)] = source 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,17 @@ 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 + + if violations.count - env_var_options.count == 1 + env_var_options.each do |option| + disable_switch option + end + else + raise OptionConflictError, violations.map(&method(:name_to_option)) + end end end diff --git a/Library/Homebrew/test/cli_parser_spec.rb b/Library/Homebrew/test/cli_parser_spec.rb index 975f5ed86b..caaa71d78f 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,21 @@ 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") + parser.parse(["--switch-b"]) + expect(Homebrew.args.switch_a?).to be_falsy + expect(Homebrew.args.switch_b?).to be true + 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