Skip to content

Lock gem psych to v3.x#8918

Merged
jekyllbot merged 2 commits intomasterfrom
psych-3.x
Dec 30, 2021
Merged

Lock gem psych to v3.x#8918
jekyllbot merged 2 commits intomasterfrom
psych-3.x

Conversation

@ashmaroli
Copy link
Copy Markdown
Member

@ashmaroli ashmaroli commented Dec 29, 2021

Summary

psych-4.x default in Ruby 3.1 breaks our Cucumber based tests.

Context

(::) failed steps (::)

Tried to load unspecified class: Date (Psych::DisallowedClass)
(eval):2:in `date'
/home/runner/work/jekyll/jekyll/features/step_definitions.rb:166:in `/^I have a configuration file with "(.*)" set to "(.*)"$/'
/home/runner/work/jekyll/jekyll/features/step_definitions.rb:175:in `block (2 levels) in <top (required)>'
/home/runner/work/jekyll/jekyll/features/step_definitions.rb:174:in `each'
/home/runner/work/jekyll/jekyll/features/step_definitions.rb:174:in `/^I have a configuration file with:$/'
features/site_configuration.feature:211:in `I have a configuration file with:'

@ashmaroli
Lock gem psych to v3.x
8c55339 
`psych-4.x` default in Ruby 3.1 breaks our Cucumber based tests.
@mattr-
Copy link
Copy Markdown
Member

mattr- commented Dec 29, 2021

If we switch to unsafe loading, can we avoid locking Psych? If we can't avoid locking Psych, would you mind adding a TODO comment of some sort explaining why we locked it?

@fauno
Copy link
Copy Markdown
Member

fauno commented Dec 29, 2021

Is it related to #8772?

@toshimaru toshimaru mentioned this pull request Dec 30, 2021
Merged
@ashmaroli
Copy link
Copy Markdown
Member Author

ashmaroli commented Dec 30, 2021

If we switch to unsafe loading, can we avoid locking Psych?

I'm not so confident of making the switch to unsafe loading, especially for a minor release Jekyll 4.3.0. Perhaps we'll get enough feedback by the time Jekyll 4.4.0 or Jekyll 5.0 materializes..?

Ruby 3.0.0 shipped with psych-3.3.0 as a default gem and based on the CI results here, the locking doesn't introduce regressions even with Ruby 3.1.0 which shipped with psych-4.0.3 instead.

Therefore, my take is that locking is the optimal route now.

Is it related to #8772?

Not really @fauno. That PR doesn't explicitly use Psych 4.0 instead relying on the default version shipped with the Ruby version in use.

mattr-
mattr- approved these changes Dec 30, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@mattr- mattr- left a comment

Choose a reason for hiding this comment

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

Appreciate the comment here. Let's :shipit:

@mattr-
Copy link
Copy Markdown
Member

mattr- commented Dec 30, 2021

@jekyllbot: merge +dev

@jekyllbot jekyllbot merged commit a7a7b7a into master Dec 30, 2021
jekyllbot added a commit that referenced this pull request Dec 30, 2021
@jekyllbot
Update history to reflect merge of #8918 [ci skip]
c34fa61
@jekyllbot jekyllbot deleted the psych-3.x branch December 30, 2021 16:02
github-actions bot pushed a commit that referenced this pull request Dec 30, 2021
@jekyllbot
Deploy docs from a7a7b7a
e3ceb6e 
Ashwin Maroli: Lock gem `psych` to `v3.x` (#8918)

Merge pull request 8918
This was referenced Sep 28, 2022
Closed
Merged
@jekyll jekyll locked and limited conversation to collaborators Dec 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@mattr- mattr- mattr- approved these changes

Assignees

No one assigned

Labels

fix frozen-due-to-age internal

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ashmaroli @mattr- @fauno @jekyllbot