Incorporating Gradle tasks for automatically installing JEP #1400

orchestr7 · 2023-12-24T23:02:15Z

What's Done:

Introduced the installJep task to facilitate the automatic installation of JEP from https://github.com/icemachined/jep-distro/.
Centralized and encapsulated the logic for JEP search and installation: all the relevant logic has been relocated to the Gradle task, separating it from the main code. The main code will now simply utilize the JEP stored in the build/jep directory.

…ng JEP binaries for the Python frontend ### What's Done: 1) Introduced the `installJep` task to facilitate the automatic installation of JEP from [https://github.com/icemachined/jep-distro/](https://github.com/icemachined/jep-distro/). 2) Centralized and encapsulated the logic for JEP search and installation: all the relevant logic has been relocated to the Gradle task, separating it from the main code. The main code will now simply utilize the JEP stored in the `build/jep` directory.

CLAassistant · 2023-12-24T23:02:20Z

All committers have signed the CLA.

orchestr7 · 2023-12-24T23:04:20Z

cpg-language-python/src/main/kotlin/de/fraunhofer/aisec/cpg/frontends/python/JepSingleton.kt

-            )
-        }
-        // try system-wide paths, too
-        // TODO: is this still needed?


No, it is not... Except your GitHub CI:

find /opt/hostedtoolcache/Python/ -name libjep.so -exec sudo cp '{}' /usr/lib/ \;

And actually you do the same with usr/lib that I did with build/jep

orchestr7 · 2023-12-24T23:05:18Z

Please check @oxisto

oxisto · 2023-12-25T13:08:04Z

cc @maximiliankaul

oxisto · 2023-12-25T13:10:48Z

Thanks for your contribution! (And signing the CLA ;) Looks like an interesting idea, I will have some time between Christmas and New Years to look at this as well.

orchestr7 · 2023-12-25T19:24:15Z

Thanks for your contribution! (And signing the CLA ;) Looks like an interesting idea, I will have some time between Christmas and New Years to look at this as well.

Thanks! Happy holidays! Please also approve a workflow (I added 1 commit, trying to fix build with JEP)

oxisto

In general, I like that idea of using the jep-distro files, but I wonder if moving the logic from runtime to compile time is a good idea for non-distro-supported systems.

oxisto · 2023-12-27T09:59:49Z

cpg-language-python/src/main/kotlin/de/fraunhofer/aisec/cpg/frontends/python/JepSingleton.kt

-                MainInterpreter.setJepLibraryPath(it.toString())
+        // To understand how JEP is installed under the hood, check `installJep` task in
+        // build.gradle.kts. But the main idea is that it is always copied to `build/jep` directory.
+        val jepLocation = FileSystems.getDefault().getPath("build", "jep")


Since you moved the detection logic from runtime to build-time I wonder what happens in the following scenario:

I am running on darwin/arm64 and if I use the CPG library as a dependency, I have no way of "injecting" my JEP into the jar that I am using. Do I then need to place my JEP locally somewhere in a build/jep folder? This seems counter-intuitive to me.

What was the reasoning behind moving this logic to the build-stage?

oxisto · 2023-12-27T10:00:23Z

cpg-language-python/build.gradle.kts

+                    | Your system architecture, identified as <${System.getProperty("os.arch")}>, is not supported in icemachined/jep-distro. 
+                    | Consequently, you are required to install it manually. Here are the potential solutions:
+                    |  1. Utilize Homebrew/pip package manager to facilitate the JEP installation process; 
+                    |  2. Create a virtual environment with the specified environment variable CPG_PYTHON_VIRTUALENV set to /(user.home)/.virtualenv/CPG_PYTHON_VIRTUALENV;


sometimes its .virtualenv and sometimes .virtualenvs

maximiliankaul · 2023-12-27T10:20:38Z

Hi @akuleshov7 ,
thanks so much for our PR. I'm a bit undecided whether I like it or not:

The entire JEP installation / configuration process has always been a pain for us and we applaud any effort to improve this situation 👍
I'm very hesitant to include a dependency to icemachined as this does not seem to be related with the JEP project / it's unclear to me how well this will be maintained.
Your PR does not work on my machine (Linux). Setting the CPG_PYTHON_VIRTUALENV=cpg env variable still results in the Your system architecture, identified as <amd64>, is not supported in icemachined/jep-distro. error message.
Can you please comment on the following usecases which are important to us:
- We are using the CPG in other projects as a dependency. How does this work with your PR?
- Your PR / icemachined only supports x86, correct? We do not want do drop ARM support.
- We have to fix a Python version in the build process (i.e. on Github) and this version is required on any client using the Github build, correct? We would like to avoid a scenario like this.

We will discuss testing this with some other projects and see if anything breaks. Any improvement in the JEP install logic is very welcome to us.

orchestr7 · 2023-12-28T09:42:16Z

I'm very hesitant to include a dependency to icemachined as this does not seem to be related with the JEP project / it's unclear to me how well this will be maintained.

