<headius> good morning
<headius> kares: I cleaned out stdlib and I'm trying again... perhaps a later jossl got unpacked over an earlier one and caused some loading oddities?
<headius> kares: still seeing it 🤔
<kares[m]> headius: if you have 0.11.0 as well as 0.12.1 around that might cause loading issues
<kares[m]> having 0.12.2 along side 0.11 or anything older should not cause them
<headius> jruby-openssl (0.12.2 java, default: 0.11.0 java)
<kares[m]> hmm that should work
<headius> I will uninstall 0.12.2 and run it again
<headius> any reason we shouldn't update 9.3.4 to 0.12.2 default though?
<headius> enebo and I were chatting about a 9.3.4 release next week or so
<headius> enebo: we could chat about that now... not sure what other things should go in that release
<enebo[m]> I will look at issues but I think we have been good in only targetting what we think we will work on
<enebo[m]> wow 11 opens (1 never counts)
<headius> I think I tagged a couple things I thought were quick fixes
<headius> that range one seemed easy but I botched something
<enebo[m]> yeah I think it is likely we call the method from Java and assume the behavior
<headius> I thought I lined up the calls right but it broke a bunch of stuff
<enebo[m]> I just tagged this for
<enebo[m]> At some time today I will look at https://github.com/jruby/jruby/issues/7102
<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> Added a comment here with the failures: https://github.com/jruby/jcodings/pull/44
<headius> and added an issue for us to add JRuby encoding tests to jcodings CI: https://github.com/jruby/jcodings/issues/56
<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 }
<ahorek[m]> headius: MRI tests are outdated, we need these https://github.com/ruby/ruby/pull/3177/files
<headius> aha so this should be ok on master
<headius> we will need to patch the test on 9.3 when we update jcodings there
<ahorek[m]> btw there're many new tests in ruby 2.6.9 vs jruby 9.3 branch https://github.com/jruby/jruby/tree/jruby-9.3/test/mri
<headius> need to update 9.3 to 2.6.9 stuff ... not sure if that should happen in 9.3.4 or not
<headius> I suppose it is better to do now than wait another several weeks to do 9.3.5
<headius> enebo: thoughts?
<headius> kares: removing 0.12.2 fixed my local issue so something is up with that
<headius> I can reproduce the same error they see now
<ahorek[m]> headius: https://github.com/jruby/jcodings/pull/57 a small fix for MAC issues
<ahorek[m]> + after updating tests from https://github.com/ruby/ruby/pull/3177 jcodings should be green
<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, &apos; 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> enebo: I retargeted https://github.com/jruby/jruby/issues/7030 at 9.4 but I'm on the fence about adding the stack search
<headius> it would fix it for 9.3 but slow down non-local breaks some amount
<enebo[m]> So we know one gem which uses this
<enebo[m]> I don't know how important that gem is 'in_threads'
<enebo[m]> It is also likely they worked around this by now too
<headius> it is only so they can catch an error condition and do something else, where what we have now just bubbles out the error
<headius> I'm not sure how critical it is
<enebo[m]> yeah
<enebo[m]> master is a safe bet for the PR and 9.3 becomes more important if we see this as needed
<enebo[m]> but I don't know either on the needed part
<headius> I asked reporter how critical it is
<headius> here's another one to discuss: https://github.com/jruby/jruby/pull/7055
<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> and still does
<enebo[m]> Ah well I guess we wait on: https://github.com/ruby/ruby/pull/5636
<enebo[m]> Then what I said will be trusih :)
<headius> $ rvm ruby-3.1 do ruby -ve 'class Foo; class << Foo; private; Foo.define_singleton_method(:foo){}; end; end; Foo.foo'
<headius> ruby 3.1.0p0 (2021-12-25 revision fb4df44d16) [x86_64-darwin20]
<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 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
<headius> I have no idea what is up with that person's environment on ahorek PR: https://github.com/jruby/jruby/pull/7071#issuecomment-1069402886
<enebo[m]> cool
<byteit101[m]> :-)
<headius> I think we should merge ahorek PR and get a snapshot for that guy to test
<headius> his build env seems completely hosed
<enebo[m]> yeah
<headius> ah he was testing master
<headius> so that's not great for master but he tested against 9.3 and it is working correclty
<headius> master works better than that though so I'm still unsure what's up with his env
<headius> enebo: I'm moving https://github.com/jruby/jruby/issues/7032 and the related PR to 9.4
<headius> I'm not comfortable changing how we calculate thread status in 9.3
<headius> and this is a very new case to try to cover
<enebo[m]> ok
<headius> for https://github.com/jruby/jruby/issues/6976 I vote we change warning in 9.3.x and remove all aliases and bad defines of java import stuff in 9.4
<enebo[m]> yeah sounds reasonable
<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
<headius> and using warn with `uplevel: 1` to get the caller line number
<enebo[m]> and you removed the odd definition?
<enebo[m]> err old
<enebo[m]> I guess you would have to since you have 'import'
<headius> I removed nothing for 9.3
<enebo[m]> I should have also said changed
<headius> well I made import into a full definition so the warning could be specific
<headius> ok yeah
<headius> I'll push a PR and you cansee
<enebo[m]> My mind just wants to remove it which you will on master right?
<headius> yeah I will remove the two deprecated forms on master
<enebo[m]> cool...does that kill that kernal file as well?
<headius> not yet... still defining the toplevel version there
<headius> that could be moved into java though
<enebo[m]> ah ok
<headius> where we define the other special toplevel methods
<enebo[m]> It is some overhead to be in Ruby for that when nothing else is at this point for JI
<enebo[m]> err not much anyways
<headius> yeah one less file
<enebo[m]> one drop in the ocean
<enebo[m]> we really need to experiment with "sealed" builtin library path
<headius> the toplevel java_import was actually getting defined on Object so it overwrote the one with the deprecation warning
<headius> toplevel method definition is weird
<enebo[m]> not for gems or lib code but just so long as we know order of loading of builtin files never change we can calc the names ahead of time
<headius> hmm
<enebo[m]> gemification makes that more difficult
<headius> this will now warn for `java_import` inside another method, where it did not warn before
<headius> er, I mean inside a method in a normal object
<enebo[m]> I have used that
<headius> a method where self is class or module should be fine
<enebo[m]> I only import the java if I call a method which may not be called
<headius> hmm maybe that form should not be deprecated, but it means all objects currently have a private `java_import`
<enebo[m]> I don't recall where but it was something where the Java feature was not normally used
<enebo[m]> There are other ways of doing that
<headius> yeah that pattern is similar to lazy require only if you use the library
<enebo[m]> yeah
<enebo[m]> In that sense it feels like something normal
<enebo[m]> but it is defining a constant somewhere
<enebo[m]> and methods can also do that but only once
<headius> enebo: so maybe we remove that deprecation since it has not actually been active until my PR
<headius> yeah this seems better
<headius> I will repush
<headius> and master will just remove the deprecated `import`
<enebo[m]> ok
<headius> hmm there are a couple failures on 9.3 that seem to be from byteit101 PR
<byteit101[m]> oh
<byteit101[m]> whoa, a new file change comparision viewer in github (beta)
<byteit101[m]> puzzling why I haven't ran into this issue before
<byteit101[m]> bundle install doesn't install rake
<byteit101[m]> Is it intentional that a Java j.l.Class represented by a Ruby Class is not coercable to a j/l/Class?
<byteit101[m]> I can do a new PR if you'd like, or you can just pick that commit
<enebo[m]> That is a fun time.
<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
<ahorek[m]> do you think it's related to these https://github.com/jruby/jruby/runs/5574528021?check_suite_focus=true ?
<byteit101[m]> headius: do you want to just pick that commit up or do you want a PR?
<ahorek[m]> ok, thanks for confirmation, it seems to be the cause
<headius> I can just grab it