Switch to Ruby 3.4.2 and 3.2.7 #1245

herwinw · 2025-02-15T08:22:14Z

Please take extra note of the second commit: the function rb_undefine_finalizer should have been enabled again in Ruby 3.4.2 (it was missing in 3.4.0/3.4.1), but I could not find a way to determine this compile time. If someone more familiar with the MRI internals could take a look at it, please do.

eregon · 2025-02-15T13:19:01Z

optional/capi/ext/finalizer_spec.c

@@ -12,7 +12,7 @@ static VALUE define_finalizer(VALUE self, VALUE obj, VALUE finalizer) {
 static VALUE undefine_finalizer(VALUE self, VALUE obj) {
 // Ruby 3.4.0 and 3.4.1 have a bug where rb_undefine_finalizer is missing
 // See: https://bugs.ruby-lang.org/issues/20981
-#if RUBY_API_VERSION_CODE == 30400 && (RUBY_VERSION_TEENY == 0 || RUBY_VERSION_TEENY == 1)


I think we shouldn't need to change this, i.e. this spec should now work on 3.4.2+

Ah the commit message explains the problem

See https://github.com/ruby/spec/blob/master/optional/capi/ext/rubyspec.h which uses RUBY_VERSION_MAJOR/MINOR/TEENY.

I wonder if we fall into

spec/optional/capi/ext/rubyspec.h

Line 40 in 6f2ec17

#define RUBY_VERSION_TEENY RUBY_API_VERSION_TEENY

and then indeed it would be 0 due to RUBY_API_VERSION_* being the ABI version, which doesn't change on minor or patch releases.

I asked on the CRuby Slack.
See https://github.com/ruby/ruby/blob/23c4ac9559a05d7be4534d8e552d8dc95e272867/version.h#L12 and https://github.com/ruby/ruby/blob/master/include/ruby/version.h.

https://github.com/ruby/ruby/blob/ruby_3_4/version.h#L12 so it should work.
Maybe the problem is the extension didn't recompile locally?

I've tested this with an rbenv install, and the version.h (the one in the root, so not the ruby/version.h does not get installed into the include path. So all we have here is the API version

~/.rbenv/versions/3.4.2/include/ruby-3.4.0$ grep -r TEENY ruby/oniguruma.h:#define ONIGURUMA_VERSION_TEENY ONIGMO_VERSION_TEENY ruby/onigmo.h:#define ONIGMO_VERSION_TEENY 3 ruby/version.h:#define RUBY_API_VERSION_TEENY 0 ruby/version.h:#define RUBY_API_VERSION_CODE (RUBY_API_VERSION_MAJOR*10000+RUBY_API_VERSION_MINOR*100+RUBY_API_VERSION_TEENY)

(And ruby-3.4.0 is the only thing in the include folder).

Like I said in the commit message: I would be perfectly fine with enabling this spec on 3.4.2, but I simply cannot find a way to do so without breaking other things. So unless we want to do something like adding a configure script to the ext folder, I'm afraid we have to skip this one on every 3.4 release.

I discussed it with @peterzhu2118 and @byroot, one idea is to add a -D with the real tiny version to the generated extconf.rb here:

spec/optional/capi/spec_helper.rb

Lines 71 to 88 in 6f2ec17

if run_mkmf_in_process

required = require 'mkmf'

# Reinitialize mkmf if already required

init_mkmf unless required

create_makefile(ext, tmpdir)

else

File.write("extconf.rb", <<-RUBY)

require 'mkmf'

$ruby = ENV.values_at('RUBY_EXE', 'RUBY_FLAGS').join(' ')

# MRI magic to consider building non-bundled extensions

$extout = nil

append_cflags '-Wno-declaration-after-statement'

create_makefile(#{ext.inspect})

RUBY

output = ruby_exe("extconf.rb")

raise "extconf failed:\n#{output}" unless $?.success?

$stderr.puts output if debug

end

Like $defs << "-DRUBY_SPEC_RUBY_PATCH_VERSION=#{RUBY_VERSION.split('.')[2]}" (or maybe RUBY_SPEC_RUBY_TEENY_VERSION).
Could you try that?

Also add a comment that RUBY_VERSION_TEENY can't be used because that's not defined for extensions, only in https://github.com/ruby/ruby/blob/ruby_3_4/version.h#L12 which is an internal header not exposed for C extensions.

BTW we should cleanup

spec/optional/capi/ext/rubyspec.h

Lines 37 to 41 in 6f2ec17

#ifndef RUBY_VERSION_MAJOR

#define RUBY_VERSION_MAJOR RUBY_API_VERSION_MAJOR

#define RUBY_VERSION_MINOR RUBY_API_VERSION_MINOR

#define RUBY_VERSION_TEENY RUBY_API_VERSION_TEENY

#endif

and not rely on RUBY_VERSION_* as they only exists within CRuby repo. We should just use RUBY_API_VERSION_* instead below. And probably drop the 3rd argument teeny since it's pointless as always 0. I can do that part.

Actually there is also a simpler solution: since ruby/spec only supports the latest release of each release branch we could unconditionally use rb_undefine_finalizer(), and let 3.4.0/3.4.1 fail as they shouldn't be used.
It seems worth the simplification in this case.

I love simple solutions ;)

Updated to the simple version. I left the version guard for 3.4.0-3.4.1 in the Ruby code, running it with these versions will show a compile warning, but will not break.

BTW we should cleanup

Done in 52178dd

eregon · 2025-02-15T13:19:15Z

optional/capi/finalizer_spec.rb

@@ -22,7 +22,7 @@
  end

  describe "rb_undefine_finalizer" do
-    ruby_bug "#20981", "3.4.0"..."3.4.2" do
+    ruby_bug "#20981", "3.4.0"..."3.5.0" do


Same here, we shouldn't need to change this

This will now cause a compilation warning with Ruby 3.4.0 and 3.4.1.

eregon

Thanks!

…side CRuby repository * This means we cannot test for teeny since it's always 0, but that should be rarely needed. * See #1245 (comment) * See d9e3e4d.

Switch CI to Ruby 3.2.7 and 3.4.2

c7e52a5

eregon reviewed Feb 15, 2025

View reviewed changes

Update finalizer spec to run with Ruby 3.4.2

c61a0dc

This will now cause a compilation warning with Ruby 3.4.0 and 3.4.1.

herwinw force-pushed the ruby_3_4_2 branch from 575abab to c61a0dc Compare February 17, 2025 16:04

eregon approved these changes Feb 17, 2025

View reviewed changes

eregon merged commit 3fd3ad7 into ruby:master Feb 17, 2025
14 checks passed

herwinw deleted the ruby_3_4_2 branch February 18, 2025 06:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch to Ruby 3.4.2 and 3.2.7 #1245

Switch to Ruby 3.4.2 and 3.2.7 #1245

herwinw commented Feb 15, 2025

eregon Feb 15, 2025

eregon Feb 15, 2025

eregon Feb 15, 2025

eregon Feb 15, 2025

eregon Feb 15, 2025

herwinw Feb 16, 2025

eregon Feb 17, 2025 •

edited

Loading

eregon Feb 17, 2025 •

edited

Loading

herwinw Feb 17, 2025

eregon Feb 17, 2025

eregon Feb 15, 2025

eregon left a comment

	if run_mkmf_in_process
	required = require 'mkmf'
	# Reinitialize mkmf if already required
	init_mkmf unless required
	create_makefile(ext, tmpdir)
	else
	File.write("extconf.rb", <<-RUBY)
	require 'mkmf'
	$ruby = ENV.values_at('RUBY_EXE', 'RUBY_FLAGS').join(' ')
	# MRI magic to consider building non-bundled extensions
	$extout = nil
	append_cflags '-Wno-declaration-after-statement'
	create_makefile(#{ext.inspect})
	RUBY
	output = ruby_exe("extconf.rb")
	raise "extconf failed:\n#{output}" unless $?.success?
	$stderr.puts output if debug
	end

	#ifndef RUBY_VERSION_MAJOR
	#define RUBY_VERSION_MAJOR RUBY_API_VERSION_MAJOR
	#define RUBY_VERSION_MINOR RUBY_API_VERSION_MINOR
	#define RUBY_VERSION_TEENY RUBY_API_VERSION_TEENY
	#endif

Switch to Ruby 3.4.2 and 3.2.7 #1245

Switch to Ruby 3.4.2 and 3.2.7 #1245

Conversation

herwinw commented Feb 15, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eregon Feb 17, 2025 • edited Loading

Choose a reason for hiding this comment

eregon Feb 17, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eregon left a comment

Choose a reason for hiding this comment

eregon Feb 17, 2025 •

edited

Loading

eregon Feb 17, 2025 •

edited

Loading