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.

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

gson: initial integration #6742

Merged
merged 5 commits into from
Nov 6, 2021
Merged

Conversation

DavidKorczynski
Copy link
Collaborator

@DavidKorczynski DavidKorczynski commented Nov 2, 2021

Integrates https://github.com/google/gson
Gson is a Google library for json serialization and is the second most used json library on mvnrepository: https://mvnrepository.com/open-source/json-libraries

@DavidKorczynski DavidKorczynski marked this pull request as ready for review November 6, 2021 13:22
@DavidKorczynski
Copy link
Collaborator Author

@inferno-chromium @jonathanmetzman @oliverchang calling on the current sheriff this one is good to go.

@inferno-chromium inferno-chromium merged commit 3a50fc6 into google:master Nov 6, 2021
Comment on lines +28 to +29
COPY pom.xml $SRC/gson/pom.xml
COPY gson/pom.xml $SRC/gson/gson/pom.xml
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably have asked this earlier, but why are you providing custom POM files?
It appears the only difference is that the copy-rename-maven-plugin and proguard-maven-plugin have been removed. Was your intention to make building Gson more efficient because you are skipping the tests anyways?
(maybe you could then also run Maven only inside the gson/gson folder to avoid building the extras Maven module)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I had a similar question. When I tried building with local sources it didn't work until I hacked at the pom.xml files to adjust things like the source and target levels.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In short, I did this to make it build without issues.

The poms I added have the following changes from the original: change target/source levels and avoid the rename+proguard plugins.

I did this because I first ran into source number issues, which suggested I increase from 1.6 to 1.6. Then, I ran into the following issue when compiling the tests: /src/gson/gson/src/test/java/com/google/gson/internal/LinkedTreeMapTest.java:[164,20] Invalid java.lang.SafeVarargs annotation. Instance method <T>assertIterationOrd er(java.lang.Iterable<T>,T...) is not final.

and I ran into this issue when I skip tests:

[ERROR] Failed to execute goal com.coderplus.maven.plugins:copy-rename-maven-plugin:1.0.1:rename (pre-obfuscate-class) on project gson: sourceFile /src/gson/gson/tar
get/test-classes/com/google/gson/functional/EnumWithObfuscatedTest.class does not exist 

As such, I moved forward from there until the errors didn't persist. Let me know if this complicates things in terms of adding false positives or similar.

Notice my knowledge of Maven is very limited.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change target/source levels

Makes sense; your docker image is apparently using JDK 15, but Gson is compiled against Java 1.6 for which compilation support was dropped in JDK 12.
But hopefully future Gson versions will be compiled against at least Java 1.7 (see google/gson#2018) which is still supported by JDK 17.

avoid the rename+proguard plugins

The problem seems to be that you are building Gson with -Dmaven.test.skip=true, so even compilation of tests is disabled (see documentation). Therefore the copy-rename-maven-plugin which is needed for the tests fails. Unfortunately that plugin has no skip parameter or similar. I assume it could be solved for Gson by putting that plugin execution into a Maven profile which is enabled by default (and disabled when maven.test.skip=true), but not sure if that is worth it.

I am not familiar with how oss-fuzz or Jazzer works, but would it be an issue if the Gson tests were compiled (but not executed)? Then maybe you can remove these copies of the Gson POMs and run:

// Move into main Gson Maven module
cd gson
mvn clean package -DskipTests "-Dmaven.compiler.release=8"

@Marcono1234 Marcono1234 mentioned this pull request Nov 16, 2021
DavidKorczynski referenced this pull request Jun 15, 2022
* java-projects: update maven 3.8.5 to 3.8.6

The 3.8.5 is no longer available so project builds are failing for those
in this commit. This fixes it.

* nit: remove jul-to-slf4j
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants