Handle Ruby 3.5 new Set class by casperisfine · Pull Request #725 · ruby/psych

casperisfine · 2025-04-30T07:15:07Z

Since Set no longer is a regular object class holding a Hash it needs to be specially handled.

FYI: @tenderlove @jeremyevans
NB: Only added tests for now to show the problem. I think ideally we'd keep the exact same payload so that we don't break people applications.

Since `Set` no longer is a regular object class holding a Hash it needs to be specially handled.

casperisfine · 2025-04-30T07:23:12Z

+if defined?(::Set) && Object.const_source_location(:Set) == ["ruby", 0]
+ class Set
+ def encode_with(coder)
+ coder["hash"] = to_h
+ end
+
+ def init_with(coder)
+ replace(coder["hash"].keys)
+ end
+ end
+end


So I added the implementation, now I'm just not sure what is the best way to check for builtin Set. I could check RUBY_VERSION >= "3.5" but I don't know if that may cause issues with JRuby / Truffle ruby?
I could check if Set#initialize has a source_location, but then what if someone decorated it?
Etc.

You can check if it has a hash ivar

Good point. if Set.new.instance_variable_get(:@hash), but I think JRuby might not like it because they already move Set into a core class a long time ago.
So perhaps I should check with Pysch.load(Pysch.dump(Set.new([1, 2]))). I'll sleep on it.

