Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Merge keys not working as expected #8

Closed
wr0ngway opened this Issue · 19 comments

7 participants

@wr0ngway

Psych doesn't seem to support merge keys ( http://yaml.org/type/merge.html ) with alias/anchor the same way that syck does. I'm not sure if this is intentional or not, but its a pretty useful behavior so I'm filing a bug. Basically, while psych does allow one to use a merge key to pull a aliased mapping into another mapping, it does not allow one to add other items to that mapping. Very frequently I have a map that is mostly in common amongst a number of keys, with a subkey that differs, and this bug makes it impossible to achieve this behavior with psych.

For the yaml snippet below:

foo: &foo
  hello: world
bar:
  << : *foo
  baz: boo

syck produces: {"foo"=>{"hello"=>"world"}, "bar"=>{"hello"=>"world", "baz"=>"boo"}}

psych produces: {"foo"=>{"hello"=>"world"}, "bar"=>{"hello"=>"world"}}

Note that in the yaml 1.1 spec ( http://yaml.org/spec/1.1/#id902561 ) it says "Note that an alias node must not specify any properties or content, as these were already specified at the first occurrence of the node.", however I think this should only apply when doing something like "bar: *foo". When using a merge key, you are creating a new node (which you don't add to), but merging it into a different mapping node - which you should be able to add to.

@tenderlove
Owner

Thanks for the bug report! Would you mind filing this bug on redmine:

http://redmine.ruby-lang.org/projects/ruby-19/issues?set_filter=1&tracker_id=1

I can fix this, but if it's not on redmine, I'm afraid it won't get backported to 1.9.2.

@tenderlove
Owner

Awesome, thank you!

@michaelklishin

Is there a way to upgrade Psych for 1.9.2-p136? p180 seems to use RubyGems 1.5 and that is not compatible with Ruby on Rails 2.3.

@michaelklishin

A follow-up: his patch isn't in 1.9.2-p180. The only way to get it for Ruby on Rails 2.3 is to use HEAD and downgrade RubyGems to 1.3.7.

Lets just say that virtually every Ruby (and especially Ruby on Rails) app I have seen since 2005 that runs in multiple environments, relies on this feature of YAML :(

@tenderlove
Owner

For now, I suggest you downgrade to use Syck. The way to do that is add this to your environment.rb:

YAML::ENGINE.yamler = 'syck'

I am thinking of releasing psych as a gem, but it must be 1.9 only. :-(

@michaelklishin

So it is possible to switch back to Syck. Great, thank you. I personally think that 1.9-only version of Psych is totally fine: on 1.8, people seem to be pretty happy with Syck.

@tenderlove
Owner

The annoying thing is that even if I release a gem, you would be forced to say:

gem 'psych'

in your environment.rb. Otherwise the stdlib one would be required. Either I can teach people how to downgrade to syck, or teach people to install a gem, then add a special command to require it. I'm not sure which is least annoying. :-(

@wr0ngway

I vote for the gem - the reason I want to try psych is that I frequently segfault in syck when running our test cases with complex VCR yaml fixtures.

@tenderlove
Owner

Fixed in ruby trunk, so I'm closing. Also, I've released a gem that is 1.9 only.

@tenderlove tenderlove closed this
@michaelklishin

@tenderlove, are there any plans to backport this to the next point release of 1.9.2? I see people hitting this issue over and over, including well-known folks in the Ruby community. That's kinda sad.

@tenderlove
Owner

@michaelklishin there is a ticket filed to backport to 1.9.2. In the mean time (as I have mentioned in the previous comments) please use the gem or downgrade to syck.

@ifesdjeen

actually, better add to application.rb, not environment.rb
otherwise rake tasks that do not load environment won't fire O_O (e.q. drop/create DB)

@contentfree

@tenderlove: What version of the Psych gem has this fixed? I get the problem with Psych 1.2.0 still.

@ndbroadbent

This is still an issue, and I'm having to fix every Rails app I run on 1.9.2 with this code in config/boot.rb:

if RUBY_VERSION.to_f >= 1.9
  # The psych engine for YAML doesn't handle `<<: *defaults` properly.
  # See: https://github.com/tenderlove/psych/issues/8
  require 'yaml'
  YAML::ENGINE.yamler = 'syck'
end

It would be great if this hack was no longer necessary.

@tenderlove
Owner

@ndbroadbent please open a new ticket. In the new ticket, post code that parses your YAML file using 1.8 and 1.9 (with psych). Post the output for 1.8 and the output for 1.9, and we can get it fixed. Thanks.

@tenderlove
Owner

Also make sure to include the YAML file along with your sample program. Thanks!

@ndbroadbent

Aha! I see that it has been fixed in 1.9.2-p290, but not 1.9.2-p180.
I shall bring all my Rails 3 apps up to p290.
Sorry for the false alarm.

@firedev

Sorry but it still not merging keys correctly as of Ruby 1.9.3p374 and Rails 3.2.11 rails/rails#9025

# Models
ru:
  activerecord:
    attributes:
      model:
        title: "Russian title"


  errors: &errors
      format: ! '%{attribute} %{message}'
      messages:
        accepted: нужно подтвердить

  activerecord:
    errors:
      <<: *errors

Start console:

I18n.locale=:ru
Model.human_attribute_name(:title)
=> "Title"
@sideci-sample sideci-sample referenced this issue from a commit in sideci-sample/sideci-sample-travis-ci
@ifesdjeen ifesdjeen Downgrading YAML to syck in order to fix 1.9.2 problem, please refer t… c07773d
@sideci-sample sideci-sample referenced this issue from a commit in sideci-sample/sideci-sample-travis-ci
@ifesdjeen ifesdjeen Revert "Downgrading YAML to syck in order to fix 1.9.2 problem, pleas…
…e refer tenderlove/psych#8 for details"

This reverts commit d8c82cdb1fa1b0cacb1bc3777bf9d3d35036a80e.
84f8511
@sideci-sample sideci-sample referenced this issue from a commit in sideci-sample/sideci-sample-travis-ci
@ifesdjeen ifesdjeen Downgrading YAML to syck in order to fix 1.9.2 problem, please refer t…
…enderlove/psych#8 for details.

Fixes #90.
aa2c4c9
@sideci-sample sideci-sample referenced this issue from a commit in sideci-sample/sideci-sample-travis-ci
@ifesdjeen ifesdjeen Revert "Downgrading YAML to syck in order to fix 1.9.2 problem, pleas…
…e refer tenderlove/psych#8 for details. "

This reverts commit 553848e93d36e0b9f1fdfab91f34a1ab5dc5f118.
1ced4ec
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.