-
Notifications
You must be signed in to change notification settings - Fork 40
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Baleen Graph doesn't build on Java 9 #79
Comments
That looks like it builds fine but there's a test failure. Those tests compare an existing file against a newly generated file and in particular it looks like the gyro.kryo version is failing and that the id is different. It has certainly been tested on linux and OpenJDK 8. So, just to be sure, check that you haven't inadvertently overwritten the reference file. The next most likely issue is that we have assumed the ordering is deterministic in the kryo file but it is not, and that something about the combination of OpenJDK and platform exposes this. If this is the case then the tests will have to change to be order independent. Another possibility is that the id is generated differently in the platform/JDK combination. Can you check the full log or breakpoint to see if you can determine if it is one of these cases? |
Yes, sorry - I meant the tests failed so a standard build failed (as opposed to a compile error). The reference file hasn't been changed - I've done a There are two tests failing in
For
|
For Expected path: |
Made the assertion weeker, to allow changes in the ordering of the lines. see dstl#79
I can't reproduce the failures here. The kyro test paths look reasonable, obviously the temp directory being platform dependent. The fail is in the For the ordering issue I've changed the test to sort the lines first before comparison, this is a weaker assertion so assuming it passes for you it would be interesting to understand why it happens for you but not anyone else. I've pushed it here so you can test it before I make a pull request. |
Ah right, I (wrongly) assumed that
The ordering issue wasn't fixed by your changes. I don't think it's the order of the lines in the file, but rather the ordering of the JSON elements on a line. It looks like |
James, I have set up a VM with the same configuration as you, and can build Baleen and run the tests of
Other than possible slight differences between Ubuntu versions, the only other difference that I can think of is that I build a fresh clone of the repo with a newly installed maven 3.3.9 and so I will have pulled all dependencies down from scratch. Is there any other relevant information you can provide regarding your setup to see if we can reproduce the issue? Alternatively, if you are able to provide a pull request that resolves it frmo your end that would be great! Thanks, John |
I compared my output to yours above, and noticed that whilst |
Ok, I'm not sure what the official line on Java 9 support for Baleen is? |
Unfortunately we currently only build and test with Java 8, however we would actively welcome open source contributions that improve compatibility with later versions of Java. I will leave this issue open in hope! |
Just a heads up. We've started a branch so the code builds with tests passing on Java 11, see https://github.com/commitd/baleen/tree/jdk11 Still need to look into this issue, but once done, we'll raise a PR (end of the week). It's largely just dependency updates (and a few trivial code changes for Odin). Hopefully it addresses and closes:
|
That sounds fantastic. I don't think that we move quickly enough to keep up with the 6 monthly releases of Java, but having support for the next Long Term Support version would be amazing. |
This has been fixed as of the latest snapshot build - any further issues, please re-open this issue or create a new one. |
I did a clean pull of the GitHub repository, but when trying to build the project it failed on the
baleen-graph
project.I'm building it on Ubuntu 16.04 with OpenJDK version 1.8.0_171.
UPDATE: Maven was actually using Java 9 (and not Java 8), and that was the cause of the problem. I've updated the issue title to reflect this.
The text was updated successfully, but these errors were encountered: