<GitHub24>
[jruby] cshupp1 opened issue #5018: open3.rb broken in JRuby https://git.io/vNSL4
xardion has quit [Remote host closed the connection]
<GitHub168>
[jruby] enebo closed pull request #4991: Update File#path to raise IOError for files opened with File::Constants::TMPFILE option (ruby-2.5...file-path-now-raises-ioerror-when-tmpfile) https://git.io/vNuXN
<GitHub108>
jruby/master fff6ad0 Benoit Daloze: Squashed 'spec/mspec/' changes from 5f563e4..5d49a6c...
<GitHub142>
[jruby] olleolleolle opened pull request #5019: Do not leak DNS Request IDs (jruby-9.1...carry-do-not-leak-dns-request-ids-to-jruby-9-1) https://git.io/vNSsU
<cshupp>
@enebo what is the maven command to build jruby's complete jar?
skbb has joined #jruby
<skbb>
I upgraded jruby for my rails application from 1.7.18--> 1.7.27 to support a TLS 1.2 related change. I am unsure what underying ruby version is running with 1.7.27? I needed help to figure out if I need to update my rails from 3.2.18 to 4 or higher.
<jruby-n00b>
Hey, this might be a beginner's error, but I'm getting some "NameError: uninitialized constant" errors when running with jruby, vs it not happening when running with MRI. What is a good starting point to figure out the underlying problem for this?
<jruby-n00b>
I'm using thrift in this case, and it's not loading some auto-gen ruby code (a thrift constant), whereas in MRI there is no such problem.
<headius>
cshupp: ok, then there's some code generation bug somewhere
<headius>
cshupp: I'll try to merge from master
<headius>
jruby-n00b: hey there!
<cshupp>
headius that is master.
<headius>
best option generally is to open a bug so we can investigate together
<headius>
cshupp: ok something's up with your env then
<cshupp>
OK, but I was hoping I was the bug.
<jruby-n00b>
I did find https://github.com/jruby/jruby/issues/3920 and not sure if that could be related. My installed version is: jruby 9.1.15.0 (2.3.3) 2017-12-07 929fde8 Java HotSpot(TM) 64-Bit Server VM 25.152-b16 on 1.8.0_152-b16 +jit [darwin-x86_64]
<GitHub175>
[jruby] headius pushed 1 new commit to jruby-9.1: https://git.io/vNSVY
<GitHub175>
jruby/jruby-9.1 dd9e5e7 Charles Oliver Nutter: Remove some defunct code references to Rubinius.
<headius>
cshupp: I'll double check here
<cshupp>
OK
<cshupp>
I am on Windows.
<cshupp>
One of the three rails peole on the planet...
<headius>
jruby-n00b: hmm it looks like that particular issue is ok on 9.x
<headius>
it could be related but I'd say you should open a new bug with your details
<headius>
cshupp: we feel your pain...hoping to catch our Windows support up again here
skbb has joined #jruby
<headius>
cshupp: hey I'm confused...you said that was master but your past shows jruby-complete-9.1.16.0
<headius>
in any case we have test suites that build and run the jruby-complete jar through a number of test suites
<cshupp>
One sec I just shut that ide down
<cshupp>
I will do a git show
<headius>
I suspect you are mixing JRuby jars from two branches, because the packed thing is only on master
<cshupp>
I was switching local branches between master and your pull request
<cshupp>
but I expected the mvn clean package to do the clean part
<headius>
it certainly should
<headius>
do you have anything JRuby or Java-related in ENV?
<jruby-n00b>
headius: is it possible it's due to lazy loading? ie, a sub-require of a file that is implicit, and a constant referenced from one of those files that isn't explicitely specified. Is that jruby behavior?
<headius>
jruby-n00b: well, there are various complexities around loading in parallel, loading with autoload etc
<headius>
we have not ironed all those out but we have not had a bug report about them in a while
<cshupp>
headius, this time I manually cleaned out the target directory
<cshupp>
re-ran against master
<cshupp>
I only got 9.2.2 snapshots
<GitHub120>
[jruby] enebo pushed 2 new commits to jruby-9.1: https://git.io/vNSo7
<GitHub120>
jruby/jruby-9.1 26005d0 Thomas E Enebo: Merge pull request #5019 from olleolleolle/carry-do-not-leak-dns-request-ids-to-jruby-9-1...
<GitHub120>
jruby/jruby-9.1 ada05fa Pavel Forkert: Do not leak DNS Request IDs...
<GitHub42>
[jruby] enebo closed pull request #5019: Do not leak DNS Request IDs (jruby-9.1...carry-do-not-leak-dns-request-ids-to-jruby-9-1) https://git.io/vNSsU
<cshupp>
Ahhh, headius, your branch makes the 9.1.16s
<cshupp>
but irb comes up this time!
<headius>
woot
<headius>
what's on the branch right now just disables some things that Windows doesn't have...the ultimate fix for this stuff will be to finish the port
claudiuinberlin has joined #jruby
<headius>
enebo: perhaps I should conservatively merge the fix from that branch to 9.1 and finish up the port for 9.2
<enebo>
cshupp: ok so you are in vso/vso and it is just the error reporting telling us that
<cshupp>
yes
<cshupp>
it is what happens when you create a vso directory to clone the vso project
<enebo>
ok
<cshupp>
then you smack yourself and live with it...
<cshupp>
Now I have your guys code in jruby/jruby
<cshupp>
so I am not improving...
<headius>
I am going to focus on this populator/dynamicmethod thing for now, bbl
<enebo>
another thing I did not connect is capture3 is working partially since you used it for other things
<cshupp>
Yes, I cannot explain that
<cshupp>
it will not run a ruby binstub
<enebo>
My guess is sub-process from a sub-process which uses IO
<cshupp>
I wonder...
<cshupp>
let me append jruby to it.
<enebo>
capture3 jruby -ve1
<cshupp>
Could it be that it is not clever enough to use cygwin superpowers?
<enebo>
If that doesn't work then we know bundler is not specific
<enebo>
but you used which
<enebo>
that is from cygwin and it worked right?
<cshupp>
yeah but that is a binary executable
<enebo>
yeah
<cshupp>
it isn't using the shebang
<enebo>
try CMD once then
<cshupp>
Which one?
<cshupp>
just 'cmd'
<cshupp>
to bring a windows shell up?
<enebo>
I am guessing there is a .bat/cmd binstub on windows bundler right?
<enebo>
yeah
<cshupp>
irb(main):006:0> sttdout, stderr, status = Open3.capture3({}, "cmd")
<cshupp>
irb(main):007:0>
<cshupp>
=> ["Microsoft Windows [Version 10.0.16299.192]\r\n(c) 2017 Microsoft Corporation. All rights reserved.\r\n\r\nC:\\work\\VSO\\vso>", "", #<Process::Status: pid 16776 exit 0>]
<enebo>
So I think things you can do to try and narrow would be capture running a jruby sub-process AND while in a CMD window try and run bundler the way you reported
<enebo>
The second thing may eliminate cygwin
<enebo>
The first thing may point to something more basic about jruby
jruby-n00b has quit [Ping timeout: 240 seconds]
<cshupp>
So the bundle stuff does work for me in a cmd shell
<cshupp>
=> ["\"The version is \"\r\n\"Setting rails war to use production\"\r\n Prefix Verb URI Pattern Controller#Action\r\n root GET / welcome#index\r\nfetch_text GET /fet
<enebo>
I would definitely paste that into the issue
<enebo>
Looks like it may just be a directory/path find the thing issue then
<cshupp>
OK, I never use cygwins shell btw
<cshupp>
I only have it on my path
<enebo>
It did say that in the error message but it is hard to know if we can trust those sorts of message
<enebo>
cshupp: ah ok so not an issue then
<enebo>
cshupp: and your test largely demonstrates that the binstub does work now too
<cshupp>
OK I will add this to the bug
<cshupp>
but not with the shebang
<enebo>
well who knows? I guess I don't follow that
<enebo>
All I see is if you use fully qualified path to bundle it works
<cshupp>
OK, let me try and explain
<cshupp>
The first line of bundle's binstub is:
<cshupp>
#! jruby
<cshupp>
#! jruby
<cshupp>
most windows people type 'jruby bundle exec rake routes'
<cshupp>
but cygwin give shebang powers
<headius>
enebo: I see what you mean about the FFI stuff
<cshupp>
The thing is I am fairly certain I opened webpacker up and add the jruby
<headius>
I had not noticed Wayne made a superclass of DynamicMethod that all the FFI-based methods extend
<cshupp>
to the command
<enebo>
cshupp: ok I missed the 'jruby ' in that snippet above
<cshupp>
and it still didn't work
<enebo>
cshupp: I skipped over that thinking all you did was add the full path to bundle
<cshupp>
I did do that to feed it to jruby
<enebo>
headius: so we can add name to that subtype to eliminate that use of setName in DynamicMethod
<headius>
well that's the problem
<headius>
these things get constructed many places without any name
<headius>
like, no name even available...a wrapper around a function pointer
<enebo>
and we do not know they are the subtype at all?
<headius>
and FFI does not track or connect name with function pointer
<headius>
at this point I'm considering just making the name be some bogus generated thing like ffi<pointer>
<enebo>
headius: Are you trying to solve the whole enchilada right now? More than just addBuiltinMethod use of setName?
jrafanie has joined #jruby
<enebo>
I do not think ffi methods make it into that
<headius>
the thread is long
<headius>
in order to make invokers set name in constructor I have to change how they're generated
<headius>
FFI uses the same interfaces to generate
<enebo>
headius: minimum level of happiness for me is removing setName from builtin method thing
<enebo>
oh
<enebo>
and FFI also uses same code
<headius>
many of the paths to generating and constructi ng invokers has no name anywhere near
<headius>
so lots of signatures have to gain it
<enebo>
yeah
<headius>
I mean even MethodFactory doesn't pass in name
<headius>
it gets it from annotation for most methods, but does not get anything on the FFI path
<enebo>
so populators could be changed but there is common code which cannot use the same construction
<enebo>
so ffi would have either a fake name or have to be somewhat different
claudiuinberlin has quit [Quit: My MacBook has gone to sleep. ZZZzzz…]
<headius>
yeah
<headius>
or I'd have to do a LOT of hacking in FFI to get the name to always be with a function, which might not even be possible
<headius>
or I have it setName
<headius>
my thought currently is that the fake name is fine
<headius>
it's only used for inspect and super and these are FFI methods so they don't super
<enebo>
headius: yeah and it is not like ffi will not setName later if it is a valid name
<enebo>
headius: probably better to know we can always return something to avoid null checking or NPE if we don't
<headius>
enebo: there's a class called MethodMissingMethod you created that is no longer used
<enebo>
headius: and long term we can still probably make FFI push that setName method down to a more specialized type
<headius>
it's from 2010...tweaks since then have been general DynamicMethod changes
<headius>
I'm deleting it FYI if you want to have a look
<enebo>
headius: yeah I doubt anyone will miss it :)
<enebo>
headius: I did notice it a couple of weeks ago
<enebo>
when I tried pushing bytelist through all methods
<enebo>
I think I did delete it during that experiment since the API would have needed to change
<headius>
well, should be easy to merge then :-)
<enebo>
I did not roll with those changes
<enebo>
but I was committed on removing :)
<headius>
I'm making the name change on 9.1 but it will likely only be visible in the Method#inspect update on master
<headius>
very little code looks at getName on 9.1
<enebo>
headius: does that mean less is looking at name passed through call() etc?
<headius>
well all the places that needed name on 9.1 had it already
<headius>
through whatever mechanism
<enebo>
yeah but I mean on master
<headius>
the change on master aligns Method#inspect logic and DynamicMethod name requirement
<headius>
with what's in MRI's method structure
<headius>
basically
<enebo>
headius: so master will largely defer to field on method vs parameter to call
<enebo>
I am hoping for always so we can eliminate the parameter for 10k :)
<headius>
well so the name passed in seems to almost never be different from the bound name
<headius>
alias passes through the old name
<cshupp>
BTW, this will work
<cshupp>
sterr, stdout, status = Open3.capture3(webpack_env, "jruby #{ENV['GEM_HOME']}\\bin\\bundle exec webpack")
<headius>
so it begs the question whether the param is even needed now
<enebo>
"almost never" - class computer talk
<cshupp>
on either 9.1.15 or the snapshot
<cshupp>
once I take jruby out it bombs
<enebo>
headius: yeah it would be awesome to know that
<headius>
enebo: yeah, I just noticed it again because I realized AliasMethod didn't need an "oldName" field...its natural name should be the old name anyway
<cshupp>
Will you be able to make shebangs work?
<headius>
so that's one more place where all we need is the original method's name
<enebo>
headius: yeah I had a merge conflict with that :)
<headius>
noticed as in during porting...they get the "natural" name from the alias and it's the old name
<headius>
so I just did that
<headius>
now I'm not sure if anything passes through a different name
<headius>
I guess define_method might
<headius>
but that could be set once too
<enebo>
cshupp: Are you sure it is not calling bundle.bat?
<headius>
it's a super thing when it differs, IF it ever differs now
<enebo>
headius: define_method though seems like it could construct something with a name?
<cshupp>
let me put an echo in and see
<headius>
enebo: yeah exactly
<enebo>
headius: yeah ProcMethod has no name field but there is no reason it couldn't
<enebo>
or I should say extra constructor
<headius>
so again, I don't know anywhere that has to have a name passed in
<headius>
but that's another adventure unrelated to this
<enebo>
headius: hey if you correct these cases and run test:mri or specs perhaps you can find something or not!
<headius>
all methods will have a non-null name on 9.1 and master, but master makes more use of it
<enebo>
if not then that is somewhat exciting
<enebo>
I think I would just change the name to _ everywhere
<headius>
yeah, just trying to narrow my scope for the 2.5 days before I travel :-)
<enebo>
then for 10k remove it
<enebo>
:)
<headius>
I'll be working in transit a lot during the drip but I need to get something together for RubyConf
<headius>
the other two confs are condensed versions of LavaOne talk
<headius>
enebo: rest of the change doesn't seem bad...I'll run some sanity checks and push it
<headius>
I believe I can make name final now also
<cshupp>
This does:
<cshupp>
sterr, stdout, status = Open3.capture3(webpack_env, "bundle.bat exec webpack")
<cshupp>
and my war is built
<cshupp>
w/o the .bat it seems to go for the binstub directly
cremes has joined #jruby
<headius>
cshupp: that is not surprising...capture3 uses popen3 which is the only one with a working Windows impl
<cshupp>
A yucky little cmd shell opens up too, but my monkey opatch has the same thing
<cshupp>
So is this a jruby bug, or should the webpacker team smell windows and call the bat file?
<cshupp>
If I type:
<cshupp>
bundle exec webpack
<cshupp>
from the shell the shell chooses the bat file
<cshupp>
copen doesn't
<enebo>
cshupp: one thing to try is similar test with MRI on windows
<cshupp>
sterr, stdout, status = Open3.capture3({}, "bundle.bat exec webpack")
<cshupp>
That works on JRUBY
<cshupp>
sterr, stdout, status = Open3.capture3({}, "bundle exec webpack")
<cshupp>
That fails on jruby
<cshupp>
They both work on MRI
<cshupp>
So I think you are right enabo.
<cshupp>
So I think you are right enebo.
<enebo>
cshupp: ok cool. Add that to the issue.
<cshupp>
Done
<enebo>
thanks. you narrowed it down quite a bit. minimal is probably just two bin stubs (.sh and .bat) and we pick .sh all the time vs .bat on windows
<headius>
cshupp: that seems to just be an issue finding executable when it's a bat
<headius>
not looking for .bat before no ext
<enebo>
oh yeah or nothing first
<headius>
I seem to remember we had special logic for executable searching in Windows
<enebo>
I know I did a ton of logic in jnr-posix for exec which was based on MRI code
<headius>
ran into a few more paths not setting name but they were all deprecated, unused, or for things like UndefinedMethod
<enebo>
I wish spawn was properly in there and we could probably leverage some of that
<headius>
yeah that's the full solution
<headius>
getting the win32 equivalent
<enebo>
headius: you going to raise on getName on Undefined?
<headius>
I found some Ruby FFI impls of subprocess launching that can probably form the baseline for our Windows spawn
<enebo>
or perhaps that is too extreme
<enebo>
EXTREME!
<headius>
setName
<headius>
and I'll just deprecate it and make it do nothing for the moment
<headius>
oh wait, I just added it didn't I?
<enebo>
setName has been there a long time I think
<enebo>
Or I thought it had been
<cshupp>
Will you be able to make it pretty w/o the cmd shell popping up?
<GitHub82>
[jruby] headius pushed 1 new commit to jruby-9.1: https://git.io/vNSF3
<GitHub82>
jruby/jruby-9.1 577d13a Charles Oliver Nutter: Force DynamicMethod.name to always be non-null and final.
<headius>
enebo: I deleted it...if you are worried about fallout I can put it back as a no-op
<headius>
enebo: setName was added when I introduced hybrid backtraces
<headius>
so yeah it's pretty old
<headius>
kinda was hacking around missing names back then too
<headius>
cshupp: does MRI have an ugly shell window?
<eregon_>
[exec] to be identical to #<Thread:0x6b30d8d0@/home/travis/build/jruby/jruby/spec/ruby/core/thread/current_spec.rb:21 run>
<eregon_>
[exec]
<eregon_>
[exec] /home/travis/build/jruby/jruby/spec/ruby/core/thread/current_spec.rb:23:in `block in (root)'
<eregon_>
This sounds worth fixing
jruby-n00b has quit [Ping timeout: 240 seconds]
<eregon_>
> <headius> In the future maybe you could do it on a branch, so we don't have master going super red until we can work around it
<eregon_>
How would that work? I could make a PR. But it needs to be merged before any other spec is added otherwise it gets complicated
<eregon_>
Tagging new specs with jruby unfortunately takes me quite a bit of time, so it'd be nice if I can leave that to you guys (and you can also judge if worth fixing immediately or not)