WIP: Experiments with liquid 5.x #9229

isimluk · 2022-12-25T17:27:04Z

Summary

Prior attempts #9036, #9030, #8662, #8760, #8615, #8535

Context

Liquid 5.x provides multiple breaking changes. But liquid 4.x won't work all that well with ruby 3.2.0.

WIP

This PR is cool but disables liquid-c. Therefore it is marked wip.

bewuethr · 2022-12-25T20:20:41Z

This would be great to have because Ruby 3.2.0 dropped taint checking, and Liquid 4.0.3 still uses it, resulting in Jekyll failing to build with Ruby 3.2.0.

We need to disable liquid-5.4 and liquid-c to have the tests pass initially.

Before Liquid 5.4, context.registers are good old plain Hash. After Liquid 5.4, context.registers is_a Liquid::Registers

The following errors is raised when jekyll-seo-tag is used with liquid-5.4.0. This is caused by the fact that context.registers no longer returns Hash, but Liquid::Registers. Addressing: /opt/hostedtoolcache/Ruby/3.1.2/x64/lib/ruby/gems/3.1.0/gems/liquid-5.4.0/lib/liquid/template.rb:176:in `render': undefined method `each' for #<Liquid::Registers:0x00007f2b142ddfc0> options[:registers]&.each do |key, register| ^^^^^^ from /opt/hostedtoolcache/Ruby/3.1.2/x64/lib/ruby/gems/3.1.0/gems/liquid-5.4.0/lib/liquid/template.rb:204:in `render!' from /opt/hostedtoolcache/Ruby/3.1.2/x64/lib/ruby/gems/3.1.0/gems/jekyll-seo-tag-2.8.0/lib/jekyll-seo-tag.rb:36:in `render' from /opt/hostedtoolcache/Ruby/3.1.2/x64/lib/ruby/gems/3.1.0/gems/liquid-5.4.0/lib/liquid/tag.rb:51:in `render_to_output_buffer' from /opt/hostedtoolcache/Ruby/3.1.2/x64/lib/ruby/gems/3.1.0/gems/liquid-5.4.0/lib/liquid/block_body.rb:80:in `render_node' from /opt/hostedtoolcache/Ruby/3.1.2/x64/lib/ruby/gems/3.1.0/gems/liquid-5.4.0/lib/liquid/block_body.rb:230:in `render_node' from /opt/hostedtoolcache/Ruby/3.1.2/x64/lib/ruby/gems/3.1.0/gems/liquid-5.4.0/lib/liquid/block_body.rb:213:in `render_to_output_buffer' from /opt/hostedtoolcache/Ruby/3.1.2/x64/lib/ruby/gems/3.1.0/gems/liquid-5.4.0/lib/liquid/document.rb:41:in `render_to_output_buffer' from /opt/hostedtoolcache/Ruby/3.1.2/x64/lib/ruby/gems/3.1.0/gems/liquid-5.4.0/lib/liquid/template.rb:194:in `render' from /opt/hostedtoolcache/Ruby/3.1.2/x64/lib/ruby/gems/3.1.0/gems/liquid-5.4.0/lib/liquid/template.rb:204:in `render!' Related to: jekyll/jekyll#9229

invalidusrname

What do you think of just relying on RUBY_VERSION to determine the version of liquid to use?

invalidusrname · 2022-12-30T16:46:47Z

jekyll.gemspec

@@ -40,7 +40,7 @@ Gem::Specification.new do |s|
  s.add_runtime_dependency("jekyll-watch",          "~> 2.0")
  s.add_runtime_dependency("kramdown",              "~> 2.3", ">= 2.3.1")
  s.add_runtime_dependency("kramdown-parser-gfm",   "~> 1.0")
-  s.add_runtime_dependency("liquid",                "~> 4.0")
+  s.add_runtime_dependency("liquid",                ">= 4.0", "< 5.4")


Suggested change

s.add_runtime_dependency("liquid", ">= 4.0", "< 5.4")

if RUBY_VERSION.to_f < 3.2.0

s.add_runtime_dependency("liquid", "~> 4.0.3")

else

s.add_runtime_dependency("liquid", "~> 5.4.0")

I am fine with this as far as you are. But we need to depend on liquid < 5.4.

- s.add_runtime_dependency("liquid", "~> 5.4.0") + s.add_runtime_dependency("liquid", "~> 5.3.0")

5.4 breaks further as you can see in the the test CI run on my repo: https://github.com/isimluk/jekyll/actions/runs/3777777021/jobs/6421776463

liquid 5.4.0 changed Registers to be a plain old ruby class whilst previously it has been Hash. I have a PR for that at Shopify/liquid#1663 but 5.4.0 remains unusable.

What do you think of just relying on RUBY_VERSION to determine the version of liquid to use?

I actually tend to think that is good idea. But how do you feel about the removal of liquid-c support that this PR currently brings ?

Yeah setting the latest allowed version to 5.4 sounds reasonable to me.

Removing liquid-c is outside my understanding. It looks like it was introduced years ago, but wasn't actually being required until #7792 came along. Maybe it's fine to remove

invalidusrname · 2022-12-30T16:47:10Z

appveyor.yml

@@ -21,6 +21,10 @@ environment:
    - RUBY_FOLDER_VER: "27"
      TZINFO_VERSION: "~> 2.0"
      TEST_SUITE: "test"
