genpaku has quit [Remote host closed the connection]
genpaku has joined #jruby
<headius> enebo: you did some work on marshaling for 9.4 right?
<enebo[m]> it is likely there were fixes to 3.x support
<headius> this CCE thing seems to be a marshaling problem
<enebo[m]> Oh I think there was a lot of work actually
<enebo[m]> I just remember we were not matching MRI in significant ways
<headius> ok
<enebo[m]> If they are unmarshalling data from an earlier JRuby it maybe won't work but I have to recall what changed
<headius> that might be
<headius> I'm not sure
<enebo[m]> 170c124 was one big commit
<enebo[m]> ah yeah we did not properly support partially marshalled stuff which referenced itself
<enebo[m]> something like that. so partials was added to UnmarshalCache and a lot of alignment with MRI
<headius> hmm
<enebo[m]> This is an interesting dilemma if we cannot load old marshal format because you could say they need to unmarshal then remarshal with new format but how would you do that?
<headius> ok
<headius> so the root problem here seems to be that an instance variable slot is overlapping an object_id slot
<enebo[m]> to be fair I am not sure it can't read old data properly
<headius> after marshaling out and back
<headius> I ran his example with CRuby 3.1 and it works
<headius> so it wouldn't be something specifically about marshal spec changing
<enebo[m]> ok so it could just be a porting issue
<headius> it's unfortunately a lot of code, I'm trying to reduce it
<enebo[m]> It was huge and I think I fixed all? of the mri tests which were broken
<headius> ok
<enebo[m]> If you can extract a repro without exposing their code I can also help
<enebo[m]> I did this a year ago but I may still have some of it in my head
<headius> yeah I'm trying but without being able to see the dump side of this I am just guessing at the problem
<enebo[m]> Hmm does MRI support multiple marshal formats?
<headius> not that I know of
<headius> not even sure when they last rev'ed the format
<headius> they added some tweaks for compact String encodings
<headius> that's the last thing I remember
<enebo[m]> I need a better git navigation mechanism
<enebo[m]> obviously the tests will be commits around this one
<enebo[m]> the history button shows that commit and back but not the next commit forward on idea
<headius> yeah can't go forward in git
<headius> oh but it should be able to see it in full history
<enebo[m]> b52d531413e53b469da21b548e5214b43813d2dd
<enebo[m]> So those are some of the fixes
<enebo[m]> I think a lot of this work was just to make recursive dump/loads work
<enebo[m]> but if you can unmarshal on Ruby you should be able to see the ruby right?
<enebo[m]> Ruby = MRI
<headius> well it works in MRI
<headius> I don't know exactly why it doens't work for us yet
<enebo[m]> I am saying if you can unmarshal the data you can see it and make a repro
<enebo[m]> that data should round trip if you remarshal on MRI too
<headius> hmm maybe
<enebo[m]> soup be back in a few
<headius> public void markAsPartialObject(IRubyObject value) {
<headius> }
<headius> partials.add(value.id());
<headius> this is where the id is sneakingin
<headius> s/sneakingin/sneaking in/
<headius> enebo: this appears to fix this particular case
<enebo[m]> so is this a case of two objects with the same id?
<enebo[m]> ah
<enebo[m]> return metaClass.getRealClass().getVariableTableManager().getObjectId(this);
<enebo[m]> Is this something like two objects having the same realclass?
<enebo[m]> If this is the fix I am pretty happy. I remember getting the proper ordering around things like encodings with their strings being really tough to unwind
<headius> yeah I don't think you want to use id here
<headius> that triggers it to create an object_id for every object passing through this partials set, and it seems like it is not being handled right by the variable table
<headius> perhaps because it is mid-marshal when the object suddenly gets an object_id
<headius> I wasn't sure how this maps to the CRuby code
<headius> the cruby code just seems to use their st hash
<enebo[m]> I don't remember why I picked id
<enebo[m]> It could have been trying to match MRI or I could have figured it was a unique id for a ruby object
<enebo[m]> java identity makes more sense to me though
<headius> aha
<enebo[m]> it dodges needing to know naything about how we implement identity
<headius> arg->partial_objects = rb_init_identtable();
<headius> so it is basically an identity st map
<enebo[m]> ok that answers that then
<enebo[m]> Just talking through it I can see that Java identity makes more sense since we do not have 2 different Java instances representing the exact same ruby instance
<enebo[m]> On top of even the notion it is constructing an unneeded fixnum
<headius> well, a Long, but yeah
<headius> and infecting every unmarshaled type with object_id along the way
<headius> INFECTING
<enebo[m]> yeah...low blow
<enebo[m]> :)
<enebo[m]> oh when I said Fixnum I mean what it did by calling id() -> return getRuntime().newFixnum(getObjectId());
<headius> I did notice things like String instances coming out of this suddenly having object_id so this may have impacted a lot of stuff
<headius> well, anything being marshal loaded anyway
<enebo[m]> String would be affected since it unmarshals encoding as a separate unmarshal
<enebo[m]> since even a already registered type is still a separate load it would still mark this in partial (I think)
<enebo[m]> no doubt though that partial support will have some impact on unmarshal performance anyways since we didn't track it at all before this point
<headius> yeah at least it will just be identity hashing now
<enebo[m]> yep. makes sense. ship it!
<headius> what else do we need for 9.3.10 and 9.4.1
<enebo[m]> I wonder why I didn't fix those last 3 fails
<enebo[m]> in the specs
<enebo[m]> well you have a PR going into 9.3 right now right?
<headius> this fixes the instance_eval yield thing: https://github.com/jruby/jruby/pull/7605
<headius> that is 9.3 yes
<enebo[m]> snakeyaml is on 9.3?
<enebo[m]> I think we agreed to update psych and it is marked
<enebo[m]> we never got any response on the indy issue with rails on 9.3 so we punt
<enebo[m]> jruby supports new arch
<enebo[m]> otherwise the rest we have been punting
<headius> snakeyaml is not even released yet
<headius> with the new engine
<headius> I have a PR against master to test it that looks ok I think
<enebo[m]> so you are using snapshot of snakeyamlengine?
<enebo[m]> I guess if it is not out we cannot include it :)
<enebo[m]> Actually https://github.com/jruby/jruby/issues/6821 might be fixable today if we isolate it
<enebo[m]> sicne they said 9.4 works
<enebo[m]> this is likely fixed already but we need confirmation
<enebo[m]> you have a mac to test it so this is probably done
<headius> sorry I mean the psych using snakeyaml engine is not yet released
<enebo[m]> oh ok
<headius> I pushed a 5.1.0.pre1 to test with
<enebo[m]> either way unless they can release today it will miss
<headius> it is not a drop-in replacement
<headius> well it might be possible
<enebo[m]> ok so the jruby-complete security tool users will just not use jruby-complete until a later date
<enebo[m]> I don't really care in the near term on this. It will get fixed in the future and jruby-complete is not even vulnerable so this is appeasing people's processes
<headius> psych 5.1 seems ok against master
<headius> if we are ok with it I could push for a non-pre release
<headius> that's the only path forward though since it requires a new build of psych ext
<headius> updating 9.3 may be problematic for other reasons though
<headius> it is a big leap in psych version and suddenly will be 1.2 yaml only
<enebo[m]> What concerns are there? different yaml version supported?
<enebo[m]> not sure what may not be the same in engine that original API had
<enebo[m]> ah 1.2 only
<enebo[m]> Does that remove features? Or refuse to parse old format?
<headius> it is mostly a superset of 1.1 but there are a few small breaking changes
<headius> no idea how far it is from 1.0 or how far 1.1 is from 1.0
<enebo[m]> yeah well we are sort of up against a wall
<headius> yeah I agree
<enebo[m]> we could put this into 9.4.1.0 and get some feedback then move it back to 9.3.11.0
<enebo[m]> I don't think people expect there to be no issues on 9.4
<headius> yeah I'd be ok with that
<headius> I can update the snakeyaml cve thing with status
<enebo[m]> so make a separate issue on this marked for .11 and we will get two milestones out of the issues system after all
<headius> ok
<enebo[m]> I am hoping to see some feedback from logstash people on the regexp regression
<headius> how about I just rebase and retarget my psych 5.1 PR against 9.3.11
<enebo[m]> 100x slower is a big deal so I hope to see that one done
<headius> we'll merge it after 9.3.10 release and then it will merge forward to 9.4 for 9.4.1
<enebo[m]> yeah that makes sense
<enebo[m]> I tried a ping there last week but nothing on it
<headius> yeah beats me
<headius> can't really help with the available data
<enebo[m]> so 3 opens are fixed I think and you have 2 prs for 2 of them and the third I think you just need to run Puma on your mac
<enebo[m]> snakeyaml to .11
<enebo[m]> require_relative with chdir is not happening for this so we punt
<enebo[m]> indy rails still has no repo punt
<enebo[m]> two JI issues which have been punted before
<enebo[m]> looooooongarch issue? https://github.com/jruby/jruby/issues/7260
<headius> yeah I need to figure out what's left on the loongarch stuff
<headius> it is almost all in there already
<enebo[m]> ok well I moved issues so we have 4 there and 3 are done? 1 open is looongarch
<headius> ok
<headius> psych 5.1 PR is rebased
<headius> oh and that rand thing needs to be fixed
<enebo[m]> I thought it was fixed
<headius> the CI issue
<headius> where it is right half the time but it is not honoring the passed random:
<headius> did you open an issue for that?
<enebo[m]> oh [0, 1].shuffle(rand: something)
<headius> I will look into it
<enebo[m]> ok yeah I guess technically this is still the same issue you fixed but something got unhooked up
<enebo[m]> I bet that is a pretty obscure option too
<headius> oh but this is only 9.4
<headius> 9.3 does not have random updates
<enebo[m]> yeah
<headius> bug for new rand issue: https://github.com/jruby/jruby/issues/7607
<headius> the kares PR is trivial
<headius> git st
<headius> ack
<headius> JNR update should fix that aarch64 thing
<enebo[m]> headius: yeah. I figured but you can test it since you have an M1 mac
<headius> yeah just realized I can just try puma
<enebo[m]> 6365 may be fixed by your change to snakeyamlengine?
<headius> yeah probably
<headius> my later comments seem to indicate all it was waiting for was a new psych release
<headius> and then we didn't upgrade 9.3 because it didn't pass a bunch of tests and I never circled back
<headius> well it fails with a JDBC connection issue (perhaps using older sqlite driver that has no native m1 in it) but puma works
<enebo[m]> cool
<enebo[m]> headius: read me comment and make sure I did what you expected
<headius> Does seem right
<headius> Not sure what updated to fix this though
<headius> Last item for 9.3 is loongarch. I'll investigate that after I land
<headius> With JNR updated I don't think there's much left
<enebo[m]> yeah so tomorrow possibly for both based on triaging 9.4 stuff rest of today
<headius> I will talk to tenderlove and hsbt about psych 5.1 release
<enebo[m]> ok. yeah itwould be nice to have it in so we can see if anyone reports anything
<enebo[m]> we can also do thursday for 9.4 too
<headius> Looks like ruby-lang mailing lists moved to Google groups
<enebo[m]> So this is something untargetted we should discuss
<enebo[m]> HAHAH
<headius> Yeah I will not have time to look into that
<enebo[m]> It is funny because I had an issue posting to ruby-talk and this was probably the reason
<enebo[m]> It feels like it has an element of risk too
<headius> Yeah it needs some time to investigate
<enebo[m]> I marked it 9.3.11.0
<headius> Ok
<enebo[m]> From the looks of this just changing to snakeyamlengine is not enough
<enebo[m]> Should this get targetted for 9.4.2.0?
<headius> enebo: that donv issue is in discussion. I have a PR that exposes those settings in Psych
<enebo[m]> I saw there is a PR over there after messaging
<enebo[m]> so I maybe will move that to 9.4.2
<headius> If it can get in 5.1 great
<enebo[m]> we can move that back to 9.3.x at some point
<headius> Still being discussed though
<enebo[m]> 9.4.1 is current so I mean later
<enebo[m]> and this will only affect jruby so I think if that options stuff lands later in 5.1 we won't complain
<headius> It certainly won't break anything
<byteit101[m]> enebo: unclear. My initial plan that I ran by headius was subspawn only takes over Process.spawn/wait in 9.5, and is PTY only (+user gem use) in 9.4. If you think we should replace it earlier, even for just windows, we should discuss that
<enebo[m]> byteit101: if you can gem install I think we can entertain installing that gem on windows for 9.4 installers
<enebo[m]> headius: I will optimistically put that donv yaml issue for 9.4.2.0 so we can keep it visible
<byteit101[m]> use is require 'subspawn/replace' for end users
<enebo[m]> byteit101: we would just somehow do that require on windows and it would be the default
<headius> enebo: that's fine
<byteit101[m]> I do think I need to expose wait/waitpid though first
<byteit101[m]> busy with stuff this week, do you have an eta for 9.4.2?
<byteit101[m]> that I should try to target time wize
<headius> 9.4.2 is not for a couple weeks at least
<headius> 9.4.1 is this week so not much else getting in
<enebo[m]> byteit101: 9.4.2 is aspirational perhaps if you are not ready it can push to another point
<byteit101[m]> ok, cool
<byteit101[m]> The thing I need the most is a proper place to run jruby's rubyspec suite on windows with my changes
<enebo[m]> If we have a 9.4.2 in a couple of weeks it is because we got a list of big issues to turnaround on quickly
<headius> Windows vbox dev images are freely available
<byteit101[m]> Oh, and I can include the arm64 build changes if headius gets around to testing the -java gem from last week
<byteit101[m]> Ah, where?
<headius> Yeah I can revisit that after release
<byteit101[m]> cool
<headius> From MS somewhere
<byteit101[m]> ("ve been developing win32 in wine so far)
<headius> It's easy to find
<byteit101[m]> k, will look next week
<byteit101[m]> currently on hold with the IRS
<headius> Lovely
<byteit101[m]> at least the wait music isn't too bad
<enebo[m]> eye of the tiger?
<byteit101[m]> no, elevator music, but tolerable elevator music unlike the last place I called
<headius> this is the only piece of the loongarch work that has not been cherry-picked back to 9.3
<headius> I think I'm going to just go ahead and do that and we can have loongarch support aligned on both branches