Skip to content

Commit

Permalink
revdep_check tweaks
Browse files Browse the repository at this point in the history
  • Loading branch information
hadley committed Oct 5, 2016
1 parent 7ac900d commit c3457d2
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 22 deletions.
15 changes: 15 additions & 0 deletions NEWS.md
@@ -1,5 +1,20 @@
# devtools 1.12.0.9000

* More tweaks to `revdep_check()` and friends to make debugging easier when
something goes wrong. This includes:

* `revdep_check()` and `revdep_check_resume()` gain a skip argument
which takes a character vector of packages to skip.

* `revdep_check()` and `check_cran()` gain a `quiet_check` argument.
You can use `quiet_check = FALSE` to see the actual text of R CMD
check as it runs (not recommending with multiple threads).

* `revdep_check_resume()` now takes `...` which can be used to
override settings from `revdep_check()`. For debugging a problem
with package checks, try
`revdep_check(threads = 1, quiet_check = FALSE)`

* New `use_gpl3_license()` sets the license field in `DESCRIPTION` and
includes a copy of the license in `LICENSE`.

Expand Down
38 changes: 26 additions & 12 deletions R/check-cran.r
Expand Up @@ -20,6 +20,9 @@
#' @param threads Number of concurrent threads to use for checking.
#' It defaults to the option \code{"Ncpus"} or \code{1} if unset.
#' @param check_dir Directory to store results.
#' @param quiet_check If \code{TRUE}, suppresses individual \code{R CMD
#' check} output and only prints summaries. Set to \code{FALSE} for
#' debugging.
#' @return Returns (invisibly) the directory where check results are stored.
#' @keywords internal
#' @inheritParams check
Expand All @@ -29,7 +32,8 @@ check_cran <- function(pkgs, libpath = file.path(tempdir(), "R-lib"),
type = getOption("pkgType"),
threads = getOption("Ncpus", 1),
check_dir = tempfile("check_cran"),
env_vars = NULL) {
env_vars = NULL,
quiet_check = TRUE) {

stopifnot(is.character(pkgs))
if (length(pkgs) == 0) return()
Expand All @@ -47,25 +51,30 @@ check_cran <- function(pkgs, libpath = file.path(tempdir(), "R-lib"),

# Add the temporary library and remove on exit
libpaths_orig <- withr::with_libpaths(libpath, {

rule("Installing dependencies") # --------------------------------------------
repos <- c(CRAN = cran_mirror())
if (bioconductor) {
check_suggested("BiocInstaller")
repos <- c(repos, BiocInstaller::biocinstallRepos())
}
available_src <- available_packages(repos, "source")

message("Determining packages to update... ", appendLF = FALSE) # ------------
rule("Installing dependencies") # ------------------------------------------

deps <- package_deps(pkgs, repos = repos, type = type, dependencies = TRUE)
message(paste(deps$package, collapse = ", "))
update(deps, Ncpus = threads, quiet = TRUE)
needed <- deps$diff != CURRENT
if (any(needed)) {
message("Installing ", sum(needed), " packages: ", comma(pkgs))
update(deps, Ncpus = threads, quiet = TRUE)
}

message("Downloading source packages for checking") #-------------------------
# Download source packages
available_src <- available_packages(repos, "source")
urls <- lapply(pkgs, package_url, repos = repos, available = available_src)
ok <- vapply(urls, function(x) !is.na(x$name), logical(1))
if (any(!ok)) {
message("Couldn't find source for: ", paste(pkgs[!ok], collapse = ", "))
message(
"Skipping ", sum(!ok), " packages without source:",
comma(pkgs[!ok])
)
urls <- urls[ok]
pkgs <- pkgs[ok]
}
Expand All @@ -75,12 +84,17 @@ check_cran <- function(pkgs, libpath = file.path(tempdir(), "R-lib"),

needs_download <- !vapply(local_urls, is_source_pkg, logical(1))
if (any(needs_download)) {
message("Downloading ", sum(needs_download), " packages")
message(
"Downloading ", sum(needs_download), " source packages: ",
comma(pkgs[needs_download])
)
Map(utils::download.file, remote_urls[needs_download],
local_urls[needs_download], quiet = TRUE)
}

rule("Checking packages") # --------------------------------------------------
rule("Checking packages") # ------------------------------------------------
message("Checking ", length(pkgs), " packages: ", comma(pkgs))

check_start <- Sys.time()
pkg_names <- format(pkgs)
check_pkg <- function(i) {
Expand All @@ -90,7 +104,7 @@ check_cran <- function(pkgs, libpath = file.path(tempdir(), "R-lib"),
args = "--no-multiarch --no-manual --no-codoc",
env_vars = env_vars,
check_dir = check_dir,
quiet = TRUE
quiet = quiet_check
)
end_time <- Sys.time()

Expand Down
27 changes: 22 additions & 5 deletions R/revdep.R
Expand Up @@ -93,6 +93,8 @@ print.maintainers <- function(x, ...) {
#'
#' @inheritParams revdep
#' @param pkg Path to package. Defaults to current directory.
#' @param skip A character vector of package names to exclude from the
#' checks.
#' @inheritParams check_cran
#' @param check_dir A temporary directory to hold the results of the package
#' checks. This should not exist as after the revdep checks complete
Expand All @@ -113,12 +115,14 @@ print.maintainers <- function(x, ...) {
#' }
revdep_check <- function(pkg = ".", recursive = FALSE, ignore = NULL,
dependencies = c("Depends", "Imports", "Suggests", "LinkingTo"),
skip = character(),
libpath = getOption("devtools.revdep.libpath"),
srcpath = libpath, bioconductor = FALSE,
type = getOption("pkgType"),
threads = getOption("Ncpus", 1),
env_vars = NULL,
check_dir = NULL) {
check_dir = NULL,
quiet_check = TRUE) {

pkg <- as.package(pkg)

Expand All @@ -145,22 +149,27 @@ revdep_check <- function(pkg = ".", recursive = FALSE, ignore = NULL,
call. = FALSE)
}

message("Computing reverse dependencies... ", appendLF = FALSE)
message("Computing reverse dependencies... ")
revdeps <- revdep(pkg$package, recursive = recursive, ignore = ignore,
bioconductor = bioconductor, dependencies = dependencies)

message(paste(revdeps, collapse = ", "))
if (length(skip) > 0) {
message("Skipping: ", comma(skip))
revdeps <- setdiff(revdeps, skip)
}

# Save arguments and revdeps to a cache
cache <- list(
pkgs = revdeps,
skip = skip,
libpath = libpath,
srcpath = srcpath,
bioconductor = bioconductor,
type = type,
threads = threads,
check_dir = check_dir,
env_vars = env_vars
env_vars = env_vars,
quiet_check = quiet_check
)
saveRDS(cache, revdep_cache_path(pkg))

Expand All @@ -169,7 +178,9 @@ revdep_check <- function(pkg = ".", recursive = FALSE, ignore = NULL,

#' @export
#' @rdname revdep_check
revdep_check_resume <- function(pkg = ".") {
#' @param ... Optionally, override original value of arguments to
#' \code{revdep_check}. Use with care.
revdep_check_resume <- function(pkg = ".", ..., skip = character()) {
pkg <- as.package(pkg)

cache_path <- revdep_cache_path(pkg)
Expand All @@ -179,6 +190,12 @@ revdep_check_resume <- function(pkg = ".") {
}

cache <- readRDS(cache_path)
cache <- utils::modifyList(cache, list(...))

if (length(cache$pkgs) > 0) {
message("Skipping: ", comma(skip))
cache$pkgs <- setdiff(cache$pkgs, skip)
}

# Don't need to check packages that we've already checked.
check_dirs <- dir(cache$check_dir, full.names = TRUE)
Expand Down
7 changes: 7 additions & 0 deletions R/utils.r
Expand Up @@ -236,3 +236,10 @@ strip_internal_calls <- function(x, package) {
x
}
}

comma <- function(x, at_most = 20) {
if (length(x) > at_most) {
x <- c(x[seq_len(at_most)], "...")
}
paste(x, collapse = ", ")
}
6 changes: 5 additions & 1 deletion man/check_cran.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 15 additions & 4 deletions man/revdep_check.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 comments on commit c3457d2

@krlmlr
Copy link
Member

@krlmlr krlmlr commented on c3457d2 Oct 5, 2016

Choose a reason for hiding this comment

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

Now #1300, #1322, #1336 and #1338 can't be merged, might be due to these changes. Might be easier to revert, apply my stuff, and then redo manually.

@hadley
Copy link
Member Author

@hadley hadley commented on c3457d2 Oct 5, 2016

Choose a reason for hiding this comment

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

Sorry about that — I'll handle the merge conflicts

@krlmlr
Copy link
Member

@krlmlr krlmlr commented on c3457d2 Oct 5, 2016

Choose a reason for hiding this comment

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

Thanks! I'll update my PRs with NEWS, then.

@hadley
Copy link
Member Author

@hadley hadley commented on c3457d2 Oct 5, 2016

Choose a reason for hiding this comment

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

I just had to make these changes in order to debug why the revdeps checks were failing with ggplot2 😢

@krlmlr
Copy link
Member

@krlmlr krlmlr commented on c3457d2 Oct 5, 2016

Choose a reason for hiding this comment

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

No worries. I could also handle the conflicts and make one big PR.

@hadley
Copy link
Member Author

@hadley hadley commented on c3457d2 Oct 5, 2016

Choose a reason for hiding this comment

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

That would be convenient for me 😄 If you do it in the next couple of days and ping me in the PR, I'll review & merge quickly.

@jcheng5
Copy link
Member

@jcheng5 jcheng5 commented on c3457d2 Oct 5, 2016

Choose a reason for hiding this comment

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

This commit seems to be causing a problem for me trying to revdep_check htmltools:

> system.time({ devtools::revdep_check(recursive = TRUE) })
Reverse dependency checks for htmltools ============================================
Saving check results in `revdep/checks/`
Saving install results in `revdep/install/`
Computing reverse dependencies... 
Installing dependencies for htmltools to ~/Development/htmltools_revdep_lib/
Installing htmltools 0.3.6 to /var/folders/n4/d2bbm6h56z1fdgnspfnhync00000gn/T//RtmptONrMC/revdep14994442c5a5c
Setting env vars -------------------------------------------------------------------
NOT_CRAN    : false
RGL_USE_NULL: true
DISPLAY     : 
 Error in (function (pkgs, libpath = file.path(tempdir(), "R-lib"), srcpath = libpath,  : 
  unused argument (skip = character(0)) 
5. (function (pkgs, libpath = file.path(tempdir(), "R-lib"), srcpath = libpath, 
    check_libpath = libpath, bioconductor = FALSE, type = getOption("pkgType"), 
    threads = getOption("Ncpus", 1), check_dir = tempfile("check_cran"), 
    install_dir = tempfile("check_cran_install"), env_vars = NULL,  ... 
4. do.call(check_cran, cache) 
3. revdep_check_from_cache(pkg, cache) 
2. devtools::revdep_check(recursive = TRUE) 
1. system.time({
    devtools::revdep_check(recursive = TRUE)
}) 

@krlmlr
Copy link
Member

@krlmlr krlmlr commented on c3457d2 Oct 6, 2016

Choose a reason for hiding this comment

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

I'll submit a fix very soon.

Please sign in to comment.