Skip to content

Support Ruby 3.3#79

Merged
ioquatix merged 5 commits into
socketry:masterfrom
k0kubun:ruby-3-3
Aug 3, 2023
Merged

Support Ruby 3.3#79
ioquatix merged 5 commits into
socketry:masterfrom
k0kubun:ruby-3-3

Conversation

@k0kubun

@k0kubun k0kubun commented Jul 26, 2023

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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
Copy Markdown
Collaborator

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

@ioquatix

Copy link
Copy Markdown
Member

Okay, I'll await your transfer.

@tarcieri

Copy link
Copy Markdown
Collaborator

It's transferred

@ioquatix

Copy link
Copy Markdown
Member

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

@tarcieri

Copy link
Copy Markdown
Collaborator

Invite sent

@ioquatix

Copy link
Copy Markdown
Member

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

@k0kubun

k0kubun commented Jul 27, 2023

Copy link
Copy Markdown
Contributor Author

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

ioquatix commented Aug 2, 2023

Copy link
Copy Markdown
Member

@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

ioquatix commented Aug 2, 2023

Copy link
Copy Markdown
Member

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

Comment thread ext/iobuffer/iobuffer.c Outdated
Comment thread ext/iobuffer/iobuffer.c Outdated
@k0kubun k0kubun requested a review from ioquatix August 2, 2023 17:42
@ioquatix

ioquatix commented Aug 2, 2023

Copy link
Copy Markdown
Member

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

k0kubun commented Aug 2, 2023

Copy link
Copy Markdown
Contributor Author

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

k0kubun commented Aug 2, 2023

Copy link
Copy Markdown
Contributor Author

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

@ioquatix ioquatix merged commit 273ca95 into socketry:master Aug 3, 2023
@ioquatix

ioquatix commented Aug 3, 2023

Copy link
Copy Markdown
Member

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

ioquatix commented Aug 3, 2023

Copy link
Copy Markdown
Member

@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 ruby/ruby-bench that referenced this pull request Aug 3, 2023
My contribution socketry/cool.io#79 has been
merged and released.
@k0kubun

k0kubun commented Aug 3, 2023

Copy link
Copy Markdown
Contributor Author

I upgraded cool.io to the released version on yjit-bench ruby/ruby-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