You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by "onichols-pivotal (GitHub)" <gi...@apache.org> on 2018/10/17 17:43:03 UTC

[GitHub] [geode] onichols-pivotal opened pull request #2643: GEODE-5846 GEODE-5850 GEODE-5860 GEODE-5862 GEODE-5878 fix JAVA_HOME

Prior to this PR, the pipeline was not running all tests under the advertised JVM.  Now:

* pipeline explicitly sets compileJavac and testJVM
* pipeline no longer sets JAVA_HOME when invoking gradlew
  * this ensures unintended java version does not leak into tests
* developers may set JAVA_HOME and it will be used as default for both

This fix exposes additional Java 11 test failures now that all tests are running with intended version of Java.

This PR also subsumes https://github.com/apache/geode/pull/2608

Thank you for submitting a contribution to Apache Geode.

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

### For all changes:
- [x] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?

- [x] Has your PR been rebased against the latest commit within the target branch (typically `develop`)?

- [x] Is your initial contribution a single, squashed commit?

- [x] Does `gradlew build` run cleanly?

- [n/a] Have you written or updated unit tests to verify your changes?

- [n/a] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?

### Note:
Please ensure that once the PR is submitted, you check travis-ci for build issues and
submit an update to your PR as soon as possible. If you need help, please send an
email to dev@geode.apache.org.

[ Full content available at: https://github.com/apache/geode/pull/2643 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] PurelyApplied commented on pull request #2643: GEODE-5846 GEODE-5850 GEODE-5860 GEODE-5862 GEODE-5878 fix JAVA_HOME

Posted by "PurelyApplied (GitHub)" <gi...@apache.org>.
I have mixed feelings about this and would like to hear other opinions...

On the one hand: I agree that restricting `JAVA_HOME` will provide a great deal of safety from accidentally leaking java versions where we don't mean to do so.

On the other hand: the `gradlew` wrapper is typically generated with the `gradle wrapper` command, and it feels off to deviate from that in a way that isn't in our `build.gradle`.

On the other other hand: `gradlew` is part of our source tree, so it's not as if we're encouraging developers to go fetch their own gradle wrapper in any event.

I think I'm still +1 on this PR overall, though.  Just a bit conflicted about it.

[ Full content available at: https://github.com/apache/geode/pull/2643 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] dickcav closed pull request #2643: GEODE-5846 GEODE-5850 GEODE-5860 GEODE-5862 GEODE-5878 fix JAVA_HOME

Posted by "dickcav (GitHub)" <gi...@apache.org>.
[ pull request closed by dickcav ]

[ Full content available at: https://github.com/apache/geode/pull/2643 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] PurelyApplied commented on pull request #2643: GEODE-5846 GEODE-5850 GEODE-5860 GEODE-5862 GEODE-5878 fix JAVA_HOME

Posted by "PurelyApplied (GitHub)" <gi...@apache.org>.
I dislike this a great deal less than actually modifying the gradlew.  Woot woot!

[ Full content available at: https://github.com/apache/geode/pull/2643 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org