[lionel-]

Addresses posit-dev/positron#16523 and posit-dev/positron#14790 (comment)

Useful context: #1429

• The local renv library is made of symlinks into the global renv cache, to avoid proliferation of package copies in every renv projects
• renv injects its local library into .libPaths()
• The Oak LSP evaluates .libPaths() via Ark when it starts up. This becomes the input for library roots.
• The Oak scanner didn't follow symlinks

Simple fix in this PR: follow symlinks. New e2e tests cover renv and targets together.

Not fixed here: Nested environments are not statically discovered because we rely on the dynamic startup value of library paths, sent from the workspace top-level directory.

A better approach would be to call .libPaths() from a subprocess for the top-level and every nested folder with environment files (.Rprofile and .Renviron). We'd refresh that when the environment files change. And we'd independently track and scan the library folders for every level, so we pick up changes (installed and deleted packages) from outside R.

Positron Release Notes

New Features

• N/A

Bug Fixes

• N/A

[jennybc]

I created 2 small repro workspaces, one using targets, one not. I confirmed the undesirable "before" behaviour and that all is well "after" I build ark from this PR.

I made one observation/suggestion and am happy for you decide whether to do anything about it.

[jennybc]

Suggested change

	/// needs no network but does need `renv` and `targets` installed. The cache is
	/// isolated so the test never writes to the user's renv cache.
	fn renv_project_with_targets(project: &Path, cache: &Path) -> PathBuf {
	std::fs::create_dir_all(project).unwrap();
	let script = r#"
	project <- commandArgs(trailingOnly = TRUE)[[1]]
	renv::init(project = project, bare = TRUE, load = FALSE, restart = FALSE)
	/// needs no network but does need `renv` and `targets` installed there. Without
	/// `targets`, `hydrate()` would fall back to installing it from the repos, as
	/// copies rather than links, so the script checks for it up front. The cache is
	fn renv_project_with_targets(project: &Path, cache: &Path) -> PathBuf {
	std::fs::create_dir_all(project).unwrap();
	let script = r#"
	if (!requireNamespace("targets", quietly = TRUE)) {
	stop("`targets` must be installed in the library of the R running this test. renv copies it from there to build the fixture.", call. = FALSE)
	}
	project <- commandArgs(trailingOnly = TRUE)[[1]]
	renv::init(project = project, bare = TRUE, load = FALSE, restart = FALSE)

[jennybc]

I know you added targets to the GHA workflow files, so in theory targets would have to be present.

But I learned the old-fashioned way 😅 (apparently not having targets installed locally) that if you don't have targets installed, this test fails in a very cryptic way.

The confusion comes from two different libraries getting mixed up. The test depends on targets being in the library of whatever Rscript builds the fixture, because renv::hydrate() copies it from there into the isolated cache and links it into the fixture project's library. That's the only library the LSP under test sees. But when targets isn't in that first library, hydrate() doesn't error. Instead it falls back to installing targets and its ~30 dependencies from the repos (so the network gets involved, despite the docstring), and those land in the fixture library as plain copies rather than links.

So the eventual failure is assertion failed: targets_entry.file_type().is_symlink(). That points you at renv's linking behaviour, which is a red herring. The actual problem is a missing prerequisite in a completely different library.

I don't know if ark has a better way of expressing this?

[lionel-]

Good point. We now fail the test early if renv or targets is not installed (fail on CI, skip locally).