cloudsoft / jclouds-vcloud-director

0 stars 9 forks source link

Fix/jclouds 2.0 compatibility #48

Closed aledsage closed 7 years ago

aledsage commented 7 years ago

See individual commits for details.

TemplateBuilderImpl is a copy, so I updated that by copy-pasting the v2.0.0 version, and then applying the changes (see comment at top of the class).

To make mvn clean install work, I needed to make lots of changes (because jclouds 2.0.0 uses google’s error-prone for additional validation checks). This required quite a few changes, including:

- in pom.xml, change error-prone’s version to 2.0.15 and disable some checks that couldn’t be found.
- in pom.xml, change “animal-sniffer” to check for java 1.7 compatibility, rather than 1.6
- add generics (even where java can infer the right type)
- remove unreachable code
- disable VCloudDirectorComputeServiceAdapter.getAvailableVMsFromVAppTemplate, which was doing `computerName.equals(computerName)`.
  TODO: What is the right fix?!
- Fields of enums must be final
- imports of classes must not use “static”
- remove @Provides from VCloudDirectorApi.getCurrentSession, because it’s not in a guice module.
- Use “L” instead of “l” for longs
- Add @Override everywhere
- Pass charset to `new String(byte[])`
- Make inner class `static` where possible
- Don’t throw exception in finally block
- Ensure `String.format(...)` has right number of args
aledsage commented 7 years ago

The travis build failure is:

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.1:compile (default-compile) on project vcloud-director: Execution default-compile of goal org.apache.maven.plugins:maven-compiler-plugin:3.1:compile failed: An API incompatibility was encountered while executing org.apache.maven.plugins:maven-compiler-plugin:3.1:compile: java.lang.UnsupportedClassVersionError: javax/tools/DiagnosticListener : Unsupported major.minor version 52.0

That suggests it's using Java 8 to compile (i.e. picking up Java 1.8 of rt.jar), and that the new pom.xml entry that validates we are 1.7 compatible then fails.

I'm now getting the same error locally (but thought that it was working for me on Friday, so not sure what's going on with my local machine!).

aledsage commented 7 years ago

@googlielmo (cc @bostko @andreaturli) are you happy for this to be merged now? We still need to do more testing, but this "moves things forward"! The live tests need some attention before they can be run properly.

bostko commented 7 years ago

@aledsage It looks good. I will look at the tests this week.