joast has quit [Quit: Leaving.]
joast has joined #jruby
viglu[m] has quit [Quit: You have been kicked for being idle]
<headius> enebo: latest push seems to have mysql2 tests running
<headius> I will start looking at how to represent the whole matrix
<enebo[m]> cool
<enebo[m]> So last JIT error on kwargs is something I understand now
<headius> oh yeah?
<enebo[m]> So originally I would more or less make an if/else looking for empty kwrest (and only that) like in this snippet.
<enebo[m]> I have to reinstate this again (or encapsulate this into some callinstr)
<enebo[m]> the issue here is specific arity 1 with 1 argument will arg error if empty but if not empty it becomes a normal arg
<headius> which would happen only if ruby2_keywords?
<enebo[m]> so I can detect this at callsite and it seems better to force this logic only to callsites with kwrest
<enebo[m]> this is ruby3 behavior
<headius> wow, weird
<enebo[m]> I think this was to not break as much legacy code
<enebo[m]> so **h does not neccesarily mean it is a keyword arg
<headius> ahhh I see
<enebo[m]> it depends on what is receiving it
<enebo[m]> I now also realize get_keywords is what unmarks the keyword flag
<enebo[m]> so even in the m(**{a: 1}) case I am not unmarking because specific arity is not calling that instr
<enebo[m]> That is simple I guess I will just unmark first parameter if it is a hash
<enebo[m]> I knew all this but I could not elucidate it the other day because I just had about a dozen debugging sessions of other kwargs issues
<headius> yeah I guess it makes sense for older code
<enebo[m]> So for this week (at least) I will go back to if/else for only kwrest callsites and eliminate passing empty kwargs hashes as a parameter
<enebo[m]> In the future we can consider if we should just have a smarter type of call which will strip the argument
<enebo[m]> The immediate solution removes any argument processing at the cost of having an if/else in IR
<enebo[m]> which should be fine for JIT but less fine for interp
<enebo[m]> What I also think is this might mean slightly less logic in receive_keywords
<headius> ok this ran but it has a lot of failures (Rails Tests): https://github.com/jruby/activerecord-jdbc-adapter/runs/6197125226?check_suite_focus=true
<headius> I don't know if these are expected to be green or not
<enebo[m]> ok neat
<enebo[m]> That is running normal mode and not --dev?
<headius> yeah just normal
<headius> against 9.3.4
<enebo[m]> looks like a few of these are a missing new argument to specific adapters
<enebo[m]> that will just be looking at the specific methods in ar source
<enebo[m]> but this is not too many failures
<enebo[m]> Although add_column and things like this probably block a lot of stuff from running
<headius> this should be locked to a Rails 6.1 version
<enebo[m]> oh I see. I thought we were ok for Rails 6.1 so I am not sure
<enebo[m]> I don't suppose travis still has archives of old builds
<enebo[m]> We have 61.x out as full releases
<enebo[m]> so I do not think these failed unless they changed an internal API during point releases
<enebo[m]> which is definitely possible
<headius> yeah I don't think we can see the archives anymore
<headius> oh I found it
<headius> 2 years ago?
<enebo[m]> If we looked at Rails out for 61.1 - July 29, 2021 java (379 KB) then we can probably set to that Rails version and see if this goes away
<headius> and it does appear to be green then
<enebo[m]> and 61.1 was put out in 2021
<headius> this is rails 6-1-STABLE, but this seems too many failures for a stable branch change
<enebo[m]> headius: and this is using jruby-head?
<headius> no
<enebo[m]> or jruby 9.3?
<headius> 9.3.4
<enebo[m]> were we actually using 9.3?
<enebo[m]> or 9.2 in travis?
<headius> 9.2.14.0
<enebo[m]> looks like 9.2
<headius> I can try to run with that and see if that is green
<enebo[m]> It is possible we have a 9.3 bug or that Rails changed something
<headius> ok I will try that
<enebo[m]> I would lay even odds
<headius> I will get these mysql2 tests running and then merge to master and do another PR to expand the matrix, so we will at least have a baseline build running
<enebo[m]> I am hoping somehow this fix for specific args has something to do with the last failure in spec:compiler
<headius> wow wtf
<headius> NumericDataTest#test_numeric_fields:
<headius> Java::JavaLang::ClassCastException: class org.jruby.ir.operands.TemporaryClosureVariable cannot be cast to class org.jruby.ir.operands.DepthCloneable (org.jruby.ir.operands.TemporaryClosureVariable and org.jruby.ir.operands.DepthCloneable are in module org.jruby.dist of loader 'app')
<headius> enebo: does that look familiar?
<headius> something we fixed after 9.2.14 perhaps?
<headius> there's 135 of these when I got 9.2.14 running in GHA
<enebo[m]> That is interesting. It is calculating zsuper scope where values come from
<enebo[m]> which we got rid of at one point since it is not how ruby works (but should have)
<headius> it looks like it is the same trace every time so it is repeatedly trying and failing to build this
<headius> I will try 9.2 latest
<enebo[m]> yeah give it a shot
<enebo[m]> This is something I remember but it has happened more than once
<enebo[m]> If this is not fixed then something in Rails 6.1 changed to use zsuper in a way which exposes a problem
<headius> that could be
<enebo[m]> I guess in a sense it is good to kow it is not in 9.3
<headius> these tests have not run in CI for a couple years at least, but I would have thought you and others would have run locally since then
<enebo[m]> yeah you would hope someone would report it
<enebo[m]> 9.2.14.0 is quite old though
<headius> looks like it is rails 6.1.5.1 on 6-1-stable
<headius> it is
<headius> and nobody would have run with that in the last two years doing arjdbc dev
<headius> probably not anyway
<enebo[m]> I definitely think people would be using newer 9.2
<enebo[m]> especially if this started happening
<enebo[m]> Let's hope it is green with latest I guess
<enebo[m]> 6.1 I pretty much don't care too much if we work on 9.2 other than we were passing before
<headius> total for 9.3.4 was like 14E
<enebo[m]> 9.3 it needs to work
<enebo[m]> yeah
<enebo[m]> those methods too are hinting at an API change I think too
<headius> yeah I just want a single config to run green before I merge back to master and try to expand the matrix
<headius> mysql2 rails tests seemed good
<enebo[m]> We tend to not strip args out of a call anywhere so missing an arg will tend to just be we are calling with the wrong number initialially
<enebo[m]> was that sqlite3?
<headius> no mysql
<headius> I wanted to do one that required db setup for the first go
<enebo[m]> oh you just meant the rails tests were green for mysql2
<enebo[m]> are those our internal tests?
<headius> I am setting up a single run of rails tests on mysql2 for this POC
<headius> all these configs were green on travis two years ago
<enebo[m]> sorry I am looking at the results
<enebo[m]> sort of looks like two changes to schema management APIs based on the errors
<enebo[m]> one return type switched to nil and another method added a parameter
<headius> yeah I saw those
<headius> two encoding failures
<headius> perhaps new tests that we never passed
<headius> I thnk these are in the 9.3.4 results too
<enebo[m]> Some of this stuff does not surprise me too much
<enebo[m]> the encoding things
<headius> not sure how to proceed with this because it isn't green but it almost is
<enebo[m]> I remember there were also some weird creation params (to db itself) or envs which also could cause weirdness
<headius> these may need fixes in ar-jdbc or JRuby itself
<enebo[m]> I think this 9.2.20.1 result looks good enough to tag those two out
<enebo[m]> but odd 9.3 has the errors it does
<headius> can we do that?
<headius> oh it does have excludes
<headius> go figure
<headius> so we are not perfect anyway
<enebo[m]> yeah we have some errors
<headius> yeah I can exclude these and do another push and it should be green for merge
<enebo[m]> dr-itz got rid of a lot over time but we had a few I think mostly for qg
<enebo[m]> err pg
<enebo[m]> So 9.3.4 and 9.2.20.1 are running the same Rails but one has extra mismatched params and return values
<enebo[m]> We can run Rails scaffold using 9.3 so I guess either we don't hit these differences or they don't matter in the context of our app
<headius> yeah apparently not
<headius> I pushed excludes, fingers crossed
<enebo[m]> Seems weird to me to see so I would like to look at those 9.3 failures but this is not really our focus
<headius> so if this is green I was planning to merge to master and then open a new PR to expand the runs... or would you prefer we branch off 6 and start running 7 stuff?
<enebo[m]> I strong suspect the 9.3 errors will not be excluded...looks to basic
<enebo[m]> well master is updated for 7 stuff already
<headius> I expect it will take some screwing around to get PG set up and working... sqlite will probably be easy
<enebo[m]> stable-6.1 is just for 6.1
<headius> ah k
<headius> I forgot how we are handling branches and merges
<enebo[m]> although once we get first 70.0 out we will make a 70-stable branch
<headius> so after this merges to master, what would be best value for my time
<headius> that's what I'm looking for
<enebo[m]> It is one huge ass set of merges but I think 50 and 51 probably can be EOL
<enebo[m]> I think we want to see what is broken on jruby-head running against Rails 7 tests
<headius> so same set up but flip to 7-0-stable and jruby-head
<enebo[m]> I thought AR would be the most logical place to start since we want to hit a database but we also need to run Rails unit tests also to see what other broken stuff we have
<enebo[m]> teag
<enebo[m]> heh yeah
<headius> teag
<headius> collar
<enebo[m]> both are no doubt in urban dictionary
<headius> if not they should be
<enebo[m]> The 9.3 stuff bugs me
<headius> as it should
<enebo[m]> The fact 9.2 runs differently is confusing
<enebo[m]> I really really did not think 9.3 did basic Ruby worse than 9.2
<enebo[m]> unless this ends up being some issue with JI changes or something
<headius> rails 6 depended on what Ruby version?
<headius> 6.1
<headius> small possibility something changed for 2.6 maybe
<enebo[m]> I don't know but we use that version as a sanity check for a simple sqlite3 app
<headius> but I would have expected them to fix anything
<headius> it's green with the two new excludes
<headius> so this is basic GHA working and I will merge PR
<enebo[m]> ok yeah I thought there was something with encoding broken in one of the adapters
<enebo[m]> I thought it was pg but hell we have to remember too much already
<enebo[m]> fantastic
<headius> MariaDB is just a different engine for MySQL yeah?
<enebo[m]> yeah
<enebo[m]> I don't really know how much they diverge in behavior and I did not set up testing maria so I don't know why both are run
<enebo[m]> I am guessing there is something different
<headius> yeah that was my thought
<enebo[m]> It could just be a nice sound engineering decision to make sure it always works though
<headius> 1) Error:
<headius> ActiveRecord::AdapterTest#test_not_specifying_database_name_for_cross_database_selects:
<headius> ActiveRecord::JDBCError: Client does not support authentication protocol requested by server. Consider upgrading MariaDB client. plugin was = caching_sha2_password
<headius> only failure setting driver to Maria
<headius> unsure if more needed on DB side to have this green
<enebo[m]> I have no idea
<enebo[m]> I would say exclude for now and see if someone complains
<headius> this may just need a newer jdbc driver
<enebo[m]> yeah
<headius> need to get this repo set up to pull drivers from maven... committing 500k binaries to the repo is no fun
<enebo[m]> yeah
<headius> I remember why I don't do server app dev anymore
<headius> I believe I have the arjdbc tests running on mysql now
<enebo[m]> looks like the if/else got rid of one error and one more which is unmarking keyword should make spec:ruby:fast:jit green
<enebo[m]> (sans wip obviously which is at ~171)
<enebo[m]> About 70 of those is fiber
<enebo[m]> are
<enebo[m]> ?
<headius> yeah
<headius> sqlite3 arjdbc tests running
<headius> sqlite3 rails tests running and expanded mysql to with/without prepared statements
<headius> the sqlite3 rails tests also fail on those same two encoding tests
<headius> I can add the same excludes
<headius> this should be six green jobs
<headius> bbiab
<headius> oh enebo I have push rights for net-http now so I will push a pre gem with the ssl tweaks and we should be able to switch to the gem on master
<enebo[m]> great...thanks for what had to be a painful day of setting that up
<headius> yeah I will try to expand to postgres when I return to the GHA stuff
<headius> for net gems, latest push here uses net-http pre gem and net-imap (it needed strscan which we have now) so assuming nothing serious breaks this PR should be ready to merge: https://github.com/jruby/jruby/pull/7088
<headius> and then we are on gems for all net-* on master
<headius> Other than postgres, let me know what you think the next steps are for GitHub actions to support Rails 7 work
<headius> Shouldn't be hard to make a PR to just switch these test runs to Rails 7 but I'm not sure if Master is ready for that or not
<enebo[m]> headius: for master of AR-JDBC I think we should use latest stable Rails 7 and jruby-head
<enebo[m]> Rails 7 will never work with 9.3 so I don't think it matters how green master is
<headius> I'm not sure how merges have been handled but these GHA changes probably should get on the 6.1 branch
<headius> As is they should copy right over and work I believe
<enebo[m]> So long as stable-61 is
<enebo[m]> yeah when we land stuff for ARJDBC it will be lowest supported branch for the fix/change and then merges until we get to master
<enebo[m]> So your master changes probably should have started on 61 branch but it is fine. You can just cp that there and we/you/me can change to jruby-head and rails 7
<enebo[m]> I am pretty curious to see how good/bad it runs
<enebo[m]> Anything serious from a Ruby perspective would trump any other work on Ruby 3 since being able to run Rails is paramount
<enebo[m]> and it would be Ruby 3 work after all
<headius> Okay, I will attempt to get postgres running and once that's green I'll copy the GHA stuff to the 6.1 branch and try switching master around
<enebo[m]> I am somewhat optimistic. I think 2.7 did not add include/prepend changes so that not existing will not be an issue
<enebo[m]> Also areg evaluation also was not 2.7
<headius> Yeah that's one piece I'm a little worried about but so far it hasn't come up
<enebo[m]> The include/prepend should not be too bad since we track subclasses already
<enebo[m]> I think anyways. We are tracking that direction already so we mostly have to just perform the activity of adding it
<headius> net-* PR looks good so I am merging
<headius> two more default gems checked off the list
ahorek[m] has joined #jruby
genpaku has joined #jruby
<headius> woot, pgsql running
<headius> what a hassle, all these databases have their own arcane ways to configure a user
byteit101[m] has quit [Ping timeout: 240 seconds]
byteit101[m] has joined #jruby
GaurishSharma[m] has quit [Ping timeout: 240 seconds]
demon36[m] has quit [Ping timeout: 240 seconds]
GaurishSharma[m] has joined #jruby
demon36[m] has joined #jruby
<headius> ok pgsql is green on arjdbc tests and has like three failures I'm not sure about on rails tests (excluding the two encoding fails the other DBs have had)
<headius> we can exclude those but maybe enebo kares have thoughts on the fails
<headius> I'm done for the night