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

Breaking change in 5.2.0 #694

Closed
tdeo opened this issue Nov 13, 2024 · 4 comments · Fixed by #695
Closed

Breaking change in 5.2.0 #694

tdeo opened this issue Nov 13, 2024 · 4 comments · Fixed by #695
Assignees

Comments

@tdeo
Copy link
Contributor

tdeo commented Nov 13, 2024

I've run into the following behavior after upgrading to psych v5.2.0:

With 5.1.2

irb(main):001> require 'yaml'
=> true
irb(main):002> Gem.loaded_specs['psych'].version
=> Gem::Version.new("5.1.2")
irb(main):003> '2020-01-01'.to_yaml
=> "--- '2020-01-01'\n"

Observe how the string is quoted in the Yaml file

With 5.2.0

irb(main):001> require 'yaml'
=> true
irb(main):002> Gem.loaded_specs['psych'].version
=> Gem::Version.new("5.2.0")
irb(main):003> '2020-01-01'.to_yaml
=> "--- 2020-01-01\n"

Now the string is unquoted, which makes it invalid to be parsed back with YAML.safe_load.

I believe this behavior originates from this commit, also because I noticed that manually requiring 'date' before calling .to_yaml still yields the expected result of v5.1.2:

irb(main):001> require 'yaml'
=> true
irb(main):002> Gem.loaded_specs['psych'].version
=> Gem::Version.new("5.2.0")
irb(main):003> require 'date'
=> true
irb(main):004> '2020-01-01'.to_yaml
=> "--- '2020-01-01'\n"

I think the following error probably hints as to where the problem originates from:

irb(main):001> require 'yaml'; Psych::ClassLoader.new.date
/Users/thierry/.asdf/installs/ruby/3.3.5/lib/ruby/gems/3.3.0/gems/psych-5.2.0/lib/psych/class_loader.rb:58:in `path2class': undefined class/module Date (ArgumentError)

        path2class(name)
                   ^^^^
	from /Users/thierry/.asdf/installs/ruby/3.3.5/lib/ruby/gems/3.3.0/gems/psych-5.2.0/lib/psych/class_loader.rb:58:in `resolve'
	from /Users/thierry/.asdf/installs/ruby/3.3.5/lib/ruby/gems/3.3.0/gems/psych-5.2.0/lib/psych/class_loader.rb:50:in `find'
	from /Users/thierry/.asdf/installs/ruby/3.3.5/lib/ruby/gems/3.3.0/gems/psych-5.2.0/lib/psych/class_loader.rb:30:in `load'
	from /Users/thierry/.asdf/installs/ruby/3.3.5/lib/ruby/gems/3.3.0/gems/psych-5.2.0/lib/psych/class_loader.rb:42:in `date'
	from (irb):1:in `<main>'

and that actually, autoload :Date, date probably rather belongs in Psych::ClassLoader than Psych::ScalarScanner. I'll try writing a regression and submit a PR if I succeed

@tdeo
Copy link
Contributor Author

tdeo commented Nov 18, 2024

@byroot did you see this issue and the related PR by any chance as you authored the commit I suspect to be responsible?

@byroot
Copy link
Member

byroot commented Nov 19, 2024

No, I'm the maintainer, but I'll have a look.

@byroot
Copy link
Member

byroot commented Nov 19, 2024

I can repro the same issue before my change.

@byroot
Copy link
Member

byroot commented Nov 19, 2024

I can repro the same issue before my change.

Nevermind, my LOAD PATH was incorrect, it's indeed a regression.

@byroot byroot self-assigned this Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants