<headius[m]>
the other form doesn't output anything at all though
<jswenson[m]>
I got output when I launched the daemon, but no out when it is already running
<jswenson[m]>
ok yeah both of those hang for me, but I dont get the `> Connecting to daemon` output
<jswenson[m]>
sidenote, with the backticks workaround we’re seeing some new failures in CI with the error `RuntimeError: implicit argument passing of super from method defined by define_method() is not supported. Specify all arguments explicitly.`
<jswenson[m]>
I haven’t looked into these at all yet, but I did see some conversation about define_method recently in this channel.
<jswenson[m]>
we haven’t found which ones are doing it yet, might be a library we’re using.
<jswenson[m]>
Will file an issue if we get closer / think it might be something new
<headius[m]>
oh that is an enebo joint
<headius[m]>
if you can figure out where that is happening it is either a bug in the Ruby code or a bug in enebo fix
<headius[m]>
we do want to know
<headius[m]>
jswenson: there is another possible workaround
<jswenson[m]>
oh yikes this is a bit of a mess here I found some suspicious code that is doing some magic, let me see if I can write up a little repro.
<headius[m]>
ok
<headius[m]>
proper ruby behavior is to reject a bare `super` that is in a `define_method` body
<headius[m]>
but we supported it (badly) until this release
<headius[m]>
ok the other workaround I thought we had does not seem to exist... I thought I had implemented an opt-in "real" chdir but can't find it now
<headius[m]>
this is complicated because we simulate chdir to avoid changing the working dir for the whole JVM (which could have unexpected consequences)
<headius[m]>
so we have to do a manual chdir when shelling out if a chdir is in play
<headius[m]>
seems we need to put this back but I want to dig into what is actually causing this to hang now
<headius[m]>
interestingly backquote always uses native process launching too, but it works
<headius[m]>
jswenson: ok I will update issue with repro information and this will be on its way to fixing
<headius[m]>
if you find valid Ruby code that is being rejected by that `super` thing open another bug for it
<headius[m]>
I have to log off for a while
<jswenson[m]>
will do! thanks
<headius[m]>
oh hey
<headius[m]>
if you want to confirm this fixes it you can git revert 378f691e1b31889e9e6a380bb678525f55c2a95b
ur5us has joined #jruby
<headius[m]>
rebuild and try that and it should go back to the old behavior for system within chdir
<headius[m]>
jswenson: let us know on the issue if that works
<jswenson[m]>
headius: enebo looks like we have a `define_method(:method_name) { super ; ... }` that is deeply nested / in some other hairy dynamic code, but it doesn’t seem at all special. That seems like the expected behavior then?
<enebo[m]1>
jswenson: that behavior was never valid ruby so we "fixed" it for .15
<enebo[m]1>
super(the, args) will still work but not zsuper
<kalenp[m]>
what if the parent method takes no args?
<enebo[m]1>
kalenp: super and super() are totally different
<kalenp[m]>
of course they are... ah, ruby
<enebo[m]1>
I think zsuper or super without argument is a major misfeature of Ruby
<enebo[m]1>
super without parens means whatever the method you are calling it from took...except if it is in define_method in which case it will raise saying you cannot do it...
<enebo[m]1>
just to cement this def foo(a); super; end ... the super will be like doing super(a)
<kalenp[m]>
what I love is that for "normal" methods, the parens are just optional, but for `super`, suddenly they're significant, at least for the 0-args case
<kalenp[m]>
* what I love is that for "normal" methods, the parens are just optional, but for `super`, suddenly they're significant, at least for the 0-args case (maybe there's no `super the args`?)
<enebo[m]1>
unless you are some fanatical liskov substitution fanatic that idiom does not age well
<headius[m]>
Definitely a misfeature
<enebo[m]1>
I even doubled the word fanatic there...I am not a fan of implicit
<kalenp[m]>
thanks for the clarification. TIL
_whitelogger has joined #jruby
<jswenson[m]>
Cool. We’ll fix that and see how it goes. Won’t open a ticket.
<headius[m]>
👍
<headius[m]>
I will be doing a survey of all the different process launching things to make sure chdir logic is correct for .16
<jswenson[m]>
Currently I’m building from a sha (with a nix tool chain), I’ll have to see if I can figure out if I can just do it from a local checkout to see if that reversion fixes it.
<jswenson[m]>
Might have to wait until tomorrow though
ur5us has quit [Ping timeout: 272 seconds]
_whitelogger has joined #jruby
<headius[m]>
I managed to fix it locally
<headius[m]>
I don't have an explanation for the hanging yet but the code I thought was broken actually wasn't being used. If I force it down that path it works fine. There's a separate path for sh and chair that results in a hang
<headius[m]>
* I don't have an explanation for the hanging yet but the code I thought was broken actually wasn't being used. If I force it down that path it works fine. There's a separate path for sh and chdir that results in a hang
daveg_lookout[m] has quit [Quit: Idle for 30+ days]
<headius[m]>
so this was a confluence of unlucky happenings
<headius[m]>
a few years ago we added code from a user that created a new process group for the sh + chdir thingy only in the path that has a single program string
<headius[m]>
I am not yet sure of the mechanism, but when that process group includes a forked daemon, the waitpid ends up waiting on the entire group
<headius[m]>
so basically, the spawning of the daemon is what causes this to hang
<headius[m]>
a new workaround would be for you to ensure the daemon is already running, or to force gradle to run without using the daemon (which seems surprisingly tricky to do)
<headius[m]>
it also limits the scope of this regression since most `system` calls will not spawn long-running daemon processes
<headius[m]>
enebo: we will need a .16 in short order
subbu is now known as subbu|away
<headius[m]>
two failures on that PR, looking into it
subbu|away is now known as subbu
<headius[m]>
should be fixed now
<headius[m]>
`system "foo || bar"`
<headius[m]>
who does that
<headius[m]>
GREEN
<headius[m]>
we have a winner
<headius[m]>
this crap needs to be rewritten on master... most of the CRuby logic is totally unnecessary
<headius[m]>
I have been pondering how to write a test for this... it would need to spawn a daemon process and test that the daemon keeps running after waitpid returns, and not leak the daemon
ur5us has joined #jruby
<jswenson[m]>
headius: to confirm, I was able to pull down your latest commit there and verified that I was able to go through that flow with these changes.
<headius[m]>
that is good news
<headius[m]>
what a freak bug
<jswenson[m]>
we love those... keeps things interesting
<headius[m]>
jswenson: we will push forward with a quick .16
ur5us has quit [Quit: Leaving]
<jswenson[m]>
Awesome. We got our full test suite passing with the backticks workaround and removal of some naked super calls. Also had to require racc explicitly. Since .16 is imminent, we’ll probably end up waiting for that.
<headius[m]>
racc thing will be fixed too
<kalenp[m]>
is there a way to get notified of upcoming releases so that we can try things out before the release is cut? I'm thinking we could make an email alias that you could send to when you're getting close so we know to check things out
<headius[m]>
kalenp: there is nothing like a prerelease list currently but it is a good idea
<headius[m]>
there's really just the two of us handling release overhead so the process is a little loose
<kalenp[m]>
for sure. I saw that you just created the post-release list in the last couple days :)
<headius[m]>
we do try to pre-release announcements on twitter and in here but probably should do mailing list somewhere
<headius[m]>
or schedule it more than a couple days in advance
<headius[m]>
if there's a way you all could run some side CI off HEAD that would be a possible path forward
<headius[m]>
but false negatives suck I know
<kalenp[m]>
our CI system is kind of a mess, so we're probably not ready to have a separate JRuby-HEAD process going (though I would love it if we could). right now we're more at the "oh, hey, let's try to manually shoehorn this in to make sure things don't break" stage
<headius[m]>
yeah maybe we should try to bake in a day or two after closing the door before we release... we just get eager to get it out
<kalenp[m]>
👍️
<kalenp[m]>
if you do set up a list, I'll make sure we get subscribed to it
<headius[m]>
actually could you open an issue about it?
<headius[m]>
I would like to discuss and keep the discussion