@icemachined, Dima, how well do you maintain JEP-distro? 😄

icemachined · 2023-12-25T11:08:20Z

cpg-language-python/src/main/kotlin/de/fraunhofer/aisec/cpg/frontends/python/JepSingleton.kt

-                MainInterpreter.setJepLibraryPath(it.toString())
+        // To understand how JEP is installed under the hood, check `installJep` task in
+        // build.gradle.kts. But the main idea is that it is always copied to `build/jep` directory.
+        val jepLocation = FileSystems.getDefault().getPath("build", "jep")


it's better to put jep in jep-distro/jep and then put jep-distro to include path.

icemachined · 2023-12-25T11:14:45Z

cpg-language-python/build.gradle.kts

+    // Based on the host OS we will determine the extension for the JEP binary
+    val os = System.getProperty("os.name")
+    val jepBinary = "libjep." + when {
+        os.contains("Mac") -> "jnilib"


more common approach
os.contains("win")
os.contains("nix") || os.contains("nux") || os.contains("aix")
os.contains("mac")

icemachined · 2023-12-25T11:20:18Z

cpg-language-python/build.gradle.kts

+        else -> throw IllegalStateException("Cannot install JEP for this operating system: [$os]")
+    }
+
+    // If JEP already exists on the current host machine, there's no need to copy it from the distribution archive.


what if you want to install jep for different python version or distro version?

icemachined · 2023-12-25T11:20:48Z

cpg-language-python/src/main/kotlin/de/fraunhofer/aisec/cpg/frontends/python/JepSingleton.kt

