diff options
author | Samuel Giddins <segiddins@segiddins.me> | 2017-07-11 19:47:44 -0500 |
---|---|---|
committer | Samuel Giddins <segiddins@segiddins.me> | 2017-07-23 12:39:01 -0500 |
commit | 1223f932c0790687d1743e5449e54b67f60f4c45 (patch) | |
tree | 1933cf97689c02fb322c9fb8d430928bf09fc997 | |
parent | a0b16613040382b5db2c3c14b4fdf04f9091e641 (diff) | |
download | bundler-1223f932c0790687d1743e5449e54b67f60f4c45.tar.gz |
Introduce the notion of settings validation
-rw-r--r-- | lib/bundler/cli/doctor.rb | 1 | ||||
-rw-r--r-- | lib/bundler/cli/install.rb | 13 | ||||
-rw-r--r-- | lib/bundler/settings.rb | 41 | ||||
-rw-r--r-- | lib/bundler/settings/validator.rb | 79 | ||||
-rw-r--r-- | spec/install/gemfile/groups_spec.rb | 6 |
5 files changed, 118 insertions, 22 deletions
diff --git a/lib/bundler/cli/doctor.rb b/lib/bundler/cli/doctor.rb index ae27983240..7f28a5eb13 100644 --- a/lib/bundler/cli/doctor.rb +++ b/lib/bundler/cli/doctor.rb @@ -62,6 +62,7 @@ module Bundler def run Bundler.ui.level = "error" if options[:quiet] + Bundler.settings.validate! check! definition = Bundler.definition diff --git a/lib/bundler/cli/install.rb b/lib/bundler/cli/install.rb index cc41cce7a6..fed2cf0b22 100644 --- a/lib/bundler/cli/install.rb +++ b/lib/bundler/cli/install.rb @@ -154,8 +154,8 @@ module Bundler check_for_group_conflicts_in_cli_options - Bundler.settings.set_command_option_if_given :with, options[:with] - Bundler.settings.set_command_option_if_given :without, options[:without] + Bundler.settings.set_command_option :with, nil if options[:with] == [] + Bundler.settings.set_command_option :without, nil if options[:without] == [] with = options.fetch(:with, []) with |= Bundler.settings[:with].map(&:to_s) @@ -189,8 +189,13 @@ module Bundler Bundler.settings.set_command_option_if_given :clean, options["clean"] - Bundler.settings.set_command_option :without, options[:without] unless Bundler.settings[:without] == options[:without] - Bundler.settings.set_command_option :with, options[:with] unless Bundler.settings[:with] == options[:with] + unless Bundler.settings[:without] == options[:without] && Bundler.settings[:with] == options[:with] + # need to nil them out first to get around validation for backwards compatibility + Bundler.settings.set_command_option :without, nil + Bundler.settings.set_command_option :with, nil + Bundler.settings.set_command_option :without, options[:without] - options[:with] + Bundler.settings.set_command_option :with, options[:with] + end disable_shared_gems = Bundler.settings[:path] ? true : nil Bundler.settings.set_command_option :disable_shared_gems, disable_shared_gems unless Bundler.settings[:disable_shared_gems] == disable_shared_gems diff --git a/lib/bundler/settings.rb b/lib/bundler/settings.rb index fa1483f804..9cc80a0f99 100644 --- a/lib/bundler/settings.rb +++ b/lib/bundler/settings.rb @@ -6,6 +6,7 @@ module Bundler class Settings autoload :Mirror, "bundler/mirror" autoload :Mirrors, "bundler/mirror" + autoload :Validator, "bundler/settings/validator" BOOL_KEYS = %w[ allow_bundler_dependency_conflicts @@ -230,6 +231,15 @@ module Bundler @app_cache_path ||= self[:cache_path] || "vendor/cache" end + def validate! + all.each do |raw_key| + [@local_config, ENV, @global_config].each do |settings| + value = converted_value(settings[key_for(raw_key)], raw_key) + Validator.validate(raw_key, value, settings.to_h.dup) + end + end + end + private def key_for(key) @@ -282,24 +292,25 @@ module Bundler array.join(":").tr(" ", ":") end - def set_key(key, value, hash, file) - value = array_to_s(value) if is_array(key) + def set_key(raw_key, value, hash, file) + raw_key = raw_key.to_s + value = array_to_s(value) if is_array(raw_key) - key = key_for(key) + key = key_for(raw_key) - unless hash[key] == value - hash[key] = value - hash.delete(key) if value.nil? - if file - SharedHelpers.filesystem_access(file) do |p| - FileUtils.mkdir_p(p.dirname) - require "bundler/yaml_serializer" - p.open("w") {|f| f.write(YAMLSerializer.dump(hash)) } - end - end - end + return if hash[key] == value + + hash[key] = value + hash.delete(key) if value.nil? + + Validator.validate(raw_key, converted_value(value, raw_key), hash) - value + return unless file + SharedHelpers.filesystem_access(file) do |p| + FileUtils.mkdir_p(p.dirname) + require "bundler/yaml_serializer" + p.open("w") {|f| f.write(YAMLSerializer.dump(hash)) } + end end def converted_value(value, key) diff --git a/lib/bundler/settings/validator.rb b/lib/bundler/settings/validator.rb new file mode 100644 index 0000000000..263e6d9325 --- /dev/null +++ b/lib/bundler/settings/validator.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +module Bundler + class Settings + class Validator + class Rule + attr_reader :description + + def initialize(keys, description, &validate) + @keys = keys + @description = description + @validate = validate + end + + def validate(key, value, settings) + instance_exec(key, value, settings, &@validate) + end + + def fail!(key, value, *reasons) + reasons.unshift @description + raise InvalidOption, "Setting `#{key}` to #{value.inspect} failed:\n#{reasons.map {|r| " - #{r}" }.join("\n")}" + end + + def set(settings, key, value, *reasons) + hash_key = k(key) + return if settings[hash_key] == value + reasons.unshift @description + Bundler.ui.info "Setting `#{key}` to #{value.inspect}, since #{reasons.join(", ")}" + if value.nil? + settings.delete(hash_key) + else + settings[hash_key] = value + end + end + + def k(key) + Bundler.settings.send(:key_for, key) + end + end + + def self.rules + @rules ||= Hash.new {|h, k| h[k] = [] } + end + private_class_method :rules + + def self.rule(keys, description, &blk) + rule = Rule.new(keys, description, &blk) + keys.each {|k| rules[k] << rule } + end + private_class_method :rule + + def self.validate(key, value, settings) + rules_to_validate = rules[key] + rules_to_validate.each {|rule| rule.validate(key, value, settings) } + end + + rule %w[path disable_shared_gems], "path and disable_shared_gems are mutually exclusive" do |key, value, settings| + if key == "path" && value + set(settings, :disabled_shared_gems, nil) + elsif key == "disable_shared_gems" && value + set(settings, :path, nil) + end + end + + rule %w[with without], "a group cannot be in both `with` & `without` simultaneously" do |key, value, settings| + with = settings.fetch(k(:with), "").split(":").map(&:to_sym) + without = settings.fetch(k(:without), "").split(":").map(&:to_sym) + + other_key = key == "with" ? :without : :with + other_setting = key == "with" ? without : with + + conflicting = with & without + if conflicting.any? + fail!(key, value, "`#{other_key}` is current set to #{other_setting}", "the `#{conflicting.join("`, `")}` groups conflict") + end + end + end + end +end diff --git a/spec/install/gemfile/groups_spec.rb b/spec/install/gemfile/groups_spec.rb index dc55f1d8d3..19c379e188 100644 --- a/spec/install/gemfile/groups_spec.rb +++ b/spec/install/gemfile/groups_spec.rb @@ -192,7 +192,7 @@ RSpec.describe "bundle install with groups" do expect(the_bundle).not_to include_gems "thin 1.0" end - it "does remove groups from without when passed at with" do + it "does remove groups from without when passed at --with", :bundler => "< 2" do bundle :install, forgotten_command_line_options(:without => "emo") bundle :install, forgotten_command_line_options(:with => "emo") expect(the_bundle).to include_gems "activesupport 2.3.5" @@ -211,12 +211,12 @@ RSpec.describe "bundle install with groups" do end it "allows the BUNDLE_WITH setting to override BUNDLE_WITHOUT" do - bundle! "config --local with debugging" + ENV["BUNDLE_WITH"] = "debugging" bundle! :install expect(the_bundle).to include_gem "thin 1.0" - bundle! "config --local without debugging" + ENV["BUNDLE_WITHOUT"] = "debugging" expect(the_bundle).to include_gem "thin 1.0" bundle! :install |