Skip to content
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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Jan 1, 2022
Merged

Put module-info.class into Multi-Release JAR folder #2013

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2d656a1
Put module-info.class into Multi-Release JAR folder
Marcono1234 Nov 8, 2021
80aaf84
Merge branch 'master' into marcono1234/module-info-multi-release-jar
Marcono1234 Dec 27, 2021
f50026a
Merge branch 'master' into marcono1234/module-info-multi-release-jar
Marcono1234 Jan 1, 2022
14972e0
Adjust pom.xml to drop support for Java 6
Marcono1234 Jan 1, 2022
fc73dd5
Change doclint setting
Marcono1234 Jan 1, 2022
c2b2aa8
Improve README Java requirements
Marcono1234 Jan 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 28 additions & 22 deletions gson/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,33 +39,18 @@
<executions>
<execution>
<id>default-compile</id>
<configuration>
<jdkToolchain>
<version>9</version>
</jdkToolchain>
<release>9</release>
</configuration>
</execution>
<execution>
<id>base-compile</id>
<goals>
<goal>compile</goal>
</goals>
<configuration>
<excludes>
<!-- module-info.java is compiled using ModiTect -->
<exclude>module-info.java</exclude>
</excludes>
</configuration>
</execution>
</executions>
<configuration>
<jdkToolchain>
<version>[1.5,9)</version>
</jdkToolchain>
<source>1.6</source>
<target>1.6</target>
</configuration>
</plugin>
<!-- Note: Javadoc plugin has to be run in combination with >= `package`
phase, e.g. `mvn package javadoc:javadoc`, otherwise it fails with
"Aggregator report contains named and unnamed modules" -->
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
Expand All @@ -86,11 +71,31 @@
<artifactId>maven-javadoc-plugin</artifactId>
<configuration>
<excludePackageNames>com.google.gson.internal:com.google.gson.internal.bind</excludePackageNames>
<links>
<link>https://docs.oracle.com/javase/6/docs/api/</link>
</links>
</configuration>
</plugin>
<!-- Add module-info to JAR, see https://github.com/moditect/moditect#adding-module-descriptors-to-existing-jar-files -->
<!-- Uses ModiTect instead of separate maven-compiler-plugin executions
for better Eclipse IDE support, see https://github.com/eclipse-m2e/m2e-core/issues/393 -->
<plugin>
<groupId>org.moditect</groupId>
<artifactId>moditect-maven-plugin</artifactId>
<version>1.0.0.RC2</version>
<executions>
<execution>
<id>add-module-info</id>
<phase>package</phase>
<goals>
<goal>add-module-info</goal>
</goals>
<configuration>
<jvmVersion>9</jvmVersion>
<module>
<moduleInfoFile>${project.build.sourceDirectory}/module-info.java</moduleInfoFile>
</module>
</configuration>
</execution>
</executions>
</plugin>
<plugin>
<groupId>biz.aQute.bnd</groupId>
<artifactId>bnd-maven-plugin</artifactId>
Expand All @@ -108,6 +113,7 @@
<artifactId>maven-jar-plugin</artifactId>
<configuration>
<archive>
<!-- Use existing manifest generated by BND plugin -->
<manifestFile>${project.build.outputDirectory}/META-INF/MANIFEST.MF</manifestFile>
</archive>
</configuration>
Expand Down
7 changes: 1 addition & 6 deletions gson/src/test/java/com/google/gson/functional/TreeTypeAdaptersTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ protected void setUp() {
.registerTypeAdapter(Id.class, new IdTreeTypeAdapter())
.create();
course = new Course<HistoryCourse>(COURSE_ID, 4,
new Assignment<HistoryCourse>(null, null), createList(STUDENT1, STUDENT2));
new Assignment<HistoryCourse>(null, null), Arrays.asList(STUDENT1, STUDENT2));
}

public void testSerializeId() {
Expand Down Expand Up @@ -171,9 +171,4 @@ public Assignment(Id<Assignment<T>> id, T data) {
private static class HistoryCourse {
int numClasses;
}

@SafeVarargs
private static <T> List<T> createList(T ...items) {
return Arrays.asList(items);
}
}
1 change: 0 additions & 1 deletion gson/src/test/java/com/google/gson/internal/LinkedTreeMapTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,6 @@ public void testJavaSerialization() throws IOException, ClassNotFoundException {
assertEquals(Collections.singletonMap("a", 1), deserialized);
}

@SafeVarargs
private <T> void assertIterationOrder(Iterable<T> actual, T... expected) {
ArrayList<T> actualList = new ArrayList<T>();
for (T t : actual) {
Expand Down
55 changes: 29 additions & 26 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@

<properties>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<java.version>1.6</java.version>
<javaVersion>6</javaVersion>
Copy link
Member

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.

Copy link
Collaborator Author

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.

Copy link
Member

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.

Copy link
Collaborator Author

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.

</properties>

<scm>
Expand Down Expand Up @@ -68,23 +68,45 @@
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<version>3.8.1</version>
<configuration>
<release>${javaVersion}</release>
Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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 @SafeVarargs from two test classes and adjusted ProtoTypeAdapter because Map.putIfAbsent was added in Java 8 while ConcurrentMap.putIfAbsent already existed in Java 1.6.

<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>
Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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.3</version>
<inherited>true</inherited>
</plugin>
</plugins>
</pluginManagement>
<plugins>
Expand All @@ -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>
8 changes: 4 additions & 4 deletions proto/src/main/java/com/google/gson/protobuf/ProtoTypeAdapter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ public static Builder newBuilder() {
private static final com.google.protobuf.Descriptors.FieldDescriptor.Type ENUM_TYPE =
com.google.protobuf.Descriptors.FieldDescriptor.Type.ENUM;

private static final ConcurrentMap<String, Map<Class<?>, Method>> mapOfMapOfMethods =
private static final ConcurrentMap<String, ConcurrentMap<Class<?>, Method>> mapOfMapOfMethods =
new MapMaker().makeMap();

private final EnumSerialization enumSerialization;
Expand Down Expand Up @@ -308,7 +308,7 @@ public Message deserialize(JsonElement json, Type typeOfT,
}
}
}
return (Message) protoBuilder.build();
return protoBuilder.build();
} catch (SecurityException e) {
throw new JsonParseException(e);
} catch (NoSuchMethodException e) {
Expand Down Expand Up @@ -396,10 +396,10 @@ private EnumValueDescriptor findValueByNameAndExtension(EnumDescriptor desc,

private static Method getCachedMethod(Class<?> clazz, String methodName,
Class<?>... methodParamTypes) throws NoSuchMethodException {
Map<Class<?>, Method> mapOfMethods = mapOfMapOfMethods.get(methodName);
ConcurrentMap<Class<?>, Method> mapOfMethods = mapOfMapOfMethods.get(methodName);
if (mapOfMethods == null) {
mapOfMethods = new MapMaker().makeMap();
Map<Class<?>, Method> previous =
ConcurrentMap<Class<?>, Method> previous =
mapOfMapOfMethods.putIfAbsent(methodName, mapOfMethods);
mapOfMethods = previous == null ? mapOfMethods : previous;
}
Expand Down