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

Fix issue #588: Support for JDK17 in Reachability Analysis using WALA #593

Merged
merged 5 commits into from
Aug 26, 2023

Conversation

mayaba
Copy link
Contributor

@mayaba mayaba commented Aug 8, 2023

Added support for analyzing JDK 17 compiled resources with WALA

Tested the contribution by adding a unit test that analyze JDK 17 jar file. The content of the jar is the same as the other unit test examplesWalaTest, and the result is identical.

TODOs

  • Tests
  • Documentation

@mayaba
Copy link
Contributor Author

mayaba commented Aug 8, 2023

Hi @serenaponta & @henrikplate,

Can you please check this PR. I added the support for analyzing JDK17 code with the goal a2c using WALA.
Thank you for your time

@henrikplate
Copy link
Contributor

1st of all thanks for the contribution @mayaba, great initiative!

Here are a few comments on the PR:

  • The next release will be 3.2.6, so I do not think it is necessary to bump the version in all the POM files to 3.2.7-SNAPSHOT. Can you please revert that change?
  • I think it makes sense to also keep the source files of what's included in examplesJdk17.jar, so people can better understand which JDK17 features are included. Can you please add them to the PR? Also, I wonder whether the compilation of those sources and the JAR creation can be done during the build (rather than having pre-built JARs in the repo). What do you think?
  • And there's a small hick-up with the license header, some line breaks got messed up, esp. SPDX-License-Identifier.

mayaba added 2 commits August 11, 2023 14:20
… 17 example class, (3) reformat the code I added to comply with Google Java Style Guide
@mayaba
Copy link
Contributor Author

mayaba commented Aug 12, 2023

1st of all thanks for the contribution @mayaba, great initiative!

Here are a few comments on the PR:

  • The next release will be 3.2.6, so I do not think it is necessary to bump the version in all the POM files to 3.2.7-SNAPSHOT. Can you please revert that change?
  • I think it makes sense to also keep the source files of what's included in examplesJdk17.jar, so people can better understand which JDK17 features are included. Can you please add them to the PR? Also, I wonder whether the compilation of those sources and the JAR creation can be done during the build (rather than having pre-built JARs in the repo). What do you think?
  • And there's a small hick-up with the license header, some line breaks got messed up, esp. SPDX-License-Identifier.

Hi @henrikplate,

Thank you very much for the feedback. I did the following:
1- Reverted the changes I did to bump the version.
2- I added the source file of the class that is contained in examplesJdk17.jar. I tried to implement your suggestion to create the JAR during the build, but that introduced more unnecessary complexity (i.e. have to add the source file in a new sub-project). Also, the version of examplesJdk17.jar is different than the one that is being used which complicated this process.
3- Reverted the code format change I did previously which corrected the license headers.

@mayaba
Copy link
Contributor Author

mayaba commented Aug 16, 2023

Hi @henrikplate,
Looking forward to your insights on my last commit at your earliest convenience.
Thank you very much for your time.

@henrikplate
Copy link
Contributor

Just ran the test with

mvn -Djacoco.skip=true -Dvulas.core.backendConnection=OFFLINE -Dtest=WalaCallGraphTest#examplesWalaTestJdk17 -pl lang-java-reach-wala test

There are many errors like

2023-08-16 22:31:54,777 [vulas-reach-1] [ERROR] org.eclipse.steady.java.JavaId - Class [wala.lambda$java$util$regex$CharPredicates$0] not found: wala.lambda$java$util$regex$CharPredicates$0

due to Lamdba classes created dynamically, which makes sense. We should maybe change message to warning...

@mayaba
Copy link
Contributor Author

mayaba commented Aug 16, 2023

Just ran the test with

mvn -Djacoco.skip=true -Dvulas.core.backendConnection=OFFLINE -Dtest=WalaCallGraphTest#examplesWalaTestJdk17 -pl lang-java-reach-wala test

There are many errors like

2023-08-16 22:31:54,777 [vulas-reach-1] [ERROR] org.eclipse.steady.java.JavaId - Class [wala.lambda$java$util$regex$CharPredicates$0] not found: wala.lambda$java$util$regex$CharPredicates$0

due to Lamdba classes created dynamically, which makes sense. We should maybe change message to warning...

@henrikplate Thank you for the feedback.
I just ran the same command and didn't face any error!

mvn -Djacoco.skip=true -Dvulas.core.backendConnection=OFFLINE -Dtest=WalaCallGraphTest#examplesWalaTestJdk17 -pl lang-java-reach-wala test

Could you elaborate more on "We should maybe change message to warning...". Do you mean in the unit test itself, or the framework in general?

@henrikplate
Copy link
Contributor

I think the PR is fine in general (thank you again), but the Jenkins build currently fails due to problems when resolving some dependencies. I will try to dig deeper into this soon.

@henrikplate
Copy link
Contributor

henrikplate commented Aug 26, 2023

According to the logs, several artifacts cannot be resolved, e.g.

[FATAL] Non-resolvable parent POM for org.eclipse.steady:rest-lib-utils:3.2.6-SNAPSHOT: Could not find artifact org.springframework.boot:spring-boot-starter-parent:pom:2.7.8 in eclipse.maven.central.mirror (https://repo.eclipse.org/content/repositories/maven_central/) and 'parent.relativePath' points at no local POM @ line 26, column 10
<snip>
[ERROR] Unresolveable build extension: Plugin com.google.cloud.tools:jib-maven-plugin:1.7.0 or one of its dependencies could not be resolved: The following artifacts could not be resolved: com.google.cloud.tools:jib-maven-plugin:jar:1.7.0, org.codehaus.plexus:plexus-utils:jar:1.1: Could not find artifact com.google.cloud.tools:jib-maven-plugin:jar:1.7.0 in eclipse.maven.central.mirror (https://repo.eclipse.org/content/repositories/maven_central/) @ 

However, both are present in https://repo.eclipse.org ...

@henrikplate henrikplate merged commit a0e1e19 into eclipse:master Aug 26, 2023
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.

2 participants