I guess a broader question is: why not move to a more general-purpose implementation completely? Have the behavior change (looks like a set -> doesn't look like a set) after 3.5. Does inter-version consistency matter?

Have the behavior change (looks like a set -> doesn't look like a set) after 3.5. Does inter-version consistency matter?
I think it does. Many shops (including Shopify) have Psych-serialized data in their databases that they expect to be able to deserialize, so the format should match. Similarly, we might need to roll-back Ruby and/or gem versions, and that should not result in the - now new - format in the database being unreadable.

Alright, another silly question: I tested psych with the new Set implementation and it passes all tests on Ruby HEAD and with psych HEAD. What new test should I be messing around with?

Oh, that's an internal implementation

casperisfine · 2025-05-02T15:12:01Z

Ref: rails/bootsnap#498

So since Pysch is a default gem, apps may be on Ruby 2.5 but run an older version of Pysch.

So perhaps this patch isn't correct and we should instead have these methods in ruby itself.

Followup: ruby/psych#725 Adding these methods from `psych` isn't enough because `psych` is a default gem, so you may run Ruby 3.5.0 with a very old `psych` version that doesn't know how to serialize Ruby 3.5 sets. It's a bit weird to implement these methods directly into Ruby, but `encode_with/init_with` is a common serialization convention so it makes sense.

casperisfine · 2025-04-30T07:15:07Z

Since Set no longer is a regular object class holding a Hash it needs to be specially handled.

FYI: @tenderlove @jeremyevans
NB: Only added tests for now to show the problem. I think ideally we'd keep the exact same payload so that we don't break people applications.

Since `Set` no longer is a regular object class holding a Hash it needs to be specially handled.

casperisfine · 2025-04-30T07:23:12Z

+if defined?(::Set) && Object.const_source_location(:Set) == ["ruby", 0]
+ class Set
+ def encode_with(coder)
+ coder["hash"] = to_h
+ end
+
+ def init_with(coder)
+ replace(coder["hash"].keys)
+ end
+ end
+end


So I added the implementation, now I'm just not sure what is the best way to check for builtin Set. I could check RUBY_VERSION >= "3.5" but I don't know if that may cause issues with JRuby / Truffle ruby?
I could check if Set#initialize has a source_location, but then what if someone decorated it?
Etc.

You can check if it has a hash ivar

Good point. if Set.new.instance_variable_get(:@hash), but I think JRuby might not like it because they already move Set into a core class a long time ago.
So perhaps I should check with Pysch.load(Pysch.dump(Set.new([1, 2]))). I'll sleep on it.

I guess a broader question is: why not move to a more general-purpose implementation completely? Have the behavior change (looks like a set -> doesn't look like a set) after 3.5. Does inter-version consistency matter?

Have the behavior change (looks like a set -> doesn't look like a set) after 3.5. Does inter-version consistency matter?
I think it does. Many shops (including Shopify) have Psych-serialized data in their databases that they expect to be able to deserialize, so the format should match. Similarly, we might need to roll-back Ruby and/or gem versions, and that should not result in the - now new - format in the database being unreadable.

Alright, another silly question: I tested psych with the new Set implementation and it passes all tests on Ruby HEAD and with psych HEAD. What new test should I be messing around with?

Oh, that's an internal implementation

casperisfine · 2025-05-02T15:12:01Z

Ref: rails/bootsnap#498

So since Pysch is a default gem, apps may be on Ruby 2.5 but run an older version of Pysch.

So perhaps this patch isn't correct and we should instead have these methods in ruby itself.

Followup: ruby/psych#725 Adding these methods from `psych` isn't enough because `psych` is a default gem, so you may run Ruby 3.5.0 with a very old `psych` version that doesn't know how to serialize Ruby 3.5 sets. It's a bit weird to implement these methods directly into Ruby, but `encode_with/init_with` is a common serialization convention so it makes sense.

casperisfine · 2025-04-30T07:15:07Z

Since Set no longer is a regular object class holding a Hash it needs to be specially handled.

FYI: @tenderlove @jeremyevans
NB: Only added tests for now to show the problem. I think ideally we'd keep the exact same payload so that we don't break people applications.

Since `Set` no longer is a regular object class holding a Hash it needs to be specially handled.

casperisfine · 2025-04-30T07:23:12Z

+if defined?(::Set) && Object.const_source_location(:Set) == ["ruby", 0]
+ class Set
+ def encode_with(coder)
+ coder["hash"] = to_h
+ end
+
+ def init_with(coder)
+ replace(coder["hash"].keys)
+ end
+ end
+end


So I added the implementation, now I'm just not sure what is the best way to check for builtin Set. I could check RUBY_VERSION >= "3.5" but I don't know if that may cause issues with JRuby / Truffle ruby?
I could check if Set#initialize has a source_location, but then what if someone decorated it?
Etc.

You can check if it has a hash ivar

Good point. if Set.new.instance_variable_get(:@hash), but I think JRuby might not like it because they already move Set into a core class a long time ago.
So perhaps I should check with Pysch.load(Pysch.dump(Set.new([1, 2]))). I'll sleep on it.

I guess a broader question is: why not move to a more general-purpose implementation completely? Have the behavior change (looks like a set -> doesn't look like a set) after 3.5. Does inter-version consistency matter?

Have the behavior change (looks like a set -> doesn't look like a set) after 3.5. Does inter-version consistency matter?
I think it does. Many shops (including Shopify) have Psych-serialized data in their databases that they expect to be able to deserialize, so the format should match. Similarly, we might need to roll-back Ruby and/or gem versions, and that should not result in the - now new - format in the database being unreadable.

Alright, another silly question: I tested psych with the new Set implementation and it passes all tests on Ruby HEAD and with psych HEAD. What new test should I be messing around with?

Oh, that's an internal implementation

casperisfine · 2025-05-02T15:12:01Z

Ref: rails/bootsnap#498

So since Pysch is a default gem, apps may be on Ruby 2.5 but run an older version of Pysch.

So perhaps this patch isn't correct and we should instead have these methods in ruby itself.

Followup: ruby/psych#725 Adding these methods from `psych` isn't enough because `psych` is a default gem, so you may run Ruby 3.5.0 with a very old `psych` version that doesn't know how to serialize Ruby 3.5 sets. It's a bit weird to implement these methods directly into Ruby, but `encode_with/init_with` is a common serialization convention so it makes sense.

casperisfine · 2025-04-30T07:15:07Z

Since Set no longer is a regular object class holding a Hash it needs to be specially handled.

FYI: @tenderlove @jeremyevans
NB: Only added tests for now to show the problem. I think ideally we'd keep the exact same payload so that we don't break people applications.

Since `Set` no longer is a regular object class holding a Hash it needs to be specially handled.

casperisfine · 2025-04-30T07:23:12Z

+if defined?(::Set) && Object.const_source_location(:Set) == ["ruby", 0]
+ class Set
+ def encode_with(coder)
+ coder["hash"] = to_h
+ end
+
+ def init_with(coder)
+ replace(coder["hash"].keys)
+ end
+ end
+end


So I added the implementation, now I'm just not sure what is the best way to check for builtin Set. I could check RUBY_VERSION >= "3.5" but I don't know if that may cause issues with JRuby / Truffle ruby?
I could check if Set#initialize has a source_location, but then what if someone decorated it?
Etc.

You can check if it has a hash ivar

Good point. if Set.new.instance_variable_get(:@hash), but I think JRuby might not like it because they already move Set into a core class a long time ago.
So perhaps I should check with Pysch.load(Pysch.dump(Set.new([1, 2]))). I'll sleep on it.

I guess a broader question is: why not move to a more general-purpose implementation completely? Have the behavior change (looks like a set -> doesn't look like a set) after 3.5. Does inter-version consistency matter?

Have the behavior change (looks like a set -> doesn't look like a set) after 3.5. Does inter-version consistency matter?
I think it does. Many shops (including Shopify) have Psych-serialized data in their databases that they expect to be able to deserialize, so the format should match. Similarly, we might need to roll-back Ruby and/or gem versions, and that should not result in the - now new - format in the database being unreadable.

Alright, another silly question: I tested psych with the new Set implementation and it passes all tests on Ruby HEAD and with psych HEAD. What new test should I be messing around with?

Oh, that's an internal implementation

casperisfine · 2025-05-02T15:12:01Z

Ref: rails/bootsnap#498

So since Pysch is a default gem, apps may be on Ruby 2.5 but run an older version of Pysch.

So perhaps this patch isn't correct and we should instead have these methods in ruby itself.

Followup: ruby/psych#725 Adding these methods from `psych` isn't enough because `psych` is a default gem, so you may run Ruby 3.5.0 with a very old `psych` version that doesn't know how to serialize Ruby 3.5 sets. It's a bit weird to implement these methods directly into Ruby, but `encode_with/init_with` is a common serialization convention so it makes sense.

casperisfine · 2025-04-30T07:15:07Z

Since Set no longer is a regular object class holding a Hash it needs to be specially handled.

FYI: @tenderlove @jeremyevans
NB: Only added tests for now to show the problem. I think ideally we'd keep the exact same payload so that we don't break people applications.

Since `Set` no longer is a regular object class holding a Hash it needs to be specially handled.

casperisfine · 2025-04-30T07:23:12Z

+if defined?(::Set) && Object.const_source_location(:Set) == ["ruby", 0]
+ class Set
+ def encode_with(coder)
+ coder["hash"] = to_h
+ end
+
+ def init_with(coder)
+ replace(coder["hash"].keys)
+ end
+ end
+end


So I added the implementation, now I'm just not sure what is the best way to check for builtin Set. I could check RUBY_VERSION >= "3.5" but I don't know if that may cause issues with JRuby / Truffle ruby?
I could check if Set#initialize has a source_location, but then what if someone decorated it?
Etc.

You can check if it has a hash ivar

Good point. if Set.new.instance_variable_get(:@hash), but I think JRuby might not like it because they already move Set into a core class a long time ago.
So perhaps I should check with Pysch.load(Pysch.dump(Set.new([1, 2]))). I'll sleep on it.

I guess a broader question is: why not move to a more general-purpose implementation completely? Have the behavior change (looks like a set -> doesn't look like a set) after 3.5. Does inter-version consistency matter?

Have the behavior change (looks like a set -> doesn't look like a set) after 3.5. Does inter-version consistency matter?
I think it does. Many shops (including Shopify) have Psych-serialized data in their databases that they expect to be able to deserialize, so the format should match. Similarly, we might need to roll-back Ruby and/or gem versions, and that should not result in the - now new - format in the database being unreadable.

Alright, another silly question: I tested psych with the new Set implementation and it passes all tests on Ruby HEAD and with psych HEAD. What new test should I be messing around with?

Oh, that's an internal implementation

casperisfine · 2025-05-02T15:12:01Z

Ref: rails/bootsnap#498

So since Pysch is a default gem, apps may be on Ruby 2.5 but run an older version of Pysch.

So perhaps this patch isn't correct and we should instead have these methods in ruby itself.

Followup: ruby/psych#725 Adding these methods from `psych` isn't enough because `psych` is a default gem, so you may run Ruby 3.5.0 with a very old `psych` version that doesn't know how to serialize Ruby 3.5 sets. It's a bit weird to implement these methods directly into Ruby, but `encode_with/init_with` is a common serialization convention so it makes sense.

casperisfine · 2025-04-30T07:15:07Z

Since Set no longer is a regular object class holding a Hash it needs to be specially handled.

FYI: @tenderlove @jeremyevans
NB: Only added tests for now to show the problem. I think ideally we'd keep the exact same payload so that we don't break people applications.

Since `Set` no longer is a regular object class holding a Hash it needs to be specially handled.

casperisfine · 2025-04-30T07:23:12Z

+if defined?(::Set) && Object.const_source_location(:Set) == ["ruby", 0]
+ class Set
+ def encode_with(coder)
+ coder["hash"] = to_h
+ end
+
+ def init_with(coder)
+ replace(coder["hash"].keys)
+ end
+ end
+end


So I added the implementation, now I'm just not sure what is the best way to check for builtin Set. I could check RUBY_VERSION >= "3.5" but I don't know if that may cause issues with JRuby / Truffle ruby?
I could check if Set#initialize has a source_location, but then what if someone decorated it?
Etc.

You can check if it has a hash ivar

Good point. if Set.new.instance_variable_get(:@hash), but I think JRuby might not like it because they already move Set into a core class a long time ago.
So perhaps I should check with Pysch.load(Pysch.dump(Set.new([1, 2]))). I'll sleep on it.

I guess a broader question is: why not move to a more general-purpose implementation completely? Have the behavior change (looks like a set -> doesn't look like a set) after 3.5. Does inter-version consistency matter?

Have the behavior change (looks like a set -> doesn't look like a set) after 3.5. Does inter-version consistency matter?
I think it does. Many shops (including Shopify) have Psych-serialized data in their databases that they expect to be able to deserialize, so the format should match. Similarly, we might need to roll-back Ruby and/or gem versions, and that should not result in the - now new - format in the database being unreadable.

Alright, another silly question: I tested psych with the new Set implementation and it passes all tests on Ruby HEAD and with psych HEAD. What new test should I be messing around with?

Oh, that's an internal implementation

casperisfine · 2025-05-02T15:12:01Z

Ref: rails/bootsnap#498

So since Pysch is a default gem, apps may be on Ruby 2.5 but run an older version of Pysch.

So perhaps this patch isn't correct and we should instead have these methods in ruby itself.

Followup: ruby/psych#725 Adding these methods from `psych` isn't enough because `psych` is a default gem, so you may run Ruby 3.5.0 with a very old `psych` version that doesn't know how to serialize Ruby 3.5 sets. It's a bit weird to implement these methods directly into Ruby, but `encode_with/init_with` is a common serialization convention so it makes sense.

casperisfine · 2025-04-30T07:15:07Z

Since Set no longer is a regular object class holding a Hash it needs to be specially handled.

FYI: @tenderlove @jeremyevans
NB: Only added tests for now to show the problem. I think ideally we'd keep the exact same payload so that we don't break people applications.

Since `Set` no longer is a regular object class holding a Hash it needs to be specially handled.

casperisfine · 2025-04-30T07:23:12Z

+if defined?(::Set) && Object.const_source_location(:Set) == ["ruby", 0]
+ class Set
+ def encode_with(coder)
+ coder["hash"] = to_h
+ end
+
+ def init_with(coder)
+ replace(coder["hash"].keys)
+ end
+ end
+end


So I added the implementation, now I'm just not sure what is the best way to check for builtin Set. I could check RUBY_VERSION >= "3.5" but I don't know if that may cause issues with JRuby / Truffle ruby?
I could check if Set#initialize has a source_location, but then what if someone decorated it?
Etc.

You can check if it has a hash ivar

Good point. if Set.new.instance_variable_get(:@hash), but I think JRuby might not like it because they already move Set into a core class a long time ago.
So perhaps I should check with Pysch.load(Pysch.dump(Set.new([1, 2]))). I'll sleep on it.

I guess a broader question is: why not move to a more general-purpose implementation completely? Have the behavior change (looks like a set -> doesn't look like a set) after 3.5. Does inter-version consistency matter?

Have the behavior change (looks like a set -> doesn't look like a set) after 3.5. Does inter-version consistency matter?
I think it does. Many shops (including Shopify) have Psych-serialized data in their databases that they expect to be able to deserialize, so the format should match. Similarly, we might need to roll-back Ruby and/or gem versions, and that should not result in the - now new - format in the database being unreadable.

Alright, another silly question: I tested psych with the new Set implementation and it passes all tests on Ruby HEAD and with psych HEAD. What new test should I be messing around with?

Oh, that's an internal implementation

casperisfine · 2025-05-02T15:12:01Z

Ref: rails/bootsnap#498

So since Pysch is a default gem, apps may be on Ruby 2.5 but run an older version of Pysch.

So perhaps this patch isn't correct and we should instead have these methods in ruby itself.

Followup: ruby/psych#725 Adding these methods from `psych` isn't enough because `psych` is a default gem, so you may run Ruby 3.5.0 with a very old `psych` version that doesn't know how to serialize Ruby 3.5 sets. It's a bit weird to implement these methods directly into Ruby, but `encode_with/init_with` is a common serialization convention so it makes sense.

casperisfine · 2025-04-30T07:15:07Z

Since Set no longer is a regular object class holding a Hash it needs to be specially handled.

FYI: @tenderlove @jeremyevans
NB: Only added tests for now to show the problem. I think ideally we'd keep the exact same payload so that we don't break people applications.

Since `Set` no longer is a regular object class holding a Hash it needs to be specially handled.

casperisfine · 2025-04-30T07:23:12Z

+if defined?(::Set) && Object.const_source_location(:Set) == ["ruby", 0]
+ class Set
+ def encode_with(coder)
+ coder["hash"] = to_h
+ end
+
+ def init_with(coder)
+ replace(coder["hash"].keys)
+ end
+ end
+end


So I added the implementation, now I'm just not sure what is the best way to check for builtin Set. I could check RUBY_VERSION >= "3.5" but I don't know if that may cause issues with JRuby / Truffle ruby?
I could check if Set#initialize has a source_location, but then what if someone decorated it?
Etc.

You can check if it has a hash ivar

Good point. if Set.new.instance_variable_get(:@hash), but I think JRuby might not like it because they already move Set into a core class a long time ago.
So perhaps I should check with Pysch.load(Pysch.dump(Set.new([1, 2]))). I'll sleep on it.

I guess a broader question is: why not move to a more general-purpose implementation completely? Have the behavior change (looks like a set -> doesn't look like a set) after 3.5. Does inter-version consistency matter?

Have the behavior change (looks like a set -> doesn't look like a set) after 3.5. Does inter-version consistency matter?
I think it does. Many shops (including Shopify) have Psych-serialized data in their databases that they expect to be able to deserialize, so the format should match. Similarly, we might need to roll-back Ruby and/or gem versions, and that should not result in the - now new - format in the database being unreadable.

Alright, another silly question: I tested psych with the new Set implementation and it passes all tests on Ruby HEAD and with psych HEAD. What new test should I be messing around with?

Oh, that's an internal implementation

casperisfine · 2025-05-02T15:12:01Z

Ref: rails/bootsnap#498

So since Pysch is a default gem, apps may be on Ruby 2.5 but run an older version of Pysch.

So perhaps this patch isn't correct and we should instead have these methods in ruby itself.

Followup: ruby/psych#725 Adding these methods from `psych` isn't enough because `psych` is a default gem, so you may run Ruby 3.5.0 with a very old `psych` version that doesn't know how to serialize Ruby 3.5 sets. It's a bit weird to implement these methods directly into Ruby, but `encode_with/init_with` is a common serialization convention so it makes sense.

Handle Ruby 3.5 new Set class
c2d185d
Since `Set` no longer is a regular object class holding a Hash it needs to be specially handled.

casperisfine force-pushed the fix-ruby-3.5-sets branch from 65d941e to c2d185dCompare April 30, 2025 07:21

casperisfine commented Apr 30, 2025
View reviewed changes

tenderlove approved these changes Apr 30, 2025
View reviewed changes

tenderlove merged commit 43f6fc6 into ruby:masterApr 30, 2025

byroot mentioned this pull request May 2, 2025
Implement Set#init_with and Set#encode_with builtin ruby/ruby#13240
Closed