Skip to content

Commit 2e020f0

Browse files
authored
Fix live Java checks and Temurin discovery (#103)
* Fix java_check_version_rjava() failing on Linux by injecting get_libjvm_path into subprocess * fix deparse bug and add regression test * fix: harden live Java checks Make Temurin version discovery asset-backed, isolate rJava version checks via subprocess env setup, and update tests and CI diagnostics for the live workflow. * test: replace parent assignment in tests * test: cover subprocess fallback branches
1 parent a6bd250 commit 2e020f0

22 files changed

Lines changed: 908 additions & 331 deletions

.github/workflows/java-live-tests.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,12 @@ jobs:
5252
extra-packages: any::rcmdcheck
5353
needs: check
5454

55+
- name: Log Java diagnostics
56+
shell: bash
57+
run: |
58+
java -version || true
59+
Rscript -e 'cat(Sys.getenv("JAVA_HOME"), "\n")'
60+
5561
- uses: r-lib/actions/check-r-package@v2
5662
with:
5763
upload-snapshots: true

R/internal_utilities.R

Lines changed: 89 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,70 @@ read_json_url <- function(url, max_simplify_lvl = "data_frame") {
7373
RcppSimdJson::fparse(content, max_simplify_lvl = max_simplify_lvl)
7474
}
7575

76+
#' Build environment variables for a Java subprocess
77+
#'
78+
#' @param java_home Path to Java home directory.
79+
#' @param rjava Logical. Whether the subprocess will initialize rJava.
80+
#'
81+
#' @return Named character vector of environment variables.
82+
#' @keywords internal
83+
#' @noRd
84+
java_subprocess_env <- function(java_home, rjava = FALSE) {
85+
checkmate::assert_string(java_home)
86+
checkmate::assert_logical(rjava, len = 1)
87+
88+
env_vars <- Sys.getenv()
89+
env_get <- function(name) {
90+
if (name %in% names(env_vars)) {
91+
env_vars[[name]]
92+
} else {
93+
NA_character_
94+
}
95+
}
96+
java_bin <- file.path(java_home, "bin")
97+
old_path <- env_get("PATH")
98+
99+
env_vars["JAVA_HOME"] <- java_home
100+
env_vars["PATH"] <- paste(java_bin, old_path, sep = .Platform$path.sep)
101+
102+
if (!isTRUE(rjava)) {
103+
return(env_vars)
104+
}
105+
106+
sysname <- Sys.info()[["sysname"]]
107+
libjvm_path <- get_libjvm_path(java_home)
108+
if (is.null(libjvm_path)) {
109+
return(env_vars)
110+
}
111+
112+
jvm_lib_dir <- dirname(libjvm_path)
113+
114+
if (identical(sysname, "Linux")) {
115+
old_ld <- env_get("LD_LIBRARY_PATH")
116+
if (is.na(old_ld)) {
117+
old_ld <- ""
118+
}
119+
env_vars["JAVA_LD_LIBRARY_PATH"] <- jvm_lib_dir
120+
env_vars["LD_LIBRARY_PATH"] <- if (nzchar(old_ld)) {
121+
paste(jvm_lib_dir, old_ld, sep = .Platform$path.sep)
122+
} else {
123+
jvm_lib_dir
124+
}
125+
} else if (identical(sysname, "Darwin")) {
126+
old_dyld <- env_get("DYLD_LIBRARY_PATH")
127+
if (is.na(old_dyld)) {
128+
old_dyld <- ""
129+
}
130+
env_vars["DYLD_LIBRARY_PATH"] <- if (nzchar(old_dyld)) {
131+
paste(jvm_lib_dir, old_dyld, sep = .Platform$path.sep)
132+
} else {
133+
jvm_lib_dir
134+
}
135+
}
136+
137+
env_vars
138+
}
139+
76140
#' Read lines from a URL or file
77141
#'
78142
#' Helper function to read lines, mainly for testability.
@@ -148,44 +212,9 @@ urls_test_all <- function() {
148212
java_version_check_rscript <- function(java_home) {
149213
result <- tryCatch(
150214
{
151-
Sys.setenv(JAVA_HOME = java_home)
152-
153-
old_path <- Sys.getenv("PATH")
154-
new_path <- file.path(java_home, "bin")
155-
Sys.setenv(PATH = paste(new_path, old_path, sep = .Platform$path.sep))
156-
157-
# On Linux, find and dynamically load libjvm.so
158-
if (Sys.info()["sysname"] == "Linux") {
159-
libjvm_path <- get_libjvm_path(java_home)
160-
161-
if (!is.null(libjvm_path) && file.exists(libjvm_path)) {
162-
tryCatch(
163-
dyn.load(libjvm_path),
164-
error = function(e) {
165-
# Use base message to avoid dependency issues in the isolated script
166-
message(sprintf(
167-
"Found libjvm.so at '%s' but failed to load it: %s",
168-
libjvm_path,
169-
e$message
170-
))
171-
}
172-
)
173-
} else {
174-
message(sprintf(
175-
"Could not find libjvm.so within the provided JAVA_HOME: %s",
176-
java_home
177-
))
178-
}
179-
}
180-
181215
suppressWarnings(rJava::.jinit())
182-
suppressWarnings(
183-
java_version <- rJava::.jcall(
184-
"java.lang.System",
185-
"S",
186-
"getProperty",
187-
"java.version"
188-
)
216+
java_version <- suppressWarnings(
217+
rJava::.jcall("java.lang.System", "S", "getProperty", "java.version")
189218
)
190219

191220
message <- cli::format_message(
@@ -202,6 +231,28 @@ java_version_check_rscript <- function(java_home) {
202231
return(result)
203232
}
204233

234+
#' Parse major Java version from a java.version string
235+
#'
236+
#' @param java_ver_str Character string containing Java version.
237+
#'
238+
#' @return Character scalar major version or NULL.
239+
#' @keywords internal
240+
#' @noRd
241+
parse_java_major_version <- function(java_ver_str) {
242+
if (is.null(java_ver_str) || !nzchar(java_ver_str)) {
243+
return(NULL)
244+
}
245+
246+
matches <- regexec("^(1\\.)?([0-9]+)", java_ver_str)
247+
parts <- regmatches(java_ver_str, matches)[[1]]
248+
249+
if (length(parts) < 3) {
250+
return(NULL)
251+
}
252+
253+
parts[3]
254+
}
255+
205256
#' Find path to libjvm dynamic library
206257
#'
207258
#' @description
@@ -394,22 +445,7 @@ java_check_current_rjava_version <- function() {
394445
return(NULL)
395446
}
396447

397-
# Parse version: "1.8.0_..." -> "8", "17.0.1" -> "17"
398-
matches <- regexec(
399-
"^(1\\.)?([0-9]+)",
400-
java_ver_str
401-
)
402-
parts <- regmatches(java_ver_str, matches)[[1]]
403-
404-
if (length(parts) < 3) {
405-
return(NULL)
406-
}
407-
408-
major <- parts[3]
409-
# Handle 1.8 -> 8 case (parts[2] is "1." and parts[3] is "8")
410-
# Handle 17 -> 17 case (parts[2] is "" and parts[3] is "17")
411-
412-
return(major)
448+
parse_java_major_version(java_ver_str)
413449
}
414450

415451
#' Find the actual extracted directory, ignoring hidden/metadata files

R/java_env.R

Lines changed: 65 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -305,32 +305,67 @@ java_check_version_rjava <- function(
305305

306306
# Original implementation (not memoised) - used when .use_cache = FALSE
307307
._java_version_check_rjava_impl_original <- function(java_home = NULL) {
308-
# Get the code of the unexported functions to use in a script
309-
# On Linux, the script depends on get_libjvm_path() to avoid crashes in .jinit()
308+
if (is.null(java_home) || !nzchar(java_home)) {
309+
return(FALSE)
310+
}
311+
312+
if (requireNamespace("callr", quietly = TRUE)) {
313+
data <- tryCatch(
314+
{
315+
callr::r(
316+
func = function() {
317+
suppressWarnings(rJava::.jinit())
318+
java_version <- suppressWarnings(
319+
rJava::.jcall(
320+
"java.lang.System",
321+
"S",
322+
"getProperty",
323+
"java.version"
324+
)
325+
)
326+
327+
list(
328+
java_version = java_version,
329+
output = cli::format_message(
330+
paste0(
331+
"rJava and other rJava/Java-based packages will use ",
332+
"Java version: {.val {java_version}}"
333+
)
334+
)
335+
)
336+
},
337+
libpath = .libPaths(),
338+
env = java_subprocess_env(java_home, rjava = TRUE),
339+
show = FALSE
340+
)
341+
},
342+
error = function(e) FALSE
343+
)
344+
345+
if (!isFALSE(data)) {
346+
major_java_ver <- parse_java_major_version(data$java_version)
347+
if (!is.null(major_java_ver)) {
348+
return(list(
349+
major_version = major_java_ver,
350+
output = data$output
351+
))
352+
}
353+
}
354+
}
310355
java_version_check_fn <- getFromNamespace(
311356
"java_version_check_rscript",
312357
"rJavaEnv"
313358
)
314-
get_libjvm_path_fn <- getFromNamespace("get_libjvm_path", "rJavaEnv")
315-
316-
# Helper to deparse and collapse multi-line expressions
317359
deparse_collapse <- function(x) {
318360
paste(deparse(x, width.cutoff = 500), collapse = "\n")
319361
}
320362

321363
java_version_check_body <- deparse_collapse(body(java_version_check_fn))
322-
get_libjvm_path_body <- deparse_collapse(body(get_libjvm_path_fn))
323364
libs_val <- deparse_collapse(as.character(.libPaths()))
324-
325-
# Create a wrapper script that includes the function definitions and calls them
326-
# Capture current libPaths to ensure subprocess can find rJava in renv/packrat environments
327365
wrapper_script <- paste0(
328366
".libPaths(",
329367
libs_val,
330368
")\n\n",
331-
"get_libjvm_path <- function(java_home) ",
332-
get_libjvm_path_body,
333-
"\n\n",
334369
"java_version_check <- function(java_home) ",
335370
java_version_check_body,
336371
"\n\n",
@@ -341,46 +376,42 @@ java_check_version_rjava <- function(
341376
"}"
342377
)
343378

344-
# Write the wrapper script to a temporary file
345379
script_file <- tempfile(fileext = ".R")
346380
writeLines(wrapper_script, script_file)
347381

348-
# Run the script in a separate R session and capture the output
349-
rscript_path <- file.path(R.home("bin"), "Rscript")
350-
output <- suppressWarnings(system2(
351-
rscript_path,
352-
args = c(script_file, java_home),
353-
stdout = TRUE,
354-
stderr = TRUE,
355-
timeout = 5
356-
))
382+
output <- tryCatch(
383+
suppressWarnings(system2(
384+
file.path(R.home("bin"), "Rscript"),
385+
args = c(script_file, java_home),
386+
stdout = TRUE,
387+
stderr = TRUE,
388+
timeout = 5,
389+
env = java_subprocess_env(java_home, rjava = TRUE)
390+
)),
391+
error = function(e) character(0)
392+
)
357393

358-
# Delete the temporary script file
359394
unlink(script_file)
360395

361-
# Process the output (no printing here)
362396
if (length(output) == 0 || any(grepl("error", tolower(output)))) {
363397
return(FALSE)
364398
}
365399

366400
output <- paste(output, collapse = "\n")
367401
cleaned_output <- cli::ansi_strip(output)
368-
major_java_ver <- sub('.*version: \\"([0-9]+).*', '\\1', cleaned_output)
402+
version_matches <- regexec('version: \\"([^\\"]+)\\"', cleaned_output)
403+
version_parts <- regmatches(cleaned_output, version_matches)[[1]]
404+
java_version <- if (length(version_parts) > 1) version_parts[2] else NULL
405+
major_java_ver <- parse_java_major_version(java_version)
369406

370-
if (!nzchar(major_java_ver) || !grepl("^[0-9]+$", major_java_ver)) {
407+
if (is.null(major_java_ver)) {
371408
return(FALSE)
372409
}
373410

374-
# Fix 1 to 8, as Java 8 prints "1.8"
375-
if (major_java_ver == "1") {
376-
major_java_ver <- "8"
377-
}
378-
379-
# Return structured data for printing in wrapper
380-
return(list(
411+
list(
381412
major_version = major_java_ver,
382413
output = output
383-
))
414+
)
384415
}
385416

386417
# Internal function: Spawn subprocess to check Java with rJava - this gets cached

R/java_list_available.R

Lines changed: 5 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -103,62 +103,16 @@ java_list_available <- function(
103103

104104
#' @keywords internal
105105
list_temurin_versions_impl <- function(platform, arch) {
106-
# Adoptium API mapping
107-
api_os <- switch(
108-
platform,
109-
"macos" = "mac",
110-
"alpine-linux" = "alpine-linux",
111-
platform
106+
candidates <- tryCatch(
107+
java_valid_major_versions_temurin(platform = platform, arch = arch),
108+
error = function(e) character(0)
112109
)
113110

114-
# We first get available major releases
115-
major_vers <- tryCatch(
116-
java_valid_major_versions_temurin(),
117-
error = function(e) return(NULL)
118-
)
119-
120-
if (is.null(major_vers)) {
111+
if (length(candidates) == 0) {
121112
return(data.frame())
122113
}
123114

124-
all_releases <- list()
125-
126-
for (v in major_vers) {
127-
url <- sprintf(
128-
"https://api.adoptium.net/v3/assets/feature_releases/%s/ga?os=%s&architecture=%s&image_type=jdk&jvm_impl=hotspot",
129-
v,
130-
api_os,
131-
arch
132-
)
133-
134-
data <- tryCatch(
135-
read_json_url(url, max_simplify_lvl = "list"),
136-
error = function(e) NULL
137-
)
138-
139-
if (is.null(data) || length(data) == 0) {
140-
next
141-
}
142-
143-
for (rel in data) {
144-
all_releases[[length(all_releases) + 1]] <- data.frame(
145-
backend = "native",
146-
vendor = "Temurin",
147-
major = as.integer(v),
148-
version = rel$version_data$semver,
149-
platform = platform,
150-
arch = arch,
151-
identifier = rel$version_data$openjdk_version,
152-
checksum_available = TRUE,
153-
stringsAsFactors = FALSE
154-
)
155-
}
156-
}
157-
158-
if (length(all_releases) == 0) {
159-
return(data.frame())
160-
}
161-
do.call(rbind, all_releases)
115+
temurin_probe_available_versions(platform, arch, candidates)$releases
162116
}
163117

164118
#' @keywords internal

0 commit comments

Comments
 (0)