-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Add Java 11 compatibility #2812
Changes from 13 commits
5c6fa6f
3c5f6b7
825b0d9
c811224
aca91ab
a19a273
bbe4c32
cbaa7bc
e41888a
ec99e1b
36d875b
d996430
00ed2b5
408cbec
f45ac81
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -78,7 +78,7 @@ | |
</scm> | ||
|
||
<properties> | ||
<geotools.version>20.1</geotools.version> | ||
<geotools.version>21.2</geotools.version> | ||
<geotools.wfs.version>16.5</geotools.wfs.version> | ||
<jackson.version>2.9.7</jackson.version> | ||
<jersey.version>2.18</jersey.version> | ||
|
@@ -172,11 +172,10 @@ | |
<plugin> | ||
<groupId>org.apache.maven.plugins</groupId> | ||
<artifactId>maven-compiler-plugin</artifactId> | ||
<version>3.1</version> | ||
<version>3.8.1</version> | ||
<configuration> | ||
<!-- Target Java versions --> | ||
<source>1.8</source> | ||
<target>1.8</target> | ||
<release>11</release> | ||
</configuration> | ||
</plugin> | ||
<plugin> | ||
|
@@ -208,15 +207,35 @@ | |
<docsDir>${project.build.directory}/site/enunciate</docsDir> | ||
</configuration> | ||
</plugin> | ||
<plugin> | ||
<groupId>org.apache.maven.plugins</groupId> | ||
<artifactId>maven-jar-plugin</artifactId> | ||
<version>3.1.2</version> | ||
<configuration> | ||
<archive> | ||
<manifestEntries> | ||
<!-- For Java 11 Modules, specify a module name. Do not create module-info.java until all | ||
our dependencies specify a module name. --> | ||
<Automatic-Module-Name>org.opentripplanner.otp</Automatic-Module-Name> | ||
</manifestEntries> | ||
</archive> | ||
</configuration> | ||
|
||
</plugin> | ||
<plugin> | ||
<!-- This plugin must be configured both here (for attach-javadoc during release) | ||
and in "reports" (for site generation), preferably with identical version numbers. --> | ||
<groupId>org.apache.maven.plugins</groupId> | ||
<artifactId>maven-javadoc-plugin</artifactId> | ||
<version>2.10.3</version> | ||
<version>3.1.1</version> | ||
<configuration> | ||
<!-- Turn off Java 8 strict Javadoc checking --> | ||
<additionalparam>-Xdoclint:none</additionalparam> | ||
<!-- Turn off strict Javadoc checking --> | ||
<doclint>none</doclint> | ||
<!-- | ||
Exclude this package as it inherits from a JAI (https://www.oracle.com/technetwork/java/iio-141084.html) type | ||
whose sources are hard to discover by the JavaDoc tool. | ||
--> | ||
<excludePackageNames>org.opentripplanner.graph_builder.module.ned:*</excludePackageNames> | ||
</configuration> | ||
<executions> | ||
<!-- Compress Javadoc into JAR and include that JAR when deploying. --> | ||
|
@@ -231,7 +250,7 @@ | |
<plugin> | ||
<groupId>org.apache.maven.plugins</groupId> | ||
<artifactId>maven-source-plugin</artifactId> | ||
<version>3.0.1</version> | ||
<version>3.1.0</version> | ||
<executions> | ||
<execution> | ||
<id>attach-sources</id> | ||
|
@@ -259,10 +278,29 @@ | |
<plugin> | ||
<groupId>org.apache.maven.plugins</groupId> | ||
<artifactId>maven-surefire-plugin</artifactId> | ||
<version>2.21.0</version> | ||
<!-- | ||
Intentionally an old version as the combination of Junit 5, Java 11 and Surefire was causing issues. | ||
https://github.com/opentripplanner/OpenTripPlanner/pull/2812/commits/c811224f8467699ee70684e9f6b00d3ad803dbeb | ||
--> | ||
<version>2.19.1</version> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see that in c42ca0c you downgraded this due to a corrupted stream error. I'd like to eventually find and fix the root problem. Can you provide a link to the ticket you referenced in your commit message? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Of course, I will try again and clarify the commit message. |
||
<configuration> | ||
<argLine>-Xmx2G</argLine> | ||
<argLine>-Dfile.encoding=UTF-8</argLine> | ||
<!-- we have to fork the JVM during tests so that the argLine is passed along --> | ||
<forkCount>3</forkCount> | ||
<!-- enable the restricted reflection under Java 11 so that the ObjectDiffer works --> | ||
<argLine> | ||
-Xmx2G | ||
-Dfile.encoding=UTF-8 | ||
--illegal-access=permit | ||
--add-opens java.base/java.lang.module=ALL-UNNAMED | ||
--add-opens java.base/jdk.internal.reflect=ALL-UNNAMED | ||
--add-opens java.base/jdk.internal.misc=ALL-UNNAMED | ||
--add-opens java.base/jdk.internal.loader=ALL-UNNAMED | ||
--add-opens java.base/jdk.internal.ref=ALL-UNNAMED | ||
--add-opens java.base/jdk.internal.util=ALL-UNNAMED | ||
--add-opens java.base/jdk.internal.util.jar=ALL-UNNAMED | ||
--add-opens java.base/jdk.internal.module=ALL-UNNAMED | ||
--add-opens java.base/java.lang=ALL-UNNAMED | ||
</argLine> | ||
<!-- Jenkins needs XML test reports to determine whether the build is stable. --> | ||
<disableXmlReport>false</disableXmlReport> | ||
</configuration> | ||
|
@@ -310,7 +348,7 @@ | |
properly if some input files are missing a terminating newline) --> | ||
<groupId>org.apache.maven.plugins</groupId> | ||
<artifactId>maven-shade-plugin</artifactId> | ||
<version>2.2</version> | ||
<version>3.2.1</version> | ||
<executions> | ||
<execution> | ||
<phase>package</phase> | ||
|
@@ -390,10 +428,10 @@ | |
(for attach-javadoc during release), preferably with identical version numbers. --> | ||
<groupId>org.apache.maven.plugins</groupId> | ||
<artifactId>maven-javadoc-plugin</artifactId> | ||
<version>2.10.3</version> | ||
<version>3.1.1</version> | ||
<configuration> | ||
<!-- Turn off Java 8 strict Javadoc checking --> | ||
<additionalparam>-Xdoclint:none</additionalparam> | ||
<doclint>false</doclint> | ||
</configuration> | ||
<reportSets> | ||
<reportSet> | ||
|
@@ -461,17 +499,17 @@ | |
<!-- GEOTOOLS includes JTS as a transitive dependency. --> | ||
<dependency> | ||
<groupId>org.geotools</groupId> | ||
<artifactId>gt-geojson</artifactId> | ||
<artifactId>gt-main</artifactId> | ||
<version>${geotools.version}</version> | ||
</dependency> | ||
<dependency> | ||
<groupId>org.geotools</groupId> | ||
<artifactId>gt-referencing</artifactId> | ||
<artifactId>gt-geojson</artifactId> | ||
<version>${geotools.version}</version> | ||
</dependency> | ||
<dependency> | ||
<groupId>org.geotools</groupId> | ||
<artifactId>gt-referencing3D</artifactId> | ||
<artifactId>gt-referencing</artifactId> | ||
<version>${geotools.version}</version> | ||
</dependency> | ||
<dependency> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -64,7 +64,7 @@ else if (CRS.getAxisOrder(destCrs) == CRS.AxisOrder.EAST_NORTH) | |
if (!destCrs.equals(sourceCrs)) { | ||
transform = true; | ||
|
||
// find the transformation, being strict about datums &c. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Personally I like ampersand-c. as a way to write "et cetera", as the ampersand is a ligature of Latin "et". It's fine to remove it though if it might confuse some people. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh I see, Javadoc is interpreting this as in HTML. Now I see why it's removed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct, Java 11 is way more strict with these HTML entities. A big problem where also the < and > characters. I got a little creative and converted to unicode characters where possible. If not I just spelled them out with words. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In my commits on the 2.x port, I used There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tip: In Intellij |
||
// find the transformation, being strict about datums etc. | ||
mathTransform = CRS.findMathTransform(sourceCrs, destCrs, false); | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,8 +23,8 @@ | |
* Uses the shapes from GTFS to determine which streets buses drive on. This is used to improve the quality of | ||
* the stop-to-street linkage. It encourages the linker to link to streets where transit actually travels. | ||
* | ||
* GTFS provides a mapping from trips->shapes. This module provides a mapping from stops->trips and shapes->edges. | ||
* Then transitively we get a mapping from stop->edges. | ||
* GTFS provides a mapping from trips→shapes. This module provides a mapping from stops→trips and shapes→edges. | ||
* Then transitively we get a mapping from stop→edges. | ||
* The edges that "belong" to a stop are favored when linking that stop to the street network. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, but I like |
||
*/ | ||
public class BusRouteStreetMatcher implements GraphBuilderModule { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see Geotools 21 was the first version with Java 11 compatibility. I assume we aren't jumping to an even newer Geotools because of the transitive dependency on JTS, which has changed its package name and will conflict with other OTP dependencies?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tought that 21.2 is the newest stable version: https://github.com/geotools/geotools/tags
Or would you like to use 22-RC?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I suppose it is. I saw 23-SNAPSHOT and must have assumed there was already a 22 release. But now I see 22 is also still in SNAPSHOT. In that case then there's no question, 21.2 is fine.