Update minimum JDK requirements.

Review Request #4253 — Created Sept. 21, 2016 and submitted

jsirois
pants
jsirois/java/update_req
3892
35f82da...
pants-reviews
stuhood, wisechengyi, zundel
The minimum was bumped to 8 in:
  https://rbcommons.com/s/twitter/r/4127/

 README.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Verified the minimum with:

$ ./pants clean-all publish.jar \
  --no-prompt \
  --no-dryrun \
  --force \
  --local=/tmp/repo src/java/::
$ mkdir /tmp/junit
$ unzip -qd /tmp/junit/ \
  /tmp/repo/org/pantsbuild/junit-runner/1.0.14-SNAPSHOT/junit-runner-1.0.14-SNAPSHOT.jar
$ find /tmp/junit -name "*.class" | \
  xargs file | \
  cut -d: -f2 | \
  sed -E "s|^\s+||" | \
  sort -u
compiled Java class data, version 52.0 (Java 1.8)

IOW, the next publish of any org.pantsbuild support jar will contain
java 8 classfiles as opposed to the current crop of jars which all
still contain java 6 classfiles.

CI went green here:
https://travis-ci.org/pantsbuild/pants/builds/161763813

  1. afaik * OpenJDK 7 or greater, Oracle JDK 6 or greater still works, and there are still integration testes for those cases, but it would be a good idea to recommend jdk8

    1. Really? A java 6 JDK should reject java 8 classfiles, no?
    2. $ /usr/lib/jvm/java-6-jdk/bin/java -cp /tmp/repo/org/pantsbuild/junit-runner/1.0.14-SNAPSHOT/junit-runner-1.0.14-SNAPSHOT.jar org.pantsbuild.tools.junit.ConsoleRunner
      Exception in thread "main" java.lang.UnsupportedClassVersionError: org/pantsbuild/tools/junit/ConsoleRunner : Unsupported major.minor version 52.0
      
    3. hmm https://github.com/pantsbuild/pants/blob/63ea982f2bb397e407c8f8ebd75876eedf5ed79e/tests/python/pants_test/projects/test_testprojects_integration.py#L86-L86 seems to give contradicting ideas.

      would pants grab a different junit runner if only oraclejdk6 is present?

    4. So, my example above simulates an attempt to run the pants-published junit runner on a machine with only java 6 installed - which fails. IE: You can't use pants to jar or run junit tests on a machine with JDK<8 as soon as we next publish the junit-runner jar, jar-tool jar, etc.

      The unit test you point to tries to ./pants test testprojects:: ..., which is non-trivial to reason about what is involved. There are junit_tests targets under testprojects/ though so digging...

    5. Right, so that test works since it overrides the default platform down to java6.  The problem with publishing is we do not have a script that does the same, and so anyone attempting to publish org.pantsbuild jars, will get java8 classfiles.  If you did not intend all this when you submitted https://rbcommons.com/s/twitter/r/4127/ then the answer is probably to create a dedicated `build-support/bin/publish-jars.sh` script or some-such that flags the platform down to 6 to ensure the jars we pblish contain java 6 classfiles.  That said there was talk of raising the default to 8, in which case this is all moot, except to point out that pants.ini for pants itself should generally avoid being bent to satisfy CI.  It should satisfy production 1st and foremost with CI overrides in the CI-specific override file .... which is option #37 here.
    6. would pants grab a different junit runner if only oraclejdk6 is present?

      No, we'd need something like scala_jar and the scala platform jar naming ~standard (_2.11) is not a java standard. I think everyone probably agrees this is not a road to go down. Better to use a retroweaver type thing, but also a road not to go down.

    7. Sgtm! It's good to know what we can do in case user requests for java 6 or 7 for some reason.

    8. It would likely be reasonable to override the platform of all of src/java/org/pantsbuild/**/* to declare 6. I don't think that Yi's change was necessarily intended to cause us to begin requiring 6.

  2. 
      
  1. Ship It!
  2. 
      
  1. Ship It!
  2. 
      
Review request changed

Status: Closed (submitted)

Change Summary:

Now on master:

git log -1 origin/master
commit 67c33a1b235896ce0dd380c265767d52c8851019
Author: John Sirois <john.sirois@gmail.com>
Date:   Fri Sep 23 09:27:33 2016 -0600

    Update minimum JDK requirements.
    
    The minimum was bumped to 8 in:
      https://rbcommons.com/s/twitter/r/4127/
    
    Testing Done:
    Verified the minimum with:
    ```
    $ ./pants clean-all publish.jar \
      --no-prompt \
      --no-dryrun \
      --force \
      --local=/tmp/repo src/java/::
    $ mkdir /tmp/junit
    $ unzip -qd /tmp/junit/ \
      /tmp/repo/org/pantsbuild/junit-runner/1.0.14-SNAPSHOT/junit-runner-1.0.14-SNAPSHOT.jar
    $ find /tmp/junit -name "*.class" | \
      xargs file | \
      cut -d: -f2 | \
      sed -E "s|^\s+||" | \
      sort -u
    compiled Java class data, version 52.0 (Java 1.8)
    ```
    
    IOW, the next publish of any org.pantsbuild support jar will contain
    java 8 classfiles as opposed to the current crop of jars which all
    still contain java 6 classfiles.
    
    CI went green here:
      https://travis-ci.org/pantsbuild/pants/builds/161763813
    
    Bugs closed: 3892
    
    Reviewed at https://rbcommons.com/s/twitter/r/4253/
Loading...