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

feature: Add support for dataflow analysis in new module spoon-dataflow #2970

Merged
merged 9 commits into from
May 24, 2019

Conversation

Egor18
Copy link
Contributor

@Egor18 Egor18 commented May 11, 2019

This PR is to add experimental spoon-dataflow module (see #2873).
Lambdas and anonymous classes are not fully supported at the moment (see tests marked with FIXME).

P.S. The main logic is inside DataFlowScanner.java

@monperrus
Copy link
Collaborator

Excellent! Welcome to spoon-dataflow!

Initial questions:

  • could we download com.microsoft.z3.jar from on a repo somewhere instead of pushing binary code to Git?
  • it would be great to activate CI for spoon-dataflow, I guess it means adding a few lines to chore/travis/travis-verify.sh. Could you have a look?

@Egor18
Copy link
Contributor Author

Egor18 commented May 12, 2019

Z3 solver itself is written in C++ and the jar file basically contains only JNI wrappers. This jar file is a part of a zip release from here: https://github.com/Z3Prover/z3/releases
Unfortunately, it seems like the developers do not provide any maven repo. I see several options here:

  1. We can build Z3 solver with java option ourselves (requires CMake and C++ compiler)
  2. We can create some repo and put it in there
  3. We can automatically download and install Z3 from the link above via gradle build script

@Egor18
Copy link
Contributor Author

Egor18 commented May 12, 2019

I'll check out CI script.

@monperrus
Copy link
Collaborator

I would go for solution 2 for now. Just created https://github.com/SpoonLabs/spoon-dependencies and added you as contributor.

@Egor18
Copy link
Contributor Author

Egor18 commented May 12, 2019

Regarding the CI script, we have to have z3 on the system to run tests, so I need to add something like this:

cd spoon-dataflow
wget https://github.com/Z3Prover/z3/releases/download/z3-4.8.4/z3-4.8.4.d6df51951f4c-x64-ubuntu-14.04.zip
unzip z3-4.8.4.d6df51951f4c-x64-ubuntu-14.04.zip
export LD_LIBRARY_PATH=./z3-4.8.4.d6df51951f4c-x64-ubuntu-14.04/bin
./gradlew build

Is it ok for you? (I can simply run ./gradlew assemble instead)

@monperrus
Copy link
Collaborator

monperrus commented May 13, 2019 via email

@Egor18
Copy link
Contributor Author

Egor18 commented May 13, 2019

spoon.MavenLauncherTest stuck, so have to reopen this PR to trigger Travis to rerun.

@Egor18 Egor18 closed this May 13, 2019
@Egor18 Egor18 reopened this May 13, 2019
@monperrus
Copy link
Collaborator

LGTM.

I propose to remove:

  • binary file spoon-dataflow/gradle/wrapper/gradle-wrapper.jar
  • unnecessary files gradlew.bat and gradlew

WDYT

@monperrus
Copy link
Collaborator

monperrus commented May 14, 2019 via email

@Egor18
Copy link
Contributor Author

Egor18 commented May 14, 2019

LGTM.

I propose to remove:

  • binary file spoon-dataflow/gradle/wrapper/gradle-wrapper.jar
  • unnecessary files gradlew.bat and gradlew

WDYT

I believe that having gradle wrapper committed in VCS is a common practice.
In fact almost every gradle-based java project on GitHub has wrapper in the repository.
For example:
https://github.com/ReactiveX/RxJava.git
https://github.com/spring-projects/spring-framework.git
https://github.com/elastic/elasticsearch
https://github.com/jMonkeyEngine/jmonkeyengine

The whole idea of a wrapper is to be able to build project even without having gradle (of the corresponding version) installed on the machine.
Moreover, it is only 50KB.

@monperrus
Copy link
Collaborator

I believe that having gradle wrapper committed in VCS is a common practice.

In Spoon, we have a tradition of daring to do things beyond the common practice :-)

The whole idea of a wrapper is to be able to build project even without having gradle (of the corresponding version) installed on the machine.

I see. Is it required for Travis?

@Egor18
Copy link
Contributor Author

Egor18 commented May 15, 2019

I think it is not required for Travis (gradle should be preinstalled since language: java is in travis.yml).

By the way, even the official gradle docs say that wrapper should be in VSC:
"The scripts generated by this task are intended to be committed to your version control system. This task also generates a small gradle-wrapper.jar bootstrap JAR file and properties file which should also be committed to your VCS. The scripts delegates to this JAR."
https://docs.gradle.org/current/dsl/org.gradle.api.tasks.wrapper.Wrapper.html

Gradle docs also say that it is recommended to always execute a build with the Wrapper to ensure reliable, controlled and standardized execution of the build.

If we still don't want to have it in VCS, I can add an additional step to generate wrapper:
gradle wrapper --gradle-version X.X
then we can run build as usual:
./gradlew build

WDYT?

@monperrus
Copy link
Collaborator

monperrus commented May 15, 2019 via email

@Egor18
Copy link
Contributor Author

Egor18 commented May 20, 2019

Tried to remove the wrapper, but unfortunately, gradle in Travis is too old (4.0) and it seems that it does not know about Java 9 versioning yet: Could not determine java version from '9.0.1'.
That's an example why it is better to use a wrapper, so I suggest to keep it.

@monperrus
Copy link
Collaborator

One option is to download the wrapper from somewhere else (as any other build dependencies that are also downloaded). I would go this way but I understand Egor's preference for the "standard" solution.

If anybody is strongly against adding a binary JAR file in the repo, raise your voice now.

@monperrus monperrus changed the title Add spoon-dataflow module feature: Add support for dataflow analysis in new module spoon-dataflow May 24, 2019
@monperrus monperrus merged commit deb0dfc into INRIA:master May 24, 2019
@monperrus
Copy link
Collaborator

Thanks a lot, this is an important milestone for Spoon.

There is a lack of a good and active symbolic engine for Java, and spoon-dataflow may be the foundation for this. That's great.

@monperrus monperrus mentioned this pull request Jul 25, 2019
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