Use Acceptors to get peer and sock names if not present in Connections #743
Conversation
| s.platform = 'java' | ||
| else | ||
| s.extensions = ["ext/extconf.rb", "ext/fastfilereader/extconf.rb"] | ||
| end |
There was a problem hiding this comment.
I don't recall how conditionals in the gemspec interact with rubygems.org?
There was a problem hiding this comment.
Hmm... A good question. I'm not sure. I wasn't aware that there were any interactions between specs and rubygems.org. I thought it was all evaluated at ruby runtime. A quick google doesn't seem to bring up anything obvious.
There was a problem hiding this comment.
What prompted your change here? I'm not a regular JRuby user, but let me know what the problem was / improvement is for?
There was a problem hiding this comment.
Setting the platform and adding the jar file will build a 'java' gem that will be preferentially searched for and installed if you're on a java platform when it's available on rubygems.
Segregating out the extensions will prevent jruby from trying to build those extensions (which it can't do) so that the gem can be installed.
There was a problem hiding this comment.
I've built and uploaded the Java gem without this, are you sure it's necessary?
There was a problem hiding this comment.
I had been using rake gem and rake package and they didn't seem to be java-specific (it was still trying to build c-extensions when I installed the local one), but I didn't try rake build. Also, entirely possible I was doing something dumb.
I'd be happy to pull that out. I found a few more things I'd like to contribute. Would you prefer those in a separate PR, or in this same one?
There was a problem hiding this comment.
Sorry rake gem is correct, but you would need to ensure you were using jruby+rake to invoke it.
There was a problem hiding this comment.
I must be doing something else wrong because this is what I get with my change reverted: https://gist.github.com/lampad/ac5ac84ea5c5d313f1af755fedee33c7
In any case, I'll go ahead and revert it.
There was a problem hiding this comment.
@lampad Before I build the 1.2.1 gem, I'll be sure to revisit this to make sure I can build the JRuby gem, but please do remove that commit for now.
|
I've got a couple more changes coming to support TLS connections (a few more missing constants, and setting some tls opts. |
|
The other TLS changes I mentioned will come in another pull request, as there's some work to be done on the java side to support it. |
|
@sodabrew Sure. Looks good at first glance. I'll dig a bit deeper when I get some free time. |
On Jruby, when starting a tcp server locally using the 0 port argument (to select a random port), I was later trying to discover what port it was on by calling:
Socket.unpack_sockaddr_in( EM.get_sockname( signature ) ).firstBecause the server doesn't get added to the Connections HashMap, I was getting an NPE. This PR appears to address it in my local tests, and it favors entries in Connections and falls back to the Acceptors.
I also added a couple things to make java builds friendlier, and a missing constant that was also causing me some pains.
Things appear to install and run fine, but when I try to run tests (using JRuby 9.1.2.0), I end up getting many errors about too many files being open. I could crank up my ulimit (it's already at 512), but I'm not sure how much further I need to go.