Skip to content

Commit

Permalink
reimplement recursive pruning so we still produce full dependency lists
Browse files Browse the repository at this point in the history
  • Loading branch information
aronatkins committed Nov 24, 2020
1 parent ad1463b commit 1a16006
Showing 1 changed file with 34 additions and 21 deletions.
55 changes: 34 additions & 21 deletions R/pkg.R
Original file line number Diff line number Diff line change
Expand Up @@ -193,10 +193,15 @@ getPackageRecords <- function(pkgNames,
# screen out empty package names that might have snuck in
pkgNames <- setdiff(pkgNames, "")

# avoid visiting other packages that have been recursively visited
pkgNames <- setdiff(pkgNames, ls(envir = .visited.packages))
# ... and then remember that we have visited these packages.
for (pkg in pkgNames) { .visited.packages[[pkg]] <- TRUE }
# Prior recursive steps may have already computed this package record and
# its recursive dependencies. Avoid constructing this package record.
priorPkgRecords <- c()
for (pkgName in pkgNames) {
if (exists(pkgName, envir = .visited.packages)) {
append(priorPkgRecords, get(pkgName, envir = .visited.packages))

This comment has been minimized.

Copy link
@kevinushey

kevinushey Nov 24, 2020

Contributor

This doesn't actually update the priorPkgRecords variable -- did you mean priorPkgRecords <- append(priorPkgRecords, get(pkgName, envir = .visited.packages))?

Or is it possible the solution is correct without actually populating this priorPkgRecords variable?

}
}
pkgNames <- setdiff(pkgNames, sapply(priorPkgRecords, "[[", "name"))

if (check.lockfile) {
lockfilePkgRecords <- getPackageRecordsLockfile(pkgNames, project = project)
Expand Down Expand Up @@ -253,6 +258,7 @@ getPackageRecords <- function(pkgNames,

# Collect the records together
allRecords <- c(
priorPkgRecords,
lockfilePkgRecords,
srcPkgRecords,
manualSrcPkgRecords,
Expand All @@ -266,24 +272,31 @@ getPackageRecords <- function(pkgNames,
# Now get recursive package dependencies if necessary
if (recursive) {
allRecords <- lapply(allRecords, function(record) {
deps <- getPackageDependencies(pkgs = record$name,
lib.loc = lib.loc,
available.packages = available)
if (!is.null(deps)) {
record$depends <- getPackageRecords(
deps,
project = project,
available,
TRUE,
lib.loc = lib.loc,
missing.package = missing.package,
check.lockfile = check.lockfile,
fallback.ok = fallback.ok,
.visited.packages = .visited.packages
)
if (exists(record$name, envir = .visited.packages)) {
# We have already processed this package and computed its recursive
# dependencies. Avoid recursively computing its dependencies.
get(record$name, envir = .visited.packages)
} else {
# We have not already processed this package.
deps <- getPackageDependencies(pkgs = record$name,
lib.loc = lib.loc,
available.packages = available)
if (!is.null(deps)) {
record$depends <- getPackageRecords(
deps,
project = project,
available,
TRUE,
lib.loc = lib.loc,
missing.package = missing.package,
check.lockfile = check.lockfile,
fallback.ok = fallback.ok,
.visited.packages = .visited.packages
)
}
.visited.packages[[record$name]] <- record
record
}

record
})
}

Expand Down

0 comments on commit 1a16006

Please sign in to comment.