search

alexdalitz / dnsruby

Dnsruby is a feature-complete DNS(SEC) client for Ruby, as used by many of the world's largest DNS registries and the OpenDNSSEC project
Other
194 stars 77 forks source link

Fix compatibility with the `--enable-string-literal` Ruby option #192

Closed casperisfine closed 3 months ago

casperisfine commented 3 months ago

Ref: https://bugs.ruby-lang.org/issues/20205

This allow users to turn on frozen string literal if they so chose and make the gem compatible with future version sof Ruby for when frozen string literals may become the default.

alexdalitz commented 3 months ago

Interesting - thank you! Sadly, the Ruby 3.3 tests had one failure - I'm not quite sure whether it's on your new "frozen" tests?

casperisfine commented 3 months ago

It failed on the worker that doesn't have string literals enabled. I think it's just a timeout or failed query that turned that way?

casperisfine commented 3 months ago

So I double checked, that test pass fine on my machine both with frozen string literal on and off.

I triggered a rebuild.

casperisfine commented 3 months ago

Actually, there seem to be an issue with some tests that consistently itmeout with string literals and don't without. I'll dig a bit more.

casperisfine commented 3 months ago

Alright, good call. I figured it out using -d. There was some FrozenError left, but it was happening in a background thread and the error was rescued.

Thanks to the debug flag I could see the errors printed even though they were rescued. It should go green now.

alexdalitz commented 3 months ago

OK, thanks very much for looking into and fixing the issue, and for your patch! If it is a particular concern for you to have this in a release, please let me know - otherwise I'll probably wait until something else comes along...

casperisfine commented 3 months ago

Not a huge concern, but running of a fork do raise some alarms. So no huge rush but depends what timeline you are thinking. If it's weeks it's fine :)

casperisfine commented 3 months ago

Oh and thanks for the quick merge ❤️

alexdalitz commented 3 months ago

Yeah no problem - I’m flat out with a commercial project right this minute, but I’ll try to get a new release out later today.

On 28 Mar 2024, at 16:30, Jean byroot Boussier @.***> wrote:

Oh and thanks for the quick merge ❤️

— Reply to this email directly, view it on GitHub https://github.com/alexdalitz/dnsruby/pull/192#issuecomment-2025632223, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB2WFWS7V5TZZDQ7NUOXKQDY2QZQ5AVCNFSM6AAAAABFNAW6B6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMRVGYZTEMRSGM. You are receiving this because you modified the open/close state.

alexdalitz commented 3 months ago

Ok that’s v1.72.0 out - please let me know of any issues.

On 28 Mar 2024, at 16:30, Jean byroot Boussier @.***> wrote:

Oh and thanks for the quick merge ❤️

— Reply to this email directly, view it on GitHub https://github.com/alexdalitz/dnsruby/pull/192#issuecomment-2025632223, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB2WFWS7V5TZZDQ7NUOXKQDY2QZQ5AVCNFSM6AAAAABFNAW6B6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMRVGYZTEMRSGM. You are receiving this because you modified the open/close state.

casperisfine commented 3 months ago

You're the best!