-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Put module-info.class
into Multi-Release JAR folder
#2013
Changes from 1 commit
2d656a1
80aaf84
f50026a
14972e0
fc73dd5
c2b2aa8
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 |
---|---|---|
|
@@ -28,7 +28,7 @@ | |
|
||
<properties> | ||
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding> | ||
<java.version>1.6</java.version> | ||
<javaVersion>6</javaVersion> | ||
</properties> | ||
|
||
<scm> | ||
|
@@ -68,23 +68,45 @@ | |
<groupId>org.apache.maven.plugins</groupId> | ||
<artifactId>maven-compiler-plugin</artifactId> | ||
<version>3.8.1</version> | ||
<configuration> | ||
<release>${javaVersion}</release> | ||
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. This change caused all Maven modules and test classes to be build using Java 1.6, which is why I removed |
||
<jdkToolchain> | ||
<!-- Require at most JDK 11 because it is the last version | ||
supporting compiling Java 1.6, see https://bugs.openjdk.java.net/browse/JDK-8028563 --> | ||
<!-- When Gson increases lowest supported Java version, can | ||
change this to an open version range (e.g. `[11,)`) --> | ||
<version>[9,11]</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. Not completely sure why this works for the GitHub CI workflow (or rather why it is ignored?). For Surefire plugin it caused build failures, see https://github.com/Marcono1234/gson/runs/4146407481. |
||
</jdkToolchain> | ||
</configuration> | ||
</plugin> | ||
<plugin> | ||
<groupId>org.apache.maven.plugins</groupId> | ||
<artifactId>maven-javadoc-plugin</artifactId> | ||
<version>3.3.1</version> | ||
<configuration> | ||
<jdkToolchain> | ||
<version>[11,)</version> | ||
</jdkToolchain> | ||
<doclint>none</doclint> | ||
<!-- Link against newer Java API Javadoc because most users likely | ||
use a newer Java version than the one used for building this project --> | ||
<detectJavaApiLink>false</detectJavaApiLink> | ||
<links> | ||
<link>https://docs.oracle.com/en/java/javase/11/docs/api/</link> | ||
</links> | ||
<!-- Disable detection of offline links between Maven modules: | ||
(1) Only `gson` module is published, so for other modules Javadoc links don't | ||
matter much at the moment; (2) The derived URL for the modules is based on | ||
the project URL (= Gson GitHub repo) which is incorrect because it is not | ||
hosting the Javadoc (3) It might fail due to https://bugs.openjdk.java.net/browse/JDK-8212233 --> | ||
<detectOfflineLinks>false</detectOfflineLinks> | ||
</configuration> | ||
</plugin> | ||
<plugin> | ||
<groupId>org.apache.maven.plugins</groupId> | ||
<artifactId>maven-jar-plugin</artifactId> | ||
<version>3.2.0</version> | ||
</plugin> | ||
<plugin> | ||
<groupId>org.apache.felix</groupId> | ||
<artifactId>maven-bundle-plugin</artifactId> | ||
<version>5.1.2</version> | ||
<inherited>true</inherited> | ||
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. Have removed this because it looks like this is not used (unless I am overlooking something). The |
||
</plugin> | ||
</plugins> | ||
</pluginManagement> | ||
<plugins> | ||
|
@@ -110,23 +132,4 @@ | |
</plugin> | ||
</plugins> | ||
</build> | ||
<profiles> | ||
<profile> | ||
<id>doclint-java8-disable</id> | ||
<activation> | ||
<jdk>[1.8,)</jdk> | ||
</activation> | ||
<build> | ||
<plugins> | ||
<plugin> | ||
<groupId>org.apache.maven.plugins</groupId> | ||
<artifactId>maven-javadoc-plugin</artifactId> | ||
<configuration> | ||
<additionalparam>-Xdoclint:none</additionalparam> | ||
</configuration> | ||
</plugin> | ||
</plugins> | ||
</build> | ||
</profile> | ||
</profiles> | ||
</project> |
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.
Would it work if you set this to 7? If so then I think it is probably time for a separate PR to fix #2018 by changing the target version to 7 everywhere.
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.
Do you mean to work around the fuzzer exception? That can probably rather be solved by adjusting the fuzzer script to use
-DjavaVersion=8
instead (which should hopefully overwrite this property here then).Changing this property to 7 would effectively change the complete build to use Java 7.
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.
Sorry, I was unclear. I was indeed suggesting that if changing the complete build to use Java 7 fixes this problem, then that's another reason to do it now. So if the CIFuzz check passes with that change, we can make a separate PR for just the Java version change, close #2018, and proceed with this PR.
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.
The fuzzer seems to run successfully now.