ManageIQ / manageiq

ManageIQ Open-Source Management Platform
https://manageiq.org
Apache License 2.0
1.35k stars 900 forks source link

Upgrading ruby to 3.3 #23142

Closed kbrock closed 2 weeks ago

kbrock commented 3 months ago

3.2 ref

Highlights:

3.3

punt:

External PRs

These were changed in external libraries and make using them easier, but are not required for our upgrade.

Related PRs

providers:

current solution

gem install --verbose ovirt-engine-sdk -v4.6.0 -- --with-cflags="-Wno-error=incompatible-function-pointer-types -Wno-error=implicit-function-declaration"
bdunne commented 3 months ago

For build, EL9 ships with ruby 3.0 by default and has dnf modules for 3.1 and 3.3, but not 3.2.

Fryguy commented 3 months ago

so weird that el9 jumped a version 😕

kbrock commented 3 months ago

The gem build failure for ovirt-engine-sdk is frustrating. I can see that @jrafanie and others had issues building it for https://github.com/oVirt/ovirt-engine-sdk-ruby/issues/11

Hopefully it shouldn't be too hard. Guess this is where it all begins. Wish they had a C guide titled "this is what changed over the past 20 years" - because that code has a bunch of stuff I do not recognize

jrafanie commented 3 months ago
ov_http_request.c:75:29: warning: excess elements in array initializer
   75 |         .reserved = { NULL, NULL }
      |                             ^~~~
ov_http_request.c:75:29: note: (near initialization for
‘ov_http_request_type.function.reserved’)
ov_http_request.c: In function ‘ov_http_request_define’:
ov_http_request.c:347:77: error: ‘rb_cData’ undeclared (first use in this
function)
347 |     ov_http_request_class = rb_define_class_under(ov_module,
"HttpRequest", rb_cData);
|                                                                         
^~~~~~~~
ov_http_request.c:347:77: note: each undeclared identifier is reported only once
for each function it appears in
ov_http_request.c: At top level:
cc1: note: unrecognized command-line option ‘-Wno-self-assign’ may have been
intended to silence earlier diagnostics
cc1: note: unrecognized command-line option ‘-Wno-parentheses-equality’ may have
been intended to silence earlier diagnostics
cc1: note: unrecognized command-line option ‘-Wno-constant-logical-operand’ may
have been intended to silence earlier diagnostics
make: *** [Makefile:248: ov_http_request.o] Error 1

This looks alittle different. There's no option reported in the output to silence it.

Fryguy commented 3 months ago

That's also an error, whereas the others were warnings.

jrafanie commented 3 months ago

That's also an error, whereas the others were warnings.

sorry, it's different than the one we had previously where it told you that you can ignore it by opting into that behavior:

implicit function declarations [-Wimplicit-function-declaration]

https://github.com/oVirt/ovirt-engine-sdk-ruby/issues/11

This one is a undeclared variable if I'm reading correctly.

micwoj92 commented 2 months ago

@jrafanie in the linked issue from your last comment you tried to build version 4.6.0 of ovirt engine sdk. This error is because the CI pulls 4.4.1 and it is unrelated. This issue has already been fixed with latest version. See https://github.com/oVirt/ovirt-engine-sdk-ruby/commit/97df88cb4c20a84632559c4d3cf1ac0d6224a2a0

kbrock commented 2 months ago

@miq-bot cross_repo_test ManageIQ#23142 including ManageIQ/manageiq-providers-ovirt#676 ManageIQ/manageiq-providers-red_hat_virtualization#54

Fryguy commented 2 months ago

@kbrock I find this site more useful for detailing changes in Ruby:

kbrock commented 3 weeks ago

update:

unsure: Do we want to force ruby => 3.1?

kbrock commented 2 weeks ago

update:

miq-bot commented 2 weeks ago

Checked commit https://github.com/kbrock/manageiq/commit/1e99a384de26844bd5c362f3f48a14d4fc5406ea with ruby 3.1.5, rubocop 1.56.3, haml-lint 0.51.0, and yamllint 1 file checked, 0 offenses detected Everything looks fine. :star:

jrafanie commented 2 weeks ago
  • Hash#shift (empty returns nil instead of default)

Is there anything we need to do here? I assume it's difficult to find.

kbrock commented 2 weeks ago

@jrafanie no. I searched through the code and it looks like we are all set, but left it as "outstanding" to let people know that a few of these may still be lurking.

ok. Ignoring specs:

jrafanie commented 2 weeks ago

@bdunne are you good with smoke tests of appliances/pods with ruby 3.3? Are we good to go?

@kbrock add radjabov if this is for backport.

jrafanie commented 2 weeks ago

LGTM, @bdunne @Fryguy merge when ready.

jrafanie commented 2 weeks ago

🎉 🕺 🏆 🎉 🕺 🏆 🎉 🕺 🏆

Fryguy commented 2 weeks ago

Backported to radjabov in commit a4e4a739cdd1feeda8948253bfdd8564b6508592.

commit a4e4a739cdd1feeda8948253bfdd8564b6508592
Author: Jason Frey <fryguy9@gmail.com>
Date:   Wed Nov 13 17:30:47 2024 -0500

    Merge pull request #23142 from kbrock/ruby33

    Upgrading ruby to 3.3

    (cherry picked from commit 3fb39c3b87d88a144d8bf03cf49b7a42dbf5bd8d)