+            jepLocation /
+                ("libjep." +
+                    when {
+                        os.contains("Mac") -> "jnilib"


more common approach
os.contains("win")
os.contains("nix") || os.contains("nux") || os.contains("aix")
os.contains("mac")

icemachined · 2023-12-25T11:22:16Z

cpg-language-python/src/main/kotlin/de/fraunhofer/aisec/cpg/frontends/python/JepSingleton.kt

        config.redirectStdErr(System.err)
        config.redirectStdout(System.out)

-        System.getenv("CPG_JEP_LIBRARY")?.let {


need to keep possibility to use jep from custom location

icemachined · 2023-12-28T10:21:03Z

I'm very hesitant to include a dependency to icemachined as this does not seem to be related with the JEP project / it's unclear to me how well this will be maintained.

@icemachined, Dima, how well do you maintain JEP-distro? 😄

I'm not modifying jep I just perform jep build on github and publish it on maven, you can see the code here https://github.com/icemachined/jep-distro. If there will be any issues I can fix it or accept your fix.
@maximiliankaul we already collaborated with you in #1032 :)

maximiliankaul · 2024-03-27T12:25:14Z

  * Your PR / icemachined only supports x86, correct? We do not want do drop ARM support.
  * We have to fix a Python version in the build process (i.e. on Github) and this version is required on any client using the Github build, correct? We would like to avoid a scenario like this.

Is there any update on these two points? @oxisto, if I remember correctly, we can live without the ARM support (although we would like to keep it as some of our team work on ARM machines), but the second point is a deal breaker for us, right?

icemachined · 2024-03-27T13:34:00Z

Your PR / icemachined only supports x86, correct? We do not want do drop ARM support.

We have to fix a Python version in the build process (i.e. on Github) and this version is required on any client using the Github build, correct? We would like to avoid a scenario like this.
Is there any update on these two points? @oxisto, if I remember correctly, we can live without the ARM support (although we would like to keep it as some of our team work on ARM machines), but the second point is a deal breaker for us, right?

Hi @maximiliankaul , jep-distro currently support ARM, latest release have ARM libraries tested on M2. so if you need we can update pr with ARM support

maximiliankaul · 2024-03-27T15:19:16Z

Your PR / icemachined only supports x86, correct? We do not want do drop ARM support.

We have to fix a Python version in the build process (i.e. on Github) and this version is required on any client using the Github build, correct? We would like to avoid a scenario like this.

Is there any update on these two points? @oxisto, if I remember correctly, we can live without the ARM support (although we would like to keep it as some of our team work on ARM machines), but the second point is a deal breaker for us, right?

Hi @maximiliankaul , jep-distro currently support ARM, latest release have ARM libraries tested on M2. so if you need we can update pr with ARM support

That's great to hear :) When we first checked this PR, there was no support / we missed this.
However, as we mentioned above, the deal breaker is fixing the Python version at compile time. Please don't get me wrong: we don't like the current solution and welcome any improvements (and this PR is an improvement in many ways), but we don't want to lose the ability to build the CPG once (e.g. here on Github) and distribute it to machines running different versions of Python. This is not achievable with this PR right now, is it?

icemachined · 2024-03-27T15:57:24Z

That's great to hear :) When we first checked this PR, there was no support / we missed this. However, as we mentioned above, the deal breaker is fixing the Python version at compile time. Please don't get me wrong: we don't like the current solution and welcome any improvements (and this PR is an improvement in many ways), but we don't want to lose the ability to build the CPG once (e.g. here on Github) and distribute it to machines running different versions of Python. This is not achievable with this PR right now, is it?

Yes you are right seems like this is not achievable in this pr. But it is achievable in general, because jep-distro contains builds for all architectures and versions of python starting from 3.6 till 3.12. You could include all required versions in distro (the size is not very big) and unpack one of them in runtime to tmp depending on current version of python.
The only restriction is glibc version, currently it is build on github with ubuntu-20.04 , if you use earlier versions of linux with older glibc there can be incompartibilities. for x86 and arm there was no problems

konradweiss · 2024-06-25T09:51:10Z

Hello everyone,

we are cleaning up old PRs and have decided to close this one as it does not really fit our target use case, as discussed above.
Shipping multiple versions of jep and unpacking them at runtime does have some drawbacks to the current approach. As stated above, we are not happy with the current mechanism either and are open to improvements. This PR, however, is too far away from what we want to achieve.

Thank you for contributing!

Best regards,
The CPG Team

orchestr7 requested review from oxisto, maximiliankaul and konradweiss as code owners December 24, 2023 23:02

orchestr7 mentioned this pull request Dec 24, 2023

Adding a path to JEP for MacOs #995

Closed

orchestr7 commented Dec 24, 2023

View reviewed changes

0x6675636b796f75676974687562 approved these changes Dec 25, 2023

View reviewed changes

oxisto assigned maximiliankaul Dec 25, 2023

oxisto added the python label Dec 25, 2023

Fixing CI action for $CPG_JEP_LIBRARY

8311852

oxisto reviewed Dec 27, 2023

View reviewed changes

icemachined suggested changes Dec 28, 2023

View reviewed changes

konradweiss closed this Jun 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Incorporating Gradle tasks for automatically installing JEP #1400

Incorporating Gradle tasks for automatically installing JEP #1400

orchestr7 commented Dec 24, 2023

CLAassistant commented Dec 24, 2023 •

edited

Loading

orchestr7 Dec 24, 2023 •

edited

Loading

orchestr7 commented Dec 24, 2023

oxisto commented Dec 25, 2023

oxisto commented Dec 25, 2023

orchestr7 commented Dec 25, 2023

oxisto left a comment

oxisto Dec 27, 2023

oxisto Dec 27, 2023 •

edited

Loading

maximiliankaul commented Dec 27, 2023

orchestr7 commented Dec 28, 2023

icemachined Dec 25, 2023

icemachined Dec 25, 2023

icemachined Dec 25, 2023

icemachined Dec 25, 2023

icemachined Dec 25, 2023

icemachined commented Dec 28, 2023 •

edited

Loading

maximiliankaul commented Mar 27, 2024

icemachined commented Mar 27, 2024 •

edited

Loading

maximiliankaul commented Mar 27, 2024

icemachined commented Mar 27, 2024 •

edited

Loading

konradweiss commented Jun 25, 2024

Incorporating Gradle tasks for automatically installing JEP #1400

Incorporating Gradle tasks for automatically installing JEP #1400

Conversation

orchestr7 commented Dec 24, 2023

What's Done:

CLAassistant commented Dec 24, 2023 • edited Loading

orchestr7 Dec 24, 2023 • edited Loading

Choose a reason for hiding this comment

orchestr7 commented Dec 24, 2023

oxisto commented Dec 25, 2023

oxisto commented Dec 25, 2023

orchestr7 commented Dec 25, 2023

oxisto left a comment

Choose a reason for hiding this comment

oxisto Dec 27, 2023

Choose a reason for hiding this comment

oxisto Dec 27, 2023 • edited Loading

Choose a reason for hiding this comment

maximiliankaul commented Dec 27, 2023

orchestr7 commented Dec 28, 2023

icemachined Dec 25, 2023

Choose a reason for hiding this comment

icemachined Dec 25, 2023

Choose a reason for hiding this comment

icemachined Dec 25, 2023

Choose a reason for hiding this comment

icemachined Dec 25, 2023

Choose a reason for hiding this comment

icemachined Dec 25, 2023

Choose a reason for hiding this comment

icemachined commented Dec 28, 2023 • edited Loading

maximiliankaul commented Mar 27, 2024

icemachined commented Mar 27, 2024 • edited Loading

maximiliankaul commented Mar 27, 2024

icemachined commented Mar 27, 2024 • edited Loading

konradweiss commented Jun 25, 2024

CLAassistant commented Dec 24, 2023 •

edited

Loading

orchestr7 Dec 24, 2023 •

edited

Loading

oxisto Dec 27, 2023 •

edited

Loading

icemachined commented Dec 28, 2023 •

edited

Loading

icemachined commented Mar 27, 2024 •

edited

Loading

icemachined commented Mar 27, 2024 •

edited

Loading