<enebo[m]>
It seems there are some PRs for some of this and some dependent actions (like a release of logger?)
<headius>
I believe the ivar stuff is ready to go
<headius>
and again it is new functionality so should not affect much existing code
<headius>
enebo: I am testing some jcodings updates but will look at 9.3.4 tags after that
<enebo[m]>
ok
<kares[m]>
oh 9.3.4 next week would be great - I'll look into updating jossl to have latest in release
<kares[m]>
https://github.com/jruby/jruby/pull/7071 is smt partially blockinf 9.3 adoption in Logstash, but it's tagged for 9.3.4 so it will hopefully get in ...
<enebo[m]>
I just pinged reporter to hopefully get them to do a clean build
<headius>
I"m fine with merging it in any case
<headius>
I think I'm going to try to set up JRuby as a jcodings CI test... the tests in there are very limited but the CRuby encoding-related tests are pretty extensive
<headius>
currently bisecting to find a recent PR or commit that broke some CRuby tests
<headius>
probably would be wise to do this for other upstream projects like joni
_whitelogger has joined #jruby
<headius>
ahorek: I went ahead and merged your jcodings CESU PR but it broke some tests in JRuby 9.3 (which I was testing with a jcodings snapshot for my toUnsigned PR)
<headius>
not important for 9.3.4 but we should revert the PR and try again unless you can look at it soon
<headius>
https://github.com/jruby/jruby/issues/7030 requires the rework of framing and non-local jumps that is in progress for master and that would definitely not be cool to land in 9.3.4
<headius>
we could potentially fix it in the short term with a stack search but do we want to do that?
<headius>
it would mean all non-local breaks have to search scope stack for a target before raising, where they currently can just go for it
<headius>
granted that search may be quick in simple cases like ary.each {|x| break if x is what I want }
<headius>
ahorek: looks like those test fixes have not been backported to ruby_2_6 branch so I will request that
<headius>
and I will apply that patch directly for now
<enebo[m]>
ahorek headius I think we should have up to date 2.6 tests which is totally fine so long as failures are excluded
<headius>
I can check stdlib, probably won't be many changes
<ahorek[m]>
aha, ' change is since ruby 3.0, so these tests won't match 2.6. Not sure if it's a problem to include more recent unicode in jruby 9.3 branch?
<enebo[m]>
I personally only see it as release work if you or me are doing that exclusion. If ahorek is doing it then 👍️
<headius>
ahorek: we ain't gonna branch jcodings so probably just have to accept any behavioral difference
<headius>
or not update jcodings anymore in 9.3
<headius>
of course sonatype does maintenance in the middle of me trying to sort this all out
<enebo[m]>
heh
<headius>
back now 👍
<enebo[m]>
Should we ask what the real negative consequence of having newer data will have in an older Ruby?
<headius>
I'll push a PR that updates these tests and uses jcodings snapshot and we will see how it looks
<enebo[m]>
I presume in most cases these are effectively fixes or new stuff
<enebo[m]>
I guess if we see specific errors from it then it may answer that question.
<enebo[m]>
Some amount of MRI tests just exercise all the cases in some mongo loop. I imagine anything with encodings probably has this somewhere.
<headius>
ahorek: confirmed that test patch runs green on these tests with jcodings master
<enebo[m]>
sweet
<ahorek[m]>
I don't think it's a big deal, but for instance new encodings like CESU8 shouldn't be suddenly available in 2.6.x
<enebo[m]>
ahorek: yeah I knew that was going to show up but what is the actual ramifications there?
<enebo[m]>
If we are running a Ruby program which was originally running on MRI 2.6.9 it would complain if it found that but if we work then the error would be that we should reject it. Seems pretty inconsequential. Perhaps this is too simplistic
<enebo[m]>
simplistic being my thinking here
<headius>
if nothing fails with updated jcodings I would say it is pretty safe... new encodings shouldn't break anyone
<ahorek[m]>
if you want to keep compatibility, even encoding bugs are "features", you don't want to change behavior in bugfix releases (except security fixes)
<enebo[m]>
ahorek: I think that is generally true but .3 or .4 for us is so early that we tend to be a bit more forgiving
<enebo[m]>
It also is a tradeoff. Should we be spending times branching jcodings and making this totally compatible vs working on 9.4?
<enebo[m]>
and I realize you may be saying you might be willing to do this...
<ahorek[m]>
yes if we accept these small changes (mostly bugs, but changes), it shouln't be a problem to include the new version in 9.3
<headius>
the alternative is explicitly avoiding adding the new encoding in JRuby because we add them programmatically right now
<enebo[m]>
I think we just accept this risk. I just don't see anyone caring
<ahorek[m]>
agreed
<enebo[m]>
I think about the universe of programs which would expect CESU8 to not exist and I think that is 0
<enebo[m]>
ok
<enebo[m]>
this seems like we are done but I am trying to think of a time we have added something additive during a release series and ended up having a problem. By that I mean like actual new methods or a new type.
<headius>
I think we have come up with some theoretical cases but no real ones emerged
<enebo[m]>
headius: can you think of a single example? I am thinking maybe something like someone expects from reflective method results and we provided a new method unexepcted
<headius>
like respond_to? X and using that to detect ruby version
<headius>
but that is more obvious than this, adding methods that don't exist in that version
<enebo[m]>
I remember in the older days doing some workshops and someone would complain he had methods like 'java' when asking for instance_methods on something
<headius>
this is largely just adding more encodings to the database
<enebo[m]>
That was not us adding a feature but it is the general pattern
<enebo[m]>
ueaj
<enebo[m]>
that == yeah
<headius>
something's screwy with GHA today... it is not submitting PR jobs until much later
<enebo[m]>
the tubes are clogged
<enebo[m]>
MRI tests looks like it took 5 minutes to start
<enebo[m]>
I am pretty sure no other org work is running right now
<enebo[m]>
We may be sharing some queue of sorts across all of jruby
<headius>
by the nature of how it is implemented, define_singleton_method almost always uses public visibility, but there's a single stupid edge case that will honor the frame visibility
<headius>
jeremyevans agrees that edge case is a bug and apparently it was introduced in 2.1
<enebo[m]>
oh yeah this one
<headius>
this is the last item for eliminating ostruct warnings on its shadowing of core methods
<headius>
if we mark this as always public then it doesn't need frame
<headius>
and so we don't warn when it is shadowed by ostruct
<enebo[m]>
I think the other part of jeremy's response is even more important. It no longer behaves this way
<enebo[m]>
So between 2.1 and 3? it flipped
<headius>
he was referring to using def
<enebo[m]>
This gives us a little cover to point out the behavior is a) inconsistent depending on scope which is weird in this case b) MRI realized they had this inconsistency and fixed it
<headius>
but since def does not do non-public the argument is that define_singleton_method shouldn't either
<headius>
MRI does honor frame visibility for that one edge case he shows since 2.1
<headius>
-e:1:in `<main>': private method `foo' called for Foo:Class (NoMethodError)
<headius>
ok let's punt to 9.3.5 then
<headius>
or just do it?
<headius>
I dunno
<headius>
it doesn't break anything
<enebo[m]>
Let's talk through this from a what could happen
<headius>
so someone does that weird pattern and their method ends up public
<enebo[m]>
Do people expect methods defined to not have public in those cases?
<headius>
it would not work that was in 2.0 or earlier and may not work that way in 3.2+ if jeremyevans PR is merged and my issue is accepted
<enebo[m]>
Reflective methods would start showing them
<headius>
yes
<headius>
that would be the most visible result
<enebo[m]>
well the public ones
<enebo[m]>
callers via send would start working
<headius>
it does not change anything about the behavior of the method
<headius>
and calls that succeeded before still succeed
<headius>
calls that failed before failed before
<enebo[m]>
in cases where it wouldn't as private
<headius>
so probably don't exist
<enebo[m]>
So it would be an error case there which is rare for working code to hit
<headius>
send always can call private though, you mean public_send
<enebo[m]>
yeah I do
<enebo[m]>
which I wonder how many people really even bother there
<headius>
so how much risk does that equate
<headius>
seems like not much
<enebo[m]>
I think it showing up in reflective calls is the biggest possible issue
<headius>
yeah
<enebo[m]>
but ostruct will work nicely and we remove a frame
<headius>
we have been dragging that ostruct issue along for quite a while
<headius>
it seems likely this behavior was introduced accidentally
<enebo[m]>
I don't want to sound cavalier but this is 9.3.4.0 and this seems like if people have this issue they may end up being forced to deal with in 3.1.x or 3.2
<headius>
full speed ahead
<enebo[m]>
yeah
<headius>
ok let's do it
<headius>
we can always release 9.3.5 a week later if it breaks everyone
<enebo[m]>
yep
<headius>
0.12.2 upgrade is green, merging
<headius>
er jruby-openssl 0.12.2
<headius>
enebo: I'm going to merge byteit101 ivar PR
<enebo[m]>
so we get some IR which will splat rest arg and merge kwargs then build compound array of those two things before splatting that compound array
<enebo[m]>
building compound array will args push which will then make [{}] which is passed as *[{}] to the method which accepts nothing. That in turn has no kwargs so it sees a single empty hash as an arg
<enebo[m]>
The good news I think this is just an error in what we emit as IR
<enebo[m]>
The bad news is digging around in that to see how this should actually work
<enebo[m]>
In 3.0 I think the fix could be different since we can just pass the {} so long as we mark it as a kwarg
<headius>
hah yeah you are likely to be eaten by a grue
<enebo[m]>
but optimally passing nothing as kwargs is not really a useful thing
<enebo[m]>
with that said I don't want to emit a branch/jump to not pass empty kwargs either :)
<enebo[m]>
3.0 does definitely help since they mark kwargs
<enebo[m]>
MRI optimizes **{} specifically in the parser
<enebo[m]>
I wonder if they continue to do that when they have their experimental AST API
<headius>
ah that makes sense
<headius>
or explains how they handle it anywa
<enebo[m]>
but it doesn't solve the original reporters issue
<enebo[m]>
just me reduced case
<enebo[m]>
theirs is **kwargs where kwargs is {}
<enebo[m]>
HEH just checked 3.0 parser. The put it back
<enebo[m]>
They still have the conditional but they make an empty frozen hash at that moment and pass it as a literal node
<ahorek[m]>
<headius> "hmm there are a couple failures..." <- have you find out the reason? I'm getting errors mostly with frozen string literals, even on a commit before byteit101's changes
<byteit101[m]>
see my commit above 5 messages ago
<byteit101[m]>
no idea why it changed, but Class vs j.l.Class causing issues.
<headius>
Yeah unsure why it wasn't caught earlier
<byteit101[m]>
I don't know how it was working for me