Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Ruby 3.3 #79

Merged
merged 5 commits into from Aug 3, 2023
Merged

Support Ruby 3.3 #79

merged 5 commits into from Aug 3, 2023

Conversation

k0kubun
Copy link
Contributor

@k0kubun k0kubun commented Jul 26, 2023

Fix #77

This follows the example fix shown in [Feature #19057] at 94dbdd9497. I also changed #if HAVE_RB_IO_T to #if defined(HAVE_RB_IO_T) so that it will not print warnings when not defined.

@ioquatix
Copy link
Member

Thanks for this!

@tarcieri if you don't want to maintain this gem, do you want to:

  1. Transfer it to the socketry org
  2. Add me as an owner on Rubygems

I can triage this issue and release.

LMK what you prefer to do.

@tarcieri
Copy link
Collaborator

Sure, that sounds good to me. Let's transfer it to socketry

@ioquatix
Copy link
Member

Okay, I'll await your transfer.

@tarcieri
Copy link
Collaborator

It's transferred

@ioquatix
Copy link
Member

Thanks @tarcieri do you mind adding me as an owner on Rubygems.org too? Thanks!

@tarcieri
Copy link
Collaborator

Invite sent

@ioquatix
Copy link
Member

@k0kubun I want to add CI before we merge this PR, please give me a couple of days to sort it out.

@k0kubun
Copy link
Contributor Author

k0kubun commented Jul 27, 2023

Sure, that makes sense. Thank you for working on that.

@ioquatix ioquatix force-pushed the ruby-3-3 branch 2 times, most recently from b3bc86f to 17b12fa Compare August 2, 2023 10:36
@ioquatix
Copy link
Member

ioquatix commented Aug 2, 2023

@k0kubun I could not get this to pass CI, but master branch is passing. Do you mind taking a look if you have time?

@ioquatix
Copy link
Member

ioquatix commented Aug 2, 2023

(it might just be flaky test suite... I tried several times, even master is a bit unreliable).

ext/iobuffer/iobuffer.c Outdated Show resolved Hide resolved
ext/iobuffer/iobuffer.c Outdated Show resolved Hide resolved
@k0kubun k0kubun requested a review from ioquatix August 2, 2023 17:42
@ioquatix
Copy link
Member

ioquatix commented Aug 2, 2023

This looks okay to me now. I think we should try to move fptr usage to old code path. Since ultimately we want to avoid it. Do you have time to make that change?

@k0kubun
Copy link
Contributor Author

k0kubun commented Aug 2, 2023

Well, two out of the three fptr references are used for rb_io_set_nonblock, which does expect fptr. I don't think it's the scope of this PR to move away from rb_io_set_nonblock.

@k0kubun
Copy link
Contributor Author

k0kubun commented Aug 2, 2023

Fixed the other one though 390ff4e. Only this one makes sense to me.

@ioquatix ioquatix merged commit 273ca95 into socketry:master Aug 3, 2023
8 checks passed
@ioquatix
Copy link
Member

ioquatix commented Aug 3, 2023

Thanks for your effort. I'll try to cut a release soon.

@k0kubun k0kubun deleted the ruby-3-3 branch August 3, 2023 05:42
@ioquatix
Copy link
Member

ioquatix commented Aug 3, 2023

@k0kubun it sounds like you may have software using cool.io - can you test it with the latest release if so?

@ioquatix ioquatix added this to the v1.8.0 milestone Aug 3, 2023
k0kubun added a commit to Shopify/yjit-bench that referenced this pull request Aug 3, 2023
My contribution socketry/cool.io#79 has been
merged and released.
@k0kubun
Copy link
Contributor Author

k0kubun commented Aug 3, 2023

I upgraded cool.io to the released version on yjit-bench Shopify/yjit-bench@2858bc3.

The extension compiles correctly on Ruby master. The fluentd benchmark finishes successfully. I'm not sure if the benchmark goes through cool.io code, but at least bundle install is not broken anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cool.io is no longer buildable on Ruby 3.3.0-dev
3 participants