Skip to content
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

add one level of additional revdeps to mirror packages in CI #6230

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ben-schwen
Copy link
Member

Currently we already mirror reverse dependencies, but these might also have reverse dependencies etc.

This results in that our CI reports things like also installing the dependencies ‘R.oo’, ‘R.methodsS3’, ‘evaluate’, ‘highr’, ‘xfun’, ‘commonmark’

This PR adds one extra level of mirroring also the c("Imports", "Depends") of the mirrored reverse dependencies (no recursion yet).

.ci/ci.R Outdated
Comment on lines 46 to 57
local.extract_dependency_package_names = function (x) {
## do not filter out R like tools:::.extract_dependency_package_names, used for web/$pkg/index.html
if (is.na(x))
return(character())
x <- unlist(strsplit(x, ",[[:space:]]*"))
x <- sub("[[:space:]]*([[:alnum:].]+).*", "\\1", x)
x[nzchar(x)]
}

rev.dependencies = function(pkg, available_pkgs, which=c("Imports", "Depends")) {
dependencies = available_pkgs[pkg, which]
x = dependencies[!is.na(dependencies)]
Copy link
Member

Choose a reason for hiding this comment

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

here can you please insert a comment explaining why tools::dependsOnPkgs is not sufficient?

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe I misunderstood something but tools:dependsOnPkgs(x) shows the relation for results depends on x (and only does so for installed packages). However, we want the inverse with x depends/imports result.

Copy link
Member

Choose a reason for hiding this comment

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

only does so for installed packages

This part is easy to overcome: tools::dependsOnPkgs("data.table", installed=available.packages()).

Anyway, you're correct that's going in the wrong direction; tools::package_dependencies() traverses the graph in the right direction:

tools::package_dependencies("data.table", recursive=TRUE, which="all") # overkill
# second-level instead
tools::package_dependencies("data.table", which="all") |>
  unlist() |> tools::package_dependencies() |> # could use which="all" here too?
  unlist(use.names=FALSE) |> setdiff(tools:::.get_standard_package_names()$base)

@tdhock
Copy link
Member

tdhock commented Jul 9, 2024

does "mirroring" mean that the ci will keep a copy of these packages between runs, rather than downloading them in each run?

@ben-schwen
Copy link
Member Author

does "mirroring" mean that the ci will keep a copy of these packages between runs, rather than downloading them in each run?

Exactly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants