Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set Psych as the YAML engine for Rubinius #16267

Merged
merged 1 commit into from
Jul 23, 2014
Merged

Set Psych as the YAML engine for Rubinius #16267

merged 1 commit into from
Jul 23, 2014

Conversation

robin850
Copy link
Member

Hello,

Since the rubysl-yaml gem doesn't ship with Psych by default because of its dependency on libyaml, on Rubinius, the default engine is Syck. However, if we want to be able to run the application safely on different rubies, we need to make people using Rubinius rely on Psych.

See http://git.io/uuLVag for further information.

Have a nice day.


comment = 'Use Psych as the YAML engine, instead of Syck, so serialized ' \
'data can be read safely from different rubies (see http://git.io/uuLVag)'
GemfileEntry.new('psych', '~> 2.0', comment)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should not we only include psych if the platform is rbx?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we do. The return statement at line 318 avoid these lines to be reached if we are on Rubinius. To be honest, I've just followed the example of Spring. :-)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but maybe we should add platform: :rbx too.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that would be GemfileEntry.new('psych', '~> 2.0', comment, { platforms: :rbx })

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, fair point! Thank you guys ; updated!

Since the rubysl-yaml gem doesn't ship with Psych by default because of
its dependency on libyaml, on Rubinius, the default engine is Syck.

However, if we want to be able to run the application safely on
different rubies, we need to make people using Rubinius rely on Psych.

See http://git.io/uuLVag for further information.
rafaelfranca added a commit that referenced this pull request Jul 23, 2014
Set Psych as the YAML engine for Rubinius
@rafaelfranca rafaelfranca merged commit bec08e7 into rails:master Jul 23, 2014
@robin850 robin850 deleted the rbx-yaml branch July 23, 2014 21:10
@gmcgibbon gmcgibbon mentioned this pull request Apr 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants