nirvdrum[m] has joined #jruby
<headius> ok not quite done
<headius> that gets us sqlite, mysql, and pgsql CI runs for both the ARJDBC suite and the ActiveRecord suite... missing are combinations of jruby-head, rails master, different JDK and DB versions, "jndi" and "jdbc" runs, and a few other permutations
<headius> this can get copied to the 6.1 branch and we can start moving master to 7
<headius> nite nite
<headius> this came across my desk late yesterday: https://github.com/docker-library/openjdk/issues/493#issuecomment-1105505718
<headius> we will need to evaluate the available docker images for openjdk builds and decide which fits best to replace the "official" openjdk image
<headius> yay, Geertjan from Azul wants to help with the Zulu issues we have been seeing
<enebo[m]> LOL. I figured out something wrong in recent JIT changes. I was pushing scope.isRuby2Keywords from the methods scope
<enebo[m]> Obviously when we compile the method this is before we set it to ruby2_keyword the method on Module. So it is always false :)
<enebo[m]> HAHA. I spent like 45 minutes looking at something else until I realized it is false all the time
<headius> 🤦‍♂️
<headius> well good that you found that
<headius> I am going to migrate the GHA workflow to arjdbc 6.1 branch and then see about flipping master to jruby-head and Rails 7
<enebo[m]> So I will push staticscope here
<enebo[m]> yeah great. I am very curious what we see with master and 7
<headius> has there been any merging going on with the branches on ARJDBC or should I just copy or cherry-pick it over?
<enebo[m]> Once I get past this particular JIT issue I will see how we unit test other parts of Rails 7 looking for more bugs
<headius> it's a lot of commits so it might be simplest to just physically copy the completed file over to 6 with a mention of master having the full history
<enebo[m]> we always start and merge up towards master
<enebo[m]> So you should only see version conflicts for version number on merges
<headius> ok
<headius> I will copy it over and merge back before changing master
<enebo[m]> unless there has been something committed to only one branch and that was not done
<enebo[m]> I have not committed anything in quite a long time
<enebo[m]> I think kares did but it was related to updating a jar
<enebo[m]> not merging that matters less since I think those things are just versioned differently
<headius> ok
<enebo[m]> I sort of wish arjdbc was organized differently
<headius> are releases of the drivers done from master?
<enebo[m]> There is no simple solution
<headius> they really ought to move out to a separate project and be sourced from maven but I'm not going to tackle that right now
<enebo[m]> I don't even know...ultimately it does really matter since they all have the same code
<enebo[m]> but where the tag was dropped is where it happened
<enebo[m]> I don't know if I have ever done a drive release
<enebo[m]> With that said I may have done all of them
<headius> yeah
<headius> I just ask because I updated the MariaDB driver trying to fix an issue (did not fix) but did that only on master
<enebo[m]> So going back to what you said yesterday. It would be nice to change drivers so they get from maven and do not commit AND probably be their own repo probably as well
<headius> we need a team
<enebo[m]> Having everything in one place is usually a good thing in my book but in this case there is way too much interdependence
<enebo[m]> like any change to common java code forces all dbs to get revved even if it had no code changed
<headius> yeah
<enebo[m]> Like anything this old it is daunting to consider changing
<enebo[m]> and not simple either. Especially in the module world we are now in
<headius> there are issues on 61-stable with the new workflow: https://github.com/jruby/activerecord-jdbc-adapter/actions/runs/2241569317
<headius> I will look into them in a bit
<enebo[m]> 2 look like missing the same excludes you made
<enebo[m]> err enabled
<enebo[m]> those 2 mysql2 encoding fails
<headius> Oh of course, I can cherry pick those over at least
<headius> ok looks green on 61-stable
<enebo[m]> I am looking at the 3F on sequel since it is obviously a bug with ruby2_keyword methods
<enebo[m]> It is all in the exact same form so 1 bug
<headius> ah cool that it is related
<enebo[m]> I have not looked at this in the past but was it always just 3?
<headius> re sequel, I have not actually looked at it recently so I can't tell ya
<headius> I set up a nightly snapshot build of master so jruby-head on GHA should only lag by a day or two
<headius> but it will lag
<enebo[m]> I went back 5 pages and it had 28000 lines of warnings and the full log is epic
<headius> wow
<enebo[m]> I am downloading it now for fun
<headius> I'm going to hack around on this rails7 branch and try to get some decent runs
<headius> bundle failed due to an old minitest req so that's first
<enebo[m]> 10.7M of lines
<enebo[m]> haha bizarely it only failed 1F
<enebo[m]> I am guessing your fix for method redefinition is why this is not huge now
<headius> oh yeah could be
<headius> well that's new
<headius> ArgumentError: tried to create Proc object without a block
<headius> register_attribute_observer at /home/runner/work/activerecord-jdbc-adapter/activerecord-jdbc-adapter/vendor/bundle/jruby/3.1.0/gems/test-unit-2.5.5/lib/test/unit/attribute.rb:110
<headius> new at org/jruby/RubyProc.java:150
<enebo[m]> yay
<enebo[m]> heh
<enebo[m]> must be a Proc.new
<headius> oh right this is not supported anymore
<headius> this is an old test-unit
<enebo[m]> yeah I was just going to say it is probably very old
<enebo[m]> Assuming this is our test suite (which I would love to whittle down to nothing)
<headius> I'm just removing the version req on these support libs like minitest, test-unit, rake
<enebo[m]> yeah not a bad idea
<headius> we can sort out the gemfile later but it is really old stuff
<enebo[m]> Most of it is copies of existing tests from ar test suite itself
<headius> sqlite arjdbc tests are green
<headius> on jruby-head
<headius> 498 errors on arjdbc pgsql 😀
<headius> nearly all of them are ArgumentError: wrong number of arguments (given 1, expected 0)
<headius> rails sqlite tests failed with "LoadError: load error: cases/helper -- java.lang.RuntimeException: IR compiler/interpreter bug: org.jruby.ir.operands.UndefinedValue should not be used as a valid value during execution."
<headius> Undefined leaking out of arg logic
<headius> and mysql rails fail: ArgumentError: wrong number of arguments (given 3, expected 1..2)
<headius> NoMethodError: undefined method `register_class_with_limit' for #<ActiveRecord::ConnectionAdapters::PostgreSQLAdapter:0x4c9235c0>
<headius> pgsql rails fail
<headius> some of these will be API changes and some are jruby-head
<enebo[m]> well that is something. I am not so surprised sqlite3 is passing
<enebo[m]> pg is the most modified of the three
<enebo[m]> The undefined is for sure part of the ** logic
<enebo[m]> That is good to find as the more of this we find the more we will figure out
<headius> for sure
<enebo[m]> The actual logic for merging and marking kwargs in IR can definitely be improved upon
<enebo[m]> It may happen sooner than later if this uncovers a big pocket
<headius> the mysql rails fails on this method: def execute(sql, name = nil)
<headius> it probably is supposed to take an additional arg or kwarg now
<headius> yield execute(sql, name, async: async)
<headius> yep
<enebo[m]> fun mandatory
<enebo[m]> Making it run is simple but knowing how to set async properly may be a bit more changing
<headius> there's a lot of new kwargs in here it seems
<headius> yeah jdbc has no async support
<enebo[m]> I did not even mean that
<headius> oh yeah
<enebo[m]> I just meant they prop that value through other methods
<headius> lots of them
<enebo[m]> but not supporting it is a problem :)
<enebo[m]> This was why I tried so hard for sqlite3 and mysql to use as much of a direct copy of their code as possible
<enebo[m]> pg is much more complicated to update
<enebo[m]> but sqlite2 should be very close sourcewise unless they radically redesigned it
<headius> yeah I am looking at the mysql diffs now
<enebo[m]> err mysql
<enebo[m]> hahah sqlite3
<enebo[m]> err 2
<enebo[m]> sqlite3 working is nice though for short term testing
<enebo[m]> I added a comment to dr-itz on your last closed PR trying to entice him
<enebo[m]> He has done so much on keeping stuff updated
<headius> I saw that
<headius> I will try some small patches to improve this... the async thing only needed to be propagated through execute
<enebo[m]> but every call to execute passes that right?
<headius> yeah
<enebo[m]> heh
<enebo[m]> If this is broken then just make super(...)
<headius> this is the rails version, I'm looking at what we need to do in ours
<headius> oh no, the rails versions just added async
<headius> so I will do that
<headius> as long as this is simple propagation of the kwarg I will keep poking
<enebo[m]> yeah I am going to poke open a beer and consider dinner
<headius> ok
<enebo[m]> It will be cool if it is just a little updating of some ruby methods
<headius> mysql now fails with java.lang.RuntimeException: IR compiler/interpreter bug: org.jruby.ir.operands.UndefinedValue should not be used as a valid value during execution.
<headius> sqlite started failing too so I will see where async needs to propagate there
<headius> oh I see
<headius> I will find that and update it
<enebo[m]> this should not be happening regardless
<headius> heh
<headius> so they started rejecting async I just added
<enebo[m]> but still
<headius> the arjdbc test runs were still using 6.1 as the base
<enebo[m]> I am not that suprised since I fixed a few problems with it yesterday and it is not super clear
<enebo[m]> I guess perhaps after things are working it will be time to boil the ocean on kws on callsite side
<enebo[m]> I really feels like AST could aggregate these in a way where we are not just doing some infinite weirdness of merging
<headius> yeah
<enebo[m]> Or at least it feels like in most cases the flow could just be a single simple linear creation
<enebo[m]> Right now it will make a hash at some point and then will merge different hashes to that hash
<enebo[m]> but some of that is just supporting **a, **b sorts of wackiness which never happens
<headius> yeah I don't expect it to be efficient for 9.4 but we are getting the right pieces in place
<enebo[m]> but even a: 1, **c is a collection of merges
<headius> wanna get this all compat and released so we can sit back and do some big perf work this summer
<enebo[m]> yeah and some of what I am saying is less perf (although I am sure it will help perf) and more just simplify the model
<headius> now sqlite fails mostly with
<headius> NoMethodError: undefined method `extract_value_from_default' for #<ActiveRecord::ConnectionAdapters::SQLite3Adapter:0x5ca896af>
<headius> Did you mean? extract_new_default_value
<headius> a new or renamed method I suppose
<enebo[m]> the kwarg above I think is hash {a: 1} merge c then likely a dup
<enebo[m]> The duping in not obvious when or when not to dup
<enebo[m]> ok this will be a) something we need @JRubyMethod for to mimic the C-ext being native b) just a missing ruby method
<headius> looks like it is in private methods... we copy the main sqlite file and modify yeah?
<enebo[m]> Generally when it is possible to match cext I have been just to make this comparison simpler
<enebo[m]> well I diff and add delta but however it works out best
<headius> ah yep it is
<enebo[m]> and I mean "diff" not literally diff
<enebo[m]> which is also why I make everything in the exact same order with a comment when something is new/different
<headius> yeah
<enebo[m]> Ultimately this is where if I had infinite spare time this would have been pushed as some PR to module all this so their connection adapters and ours would be the same then our file cuold go away
<enebo[m]> anyways too much
<enebo[m]> afk for reals
<headius> yeah that would be nice
<headius> woah!
<headius> after copying sqlite stuff in, arjdbc tests: 146 tests, 333 assertions, 0 failures, 10 errors, 1 pendings, 4 omissions, 0 notifications
<headius> not bad
<headius> the rails tests are all blowing up with that Undefined error