Skip to content

Always list base packages in workspace_dependencies()#1329

Merged
DavisVaughan merged 4 commits into
mainfrom
feature/base-packages-as-workspace-dependencies
Jul 14, 2026
Merged

Always list base packages in workspace_dependencies()#1329
DavisVaughan merged 4 commits into
mainfrom
feature/base-packages-as-workspace-dependencies

Conversation

@DavisVaughan

Copy link
Copy Markdown
Contributor

I noticed that if you open purrr to https://github.com/tidyverse/purrr/blob/481e829f297fd4315b386518215157f361475ad0/R/adverb-safely.R#L42 and just try to goto definition on force(), then nothing happens.

That's because force() is a base function, but base isn't listed in Imports or Depends so it doesn't get picked up in workspace_dependencies() and we never try and set its package R/ sources.

Similar story if you open an arbitrary workspace of scripts where you use any package from the default R search() path without first calling library() or :: on that package somewhere in the file.

I think the right thing to do is to just ensure that the default search() packages are always listed in workspace_dependencies(). This is definitely right for workspace scripts. For workspace packages technically an individual package file would only need {base}, but if you add any extra scripts into the package, like inst/data.R, then we'd require the rest of the base packages anyways, so I think it is fine to just default to all of the default search() packages for workspace packages too. It doesn't affect diagnostics there, just the set of packages we try and download sources for, so over approximation is pretty harmless.

@DavisVaughan DavisVaughan requested a review from lionel- July 9, 2026 15:39

@lionel- lionel- left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For workspace packages technically an individual package file would only need {base}, but if you add any extra scripts into the package, like inst/data.R, then we'd require the rest of the base packages anyways, so I think it is fine to just default to all of the default search() packages for workspace packages too. It doesn't affect diagnostics there, just the set of packages we try and download sources for, so over approximation is pretty harmless.

For scripts in packages, I agree. For files in R/, we should only provide base by default. Is that still the case (or was the case to begin with? I don't remember)?

This will affect diagnostics later on so I would rather get this right now.

Comment thread crates/oak_db/src/workspace.rs Outdated
Comment thread crates/oak_db/src/workspace.rs
@DavisVaughan DavisVaughan force-pushed the feature/base-packages-as-workspace-dependencies branch from 2ab813e to 48bc1b1 Compare July 14, 2026 13:58
@DavisVaughan DavisVaughan merged commit bd50309 into main Jul 14, 2026
17 checks passed
@DavisVaughan DavisVaughan deleted the feature/base-packages-as-workspace-dependencies branch July 14, 2026 14:07
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 14, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants