feat(GradleInspector): Custom JDK bootstrap download #11039
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
heliocastro
force-pushed
the
feat/custom_bootstrap_jdk
branch
3 times, most recently
from
1c3de0d to
8d0ae94
Compare
heliocastro
force-pushed
the
feat/custom_bootstrap_jdk
branch
2 times, most recently
from
d8d4bcd to
dc6cb04
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #11039 +/- ##
=========================================
Coverage 57.42% 57.42%
Complexity 1701 1701
=========================================
Files 346 346
Lines 12835 12835
Branches 1215 1215
=========================================
Hits 7370 7370
Misses 4998 4998
Partials 467 467
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
plugins/package-managers/gradle-inspector/src/main/kotlin/GradleInspector.kt
Outdated
Show resolved
Hide resolved
| val javaHome: String?, | ||
|
|
||
| /** | ||
| * Custom JDK URL for direct download instead of the Disco service. It is valid if the javaVersion is set together. |
There was a problem hiding this comment.
Why is the feature designed so that the javaVersion is actually required? The version only seem to be used to name the download directory, which is not strictly necessary. Not requiring the javaVersion to be set would make this more similar to setting javaHome, which also does not require the version.
There was a problem hiding this comment.
I changed the logic to use a unique hashed directory for all cases, so the download function will always generate a directory based on the URL passed, whether from Disco or custom
Move the download logic to a separate function, allowing direct download of a specific JDK and not depending solely on Foojay Disco results. Directory creation is simplified to use a hash from the url provided. Signed-off-by: Helio Chissini de Castro <[email protected]>
Firewalled environments can't rely on open internet access, leading to invalid request to external Foojay Disco service. Adding customJdkUrl to enable the capability of direct download of an specified JDK. Signed-off-by: Helio Chissini de Castro <[email protected]>
Temurin OpenJDK native macOS install follows system placement, requiring path adjustments, not necessarily for Linux environments. Signed-off-by: Helio Chissini de Castro <[email protected]>
heliocastro
force-pushed
the
feat/custom_bootstrap_jdk
branch
from
dc6cb04 to
e3c5499
Compare
| val safeHash = | ||
| MessageDigest.getInstance("SHA-256") | ||
| .digest(sdkUrl.toByteArray()) | ||
| .joinToString("") { "%02x".format(it) } | ||
| .take(16) |
There was a problem hiding this comment.
Hmm, I liked the fact that you previously could tell from the JDK directory what distribution / version it is. Maybe that's not a strictly needed feature, but I'd like @oss-review-toolkit/tsc to share their views here.
There was a problem hiding this comment.
Actually, that was redundant information, since you already know which SDK is there anyway:
Example:
13:25:00.020 [DefaultDispatcher-worker-1] INFO org.gradle.tooling.ModelBuilder - Setting Java home for project analysis to '.../.ort/tools/jdks/c00aa1a4c19d36aa/jdk-17.0.17+10/Contents/Home'.So, Java 17.0.17+10 is there, and anticipating the next question, why not use the direct JDK directory? It's because the jdk-170.17+10 will be the same unpacked dir from any of the archs, but he url diffs.
You can arg to add maybe the architecture if you have different "runners" with shared home folder, but then is a very specific corner case
There was a problem hiding this comment.
Actually, that was redundant information, since you already know which SDK is there anyway:
I was meaning without the need to run ORT.
There was a problem hiding this comment.
But the directory is there, one folder below. What could be done is taking the actual last path from the url minus tar.gz, but then it's not fun and always error-prone to manipulate urls that can come with, for example, a + in the path.
2 participants
Environments that are not allowed access to the open internet cannot use the Foojay discovery service, rendering the bootstrap feature invalid.
This pull request allows specifying a custom JDK URL to be downloaded tied to the javaVersion specified.