Skip to content

feat(GradleInspector): Custom JDK bootstrap download #11039

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+41 −12
Open

feat(GradleInspector): Custom JDK bootstrap download #11039

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dc7326f
feat(utils): Split installJdk in two functions
heliocastro Nov 3, 2025
550ab36
feat(GradleInspector): Add custom JDK download capability
heliocastro Nov 3, 2025
e3c5499
feat(GradleInspector): Proper set Temurin OpenJDK for macOS
heliocastro Nov 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 23 additions & 8 deletions plugins/package-managers/gradle-inspector/src/main/kotlin/GradleInspector.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,12 @@ data class GradleInspectorConfig(
* The directory of the Java home to use when analyzing projects. By default, the same Java home as for ORT itself
* is used.
*/
val javaHome: String?
val javaHome: String?,

/**
* Custom JDK URL for direct download instead of the Disco service. It is valid if the javaVersion is set together.
Copy link
Member

@sschuberth sschuberth Nov 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@heliocastro heliocastro Nov 3, 2025

Choose a reason for hiding this comment

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

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

*/
val customJdkUrl: String?
)

/**
Expand Down Expand Up @@ -160,17 +165,27 @@ class GradleInspector(
addProgressListener(ProgressListener { logger.debug(it.displayName) })
}

val javaHome = config.javaVersion
?.takeUnless { JavaBootstrapper.isRunningOnJdk(it) }
?.let {
JavaBootstrapper.installJdk("TEMURIN", it).onFailure { e ->
val javaHome = config.customJdkUrl
?.let { url ->
JavaBootstrapper.downloadJdk(url).onFailure { e ->
issues += createAndLogIssue(e.collectMessages())
}.getOrNull()
} ?: config.javaHome?.let { File(it) }
}

?: config.javaVersion
?.takeUnless { JavaBootstrapper.isRunningOnJdk(it) }
?.let { version ->
JavaBootstrapper.installJdk("TEMURIN", version).onFailure { e ->
issues += createAndLogIssue(e.collectMessages())
}.getOrNull()
}

?: config.javaHome?.let { File(it) }

javaHome?.also {
logger.info { "Setting Java home for project analysis to '$it'." }
setJavaHome(it)
val javaHomePath = if (Os.isMac) it / "Contents" / "Home" else it
logger.info { "Setting Java home for project analysis to '$javaHomePath'." }
setJavaHome(javaHomePath)
}
}
.setJvmArguments(jvmArgs)
Expand Down
22 changes: 18 additions & 4 deletions utils/ort/src/main/kotlin/JavaBootstrapper.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@
package org.ossreviewtoolkit.utils.ort

import java.io.File
import java.security.MessageDigest

import kotlin.text.toByteArray
import kotlin.time.measureTime
import kotlin.time.measureTimedValue

Expand Down Expand Up @@ -131,7 +133,20 @@ object JavaBootstrapper {
return Result.failure(it)
}

val installDir = (ortToolsDirectory / "jdks" / pkg.distribution / pkg.distributionVersion)
return downloadJdk(pkg.links.pkgDownloadRedirect)
}

/**
* Download the resolved JDK Url and return its directory on success, or an exception on failure.
*/
fun downloadJdk(sdkUrl: String): Result<File> {
val safeHash =
MessageDigest.getInstance("SHA-256")
.digest(sdkUrl.toByteArray())
.joinToString("") { "%02x".format(it) }
.take(16)
Comment on lines +143 to +147
Copy link
Member

@sschuberth sschuberth Nov 3, 2025

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@heliocastro heliocastro Nov 3, 2025

Choose a reason for hiding this comment

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

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

Copy link
Member

@sschuberth sschuberth Nov 3, 2025

Choose a reason for hiding this comment

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

Actually, that was redundant information, since you already know which SDK is there anyway:

I was meaning without the need to run ORT.

Copy link
Contributor Author

@heliocastro heliocastro Nov 3, 2025

Choose a reason for hiding this comment

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

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.


val installDir = (ortToolsDirectory / "jdks" / safeHash)
.apply {
if (isDirectory) {
logger.info { "Not downloading the JDK again as the directory '$this' already exists." }
Expand All @@ -141,11 +156,10 @@ object JavaBootstrapper {
safeMkdirs()
}

val url = pkg.links.pkgDownloadRedirect
logger.info { "Downloading the JDK package from $url..." }
logger.info { "Downloading the JDK package from $sdkUrl..." }

val (archive, downloadDuration) = measureTimedValue {
okHttpClient.downloadFile(url, installDir).getOrElse {
okHttpClient.downloadFile(sdkUrl, installDir).getOrElse {
return Result.failure(it)
}
}
Expand Down
Loading