+    - RUBY_FOLDER_VER: "27"
+      LIQUID_VERSION: "~> 4.0"


Suggested change

LIQUID_VERSION: "~> 4.0"

invalidusrname · 2022-12-30T16:48:05Z

Gemfile

+if ENV["LIQUID_VERSION"].to_s.empty?
+  gem "liquid", "~> 5.3.0"
+else
+  gem "liquid", ENV["LIQUID_VERSION"]
+end


Suggested change

if ENV["LIQUID_VERSION"].to_s.empty?

gem "liquid", "~> 5.3.0"

else

gem "liquid", ENV["LIQUID_VERSION"]

end

if RUBY_VERSION.to_f < 3.2.0

gem "liquid", "~> 4.0.3"

else

gem "liquid", "~> 5.4.0"

end

invalidusrname · 2022-12-30T16:48:40Z

.github/workflows/ci.yml

+          - label: Ruby 2.7 with Liquid v4
+            ruby_version: "2.7"
+            liquid_version: "~> 4.0"


Suggested change

- label: Ruby 2.7 with Liquid v4

ruby_version: "2.7"

liquid_version: "~> 4.0"

invalidusrname · 2022-12-30T16:48:48Z

.github/workflows/ci.yml

+          - label: Ruby 3.1.2 with Liquid v4
+            ruby_version: "3.1.2"
+            liquid_version: "~> 4.0"


Suggested change

- label: Ruby 3.1.2 with Liquid v4

ruby_version: "3.1.2"

liquid_version: "~> 4.0"

ashmaroli added 3 commits December 25, 2022 17:26

Support Liquid v4 nd v5

bc0adc2

Filter ENV with empty strings

b33dd6e

Load monkey-patch after loading liquid-c

0fbed5c

isimluk closed this Dec 25, 2022

isimluk changed the title ~~Pr 9036~~ WIP: Experiments with liquid 5.x Dec 25, 2022

isimluk reopened this Dec 25, 2022

isimluk force-pushed the pr-9036 branch from 7c6e4f8 to 5629f7b Compare December 25, 2022 19:32

Fix tests to pass with liquid 4 or liquid <= 5.3

7cb6c87

We need to disable liquid-5.4 and liquid-c to have the tests pass initially.

isimluk force-pushed the pr-9036 branch from 5629f7b to b2c30dd Compare December 25, 2022 22:02

liquid 5 support: be flexible when reading registers

22961f5

Before Liquid 5.4, context.registers are good old plain Hash. After Liquid 5.4, context.registers is_a Liquid::Registers

isimluk force-pushed the pr-9036 branch from b2c30dd to 22961f5 Compare December 25, 2022 22:56

This was referenced Dec 25, 2022

WIP: Support liquid 5.4.0 in order to support ruby 3.2 jekyll/jekyll-seo-tag#479

Closed

Support liquid 5.4.0 in order to support ruby 3.2 isimluk/jekyll-seo-tag#1

Open

invalidusrname mentioned this pull request Dec 30, 2022

[Bug]: Can't serve and build pages with ruby 3.2 #9231

Closed

invalidusrname reviewed Dec 30, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Experiments with liquid 5.x #9229

WIP: Experiments with liquid 5.x #9229

isimluk commented Dec 25, 2022 •

edited

bewuethr commented Dec 25, 2022

invalidusrname left a comment

invalidusrname Dec 30, 2022

isimluk Dec 30, 2022 •

edited

isimluk Dec 30, 2022

invalidusrname Dec 30, 2022

invalidusrname Dec 30, 2022

invalidusrname Dec 30, 2022

invalidusrname Dec 30, 2022

invalidusrname Dec 30, 2022

-  s.add_runtime_dependency("liquid",                ">= 4.0", "< 5.4")
+  if RUBY_VERSION.to_f < 3.2.0
+    s.add_runtime_dependency("liquid", "~> 4.0.3")
+  else
+    s.add_runtime_dependency("liquid", "~> 5.4.0")

	- label: Ruby 2.7 with Liquid v4
	ruby_version: "2.7"
	liquid_version: "~> 4.0"

	- label: Ruby 3.1.2 with Liquid v4
	ruby_version: "3.1.2"
	liquid_version: "~> 4.0"

WIP: Experiments with liquid 5.x #9229

Are you sure you want to change the base?

WIP: Experiments with liquid 5.x #9229

Conversation

isimluk commented Dec 25, 2022 • edited

Summary

Context

WIP

bewuethr commented Dec 25, 2022

invalidusrname left a comment

Choose a reason for hiding this comment

invalidusrname Dec 30, 2022

Choose a reason for hiding this comment

isimluk Dec 30, 2022 • edited

Choose a reason for hiding this comment

isimluk Dec 30, 2022

Choose a reason for hiding this comment

invalidusrname Dec 30, 2022

Choose a reason for hiding this comment

invalidusrname Dec 30, 2022

Choose a reason for hiding this comment

invalidusrname Dec 30, 2022

Choose a reason for hiding this comment

invalidusrname Dec 30, 2022

Choose a reason for hiding this comment

invalidusrname Dec 30, 2022

Choose a reason for hiding this comment

isimluk commented Dec 25, 2022 •

edited

isimluk Dec 30, 2022 •

edited