Set platform to "java" under JRuby

[JasonLunn]

Proposed fix for #1594

[grpc-kokoro]

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

[JasonLunn]

Gemfile.lock has been removed in this PR because it was leading to broken tests. It is worth noting that the breakage seen in travis before it was removed was not reproducible locally while I was working under JRuby 9.1.5.0 but was immediately reproducible when I changed back to 1.7.x.

Currently the Gemfile delegates all dependency management togoogle-protobuf.gemspec via the gemspec directive. If there are concerns about having unpinned development dependencies, I propose that be solved by adding additional parameters to the calls to add_development_dependency.

[JasonLunn]

CC: @blowmage

[blowmage]

Whoah! Does this work? Can the gem be installed on JRuby with this change?

[JasonLunn]

@blowmage - Works for me locally, and passed the travis test. I am still waiting for an answer on what impact this would have, if any, on the gem publishing process. Also, RubyGems has gone to v3.1.0, so I expect there is some large upstream merge about to land here that could delay this getting merged.

[JasonLunn]

CC: @apolcyn

[apolcyn]

I tried this change locally and it to work for me too! That is, I'm building the gem as described in https://github.com/google/protobuf/tree/master/ruby, and the -java suffix does appear now.
cc @haberman I think this is what's needed for jruby publish to work, though I don't think I can approve/merge this.

[JasonLunn]

@apolcyn - is there anything about the code quality or style of the PR that needs improvement from your perspective in order to be eligible to be merged, or are you blocked by an authorization / policy issue?

[apolcyn]

@JasonLunn Code wise, I would think we'd want to keep the Gemfile.lock checked in, to keep actual versions used.
Merging this is an authorization issue for me though.

[JasonLunn]

@apolcyn - In your opinion is enhancing the specificity of the calls to add_development_dependency as suggested above an adequate alternative to restoring Gemfile.lock?

[apolcyn]

@JasonLunn - thanks I had missed that. In the case I think that making development dependencies more specific would work. That said, it looked to me like the travis failure was due to not finding the actual google-protobuf gem on jruby, wondering if it has to do with the gem just not being built or available somehow, in that test?

anyways though, I don't have authorization for this - I haven't been working on this codebase. cc @haberman

[JasonLunn]

Travis seems a little backed up at the moment, but I've added the requirements to the development dependencies. Note that this does not result in the exact same patch level versions as appeared in the Gemfile.lock. I could lock it down even further if that is perceived as a defect.

[JasonLunn]

Checks all passed - any other feedback?

[JasonLunn]

@haberman - any questions or concerns?

[JasonLunn]

@apolcyn - Is there an internal discussion going on about this PR out of band?

[apolcyn]

@JasonLunn From what I know, this change looks good; I think it should fix the jruby package issue. But I don't have further approval to make.

[haberman]

Is it necessary to specify versions? When/why will we update these versions?

[JasonLunn]

It was added in response to commentary above about the removal of Gemfile.lock. The tests pass with the latest versions of all the gems that have version requirements specified. I believe the concern is that to have completely unpinned development dependencies could introduce flakiness in the automated tests.

[JasonLunn]

Thanks all - is there any other work need from my end to change the distribution process so that both gems for both platforms are published during subsequent releases?

[JasonLunn]

@haberman - Is there anything keeping the 3.1.0 build of the gem being published for the JRuby platform?

[TeBoring]

I think this PR breaks the jenkins test for ruby.
https://grpc-testing.appspot.com/job/protobuf_branch/206/

[JasonLunn]

@TeBoring - It is hard to tell from the just the output, but since an essential part of this PR was to remove Gemfile.lock (because it cannot be right for both JRuby and non-JRuby platforms simultaneously), my guess is that Gemfile.lock is being created by bundle install under one Ruby and not being cleared out before bundle install is rerun under JRuby.

[JasonLunn]

Travis appears to be configured to run the various ruby tests individually, rather than invoking ruby_all in test.sh. We could solve the failure in Jenkins a couple of different ways I think -

1. Change Jenkins to run each test separately and remove the broken ruby_all since its current implementation assumes that you can repeatedly run bundle install in the same directory with different Ruby interpreters without managing Gemfile.lock
2. Fix ruby_all by explicitly removing Gemfile.lock in each of build_ruby_21, build_ruby_22, and build_jruby
3. Fix ruby_all by running git clean

Preferences?

[xfxyjwf]

I think we need a fix that allows us to run:

$ ./tests.sh ruby21
$ ./tests.sh ruby22
$ ./tests.sh jruby

without failing tests. Only (2) can satisfy this?

[JasonLunn]

I think (3) should remove a superset of the files removed by (2) - I'm theorizing that Gemfile.lock is enough but I'm not sure. I'll put up a new PR and link it here.

[JasonLunn]

#2254 is running through Travis now - are Jenkins builds kicked off automatically?

[skull-squadron]

BUNDLE_GEMFILE env var to set the gemfile.lock filename separately per each ruby, if necessary, but it's overkill.

Remove the Gemfile.lock from the repo, deps should be specified in the gemspec and the Gemfile should only contain:

source "https://rubygems.org"
gemspec

Gemfile.lock is ignored by rubygems because Gemfile and Gemfile.lock are bundler artifacts. End-users only see gemspecs.