Permalink
Browse files

adding requires and default yaml parser to psych if possible

  • Loading branch information...
tenderlove committed Jan 19, 2011
1 parent f2ca897 commit a318f9180811fee3f0fd9db98c39916695502091
Showing with 12 additions and 0 deletions.
  1. +6 −0 lib/bundler.rb
  2. +1 −0 lib/bundler/cli.rb
  3. +1 −0 lib/bundler/source.rb
  4. +2 −0 lib/bundler/ui.rb
  5. +2 −0 spec/support/rubygems_ext.rb
View
@@ -1,6 +1,12 @@
require 'rbconfig'
require 'fileutils'
require 'pathname'
+
+begin
+ require 'psych'

This comment has been minimized.

Show comment
Hide comment
@cris

cris Feb 3, 2011

It's a very bad idea to add 'psych' instead of default 'syck' for 'yaml'. I have an issue with this in bundler 1.0.10 and ruby 1.9.2 because psych has a big issue with aliased sections in yaml. Now, to fix it, I should do require 'yaml' before require 'bunler' in rails-script.

See https://gist.github.com/809309 for reproduce. I've created bug https://github.com/carlhuda/bundler/issues/issue/1010

@cris

cris Feb 3, 2011

It's a very bad idea to add 'psych' instead of default 'syck' for 'yaml'. I have an issue with this in bundler 1.0.10 and ruby 1.9.2 because psych has a big issue with aliased sections in yaml. Now, to fix it, I should do require 'yaml' before require 'bunler' in rails-script.

See https://gist.github.com/809309 for reproduce. I've created bug https://github.com/carlhuda/bundler/issues/issue/1010

+rescue LoadError
+end
+
require 'yaml'
require 'bundler/rubygems_ext'
require 'bundler/version'
View
@@ -1,6 +1,7 @@
$:.unshift File.expand_path('../vendor', __FILE__)
require 'thor'
require 'thor/actions'
+require 'rubygems/user_interaction'
require 'rubygems/config_file'
# Work around a RubyGems bug
View
@@ -1,4 +1,5 @@
require "uri"
+require 'rubygems/user_interaction'
require "rubygems/installer"
require "rubygems/spec_fetcher"
require "rubygems/format"
View
@@ -1,3 +1,5 @@
+require 'rubygems/user_interaction'
+
module Bundler
class UI
def warn(message)
@@ -1,3 +1,5 @@
+require 'rubygems/user_interaction'
+
module Spec
module Rubygems
def self.setup

2 comments on commit a318f91

@stephank

This comment has been minimized.

Show comment
Hide comment
@stephank

stephank Feb 1, 2011

This commit changes the default YAML parser to Psych for all gems, which may introduce subtle breakage. I personally ran into this in Rails 2.3: https://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/6354-psych-does-not-handle-symbols-in-yaml-used-in-activesupport

I'm not sure if this is a Bundler issue, or if any problems should be reported to the application or Psych. FWIW, Rails 3.0 seems to have worked around it, so I assume they acknowledge it to be an issue there, and I've reported one for 2.3.

This commit changes the default YAML parser to Psych for all gems, which may introduce subtle breakage. I personally ran into this in Rails 2.3: https://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/6354-psych-does-not-handle-symbols-in-yaml-used-in-activesupport

I'm not sure if this is a Bundler issue, or if any problems should be reported to the application or Psych. FWIW, Rails 3.0 seems to have worked around it, so I assume they acknowledge it to be an issue there, and I've reported one for 2.3.

@ndbroadbent

This comment has been minimized.

Show comment
Hide comment
@ndbroadbent

ndbroadbent Mar 1, 2011

Contributor

Is it possible that 'psych' is breaking the YAML output of ".bundle/config"?
It appears to us that between 1.0.9 and 1.0.10, the command "bundle install --deployment" has been broken, and no longer writes to ".bundle/config".
It could be an issue with our server configuration though, so it would be great to have this verified by someone else.

Contributor

ndbroadbent replied Mar 1, 2011

Is it possible that 'psych' is breaking the YAML output of ".bundle/config"?
It appears to us that between 1.0.9 and 1.0.10, the command "bundle install --deployment" has been broken, and no longer writes to ".bundle/config".
It could be an issue with our server configuration though, so it would be great to have this verified by someone else.

Please sign in to comment.