Repository navigation
feat: isolate sess installation in managed libraries #1856
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
Open
eitsupi
wants to merge
21
commits into
REditorSupport:main
Choose a base branch
from
eitsupi:fix/managed-sess-installation
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+3,200
−744
Open
Changes from all commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
05ef11b
fix: isolate sess installation in managed libraries
eitsupi 771ff7c
fix: prepare sess in the console R process
eitsupi 15e630f
style: align sess setup continuation arguments
eitsupi 5b6a415
feat: allow suppressing sess prompts for the current revision
eitsupi 38b5b4d
fix: serialize managed sess setup
eitsupi ee36b35
fix: require a ready marker for managed sess
eitsupi 0bc28da
docs: note managed sess dependency limitations
eitsupi c662787
fix: keep terminal input pending during sess setup
eitsupi 8e2e433
test: isolate terminal startup timers and cleanup
eitsupi 827c1a5
test: separate sess attach integration checks
eitsupi 8b0310d
test: wait for R evaluation before interrupting
eitsupi 06489f3
fix: honor sess startup state before sending terminal input
eitsupi 9986ef1
test: provide ready status for managed terminal fixtures
eitsupi 4f9d24f
test: keep interrupt synchronization within one R expression
eitsupi 67b085a
test: simplify managed sess regression coverage
eitsupi 521c33d
style: use consistent braces in sess test fixture
eitsupi 88d8e09
fix: separate terminal startup from sess setup
eitsupi 3aa6dc4
style: fix terminal startup R lint warnings
eitsupi 1f646af
test: synchronize terminal and hover fixtures
eitsupi b30991f
fix: publish sess startup state on automatic reconnect
eitsupi 8a05ab1
test: read Interactive workspace from active extension
eitsupi File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,223 @@ | ||
| # Shared terminal and manual-attach preparation. Installation requires a | ||
| # single-use grant from the extension process; .libPaths() is never changed. | ||
| vscode_r_prepare_sess <- function(pkg_path, managed_root, consent_dir, | ||
| source_helper, installer_helper, | ||
| timeout_seconds = 180, | ||
| setup_timeout_seconds = 300) { | ||
| source(source_helper, local = TRUE) | ||
| expected <- sess_source_revision(file.path(pkg_path, "DESCRIPTION")) | ||
| if (is.null(expected)) { | ||
| stop("Bundled sess has no valid source revision.") | ||
| } | ||
| runtime <- sess_runtime_identity() | ||
| managed_library <- sess_managed_library(managed_root, expected) | ||
|
|
||
| loaded <- "sess" %in% loadedNamespaces() | ||
| if (loaded) { | ||
| loaded_revision <- sess_loaded_source_revision() | ||
| if (!identical(loaded_revision, expected)) { | ||
| stop("A different sess namespace is already loaded. Restart R before attaching the session watcher.") | ||
| } | ||
| ns <- asNamespace("sess") | ||
| } else { | ||
| library <- sess_find_source_library(expected, .libPaths()) | ||
| if (!is.null(library)) { | ||
| ns <- sess_load_namespace(library, expected) | ||
| } else { | ||
| lock_parent <- dirname(managed_library) | ||
| dir.create(lock_parent, recursive = TRUE, showWarnings = FALSE) | ||
| if (!dir.exists(lock_parent) || file.access(lock_parent, 2L) != 0L) { | ||
| stop("The vscode-R managed sess setup directory is not writable.") | ||
| } | ||
| lock_path <- file.path(lock_parent, ".setup-lock") | ||
| # Keep lock ownership rules in sync with src/interactive/backends/sessPreparation.ts: | ||
| # mkdir claims atomically; only the owner releases via on.exit; timeout never clears a stale lock. | ||
| # This revision lock covers consent, install, and load; a follower without a ready copy does not prompt. | ||
| # Publish .ready with the source revision only after the exact namespace has loaded successfully. | ||
| lock_deadline <- Sys.time() + setup_timeout_seconds | ||
| lock_timeout_message <- paste( | ||
| "Timed out waiting for another sess setup. Its owner may have crashed;", | ||
| "retrying alone will not clear the stale lock. Remove", | ||
| shQuote(lock_path), "only if its owner has exited, then retry." | ||
| ) | ||
| followed_setup <- FALSE | ||
| repeat { | ||
| acquired <- dir.create(lock_path, showWarnings = FALSE, mode = "0700") | ||
| if (isTRUE(acquired)) { | ||
| on.exit(unlink(lock_path, recursive = TRUE, force = TRUE), add = TRUE) | ||
| break | ||
| } | ||
| followed_setup <- TRUE | ||
| if (!file.exists(lock_path)) { | ||
| if (file.access(lock_parent, 2L) != 0L) { | ||
| stop("The vscode-R managed sess setup directory is not writable.") | ||
| } | ||
| if (Sys.time() >= lock_deadline) { | ||
| stop(lock_timeout_message) | ||
| } | ||
| Sys.sleep(0.1) | ||
| next | ||
| } | ||
| if (!dir.exists(lock_path)) { | ||
| stop("A file is blocking the vscode-R managed sess setup lock.") | ||
| } | ||
| if (Sys.time() >= lock_deadline) { | ||
| stop(lock_timeout_message) | ||
| } | ||
| Sys.sleep(0.1) | ||
| } | ||
|
|
||
| ready_path <- file.path(lock_parent, ".ready") | ||
| ready_revision <- tryCatch( | ||
| readLines(ready_path, warn = FALSE, n = 2L), | ||
| warning = function(e) character(), | ||
| error = function(e) character() | ||
| ) | ||
| installed <- sess_find_source_library(expected, managed_library) | ||
| if (length(ready_revision) == 1L && identical(ready_revision, expected) && | ||
| !is.null(installed)) { | ||
| ns <- sess_load_namespace(installed, expected) | ||
| } else if (followed_setup) { | ||
| return(NULL) | ||
| } else { | ||
| existing <- any(vapply(.libPaths(), function(library) { | ||
| file.exists(file.path(library, "sess", "DESCRIPTION")) | ||
| }, FALSE)) | ||
| reason <- if (existing) { | ||
| "mismatch" | ||
| } else { | ||
| "missing" | ||
| } | ||
| if (!dir.exists(consent_dir)) { | ||
| stop("The extension's sess consent service is unavailable. Restart VS Code and try again.") | ||
| } | ||
| new_id_part <- function() { | ||
| temporary <- basename(tempfile(pattern = "request-", tmpdir = consent_dir)) | ||
| gsub("[^A-Za-z0-9_-]", "", sub("^request-", "", temporary)) | ||
| } | ||
| id <- paste0(new_id_part(), new_id_part()) | ||
| if (!grepl("^[A-Za-z0-9_-]{16,64}$", id)) { | ||
| stop("Could not create a unique sess installation request.") | ||
| } | ||
| request <- paste( | ||
| "vscode-r-sess-consent-v1", id, expected, runtime, reason, sep = "\n") | ||
| request_path <- file.path(consent_dir, paste0(id, ".request")) | ||
| response_path <- file.path(consent_dir, paste0(id, ".response")) | ||
| temporary_path <- tempfile(pattern = paste0(id, "-"), tmpdir = consent_dir) | ||
| on.exit(unlink(c(temporary_path, request_path, response_path)), add = TRUE) | ||
| writeLines(request, temporary_path, useBytes = TRUE) | ||
| if (.Platform$OS.type == "unix") { | ||
| Sys.chmod(temporary_path, "0600") | ||
| } | ||
| if (!file.rename(temporary_path, request_path)) { | ||
| stop("Could not request permission to install bundled sess.") | ||
| } | ||
|
|
||
| deadline <- Sys.time() + timeout_seconds | ||
| response <- "" | ||
| while (Sys.time() < deadline && dir.exists(consent_dir) && !nzchar(response)) { | ||
| if (file.exists(response_path)) { | ||
| lines <- tryCatch( | ||
| readLines(response_path, warn = FALSE, n = 2L), | ||
| error = function(e) character()) | ||
| if (length(lines) == 1L && lines %in% c("approve", "decline")) { | ||
| response <- lines | ||
| } else { | ||
| stop("Invalid response to the sess installation request.") | ||
| } | ||
| } else { | ||
| Sys.sleep(0.2) | ||
| } | ||
| } | ||
| if (!identical(response, "approve")) { | ||
| message("Bundled sess was not installed. The session watcher was not attached.") | ||
| return(NULL) | ||
| } | ||
|
|
||
| unlink(ready_path, force = TRUE) | ||
| ready_link <- Sys.readlink(ready_path) | ||
| if (file.exists(ready_path) || dir.exists(ready_path) || | ||
| (length(ready_link) && !is.na(ready_link) && nzchar(ready_link))) { | ||
| stop("Could not clear the previous vscode-R managed sess completion marker.") | ||
| } | ||
| configured <- getOption("repos") | ||
| repo <- if ("CRAN" %in% names(configured)) { | ||
| configured[["CRAN"]] | ||
| } else if (length(configured)) { | ||
| configured[[1L]] | ||
| } else { | ||
| "https://cloud.r-project.org" | ||
| } | ||
| if (!length(repo) || is.na(repo) || !nzchar(repo) || identical(repo, "@CRAN@")) { | ||
| repo <- "https://cloud.r-project.org" | ||
| } | ||
| dir.create(managed_library, recursive = TRUE, showWarnings = FALSE) | ||
| if (file.access(managed_library, 2L) != 0L) { | ||
| stop("The vscode-R managed sess library is not writable.") | ||
| } | ||
| installer <- new.env(parent = baseenv()) | ||
| sys.source(installer_helper, envir = installer) | ||
| installer$sess_install(pkg_path, managed_library, repo) | ||
| ns <- sess_load_namespace(managed_library, expected) | ||
| ready_temporary <- tempfile(pattern = ".ready-", tmpdir = lock_parent) | ||
| on.exit(unlink(ready_temporary), add = TRUE) | ||
| writeLines(expected, ready_temporary, useBytes = TRUE) | ||
| if (.Platform$OS.type == "unix") { | ||
| Sys.chmod(ready_temporary, "0600") | ||
| } | ||
| if (!file.rename(ready_temporary, ready_path)) { | ||
| stop("Could not publish the vscode-R managed sess completion marker.") | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| ns | ||
| } | ||
|
|
||
| vscode_r_attach_sess <- function(endpoint, pkg_path, managed_root, consent_dir, | ||
| source_helper, installer_helper, plot_backend, | ||
| timeout_seconds = 180, | ||
| setup_timeout_seconds = 300) { | ||
| registered <- getOption("vscodeR.terminalStartup") | ||
| profile_process <- is.list(registered) && identical(registered$pid, Sys.getpid()) | ||
| startup_context <- NULL | ||
| if (profile_process) { | ||
| notifier_available <- exists("vscode_r_startup_existing", mode = "function") && | ||
| exists("vscode_r_startup_run", mode = "function") | ||
| if (!notifier_available) { | ||
| startup_helper <- Sys.getenv("VSCODE_R_SESS_STARTUP_HELPER", unset = "") | ||
| if (nzchar(startup_helper) && file.exists(startup_helper)) { | ||
| tryCatch(source(startup_helper, local = TRUE), error = function(error) { | ||
| message("vscode-R could not load terminal startup notifier: ", conditionMessage(error)) | ||
| }) | ||
| } | ||
| notifier_available <- exists("vscode_r_startup_existing", mode = "function") && | ||
| exists("vscode_r_startup_run", mode = "function") | ||
| } | ||
| if (!notifier_available) { | ||
| message("vscode-R terminal startup notifier is unavailable; the session watcher was not attached.") | ||
| return(invisible(FALSE)) | ||
| } | ||
| startup_context <- vscode_r_startup_existing(endpoint) | ||
| if (is.null(startup_context)) { | ||
| message("vscode-R could not validate terminal startup status; the session watcher was not attached.") | ||
| return(invisible(FALSE)) | ||
| } | ||
| } | ||
|
|
||
| attach <- function() { | ||
| ns <- vscode_r_prepare_sess(pkg_path, managed_root, consent_dir, source_helper, installer_helper, | ||
| timeout_seconds, setup_timeout_seconds) | ||
| if (is.null(ns)) { | ||
| return(invisible(FALSE)) | ||
| } | ||
| connect <- get("connect", envir = ns, inherits = FALSE) | ||
| connect(endpoint = endpoint, plot_backend = plot_backend) | ||
| } | ||
|
|
||
| if (profile_process) { | ||
| return(vscode_r_startup_run(startup_context, attach)) | ||
| } | ||
| attach() | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One potential issue of reusing
sessis withrenv. If multiplerenvprojects exist, their isolated environments may not have the dependencies required bysessinstalled, even if those dependencies are available in another project environment.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Fred-Wu How about 0bc28da?
While it may be possible to resolve this in the future, the implementation would be extremely complex at this point, so I felt it should be noted as a known limitation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potentially could be resolved if using one managed library for all vscode-R dependent packages. Leave it to later time then.