Java 11, OpenJDK, ANTLR 4, and Travis #1
Conversation
…rain. Moves to Java11 and OpenJDK via AdoptOpenJDK within the processing4 train using a squash of sampottinger processing fork's java11 branch. **Primary required changes:** Some changes directly support OpenJFX / OpenJDK 11: - Response to image loading changes caused by [JEP 320](https://openjdk.java.net/jeps/320) - Use of jmodules as necessitated by [JEP 261](https://openjdk.java.net/jeps/261) - Reponse to largely changed file paths caused by [JEP 220](https://openjdk.java.net/jeps/220). - Modifications in build system related to AdoptOpenJDK and Java 11 which have a different naming structure for downloads. - Allowing use of non-Oracle Java within internal Processing checks. **Secondary required changes:** There were some secondary required changes that impacted the usability of Processing after having moved to OpenJFX / OpenJDK 11: - Removal of com.apple.eawt calls related to [JEP 272](http://openjdk.java.net/jeps/272) - Response to HiDPI support on Windows and Linux in [JEP 263](https://openjdk.java.net/jeps/263) - Removal of `java.ext.dirs`. Would be forced by [JEP 220](http://openjdk.java.net/jeps/220). - Due to bugs on Windows, updated the JNA jars. - Changes in downloader build tasks to support AdoptOpenJDK and OpenJFX. - Updated org.eclipse.* / equinox jars. - Some optimization around size of distribution. - Update of AppBundler. - Some changes in formulation of classpath and modifications in PreprocessingService given [JEP 261](https://openjdk.java.net/jeps/261). **Incidental changes:** This was (ahem) a bit of a larger PR with the above modifications. So, I wanted to introduce automated tests when possible and convenient along with a few changes for platform sustainability in order to support development: - Addition of cross-building capability (!) made possible by AdoptOpenJDK. - Addition of mockito for testing. - Upgrade of junit. - Addition of ant-contrib. - Standardized nomenclature around JRE / JDK in `build/build.xml` - Deduplication of code in `jre/build.xml`. - Addition of JavaDoc in a few places. - Some refactoring of PImage / Shape to support increased testing and readability in image manipulation code.
Moves to Java11 and OpenJDK via AdoptOpenJDK
* Adding travis support (hopefully without osx). Based off of processing 5753, demonstrates the use of Travis to build and test processing Requires use of OpenJDK (processing 5753). Will address processing 2747. Will redirect this to the original processing repo after processing 5753 is resolved. Note that this supersedes sampottinger/processing's #7 and benfry#70 - this does not include the deploy step. * Attempt explicit lib addition for linux-based travis build. * Ask travis for ant optional install.
License links in top level license file to include third party information. Specifically, in follow up to processing 5753, some updates to the license.txt file, reformatting to make it easier to find third party license information and to navigate the processing license itself.
Updated license text.
* Move to ANTLR 4 with Java 11 lang features and localization. Introduces ANTLR4 and Java 8 language feature support within IDE while also adding additional hooks for localization of syntax error messages, addressing processing/processing#3054 and processing/processing#3055. The PR is broadly a continuation of processing/processing#3055, bringing it up to speed with the latest Processing master plus the changes introduced at processing/processing#5753. **Requires processing/processing#5753 as pre-requisite.** This introduces a number of edits beyond processing/processing#3055 beyond compatibility with current Processing master and processing/processing#5753 including: - Update to the grammar itself - Change ANTLR listeners to emit `TextTransform.Edit` to unify JDT-based `PreprocessingService` and `JavaBuild`, removing code with duplicate purpose. - Introduction of syntax error rewriting with support for localization. - Addition of complete localized strings set for English and Spanish. - Addition of partial localized strings set for other languages. - Refactor of ANTLR-related code for testability and readability - Expansion of tests including full parse tests for new Java features (type inference, lambdas). * Ask travis for ant upgrade prior to run. * Ask Travis for java11 update. * Add openjdk ppa * Travis no confirmation on add ppa. * Force newer ant on travis. * Swtich ant download to www-us mirror. * Switch ant to 1.10.7 * Start x for unit tests in travis. * More complete start x in travis. * Revert x in travis. * Try x in services.
sampottinger
changed the title
|
We may break this PR up but folks have been testing with all of them put together on my master at https://github.com/sampottinger/processing/ so... let me know what you prefer @benfry. |
| */ | ||
| package com.oracle.appbundler; | ||
|
|
||
| public class Environment extends Option { |
There was a problem hiding this comment.
This is a ton of new classes for code that's a barely modified fork of another project… Are these really necessary? The changes make it harder to use anything upstream, and mean more maintenance for me.
There was a problem hiding this comment.
I backported the changes infinitekind made upstream into our copy of it. A pretty decent number of changes were made to support Java 11 and I wasn't sure if I would be able to cherry pick those changes without accepting some of the architectural shifts like this. I can try to narrow this down if you'd like but doing so likely makes it difficult to grab patches from them (and they are still pretty active: https://github.com/TheInfiniteKind/appbundler).
There was a problem hiding this comment.
So these are mostly their changes, not yours?
| @@ -6940,8 +6940,6 @@ public void specular(int rgb) { | |||
|
|
|||
| /** | |||
| * gray number specifying value between white and black | |||
| * | |||
| * @param gray value between black and white, by default 0 to 255 | |||
There was a problem hiding this comment.
Sorry this must have been a mistake. I'll double check what was up here.
There was a problem hiding this comment.
Reverted - sampottinger#6
|
|
||
|
|
||
| /** | ||
| * Deal with issues related to thinking differently. | ||
| * Deal with issues related to Mac OS window behavior. |
There was a problem hiding this comment.
Sorry! I don't think I got the ThinkDifferent reference and wanted to leave something in to hint more directly what it was doing. I can revert if you'd like.
| @@ -160,23 +160,23 @@ public String toString() { | |||
| } | |||
|
|
|||
|
|
|||
| protected static class Edit { | |||
| public static class Edit { | |||
There was a problem hiding this comment.
Why are all these being made public?
There was a problem hiding this comment.
I need to make edits (specifically Edits later applied in preprocessing) within the new ANTLR stuff. In general, there was a lot of duplicate code for preprocessing and I tried to consolidate to ANTLR tree walking instead of regex when possible. However, I didn't want to overhaul the TextTransform / Edit infrastructure.
There was a problem hiding this comment.
Ok, sounds fine… two general rules to note here for related edits:
- If it's an inner class that has this many
publicmethods/fields, I lean toward breaking it out as a separate class, so that folks aren't digging inside of the parent to find out what's happening inside some other code. - Always wary of any internal interfaces being made
publicsince that means any Mode or Tool author might make use of them and expect them to work in future releases. Sopublicis used sparingly (you see a lot ofprotectedfor this reason, for instance…)
| /** | ||
| * The modules comprising the Java standard modules. | ||
| */ | ||
| public static final String[] STANDARD_MODULES = { |
There was a problem hiding this comment.
These can live in Runtime… They don't need to be another class that someone else has to find. If they were non-static, had accessors, or were designed to be overridden, that would make more sense.
There was a problem hiding this comment.
Sounds good. They just felt really big.
There was a problem hiding this comment.
Potential resolution in sampottinger#8. Let me know if you want me to merge that in.
There was a problem hiding this comment.
Sounds good, yeah—let's go ahead and merge that in. Obviously lots of future work to do on this (as mentioned via email) it's a situation where a more condensed starting point will actually be better.
| */ | ||
| public static List<String> sanitizeClassPath(String classPathString) { | ||
| // Make sure class path does not contain empty string (home dir) | ||
| return Arrays.stream(classPathString.split(File.pathSeparator)) |
There was a problem hiding this comment.
Please move this back into where it came from… It's a single line. The new class is 48 lines. I understand folks like abstraction, but there's also a lot of overhead to keeping track of many classes and how they interact with one another.
Same goes for a few more of these below… This doesn't fix something that was broken, but adds a lot of overhead for anyone trying to understand how parts of the code interact, and doesn't get us closer to actually cleaning all this code for Language Server and that sort of thing.
There was a problem hiding this comment.
I think this particular piece of code got pulled up because it's used in a few different places and, even though it is one line, it does carry enough complexity that I wanted to re-use it but building inheritance around it felt heavy handed. That said, I can roll all of the runtime stuff up together into class if that is preferable. Although, I should mention that the path logic has gotten more complex with Java 11, mostly due to JEP 261 and the JavaFX jettison.
In general though... Sorry about that :( . I think some of this was formulated relatively early on when I was writing to architectural guidance for projects that I'm in outside of Processing which tend to encourage this kind of decomposition but, for the most part, I know that is cultural and project coherence is more important. I'll try to address this here in Runtime* and preproc to conform more closely to Processing norms.
There was a problem hiding this comment.
I have a possible iteration for you at sampottinger#8. Please let me know if you want to merge or further consolidate.
There was a problem hiding this comment.
Ok, for things that are used in that many places, I'm certainly up for a general utility class being in place, but just want that to be more than a single one-liner.
|
Hey there! Just a heads up re consolidating classes... I don't know if either of these are desired but see:
If you like one or both, I'll merge into my master which will be reflected on this PR. Thanks, |
Fix issue reported by @codeanticode within PreprocessorResult.java where the height was not being returned correctly.
Minor fix in PreprocessorResult dimension reporting.
Refactor ANTLR message simplification infrastructure.
Consolidate logic for runtime path generation into a single class.
|
Just merged sampottinger#8 and sampottinger#9. Let me know if there's anything else I can do! |
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Roughly the equivalent of merging
masterhttps://github.com/sampottinger/processing intomasterat https://github.com/processing/processing except that it targets the processing4 fork, this PR introduces a number of changes (and supersedes the following):