From a2c547902d8f8560b2b1e0d839b00776f09026f9 Mon Sep 17 00:00:00 2001 From: Luis de la Rosa Date: Sat, 5 Apr 2014 12:29:05 -0400 Subject: [PATCH 1/3] Show informative error message when a merge conflict is detected in a YAML file. This fixes issue #69. --- lib/cocoapods-core.rb | 2 +- lib/cocoapods-core/lockfile.rb | 4 +- lib/cocoapods-core/podfile.rb | 2 +- .../specification/set/statistics.rb | 2 +- .../{yaml_converter.rb => yaml_helper.rb} | 21 +++- spec/lockfile_spec.rb | 10 +- spec/podfile_spec.rb | 2 +- spec/specification/set/statistics_spec.rb | 8 +- ..._converter_spec.rb => yaml_helper_spec.rb} | 107 ++++++++++++++---- 9 files changed, 120 insertions(+), 38 deletions(-) rename lib/cocoapods-core/{yaml_converter.rb => yaml_helper.rb} (90%) rename spec/{yaml_converter_spec.rb => yaml_helper_spec.rb} (61%) diff --git a/lib/cocoapods-core.rb b/lib/cocoapods-core.rb index ba9cae3d3..19030aa70 100644 --- a/lib/cocoapods-core.rb +++ b/lib/cocoapods-core.rb @@ -28,7 +28,7 @@ class Informative < PlainInformative; end autoload :Source, 'cocoapods-core/source' autoload :Specification, 'cocoapods-core/specification' autoload :StandardError, 'cocoapods-core/standard_error' - autoload :YAMLConverter, 'cocoapods-core/yaml_converter' + autoload :YAMLHelper, 'cocoapods-core/yaml_helper' # TODO: Fix # diff --git a/lib/cocoapods-core/lockfile.rb b/lib/cocoapods-core/lockfile.rb index c4e970dad..4d1fd8091 100644 --- a/lib/cocoapods-core/lockfile.rb +++ b/lib/cocoapods-core/lockfile.rb @@ -40,7 +40,7 @@ def initialize(hash) def self.from_file(path) return nil unless path.exist? require 'yaml' - hash = File.open(path) { |f| YAML.load(f) } + hash = File.open(path) { |f| YAMLHelper.load(f) } unless hash && hash.is_a?(Hash) raise Informative, "Invalid Lockfile in `#{path}`" end @@ -312,7 +312,7 @@ def to_yaml 'SPEC CHECKSUMS', 'COCOAPODS', ] - YAMLConverter.convert_hash(to_hash, keys_hint, "\n\n") + YAMLHelper.convert_hash(to_hash, keys_hint, "\n\n") end #-------------------------------------------------------------------------# diff --git a/lib/cocoapods-core/podfile.rb b/lib/cocoapods-core/podfile.rb index fd55d3de2..767c9f32b 100644 --- a/lib/cocoapods-core/podfile.rb +++ b/lib/cocoapods-core/podfile.rb @@ -266,7 +266,7 @@ def self.from_yaml(path) if string.respond_to?(:encoding) && string.encoding.name != 'UTF-8' string.encode!('UTF-8') end - hash = YAML.load(string) + hash = YAMLHelper.load(string) from_hash(hash, path) end diff --git a/lib/cocoapods-core/specification/set/statistics.rb b/lib/cocoapods-core/specification/set/statistics.rb index f37f4e732..26c9ac4a9 100644 --- a/lib/cocoapods-core/specification/set/statistics.rb +++ b/lib/cocoapods-core/specification/set/statistics.rb @@ -155,7 +155,7 @@ def github_pushed_at(set) def cache unless @cache if cache_file && cache_file.exist? - @cache = YAML.load(cache_file.read) + @cache = YAMLHelper.load(cache_file.read) else @cache = {} end diff --git a/lib/cocoapods-core/yaml_converter.rb b/lib/cocoapods-core/yaml_helper.rb similarity index 90% rename from lib/cocoapods-core/yaml_converter.rb rename to lib/cocoapods-core/yaml_helper.rb index 50ed5ad31..a5136334c 100644 --- a/lib/cocoapods-core/yaml_converter.rb +++ b/lib/cocoapods-core/yaml_helper.rb @@ -15,7 +15,7 @@ module Pod # The missing features include: # - Strings are never quoted even when ambiguous. # - class YAMLConverter + class YAMLHelper class << self @@ -41,6 +41,18 @@ def convert_hash(value, hash_keys_hint, line_separator = "\n") result << "\n" end + # Load a YAML file and provide more informative error messages in special cases like merge conflict. + # @param A YAML string. + def load(yaml_string) + YAML.load(yaml_string) + rescue Exception => exception + if yaml_has_merge_error(yaml_string) + raise Informative, 'Merge conflict(s) detected' + else + raise exception + end + end + #-----------------------------------------------------------------------# private @@ -120,6 +132,13 @@ def process_hash(hash, hash_keys_hint = nil, line_separator = "\n") key_lines * line_separator end + # Check for merge errors in a YAML string. + # @param A YAML string. + # @return If a merge error was detected or not. + def yaml_has_merge_error(yaml_string) + yaml_string.include?('<<<<<<< HEAD') + end + #-----------------------------------------------------------------------# private diff --git a/spec/lockfile_spec.rb b/spec/lockfile_spec.rb index 7f0c55d00..aab4e3386 100644 --- a/spec/lockfile_spec.rb +++ b/spec/lockfile_spec.rb @@ -65,14 +65,14 @@ def self.specs end it 'stores the initialization hash' do - lockfile = Lockfile.new(YAML.load(Sample.yaml)) - lockfile.internal_data.should == YAML.load(Sample.yaml) + lockfile = Lockfile.new(YAMLHelper.load(Sample.yaml)) + lockfile.internal_data.should == YAMLHelper.load(Sample.yaml) end it 'loads from a file' do File.open(@tmp_path, 'w') { |f| f.write(Sample.yaml) } lockfile = Lockfile.from_file(@tmp_path) - lockfile.internal_data.should == YAML.load(Sample.yaml) + lockfile.internal_data.should == YAMLHelper.load(Sample.yaml) end it "returns nil if it can't find the initialization file" do @@ -331,7 +331,7 @@ def self.specs end it 'generates a valid YAML representation' do - YAML.load(@lockfile.to_yaml).should == YAML.load(Sample.yaml) + YAMLHelper.load(@lockfile.to_yaml).should == YAMLHelper.load(Sample.yaml) end it "serializes correctly `:head' dependencies" do @@ -414,7 +414,7 @@ def self.specs end it 'it includes all the information that it is expected to store' do - @lockfile.internal_data.should == YAML.load(Sample.yaml) + @lockfile.internal_data.should == YAMLHelper.load(Sample.yaml) end end diff --git a/spec/podfile_spec.rb b/spec/podfile_spec.rb index 4dc69c7b4..77f92b8f1 100644 --- a/spec/podfile_spec.rb +++ b/spec/podfile_spec.rb @@ -196,7 +196,7 @@ module Pod generate_bridge_support: true set_arc_compatibility_flag: true EOF - YAML.load(podfile.to_yaml).should == YAML.load(expected) + YAMLHelper.load(podfile.to_yaml).should == YAMLHelper.load(expected) end it 'includes inhibit warnings per pod' do diff --git a/spec/specification/set/statistics_spec.rb b/spec/specification/set/statistics_spec.rb index 8d30e1141..ac8df0400 100644 --- a/spec/specification/set/statistics_spec.rb +++ b/spec/specification/set/statistics_spec.rb @@ -95,14 +95,14 @@ module Pod it 'saves the cache after computing the creation date of a set' do @stats.creation_date(@set) - cache_hash = YAML.load(@cache_file.read) + cache_hash = YAMLHelper.load(@cache_file.read) cache_hash['JSONKit'][:creation_date].should == Time.parse('2011-09-12 10:49:04 +0200') end it 'saves the cache after computing the creation date of many sets' do sets = [@set, @source.search_by_name('libPusher').first] @stats.creation_dates(sets) - cache_hash = YAML.load(@cache_file.read) + cache_hash = YAMLHelper.load(@cache_file.read) cache_hash['JSONKit'][:creation_date].should == Time.parse('2011-09-12 10:49:04 +0200') cache_hash['libPusher'][:creation_date].should == Time.parse('2012-02-01 17:05:58 +0100') end @@ -128,7 +128,7 @@ module Pod it 'saves the cache after retrieving GitHub information' do @stats.github_watchers(@set) - saved_cache = YAML.load(@cache_file.read) + saved_cache = YAMLHelper.load(@cache_file.read) saved_cache['JSONKit'][:gh_date] = nil @cache_hash['JSONKit'][:gh_date] = nil saved_cache.should == @cache_hash @@ -141,7 +141,7 @@ module Pod it 'stores in the cache time of the last access to the GitHub API' do @stats.github_watchers(@set) - saved_cache = YAML.load(@cache_file.read) + saved_cache = YAMLHelper.load(@cache_file.read) time_delta = (Time.now - saved_cache['JSONKit'][:gh_date]) time_delta.should < 60 end diff --git a/spec/yaml_converter_spec.rb b/spec/yaml_helper_spec.rb similarity index 61% rename from spec/yaml_converter_spec.rb rename to spec/yaml_helper_spec.rb index 69e6815fa..ed75a0f38 100644 --- a/spec/yaml_converter_spec.rb +++ b/spec/yaml_helper_spec.rb @@ -24,50 +24,91 @@ def sample_yaml LOCKFILE end +def yaml_with_merge_conflict + text = <<-LOCKFILE.strip_heredoc + PODS: + - Kiwi (2.2) + - ObjectiveSugar (1.1.1) + + DEPENDENCIES: + - Kiwi + - ObjectiveSugar (from `../`) + + EXTERNAL SOURCES: + ObjectiveSugar: + :path: ../ + + SPEC CHECKSUMS: + <<<<<<< HEAD + Kiwi: 05f988748c5136c6daed8dab3563eca929399a72 + ObjectiveSugar: 7377622e35ec89ce893b05dd0af4bede211b01a4 + ======= + Kiwi: db174bba4ee8068b15d7122f1b22fb64b7c1d378 + ObjectiveSugar: 27c680bb74f0b0415e9e743d5d61d77bc3292d3f + >>>>>>> b65623cbf5e105acbc3e2dec48f8024fa82003ce + + COCOAPODS: 0.29.0 + LOCKFILE +end + +def bad_yaml + text = <<-LOCKFILE.strip_heredoc + PODS: + - Kiwi (2.2) + SOME BAD TEXT + + DEPENDENCIES: + - Kiwi + - ObjectiveSugar (from `../`) + + COCOAPODS: 0.29.0 + LOCKFILE +end + #-----------------------------------------------------------------------------# module Pod describe 'In general' do - describe YAMLConverter do + describe YAMLHelper do it 'converts a string' do value = 'Value' - result = YAMLConverter.convert(value) + result = YAMLHelper.convert(value) result.should == "Value\n" end it 'converts a symbol' do value = :value - result = YAMLConverter.convert(value) + result = YAMLHelper.convert(value) result.should == ":value\n" end it 'converts the true class' do - result = YAMLConverter.convert(true) + result = YAMLHelper.convert(true) result.should == "true\n" end it 'converts the false class' do - result = YAMLConverter.convert(false) + result = YAMLHelper.convert(false) result.should == "false\n" end it 'converts an array' do value = %w(Value_1 Value_2) - result = YAMLConverter.convert(value) + result = YAMLHelper.convert(value) result.should == "- Value_1\n- Value_2\n" end it 'converts an hash' do value = { 'Key' => 'Value' } - result = YAMLConverter.convert(value) + result = YAMLHelper.convert(value) result.should == "Key: Value\n" end it 'converts an hash which contains and array as one of the values' do value = { 'Key' => %w(Value_1 Value_2) } - result = YAMLConverter.convert(value) + result = YAMLHelper.convert(value) result.should == <<-EOT.strip_heredoc Key: - Value_1 @@ -77,7 +118,7 @@ module Pod it 'converts an hash which contains and array as one of the values' do value = { 'Key' => { 'Subkey' => %w(Value_1 Value_2) } } - result = YAMLConverter.convert(value) + result = YAMLHelper.convert(value) result.should == <<-EOT.strip_heredoc Key: Subkey: @@ -89,8 +130,30 @@ module Pod it "raises if it can't handle the class of the given object" do value = Pathname.new('a-path') should.raise StandardError do - YAMLConverter.convert(value) + YAMLHelper.convert(value) end.message.should.match /Unsupported class/ + end + end + + #-------------------------------------------------------------------------# + + describe 'Loading' do + it "raises an Informative error when it encounters a merge conflict" do + should.raise Informative do + YAMLHelper.load(yaml_with_merge_conflict) + end.message.should.match /Merge conflict\(s\) detected/ + end + + it "raises another error when it encounters an error that is not a merge conflict" do + should.raise do + YAMLHelper.load(bad_yaml) + end + end + + it "should not raise when there is no merge conflict" do + should.not.raise do + YAMLHelper.load(sample_yaml) + end end end @@ -101,27 +164,27 @@ module Pod describe '#sorted_array_with_hint' do it 'sorts an array according to its string representation' do values = %w(JSONKit BananaLib) - result = YAMLConverter.send(:sorted_array, values) + result = YAMLHelper.send(:sorted_array, values) result.should == %w(BananaLib JSONKit) end it 'sorts an array containing strings and hashes according to its string representation' do values = ['JSONKit', 'BananaLib', { 'c_hash_key' => 'a_value' }] - result = YAMLConverter.send(:sorted_array, values) + result = YAMLHelper.send(:sorted_array, values) result.should == ['BananaLib', { 'c_hash_key' => 'a_value' }, 'JSONKit'] end it 'sorts an array with a given hint' do values = %w(non-hinted second first) hint = %w(first second hinted-missing) - result = YAMLConverter.send(:sorted_array_with_hint, values, hint) + result = YAMLHelper.send(:sorted_array_with_hint, values, hint) result.should == %w(first second non-hinted) end it 'sorts an array with a given nil hint' do values = %w(JSONKit BananaLib) hint = nil - result = YAMLConverter.send(:sorted_array_with_hint, values, hint) + result = YAMLHelper.send(:sorted_array_with_hint, values, hint) result.should == %w(BananaLib JSONKit) end end @@ -130,25 +193,25 @@ module Pod it 'returns the empty string if a nil value is passed' do value = nil - result = YAMLConverter.send(:sorting_string, value) + result = YAMLHelper.send(:sorting_string, value) result.should == '' end it 'sorts strings ignoring case' do value = 'String' - result = YAMLConverter.send(:sorting_string, value) + result = YAMLHelper.send(:sorting_string, value) result.should == 'string' end it 'sorts symbols ignoring case' do value = :Symbol - result = YAMLConverter.send(:sorting_string, value) + result = YAMLHelper.send(:sorting_string, value) result.should == 'symbol' end it 'sorts arrays using the first element ignoring case' do value = %w(String_2 String_1) - result = YAMLConverter.send(:sorting_string, value) + result = YAMLHelper.send(:sorting_string, value) result.should == 'string_2' end @@ -157,7 +220,7 @@ module Pod :key_2 => 'a_value', :key_1 => 'a_value', } - result = YAMLConverter.send(:sorting_string, value) + result = YAMLHelper.send(:sorting_string, value) result.should == 'key_1' end end @@ -168,10 +231,10 @@ module Pod describe 'Lockfile generation' do it 'converts a complex file' do - value = YAML.load(sample_yaml) + value = YAMLHelper.load(sample_yaml) sorted_keys = ['PODS', 'DEPENDENCIES', 'EXTERNAL SOURCES', 'SPEC CHECKSUMS', 'COCOAPODS'] - result = YAMLConverter.convert_hash(value, sorted_keys, "\n\n") - YAML.load(result).should == value + result = YAMLHelper.convert_hash(value, sorted_keys, "\n\n") + YAMLHelper.load(result).should == value result.should == sample_yaml end end From 9211488ca406b7b2538e5cc0bfc3423a129afa5d Mon Sep 17 00:00:00 2001 From: Luis de la Rosa Date: Sat, 5 Apr 2014 18:00:30 -0400 Subject: [PATCH 2/3] Added this enhancement to the changelog. --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 875aefb24..65f6206ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,11 @@ ##### Enhancements +* Show informative error message when a merge conflict is detected in a YAML file. + [Luis de la Rosa](https://github.com/luisdelarosa) + [#69](https://github.com/CocoaPods/Core/issues/69) + [#100](https://github.com/CocoaPods/Core/pull/100) + * Added a check to the linter to ensure that the `social_media_url` has been changed from the example value. [Richard Lee](https://github.com/dlackty) From 883de73928dfe2db55f222794b4880b4f2e3c036 Mon Sep 17 00:00:00 2001 From: Luis de la Rosa Date: Sat, 5 Apr 2014 18:07:18 -0400 Subject: [PATCH 3/3] Use generic Exception to make tests pass consistently. --- spec/yaml_helper_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/yaml_helper_spec.rb b/spec/yaml_helper_spec.rb index ed75a0f38..e4237927f 100644 --- a/spec/yaml_helper_spec.rb +++ b/spec/yaml_helper_spec.rb @@ -145,7 +145,7 @@ module Pod end it "raises another error when it encounters an error that is not a merge conflict" do - should.raise do + should.raise Exception do YAMLHelper.load(bad_yaml) end end