From dee1a915b631b41991a22aef75752a3528220805 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 6 Jul 2026 12:38:03 +0000 Subject: [PATCH 1/3] Production-harden loader, multi-package dependency tracking, exhaustive tests Loader (R/loadfast.R): - Track cross-package dependencies: reloading a package flags every cached package that (transitively) imports it; the flagged package's next load_fast() auto-upgrades to a full reload, and full loads recursively reload flagged dependencies first, so a chain C -> B -> A converges with a single load_fast(C) after A is reloaded. No-change loads do not invalidate. - Attach DESCRIPTION Depends packages during full loads (with a clear error when one is missing), matching library()/load_all() semantics. - Run .onUnload best-effort when unloadNamespace() refuses (dependency imported by other loaded packages) before force-replacing the registry entry. - Clean up half-built namespaces when a full load fails partway (unregister and detach), while keeping failed-load recovery on the next call. - Detach/unload the old identity when the Package field changes in place for a cached path. - Re-source incremental changes in Collate-aware full-load order instead of alphabetical basename order. - Seed the namespace spec version from DESCRIPTION so getNamespaceVersion() reports correctly. - Replace the boolean re-entrance flag with a path stack (public load_fast() stays strict; internal recursion is allowed). - Replace library("testthat") with a guarded attachNamespace(), inline the tiny methods::: metadata helpers (.hasS4MetaData/.getGenerics/ .TableMetaPrefix equivalents), move the top-level message to .onAttach, and make the sources ASCII-only -- R CMD check --as-cran is now clean (0 errors, 0 warnings; only the inherent attach() NOTE remains). Packaging: - DESCRIPTION: version 1.0.0, declare methods/tools in Imports, move dev dependencies from the non-standard DevDeps field to Suggests. - roxygen2-generated NAMESPACE and man/ pages for both exports. - New tests/testthat suite that runs against the installed package during R CMD check (smoke, incremental, dependency invalidation, error paths). - .Rbuildignore covers all non-package files; renv settings track Suggests. Tests (test_loadfast.R, 299 checks total): - Stage 6: multi-package dependency invalidation -- direct importer invalidation, transitive chain convergence, no-change loads, whole- namespace import(), S4 class redefinition across packages, forced unregister path incl. .onUnload. - Stage 7: production hardening -- Depends attachment and failure, rename- in-place cleanup, failed-full-load cleanup and recovery, namespace version, Collate order on incremental reload, re-entrance guard, verbose logs. CI: harness now also runs on macOS; new R CMD check --as-cran job on Linux/Windows/macOS (R release) plus oldrel-1 on Linux. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01S8v6K52c7AJwuDkyyETMU7 --- .Rbuildignore | 10 + .github/workflows/ci.yml | 33 ++- .gitignore | 2 + AGENTS.md | 14 +- DESCRIPTION | 15 +- NAMESPACE | 2 + R/loadfast.R | 342 ++++++++++++++++++---- README.md | 57 +++- TECHNICAL_DEBT.md | 144 ++++------ justfile | 10 + man/load_fast.Rd | 60 ++++ man/load_fast_register_reload.Rd | 22 ++ renv/settings.json | 2 +- test_loadfast.R | 474 +++++++++++++++++++++++++++++++ tests/testthat.R | 4 + tests/testthat/test-load_fast.R | 147 ++++++++++ 16 files changed, 1173 insertions(+), 165 deletions(-) create mode 100644 man/load_fast.Rd create mode 100644 man/load_fast_register_reload.Rd create mode 100644 tests/testthat.R create mode 100644 tests/testthat/test-load_fast.R diff --git a/.Rbuildignore b/.Rbuildignore index 0e9fd07..054ea3c 100644 --- a/.Rbuildignore +++ b/.Rbuildignore @@ -1,2 +1,12 @@ ^renv$ ^renv\.lock$ +^\.Rprofile$ +^\.github$ +^\.gitignore$ +^test_loadfast\.R$ +^devpackage$ +^AGENTS\.md$ +^TECHNICAL_DEBT\.md$ +^justfile$ +^pkgload$ +^devtools$ diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a97aa36..e843d7e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -8,12 +8,13 @@ on: jobs: test: + name: harness (${{ matrix.os }}) runs-on: ${{ matrix.os }} strategy: fail-fast: false matrix: - os: [ubuntu-latest, windows-latest] + os: [ubuntu-latest, windows-latest, macos-latest] steps: - uses: actions/checkout@v4 @@ -22,3 +23,33 @@ jobs: r-version: "4.5.2" - uses: r-lib/actions/setup-renv@v2 - run: Rscript test_loadfast.R + + check: + name: R CMD check (${{ matrix.os }}, R ${{ matrix.r }}) + runs-on: ${{ matrix.os }} + + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, windows-latest, macos-latest] + r: ["release"] + include: + - os: ubuntu-latest + r: "oldrel-1" + + env: + RENV_CONFIG_AUTOLOADER_ENABLED: "FALSE" + + steps: + - uses: actions/checkout@v4 + - uses: r-lib/actions/setup-r@v2 + with: + r-version: ${{ matrix.r }} + use-public-rspm: true + - uses: r-lib/actions/setup-r-dependencies@v2 + with: + extra-packages: any::rcmdcheck + needs: check + - uses: r-lib/actions/check-r-package@v2 + with: + args: 'c("--no-manual", "--as-cran")' diff --git a/.gitignore b/.gitignore index 33af910..74f1a36 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,4 @@ pkgload/ devtools/ +loadfast_*.tar.gz +loadfast.Rcheck/ diff --git a/AGENTS.md b/AGENTS.md index db9ed93..09fdc0c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -12,9 +12,9 @@ This `AGENT.md` file is read by every agent session. !!!Keep them high-signal!!! ## Project layout - This repo **is** the source-of-truth edit target for the `loadfast` package. If you are changing loader behavior, docs, or tests for `loadfast`, make the change here. -- **`R/loadfast.R`** contains the canonical loader implementation exported by the package. It provides MD5-based incremental reloading: on first call it does a full teardown+rebuild; on subsequent calls for the same path it re-sources only files whose MD5 hash changed. See "Incremental loader" section below. -- **`DESCRIPTION`**, **`NAMESPACE`**, and **`inst/rstudio/addins.dcf`** define the installable package and its RStudio addin registration. -- **`test_loadfast.R`** is the single custom test runner. It sources `R/loadfast.R` directly and remains the single command users run. +- **`R/loadfast.R`** contains the canonical loader implementation exported by the package. It provides MD5-based incremental reloading: on first call it does a full teardown+rebuild; on subsequent calls for the same path it re-sources only files whose MD5 hash changed. The public `load_fast()` is a thin re-entrance guard around the internal `.loadfast.load()`, which can recurse to reload flagged dependencies. See "Incremental loader" section below. +- **`DESCRIPTION`**, **`NAMESPACE`**, and **`inst/rstudio/addins.dcf`** define the installable package and its RStudio addin registration. `NAMESPACE` and `man/` are **generated by roxygen2** (`roxygen2::roxygenize(".")`) — edit the roxygen comments in `R/loadfast.R`, not the generated files. Dev/test-only packages live in `Suggests`. +- **`test_loadfast.R`** is the main custom test runner (~300 checks, stages 1-7). It sources `R/loadfast.R` directly and remains the single command users run. **`tests/testthat/`** additionally holds a small self-contained suite that runs against the *installed* package during `R CMD check`. - **`devpackage/`** is the single frozen baseline package snapshot used by the tests. Contains `DESCRIPTION`, `NAMESPACE`, `R/` (source files), and `tests/testthat/` (testthat tests + helpers). Package name is `devpackage`. All code mutations for reload/incremental testing are applied ad-hoc to temp copies at test time. - `R/base.R` — plain functions (`add`, `scale_vector`, `summarize_values`, `mutate_dt`) — `mutate_dt` exercises `data.table`'s `:=` and `as.data.table` via `importFrom` - `R/s3_classes.R` — S3 constructor (`new_temperature`), a package-defined generic (`describe_s3`), and methods on both base generics (`print`/`format`/`as.character`) and the local generic. Registered via `S3method()` in NAMESPACE, none exported by name — so dispatch depends on the loader populating the S3 methods table. @@ -40,7 +40,11 @@ This `AGENT.md` file is read by every agent session. !!!Keep them high-signal!!! ## Incremental loader (`R/loadfast.R`) -- **`.loadfast.cache`** is a module-level environment (`parent = emptyenv()`) keyed by `normalizePath(path)`. Each entry currently stores the namespace env, file hashes, `renv.lock` hash baseline, any registered reload files, and the pending reload message to emit on the next load. +- **`.loadfast.cache`** is a module-level environment (`parent = emptyenv()`) keyed by `normalizePath(path)`. Each entry stores the namespace env, `pkg_name`, file hashes, `renv.lock` hash baseline, the S3 methods matrix, `import_pkgs` (names of packages this package imports), `needs_full` (NULL, or a reason string set by dependency invalidation), any registered reload files, and the pending reload message to emit on the next load. +- **Dependency invalidation**: after any load that re-sources files, `.loadfast.invalidate_dependents()` flags every cached package that (transitively) imports the reloaded package by setting `needs_full`. A flagged package's next `load_fast()` is upgraded to a full reload, and the full-load path first recursively reloads flagged *dependencies* (via `.loadfast.load()`, guarded by the `.loadfast.state$stack` path stack), so one call on the top of a chain converges the whole chain. No-change loads do not invalidate. +- **Depends**: `.loadfast.parse_depends()` reads `Depends` from DESCRIPTION (minus `R (...)`), and the full-load path attaches each with `library()` if not already attached; a missing Depends package is a clear error. +- **Teardown** (`.loadfast.teardown()`): detach + `unloadNamespace()`. When the namespace is imported by other loaded namespaces, `unloadNamespace()` errors — the fallback runs the old namespace's `.onUnload` best-effort and force-removes it from the registry via `rlang::ns_registry_env()`. A failed full load cleans up after itself (unregisters the half-built ns, detaches) via an `on.exit` guard. +- **Package identity**: if the `Package:` field changes in place for a cached path, the old name is detached/unloaded and the cache entry dropped before the full load of the new name. - **Change detection**: `tools::md5sum()` on all `R/*.R` files every call. Compared against cached hashes to classify files as changed or added. - **No per-file symbol tracking**: the incremental path does **not** track which symbols came from which file, and does **not** remove stale symbols when files are deleted or functions are removed. This avoids the O(n²) `ls()` overhead that dominated load time (27.5s of 33.6s in a 475-file project). Stale symbols linger until the user calls `load_fast(path, full = TRUE)`. - **Package env sync**: after incremental re-sourcing, all symbols from `ns_env` are bulk-copied to the `package:pkg` environment (one `ls()` call total). @@ -64,6 +68,8 @@ This `AGENT.md` file is read by every agent session. !!!Keep them high-signal!!! - 3d: add a `Collate` field plus deliberately misordered filenames, then verify the loader respects `Collate` ordering rather than plain alphabetical order. - **Stage 4**: Copy `devpackage/` to a third temp dir, test incremental-specific behaviors: no-change short-circuit, function removal (stale symbols linger), function addition, function modification, new file, file deletion, explicit file reload registration, failed incremental reload recovery, runtime S4 method patch reload registration, persistent `renv.lock` change warnings, `.onLoad`/`.onAttach` hook execution (4l), and S3 method table refresh on incremental reload (4m). Verifies that `full = TRUE` properly cleans up stale symbols. - **Stage 5**: Build ad-hoc packages (distinct class names, so nothing collides) to test cross-package fidelity: `exportPattern()` populating exports and `pkg::fn` resolving (5a), `pkg::missing` giving a proper not-exported error rather than the `lazydata` internal (5b), S3 dispatch across package boundaries for both base and package-defined generics (5c), and `exportClasses()`/`exportMethods()` + `importClassesFrom()`/`importMethodsFrom()` for S4 (5d). + - **Stage 6**: Multi-package dependency invalidation: direct importer auto-upgraded to full reload (6a), transitive chain convergence with recursive dependency reload (6b), no-change loads do not invalidate (6c), whole-namespace `import()` invalidation (6d), S4 class redefinition propagating to importers (6e), and the forced-unregister path when `unloadNamespace()` refuses (including `.onUnload` execution) (6f). + - **Stage 7**: Production hardening: `Depends` attachment + missing-Depends error (7a), rename-in-place cleanup (7b), failed-full-load cleanup + recovery (7c), namespace version from DESCRIPTION (7d), Collate order on incremental re-source (7e), re-entrance guard (7f), verbose timing logs (7g). - Stage 2 always triggers a full load (different path from Stage 1 = cache miss). Stage 3 exercises the incremental path for `load_fast()` on a stable temp path while mutating files in place. The multi-package stages (3e, 3h) call `strip_s3_fixture()` on their copies so the shared `temperature` S3 fixture from the baseline does not register colliding methods across renamed packages. ## R namespace machinery gotchas diff --git a/DESCRIPTION b/DESCRIPTION index 04a0d56..a688927 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Package: loadfast Title: Fast Incremental Package Loader for Interactive Development -Version: 0.0.1 +Version: 1.0.0 Authors@R: person(given = "Finccam", family = "Team", email = "engineering@finccam.com", role = c("aut", "cre")) Description: Fast incremental reloading for interactive development on large R packages. Intended for edit-reload-test workflows where @@ -12,9 +12,12 @@ Encoding: UTF-8 Roxygen: list(markdown = TRUE) RoxygenNote: 7.3.2 Imports: - rlang -DevDeps: - testthat, - R6, + methods, + rlang, + tools +Suggests: data.table, - digest + digest, + pkgload, + R6, + testthat diff --git a/NAMESPACE b/NAMESPACE index ccb6310..4d69b1b 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -1,2 +1,4 @@ +# Generated by roxygen2: do not edit by hand + export(load_fast) export(load_fast_register_reload) diff --git a/R/loadfast.R b/R/loadfast.R index 188d5ad..b36a18f 100644 --- a/R/loadfast.R +++ b/R/loadfast.R @@ -1,8 +1,10 @@ -message("Incremental reload is available via load_fast().") - .loadfast.cache <- new.env(parent = emptyenv()) .loadfast.state <- new.env(parent = emptyenv()) -.loadfast.state$loading <- FALSE +.loadfast.state$stack <- character(0) + +.onAttach <- function(libname, pkgname) { + packageStartupMessage("Incremental reload is available via load_fast().") +} #' Load a package from source with MD5-based incremental reloading #' @@ -11,6 +13,17 @@ message("Incremental reload is available via load_fast().") #' On subsequent calls for that same path it re-sources only `R/` files whose #' MD5 hashes changed. #' +#' Several interdependent source packages can be loaded in the same session: +#' load the dependency first, then the packages that import from it. When a +#' package is reloaded, every loaded package that (directly or transitively) +#' imports from it is flagged, and its next `load_fast()` call is +#' automatically upgraded to a full reload so importers never keep running +#' against a stale snapshot. A full reload also reloads flagged dependencies +#' first, in dependency order. +#' +#' Packages listed in the `Depends` field of `DESCRIPTION` are attached to +#' the search path during a full load, mirroring `library()` semantics. +#' #' @param path Path to a package root containing `DESCRIPTION`, `NAMESPACE`, #' and `R/`. If `path` points inside a package, `load_fast()` walks upward to #' the package root. @@ -21,13 +34,26 @@ message("Incremental reload is available via load_fast().") #' @param full If `TRUE`, force a complete teardown and rebuild. #' @param verbose If `TRUE`, emit per-phase timing logs. #' -#' @return Invisibly returns the namespace environment. +#' @return Invisibly returns the namespace environment (or `NULL` for a +#' package without `R/` files). +#' +#' @examples +#' \dontrun{ +#' # Load the package containing the current working directory +#' load_fast() +#' +#' # Load a specific package and force a clean rebuild +#' load_fast("path/to/pkg", full = TRUE) +#' } #' @export load_fast <- function(path = ".", helpers = TRUE, attach_testthat = NULL, full = FALSE, verbose = FALSE) { - if (.loadfast.state$loading) stop("load_fast() re-entrance detected — a sourced file is calling load_fast()") - .loadfast.state$loading <- TRUE - on.exit(.loadfast.state$loading <- FALSE, add = TRUE) + if (length(.loadfast.state$stack) > 0L) { + stop("load_fast() re-entrance detected -- a sourced file is calling load_fast()") + } + .loadfast.load(path, helpers = helpers, attach_testthat = attach_testthat, full = full, verbose = verbose) +} +.loadfast.load <- function(path = ".", helpers = TRUE, attach_testthat = NULL, full = FALSE, verbose = FALSE) { if (verbose) { .t0 <- proc.time()["elapsed"] .t_last <- .t0 @@ -41,6 +67,15 @@ load_fast <- function(path = ".", helpers = TRUE, attach_testthat = NULL, full = } abs_path <- .loadfast.find_package_root(path) + if (abs_path %in% .loadfast.state$stack) { + stop("load_fast() re-entrance detected for ", abs_path) + } + .loadfast.state$stack <- c(.loadfast.state$stack, abs_path) + on.exit( + .loadfast.state$stack <- setdiff(.loadfast.state$stack, abs_path), + add = TRUE + ) + path_input <- normalizePath(path, mustWork = TRUE) path_display <- if (identical(path, ".")) { basename(abs_path) @@ -122,7 +157,13 @@ load_fast <- function(path = ".", helpers = TRUE, attach_testthat = NULL, full = NULL } + needs_full_reason <- if (is.null(cached)) NULL else cached$needs_full + if (!is.null(needs_full_reason)) { + message("Full reload of '", pkg_name, "': ", needs_full_reason, ".") + } + can_incremental <- !is.null(cached) && + is.null(needs_full_reason) && !is.null(active_ns_env) && identical(cached$ns_env, active_ns_env) && pkg_env_name %in% search() @@ -158,7 +199,9 @@ load_fast <- function(path = ".", helpers = TRUE, attach_testthat = NULL, full = registered_added_reload_files <- new_files[new_files_cmp %in% intersect(new_files_cmp, registered_added_reload_files)] files_to_source <- unique(c(changed_files, added_files, registered_existing_reload_files, registered_added_reload_files)) - files_to_source <- files_to_source[order(basename(files_to_source), files_to_source)] + # Re-source in the same order as a full load (Collate-aware r_files order), + # so class definitions still come before methods when both files changed. + files_to_source <- files_to_source[order(match(files_to_source, r_files))] if (length(files_to_source) == 0L) { if (!is.null(pending_reload_message)) { @@ -170,6 +213,8 @@ load_fast <- function(path = ".", helpers = TRUE, attach_testthat = NULL, full = hashes = current_hashes, lock_hash = old_lock_hash, s3_methods = s3_methods_matrix, + import_pkgs = cached$import_pkgs, + needs_full = NULL, registered_reload_files = character(0), pending_reload_message = NULL ) @@ -204,9 +249,12 @@ load_fast <- function(path = ".", helpers = TRUE, attach_testthat = NULL, full = hashes = current_hashes, lock_hash = old_lock_hash, s3_methods = s3_methods_matrix, + import_pkgs = cached$import_pkgs, + needs_full = NULL, registered_reload_files = character(0), pending_reload_message = NULL ) + .loadfast.invalidate_dependents(pkg_name, abs_path) n_changed <- length(changed_files) n_added <- length(added_files) @@ -246,17 +294,73 @@ load_fast <- function(path = ".", helpers = TRUE, attach_testthat = NULL, full = # ---- FULL LOAD ---- - if (pkg_env_name %in% search()) { - detach(pkg_env_name, character.only = TRUE, unload = FALSE, force = TRUE) + # If the Package field changed in place, the cache entry (and the loaded + # namespace it references) describe a different logical package; tear the old + # identity down so it does not linger on the search path. + prior <- if (exists(abs_path, envir = .loadfast.cache, inherits = FALSE)) { + .loadfast.cache[[abs_path]] + } else { + NULL } - if (pkg_name %in% loadedNamespaces()) { - tryCatch(unloadNamespace(pkg_name), error = function(e) { - reg <- rlang::ns_registry_env() - if (exists(pkg_name, envir = reg, inherits = FALSE)) { - rm(list = pkg_name, envir = reg) - } - }) + if (!is.null(prior) && !is.null(prior$pkg_name) && !identical(prior$pkg_name, pkg_name)) { + old_name <- prior$pkg_name + message( + "Package name at ", path_display, " changed from '", old_name, "' to '", + pkg_name, "'; unloading '", old_name, "'." + ) + .loadfast.teardown(old_name, abs_path) + rm(list = abs_path, envir = .loadfast.cache) + } + + depends_pkgs <- .loadfast.parse_depends(desc_fields) + for (dep in depends_pkgs) { + if (!paste0("package:", dep) %in% search()) { + tryCatch( + library(dep, character.only = TRUE, warn.conflicts = FALSE), + error = function(e) { + stop( + "Failed to attach Depends package '", dep, "': ", + conditionMessage(e), + "\nIf it is a source package, load it with load_fast() first.", + call. = FALSE + ) + } + ) + } } + .timer("attach Depends") + + ns_file <- file.path(abs_path, "NAMESPACE") + nsInfo <- NULL + dep_pkg_names <- character(0) + if (file.exists(ns_file)) { + nsInfo <- parseNamespaceFile( + basename(abs_path), + dirname(abs_path), + mustExist = FALSE + ) + dep_pkg_names <- unique(c( + vapply(nsInfo$imports, function(i) as.character(i[[1L]]), character(1L)), + vapply(nsInfo$importClasses, function(i) as.character(i[[1L]]), character(1L)), + vapply(nsInfo$importMethods, function(i) as.character(i[[1L]]), character(1L)) + )) + } + .timer("parseNamespaceFile") + + # Recursively reload cached dependencies that were flagged as needing a full + # reload, so this package is rebuilt against fresh namespaces rather than + # stale ones (dependency order converges in one call). + for (key in ls(.loadfast.cache, all.names = TRUE)) { + entry <- .loadfast.cache[[key]] + if (identical(key, abs_path) || is.null(entry$needs_full)) next + if (key %in% .loadfast.state$stack) next + if (!(entry$pkg_name %in% dep_pkg_names)) next + message("Reloading dependency '", entry$pkg_name, "' first: ", entry$needs_full, ".") + .loadfast.load(key, helpers = FALSE, attach_testthat = FALSE, full = TRUE, verbose = verbose) + } + .timer("reload flagged dependencies") + + .loadfast.teardown(pkg_name, abs_path) .timer("detach + unload old ns") impenv <- new.env(parent = .BaseNamespaceEnv, hash = TRUE) @@ -265,9 +369,16 @@ load_fast <- function(path = ".", helpers = TRUE, attach_testthat = NULL, full = ns_env <- new.env(parent = impenv, hash = TRUE) ns_env$.packageName <- pkg_name + pkg_version <- if ("Version" %in% colnames(desc_fields)) { + unname(trimws(desc_fields[1L, "Version"])) + } else { + "0.0.0" + } + if (is.na(pkg_version) || !nzchar(pkg_version)) pkg_version <- "0.0.0" + info <- new.env(hash = TRUE, parent = baseenv()) ns_env[[".__NAMESPACE__."]] <- info - info[["spec"]] <- c(name = pkg_name, version = "0.0.0") + info[["spec"]] <- c(name = pkg_name, version = pkg_version) setNamespaceInfo(ns_env, "exports", new.env(hash = TRUE, parent = baseenv())) # `lazydata` must exist even for packages that ship no data: base's # `getExportedValue()` (used by `pkg::name`) falls through to it for any name @@ -287,20 +398,36 @@ load_fast <- function(path = ".", helpers = TRUE, attach_testthat = NULL, full = reg <- rlang::ns_registry_env() reg[[pkg_name]] <- ns_env + # If the load fails from here on (bad source file, failed import), do not + # leave a half-built namespace registered or attached: the next call would + # recover anyway, but `loadedNamespaces()` / `pkg::` must not expose a + # partially loaded package in the meantime. + full_load_ok <- FALSE + on.exit( + { + if (!full_load_ok) { + if (pkg_env_name %in% search()) { + tryCatch( + detach(pkg_env_name, character.only = TRUE, unload = FALSE, force = TRUE), + error = function(e) NULL + ) + } + cleanup_reg <- rlang::ns_registry_env() + if (exists(pkg_name, envir = cleanup_reg, inherits = FALSE) && + identical(cleanup_reg[[pkg_name]], ns_env)) { + rm(list = pkg_name, envir = cleanup_reg) + } + } + }, + add = TRUE + ) + if (isNamespaceLoaded("methods")) { methods::setPackageName(pkg_name, ns_env) } .timer("create + register ns env") - ns_file <- file.path(abs_path, "NAMESPACE") - if (file.exists(ns_file)) { - nsInfo <- parseNamespaceFile( - basename(abs_path), - dirname(abs_path), - mustExist = FALSE - ) - .timer("parseNamespaceFile") - + if (!is.null(nsInfo)) { for (i in nsInfo$imports) { imp_label <- if (is.character(i)) i else i[[1L]] tryCatch( @@ -400,7 +527,7 @@ load_fast <- function(path = ".", helpers = TRUE, attach_testthat = NULL, full = .timer(paste0("source ", length(r_files), " files")) s3_methods_matrix <- matrix(NA_character_, 0L, 4L) - if (file.exists(ns_file)) { + if (!is.null(nsInfo)) { exports <- .loadfast.compute_exports(ns_env, nsInfo, pkg_name) if (length(exports) > 0L) { namespaceExport(ns_env, exports) @@ -410,17 +537,9 @@ load_fast <- function(path = ".", helpers = TRUE, attach_testthat = NULL, full = } .timer("exports + S3 registration") - uses_testthat <- local({ - test_dirs <- c( - file.path(abs_path, "inst", "tests"), - file.path(abs_path, "tests", "testthat") - ) - any(dir.exists(test_dirs)) && requireNamespace("testthat", quietly = TRUE) - }) + uses_testthat <- .loadfast.uses_testthat(abs_path) if (is.null(attach_testthat)) attach_testthat <- uses_testthat - if (isTRUE(attach_testthat) && pkg_name != "testthat") { - library("testthat", warn.conflicts = FALSE) - } + .loadfast.attach_testthat(attach_testthat, pkg_name) .timer("attach testthat") .loadfast.run_hook(".onLoad", ns_env, abs_path, pkg_name) @@ -448,10 +567,14 @@ load_fast <- function(path = ".", helpers = TRUE, attach_testthat = NULL, full = hashes = current_hashes, lock_hash = current_lock_hash, s3_methods = s3_methods_matrix, + import_pkgs = setdiff(dep_pkg_names, "base"), + needs_full = NULL, registered_reload_files = character(0), pending_reload_message = NULL ) + .loadfast.invalidate_dependents(pkg_name, abs_path) + full_load_ok <- TRUE message("Load ", length(r_files), " file(s) from ", r_dir_display, ".") .timer("TOTAL (full load)") invisible(ns_env) @@ -543,8 +666,7 @@ load_fast_register_reload <- function(path = ".", files, reason = NULL) { exports <- c(ls(ns_env, pattern = p, all.names = TRUE), exports) } - has_s4 <- isNamespaceLoaded("methods") && - isTRUE(tryCatch(methods:::.hasS4MetaData(ns_env), error = function(e) FALSE)) + has_s4 <- isNamespaceLoaded("methods") && .loadfast.has_s4_metadata(ns_env) if (has_s4 && pkg_name != "methods") { methods::cacheMetaData(ns_env, TRUE, ns_env) @@ -577,8 +699,8 @@ load_fast_register_reload <- function(path = ".", files, reason = NULL) { } allGenerics <- unique(c( - methods:::.getGenerics(ns_env), - methods:::.getGenerics(parent.env(ns_env)) + .loadfast.s4_generic_names(ns_env), + .loadfast.s4_generic_names(parent.env(ns_env)) )) expMethods <- nsInfo$exportMethods addGenerics <- expMethods[is.na(match(expMethods, exports))] @@ -593,10 +715,10 @@ load_fast_register_reload <- function(path = ".", files, reason = NULL) { expTables <- character() if (length(allGenerics)) { expMethods <- unique(c(expMethods, exports[!is.na(match(exports, allGenerics))])) - tPrefix <- methods:::.TableMetaPrefix() + tPrefix <- ".__T__" allMethodTables <- unique(c( - methods:::.getGenerics(ns_env, tPrefix), - methods:::.getGenerics(parent.env(ns_env), tPrefix) + .loadfast.s4_method_tables(ns_env), + .loadfast.s4_method_tables(parent.env(ns_env)) )) needMethods <- (exports %in% allGenerics) & !(exports %in% expMethods) if (any(needMethods)) expMethods <- c(expMethods, exports[needMethods]) @@ -634,9 +756,30 @@ load_fast_register_reload <- function(path = ".", files, reason = NULL) { unique(exports) } +# Inlined equivalents of the small methods-internal helpers used by +# base::loadNamespace() for export computation (.hasS4MetaData, .getGenerics, +# .TableMetaPrefix), so the package does not reach into methods:::. They rely +# on the stable `.__C__` / `.__T__` / `.__A__` S4 metadata naming convention +# that setClass()/setGeneric()/setMethod() write into the namespace. +.loadfast.has_s4_metadata <- function(env) { + nms <- names(env) + any(startsWith(nms, ".__C__")) || + any(startsWith(nms, ".__T__")) || + any(startsWith(nms, ".__A__")) +} + +.loadfast.s4_method_tables <- function(env) { + these <- ls(env, all.names = TRUE) + these[startsWith(these, ".__T__")] +} + +.loadfast.s4_generic_names <- function(env) { + gsub("^\\.__T__(.*):([^:]+)", "\\1", .loadfast.s4_method_tables(env)) +} + # Register S3 methods declared via S3method() in NAMESPACE, mirroring the # registerS3methods() call inside base::loadNamespace(). Without this, S3 -# dispatch fails for any method not resolvable in the calling frame — notably +# dispatch fails for any method not resolvable in the calling frame -- notably # methods on generics defined in other namespaces (e.g. base's `print`) and # methods declared with S3method() but not exported by name. .loadfast.register_s3 <- function(s3_methods, pkg_name, ns_env, reset = FALSE) { @@ -654,6 +797,91 @@ load_fast_register_reload <- function(path = ".", files, reason = NULL) { invisible(NULL) } +# Detach and unload a package by name. `unloadNamespace()` refuses to unload a +# namespace that other loaded namespaces import, which is the *normal* case in +# multi-package sessions; the fallback then runs `.onUnload` best-effort and +# force-removes the namespace from R's registry so a fresh one can take over. +# Importers keep references into the old (now unregistered) namespace until +# they are reloaded themselves; see .loadfast.invalidate_dependents(). +.loadfast.teardown <- function(pkg_name, abs_path) { + pkg_env_name <- paste0("package:", pkg_name) + if (pkg_env_name %in% search()) { + detach(pkg_env_name, character.only = TRUE, unload = FALSE, force = TRUE) + } + if (pkg_name %in% loadedNamespaces()) { + old_ns <- tryCatch(asNamespace(pkg_name), error = function(e) NULL) + tryCatch(unloadNamespace(pkg_name), error = function(e) { + if (!is.null(old_ns) && exists(".onUnload", envir = old_ns, inherits = FALSE)) { + tryCatch( + get(".onUnload", envir = old_ns, inherits = FALSE)(dirname(abs_path)), + error = function(e2) { + warning( + "Error in .onUnload() for '", pkg_name, "': ", conditionMessage(e2), + call. = FALSE + ) + } + ) + } + reg <- rlang::ns_registry_env() + if (exists(pkg_name, envir = reg, inherits = FALSE)) { + rm(list = pkg_name, envir = reg) + } + }) + } + invisible(NULL) +} + +# Depends packages must be attached to the search path (not just loaded): +# package code resolves them through the environment chain below the base +# namespace, exactly as for an installed package loaded via library(). +.loadfast.parse_depends <- function(desc_fields) { + if (!("Depends" %in% colnames(desc_fields))) return(character(0)) + raw <- desc_fields[1L, "Depends"] + if (is.na(raw) || !nzchar(trimws(raw))) return(character(0)) + entries <- strsplit(raw, ",", fixed = TRUE)[[1L]] + entries <- gsub("\\([^)]*\\)", "", entries) + entries <- trimws(gsub("[[:space:]]+", " ", entries)) + entries <- entries[nzchar(entries)] + setdiff(entries, "R") +} + +# After a package is re-sourced (fully or incrementally), every cached package +# that imports from it -- directly or through other cached packages -- holds +# stale references: namespaceImportFrom() copies bindings at import time, so +# importers do not see re-sourced definitions. Flag them so their next +# load_fast() call is upgraded to a full reload. +.loadfast.invalidate_dependents <- function(pkg_name, skip_path) { + keys <- ls(.loadfast.cache, all.names = TRUE) + flagged <- pkg_name + repeat { + changed <- FALSE + for (key in keys) { + if (identical(key, skip_path)) next + entry <- .loadfast.cache[[key]] + if (is.null(entry$pkg_name) || entry$pkg_name %in% flagged) next + if (!any(entry$import_pkgs %in% flagged)) next + flagged <- c(flagged, entry$pkg_name) + if (is.null(entry$needs_full)) { + entry$needs_full <- paste0("dependency '", pkg_name, "' was reloaded") + .loadfast.cache[[key]] <- entry + } + changed <- TRUE + } + if (!changed) break + } + invisible(NULL) +} + +.loadfast.attach_testthat <- function(attach_testthat, pkg_name) { + if (isTRUE(attach_testthat) && + pkg_name != "testthat" && + !("package:testthat" %in% search()) && + requireNamespace("testthat", quietly = TRUE)) { + attachNamespace(loadNamespace("testthat")) + } + invisible(NULL) +} + .loadfast.run_hook <- function(hook, ns_env, abs_path, pkg_name) { if (!exists(hook, envir = ns_env, inherits = FALSE)) return(invisible(NULL)) tryCatch( @@ -701,20 +929,18 @@ load_fast_register_reload <- function(path = ".", files, reason = NULL) { ) } +.loadfast.uses_testthat <- function(abs_path) { + test_dirs <- c( + file.path(abs_path, "inst", "tests"), + file.path(abs_path, "tests", "testthat") + ) + any(dir.exists(test_dirs)) && requireNamespace("testthat", quietly = TRUE) +} + .loadfast.source_helpers <- function(abs_path, pkg_env, helpers, attach_testthat, pkg_name) { - uses_testthat <- local({ - test_dirs <- c( - file.path(abs_path, "inst", "tests"), - file.path(abs_path, "tests", "testthat") - ) - any(dir.exists(test_dirs)) && requireNamespace("testthat", quietly = TRUE) - }) + uses_testthat <- .loadfast.uses_testthat(abs_path) if (is.null(attach_testthat)) attach_testthat <- uses_testthat - if (isTRUE(attach_testthat) && pkg_name != "testthat") { - if (!paste0("package:testthat") %in% search()) { - library("testthat", warn.conflicts = FALSE) - } - } + .loadfast.attach_testthat(attach_testthat, pkg_name) if (isTRUE(helpers) && uses_testthat) { .loadfast.do_source_helpers(abs_path, pkg_env) } diff --git a/README.md b/README.md index c04ce1b..96b49ae 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,10 @@ # loadfast > [!NOTE] -> The majority of this codebase was generated by AI. While it lacks the manual scrutiny of hand-crafted code, we still rely on it daily. +> The majority of this codebase was written with AI assistance. It is covered by +> an extensive behavioral test suite (`test_loadfast.R`, plus a `testthat` suite +> that runs under `R CMD check`) and a multi-platform CI matrix; we rely on it +> daily. `loadfast` is an R package for interactive development on large R packages. It is intended as a drop-in replacement for `devtools::load_all()` when reload time is a bottleneck. @@ -31,9 +34,9 @@ loadfast::load_fast() loadfast::load_fast("path/to/your/package") ``` -`load_fast()` reads the package name from `DESCRIPTION`, builds a namespace, processes `NAMESPACE` imports, sources `R/` files, attaches the package to the search path, and optionally sources testthat helpers. +`load_fast()` reads the package name from `DESCRIPTION`, attaches `Depends` packages, builds a namespace, processes `NAMESPACE` imports, sources `R/` files (respecting the `Collate` field), attaches the package to the search path, and optionally sources testthat helpers. -It supports packages that rely on standard R namespace behavior, including imports, S3 methods (registered via `S3method()`), S4 classes, and R6 classes. Exports declared with `export()`, `exportPattern()`, `exportClasses()`, and `exportMethods()` are all honored, so a `load_fast()`-loaded package can be depended on by another package in the same session. +It supports packages that rely on standard R namespace behavior, including imports, S3 methods (registered via `S3method()`), S4 classes, and R6 classes. Exports declared with `export()`, `exportPattern()`, `exportClasses()`, and `exportMethods()` are all honored, so a `load_fast()`-loaded package can be depended on by another package in the same session. `.onLoad`, `.onAttach`, and `.onUnload` hooks run at the same points as they do for `library()` / `load_all()`. ### Options @@ -65,18 +68,34 @@ Use `full = TRUE` after deleting files, removing functions, or whenever you need ## Working across multiple packages -`load_fast()` can load several interdependent packages in the same session: load -the dependency first, then the packages that import from it. Because each -namespace is built with the same metadata a real installed package has -(including the `lazydata` field, S3 method tables, and full export +`load_fast()` can load several interdependent packages from different paths in +the same session: load the dependency first, then the packages that import from +it. Because each namespace is built with the same metadata a real installed +package has (including the `lazydata` field, S3 method tables, and full export processing), cross-package access works — whether through `importFrom()`, `import()`, `pkg::name`, S3 dispatch, or `importClassesFrom()` / `importMethodsFrom()`. As with `devtools::load_all()`, a package imports its dependencies' symbols as a -snapshot taken at its own load time. After editing a dependency, reload the -dependency **and** the packages that import from it so the importers pick up the -change. +snapshot taken at its own load time. `load_fast()` tracks these relationships: + +- When you reload a package, every loaded package that imports from it + (directly or transitively) is **flagged**. Their next `load_fast()` call is + automatically upgraded to a full reload — a plain `load_fast()` on an importer + never silently reports "No changes" while it is running against a stale + dependency snapshot. +- A full reload also reloads flagged dependencies first, in dependency order. + In a chain `C -> B -> A`, editing and reloading `A` followed by a single + `load_fast()` of `C` rebuilds `B` and `C` against the fresh namespaces. + +So the workflow is simply: reload the package you edited, then reload whatever +package you are working in — everything in between converges automatically. + +Reloading a dependency that other loaded packages import cannot use +`unloadNamespace()` (R refuses to unload an imported namespace); `load_fast()` +then runs the old namespace's `.onUnload` hook and force-replaces it in the +namespace registry. This is tested, including S4 class redefinition while the +class is imported elsewhere. > Earlier versions failed here with `Error: object 'lazydata' not found` when a > loaded package was accessed with `::`. That is fixed. @@ -122,9 +141,17 @@ You can bind `loadfast::load_fast()` in the Zed keymap: ## Testing -This repository uses a custom test harness. +Testing happens at two levels: -Run the full test suite with: +1. **Behavioral harness** (`test_loadfast.R`): ~300 checks across seven stages + covering full loads, incremental reloads, cross-file and cross-package + dependencies, S3/S4/R6 fidelity, multi-package dependency invalidation, and + production hardening (failed-load recovery, rename-in-place, `Depends` + attachment, `Collate` ordering, re-entrance). +2. **`testthat` suite** (`tests/testthat/`): self-contained smoke and error-path + tests that run against the *installed* package during `R CMD check`. + +Run the full harness with: ```sh Rscript test_loadfast.R @@ -137,3 +164,9 @@ Example: ```sh LOADFAST_TEST_FILTER="inter-pkg|dep-order" Rscript test_loadfast.R ``` + +CI runs the harness on Linux, Windows, and macOS, plus `R CMD check --as-cran` +on all three platforms (and one older R release). The check is clean apart from +two expected NOTEs: the `attach()` call (inherent to what a loader does — the +same NOTE `pkgload` carries) and, on some runners, an environment-specific +timestamp note. diff --git a/TECHNICAL_DEBT.md b/TECHNICAL_DEBT.md index 5c338e5..efb03c5 100644 --- a/TECHNICAL_DEBT.md +++ b/TECHNICAL_DEBT.md @@ -4,84 +4,41 @@ This document tracks known implementation debt and conscious tradeoffs in the `l ## Current status -- The package implementation under `R/` passes the current repo test suite. -- The loader design is broadly sound for the target use case. -- Most debt is in edge-case correctness and maintainability rather than basic functionality. -- Namespace fidelity has been brought in line with `loadNamespace()` for the - areas that affect real packages and multi-package sessions: S3 method - registration (`S3method()`), full export processing (`export()`, - `exportPattern()`, `exportClasses()`, `exportMethods()`), the `lazydata` / - `nativeRoutines` namespace-info fields, and the `.onAttach` hook. See the - namespace-machinery notes in `AGENTS.md`. - -## Medium-priority debt - -### 1. Incremental cache validity is inferred too loosely -**Why this matters** - -Incremental reload currently checks whether: -- a cache entry exists for the normalized path -- the package name is in `loadedNamespaces()` -- the attached package env is on the search path - -That does not fully prove that the cached namespace env is still the active registered namespace for the package. - -**Risk** -- Reusing stale env references after manual unloads or namespace manipulation -- Hard-to-debug edge cases in interactive sessions - -**Preferred fix** -- Store `pkg_name` in the cache entry -- Verify the cached namespace env matches the currently registered namespace before taking the incremental path - -**Priority** -- Medium - -### 2. Cache is keyed only by normalized path -**Why this matters** - -If the `Package:` field in `DESCRIPTION` changes in place for the same directory, the path-based cache may no longer describe the same logical package. - -**Risk** -- Mismatch between cached namespace state and current package identity - -**Preferred fix** -- Store `pkg_name` alongside the cache entry -- Force a full reload when the cached package name differs from the current `DESCRIPTION` - -**Priority** -- Medium - -## Low-priority debt - -### 3. Testthat detection logic is duplicated -**Why this matters** - -Logic for detecting testthat usage and helper sourcing is repeated in multiple places. The runtime cost is trivial, but the duplication increases maintenance cost. - -**Risk** -- Small drift between code paths -- Unnecessary repetition in package source that now lives in one canonical implementation file - -**Preferred fix** -- Extract a small helper for detecting whether testthat helpers should be considered available - -**Priority** -- Low - -### 4. Package env sync logic is duplicated conceptually -**Why this matters** - -The full-load and incremental-load paths both bulk-copy namespace and imports into the attached package env. The duplication is reasonable, but it is a maintenance seam. - -**Risk** -- Future edits may update one path but not the other - -**Preferred fix** -- Optionally extract a small helper for package env synchronization - -**Priority** -- Low +- The package implementation under `R/` passes the repo harness (`test_loadfast.R`, + ~300 checks) and the `testthat` suite that runs under `R CMD check`. +- `R CMD check --as-cran` is clean apart from two expected NOTEs: the `attach()` + call (inherent to a loader that manages the search path) and an + environment-specific timestamp note on some machines. +- Namespace fidelity matches `loadNamespace()` for the areas that affect real + packages and multi-package sessions: S3 method registration (`S3method()`), + full export processing (`export()`, `exportPattern()`, `exportClasses()`, + `exportMethods()`), the `lazydata` / `nativeRoutines` namespace-info fields, + the namespace version from `DESCRIPTION`, `Depends` attachment, and the + `.onLoad` / `.onAttach` / `.onUnload` hooks. See the namespace-machinery notes + in `AGENTS.md`. + +## Resolved (previously tracked as debt) + +### Incremental cache validity (was medium priority) +The incremental path verifies that the cached namespace env is identical to the +currently registered namespace (`asNamespace(pkg)`) and that the package env is +on the search path. Stale env references after manual unloads fall back to a +full load. + +### Cache keyed only by normalized path (was medium priority) +Cache entries store `pkg_name`. When the `Package:` field changes in place for +the same directory, the loader detaches and unloads the old identity, drops the +stale cache entry, and does a full load under the new name (tested in stage 7b). + +### Testthat detection duplication (was low priority) +Extracted into `.loadfast.uses_testthat()` / `.loadfast.attach_testthat()`. + +## Remaining low-priority debt + +### Package env sync logic is duplicated conceptually +The full-load and incremental-load paths both bulk-copy namespace and imports +into the attached package env. The duplication is reasonable, but it is a +maintenance seam. Optionally extract a small helper if the file grows further. ## Conscious tradeoffs, not bugs @@ -116,7 +73,9 @@ not worth the cost for the edit-reload loop. - Use `full = TRUE` after editing `NAMESPACE` (new exports, S3 methods, imports) ### 4. `Collate` support is intentionally narrow -The loader now respects the `Collate` field from `DESCRIPTION` when ordering `R/*.R` files, and this behavior is covered by the test suite. +The loader respects the `Collate` field from `DESCRIPTION` when ordering +`R/*.R` files, on both full loads and incremental re-sourcing, and this +behavior is covered by the test suite (stages 3d and 7e). This is still intentionally lightweight rather than a full reproduction of every package-loading edge case. Future changes should preserve the current `Collate` behavior without overcomplicating the package implementation. @@ -124,11 +83,30 @@ This is still intentionally lightweight rather than a full reproduction of every - Treat `Collate` ordering for `R/*.R` as supported behavior - Be cautious about expanding this area unless a concrete incompatibility appears -## Suggested implementation order +### 5. Dependency invalidation forces a full reload of importers +When a package is re-sourced (fully or incrementally), every cached package +that imports from it — directly or transitively — is flagged, and its next +`load_fast()` call is upgraded to a full reload (which in turn reloads flagged +dependencies first, in dependency order). This matches `load_all()`'s cost +model: importers always re-process imports against fresh namespaces. + +A cheaper in-place refresh of importer imports envs was considered and +rejected for now: it would have to reproduce `namespaceImportFrom()` / +`importMethodsFrom()` merge semantics against live namespaces, which is exactly +the class of subtle S4/S3 state this package tries not to reimplement. -1. Harden incremental cache validation -2. Store `pkg_name` in cache entries and invalidate on package identity changes -3. Extract tiny helpers for testthat detection and package env sync if the file grows further +**Current rule** +- Correctness first: flagged importers do a full reload; the within-package + incremental path remains the performance win + +### 6. S4 metadata helpers are inlined, not imported from methods::: +`.loadfast.has_s4_metadata()`, `.loadfast.s4_method_tables()`, and +`.loadfast.s4_generic_names()` replicate tiny `methods:::` internals +(`.hasS4MetaData`, `.getGenerics`, `.TableMetaPrefix`) using the stable +`.__C__` / `.__T__` / `.__A__` metadata naming convention. This keeps +`R CMD check --as-cran` free of the `:::` warning. If a future R version +changes these conventions (unlikely; they are decades old), stages 5d/6e of the +harness will catch it. ## Notes for future reviewers diff --git a/justfile b/justfile index e5634a1..08e47ad 100644 --- a/justfile +++ b/justfile @@ -1,9 +1,19 @@ default: @just --list +# run the behavioral test harness test: Rscript test_loadfast.R +# regenerate NAMESPACE and man/ from roxygen comments +document: + Rscript -e 'roxygen2::roxygenize(".")' + +# build the package tarball and run R CMD check on it +check: + R CMD build . + R CMD check --no-manual --as-cran loadfast_*.tar.gz + # clone reference packages setup: git clone https://github.com/r-lib/pkgload diff --git a/man/load_fast.Rd b/man/load_fast.Rd new file mode 100644 index 0000000..292d6f1 --- /dev/null +++ b/man/load_fast.Rd @@ -0,0 +1,60 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/loadfast.R +\name{load_fast} +\alias{load_fast} +\title{Load a package from source with MD5-based incremental reloading} +\usage{ +load_fast( + path = ".", + helpers = TRUE, + attach_testthat = NULL, + full = FALSE, + verbose = FALSE +) +} +\arguments{ +\item{path}{Path to a package root containing \code{DESCRIPTION}, \code{NAMESPACE}, +and \verb{R/}. If \code{path} points inside a package, \code{load_fast()} walks upward to +the package root.} + +\item{helpers}{If \code{TRUE}, source \code{tests/testthat/helper*.R} when testthat is +available.} + +\item{attach_testthat}{If \code{NULL}, auto-detect whether \code{testthat} should be +attached. If \code{TRUE}, attach \code{testthat}.} + +\item{full}{If \code{TRUE}, force a complete teardown and rebuild.} + +\item{verbose}{If \code{TRUE}, emit per-phase timing logs.} +} +\value{ +Invisibly returns the namespace environment (or \code{NULL} for a +package without \verb{R/} files). +} +\description{ +\code{load_fast()} is a lightweight alternative to \code{devtools::load_all()}. +On the first call for a package path it performs a full teardown and rebuild. +On subsequent calls for that same path it re-sources only \verb{R/} files whose +MD5 hashes changed. +} +\details{ +Several interdependent source packages can be loaded in the same session: +load the dependency first, then the packages that import from it. When a +package is reloaded, every loaded package that (directly or transitively) +imports from it is flagged, and its next \code{load_fast()} call is +automatically upgraded to a full reload so importers never keep running +against a stale snapshot. A full reload also reloads flagged dependencies +first, in dependency order. + +Packages listed in the \code{Depends} field of \code{DESCRIPTION} are attached to +the search path during a full load, mirroring \code{library()} semantics. +} +\examples{ +\dontrun{ +# Load the package containing the current working directory +load_fast() + +# Load a specific package and force a clean rebuild +load_fast("path/to/pkg", full = TRUE) +} +} diff --git a/man/load_fast_register_reload.Rd b/man/load_fast_register_reload.Rd new file mode 100644 index 0000000..e34228c --- /dev/null +++ b/man/load_fast_register_reload.Rd @@ -0,0 +1,22 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/loadfast.R +\name{load_fast_register_reload} +\alias{load_fast_register_reload} +\title{Register one or more files for reload on the next \code{load_fast()} call} +\usage{ +load_fast_register_reload(path = ".", files, reason = NULL) +} +\arguments{ +\item{path}{Package root path.} + +\item{files}{File paths to reload on the next call.} + +\item{reason}{Optional human-readable reason shown in messages.} +} +\value{ +Invisibly returns \code{TRUE} when a reload was registered and \code{FALSE} +when there is no active cache for the package path. +} +\description{ +Register one or more files for reload on the next \code{load_fast()} call +} diff --git a/renv/settings.json b/renv/settings.json index 13007ee..3d3ed6d 100644 --- a/renv/settings.json +++ b/renv/settings.json @@ -2,7 +2,7 @@ "bioconductor.version": null, "external.libraries": [], "ignored.packages": [], - "package.dependency.fields": ["Imports", "DevDeps", "Depends", "LinkingTo"], + "package.dependency.fields": ["Imports", "Suggests", "Depends", "LinkingTo"], "ppm.enabled": null, "ppm.ignored.urls": [], "r.version": null, diff --git a/test_loadfast.R b/test_loadfast.R index 025f310..26c8b9a 100644 --- a/test_loadfast.R +++ b/test_loadfast.R @@ -2414,6 +2414,480 @@ check("xpkg-s4: consumer dispatches an imported primitive-generic S4 method", qu get("count_teeth", envir = s4cons_load)(c(3, 5, 7)) == 3 )) +# ============================================================================ +# STAGE 6: Multi-package dependency invalidation and convergence +# When a package is reloaded, cached packages that import from it (directly +# or transitively) must be flagged and their next load upgraded to a full +# reload; a full reload must also reload flagged dependencies first. +# ============================================================================ +cat("\n--- Stage 6: multi-package dependency invalidation ---\n\n") + +# --- 6a: direct importer is auto-upgraded to a full reload --- +cat("\n--- 6a: direct importer invalidation ---\n\n") + +tmp_inv_a <- tempfile("loadfast_inv_a_") +tmp_inv_b <- tempfile("loadfast_inv_b_") +write_pkg( + tmp_inv_a, "invdep", + desc_extra = character(0), + ns_lines = "export(inv_value)", + r_files = list("a.R" = 'inv_value <- function() "one"') +) +write_pkg( + tmp_inv_b, "invuser", + desc_extra = character(0), + ns_lines = c("importFrom(invdep, inv_value)", "export(inv_wrap)"), + r_files = list("b.R" = 'inv_wrap <- function() paste0("wrapped:", inv_value())') +) +invisible(load_fast(tmp_inv_a, helpers = FALSE, attach_testthat = FALSE)) +ns_inv_b <- load_fast(tmp_inv_b, helpers = FALSE, attach_testthat = FALSE) + +check("inv-direct: importer works before dependency change", quote( + get("inv_wrap", envir = ns_inv_b)() == "wrapped:one" +)) + +writeLines('inv_value <- function() "two"', file.path(tmp_inv_a, "R", "a.R")) +invisible(load_fast(tmp_inv_a, helpers = FALSE, attach_testthat = FALSE)) + +check("inv-direct: dependency itself is fresh after incremental reload", quote( + getExportedValue("invdep", "inv_value")() == "two" +)) + +check("inv-direct: importer is stale until it reloads (snapshot semantics)", quote( + get("inv_wrap", envir = ns_inv_b)() == "wrapped:one" +)) + +inv_b_reload <- capture_messages( + load_fast(tmp_inv_b, helpers = FALSE, attach_testthat = FALSE) +) + +check("inv-direct: importer load is auto-upgraded to a full reload", quote( + any(grepl("Full reload of 'invuser': dependency 'invdep' was reloaded", inv_b_reload$messages, fixed = TRUE)) +)) + +check("inv-direct: importer sees the new dependency value", quote( + get("inv_wrap", envir = inv_b_reload$value)() == "wrapped:two" +)) + +inv_b_again <- capture_messages( + load_fast(tmp_inv_b, helpers = FALSE, attach_testthat = FALSE) +) + +check("inv-direct: importer short-circuits again after the forced full reload", quote( + any(grepl("No changes", inv_b_again$messages, fixed = TRUE)) +)) + +# --- 6b: transitive chain converges with a single load of the top package --- +cat("\n--- 6b: transitive chain convergence ---\n\n") + +tmp_inv_c <- tempfile("loadfast_inv_c_") +write_pkg( + tmp_inv_c, "invtop", + desc_extra = character(0), + ns_lines = c("importFrom(invuser, inv_wrap)", "export(inv_top)"), + r_files = list("c.R" = 'inv_top <- function() paste0("top:", inv_wrap())') +) +ns_inv_c <- load_fast(tmp_inv_c, helpers = FALSE, attach_testthat = FALSE) + +check("inv-chain: top of the chain works initially", quote( + get("inv_top", envir = ns_inv_c)() == "top:wrapped:two" +)) + +writeLines('inv_value <- function() "three"', file.path(tmp_inv_a, "R", "a.R")) +invisible(load_fast(tmp_inv_a, helpers = FALSE, attach_testthat = FALSE)) + +inv_c_reload <- capture_messages( + load_fast(tmp_inv_c, helpers = FALSE, attach_testthat = FALSE) +) + +check("inv-chain: transitive importer is flagged too", quote( + any(grepl("Full reload of 'invtop'", inv_c_reload$messages)) +)) + +check("inv-chain: flagged middle dependency is reloaded first", quote( + any(grepl("Reloading dependency 'invuser' first", inv_c_reload$messages)) +)) + +check("inv-chain: one load of the top package converges the whole chain", quote( + get("inv_top", envir = inv_c_reload$value)() == "top:wrapped:three" +)) + +check("inv-chain: middle package is fresh as well", quote( + getExportedValue("invuser", "inv_wrap")() == "wrapped:three" +)) + +# --- 6c: no-change loads do not invalidate dependents --- +cat("\n--- 6c: no-change loads do not invalidate ---\n\n") + +invisible(load_fast(tmp_inv_a, helpers = FALSE, attach_testthat = FALSE)) +inv_c_nochange <- capture_messages( + load_fast(tmp_inv_c, helpers = FALSE, attach_testthat = FALSE) +) + +check("inv-nochange: importer short-circuits after a no-change dependency load", quote( + any(grepl("No changes", inv_c_nochange$messages, fixed = TRUE)) +)) + +# --- 6d: whole-namespace import() is invalidated too --- +cat("\n--- 6d: whole-namespace import() invalidation ---\n\n") + +tmp_wns_a <- tempfile("loadfast_wns_a_") +tmp_wns_b <- tempfile("loadfast_wns_b_") +write_pkg( + tmp_wns_a, "wnsdep", + desc_extra = character(0), + ns_lines = "export(wns_value)", + r_files = list("a.R" = 'wns_value <- function() 1') +) +write_pkg( + tmp_wns_b, "wnsuser", + desc_extra = character(0), + ns_lines = c("import(wnsdep)", "export(wns_wrap)"), + r_files = list("b.R" = 'wns_wrap <- function() wns_value() + 100') +) +invisible(load_fast(tmp_wns_a, helpers = FALSE, attach_testthat = FALSE)) +ns_wns_b <- load_fast(tmp_wns_b, helpers = FALSE, attach_testthat = FALSE) + +check("inv-import: whole-namespace importer works initially", quote( + get("wns_wrap", envir = ns_wns_b)() == 101 +)) + +writeLines('wns_value <- function() 2', file.path(tmp_wns_a, "R", "a.R")) +invisible(load_fast(tmp_wns_a, helpers = FALSE, attach_testthat = FALSE)) +ns_wns_b2 <- suppressMessages(load_fast(tmp_wns_b, helpers = FALSE, attach_testthat = FALSE)) + +check("inv-import: whole-namespace importer picks up the change", quote( + get("wns_wrap", envir = ns_wns_b2)() == 102 +)) + +# --- 6e: S4 class redefinition in a dependency propagates to importers --- +cat("\n--- 6e: S4 class redefinition across packages ---\n\n") + +tmp_s4x_a <- tempfile("loadfast_s4x_a_") +tmp_s4x_b <- tempfile("loadfast_s4x_b_") +write_pkg( + tmp_s4x_a, "s4xdep", + desc_extra = character(0), + ns_lines = c("import(methods)", "exportClasses(Rotor)", "export(make_rotor)"), + r_files = list("a.R" = c( + 'setClass("Rotor", representation(blades = "numeric"))', + 'make_rotor <- function(n) new("Rotor", blades = n)' + )) +) +write_pkg( + tmp_s4x_b, "s4xuser", + desc_extra = character(0), + ns_lines = c( + "import(methods)", "importClassesFrom(s4xdep, Rotor)", + "importFrom(s4xdep, make_rotor)", "export(rotor_slots)" + ), + r_files = list("b.R" = 'rotor_slots <- function() methods::slotNames(class(make_rotor(2)))') +) +invisible(load_fast(tmp_s4x_a, helpers = FALSE, attach_testthat = FALSE)) +ns_s4x_b <- load_fast(tmp_s4x_b, helpers = FALSE, attach_testthat = FALSE) + +check("inv-s4: importer sees the original slot set", quote( + identical(get("rotor_slots", envir = ns_s4x_b)(), "blades") +)) + +writeLines(c( + 'setClass("Rotor", representation(blades = "numeric", pitch = "numeric"),', + ' prototype = list(pitch = 0))', + 'make_rotor <- function(n) new("Rotor", blades = n)' +), file.path(tmp_s4x_a, "R", "a.R")) +invisible(load_fast(tmp_s4x_a, helpers = FALSE, attach_testthat = FALSE)) +ns_s4x_b2 <- suppressMessages(load_fast(tmp_s4x_b, helpers = FALSE, attach_testthat = FALSE)) + +check("inv-s4: importer sees the redefined class with the new slot", quote( + setequal(get("rotor_slots", envir = ns_s4x_b2)(), c("blades", "pitch")) +)) + +# --- 6f: full reload of a dependency that cannot be unloadNamespace()d --- +cat("\n--- 6f: forced unregister path while imported ---\n\n") + +tmp_fu_a <- tempfile("loadfast_fu_a_") +tmp_fu_b <- tempfile("loadfast_fu_b_") +write_pkg( + tmp_fu_a, "fudep", + desc_extra = character(0), + ns_lines = "export(fu_value)", + r_files = list("a.R" = c( + 'fu_value <- function() "before"', + '.onUnload <- function(libpath) assign(".fu_unload_ran", TRUE, envir = globalenv())' + )) +) +write_pkg( + tmp_fu_b, "fuuser", + desc_extra = character(0), + ns_lines = c("importFrom(fudep, fu_value)", "export(fu_wrap)"), + r_files = list("b.R" = 'fu_wrap <- function() fu_value()') +) +invisible(load_fast(tmp_fu_a, helpers = FALSE, attach_testthat = FALSE)) +invisible(load_fast(tmp_fu_b, helpers = FALSE, attach_testthat = FALSE)) + +writeLines(c( + 'fu_value <- function() "after"', + '.onUnload <- function(libpath) assign(".fu_unload_ran", TRUE, envir = globalenv())' +), file.path(tmp_fu_a, "R", "a.R")) + +fu_reload <- capture_warnings( + load_fast(tmp_fu_a, helpers = FALSE, attach_testthat = FALSE, full = TRUE) +) + +check("forced-unregister: full reload of an imported dependency succeeds", quote( + is.environment(fu_reload$value) && isNamespace(fu_reload$value) +)) + +check("forced-unregister: .onUnload ran during the forced teardown", quote( + isTRUE(get0(".fu_unload_ran", envir = globalenv())) +)) + +check("forced-unregister: :: resolves the fresh namespace", quote( + getExportedValue("fudep", "fu_value")() == "after" +)) + +fu_user_reload <- suppressMessages(load_fast(tmp_fu_b, helpers = FALSE, attach_testthat = FALSE)) + +check("forced-unregister: importer converges after its flagged reload", quote( + get("fu_wrap", envir = fu_user_reload)() == "after" +)) + +# ============================================================================ +# STAGE 7: Production hardening +# Depends attachment, rename-in-place cleanup, failed-load cleanup, +# namespace version fidelity, Collate-aware incremental ordering, +# re-entrance guard, and verbose logging. +# ============================================================================ +cat("\n--- Stage 7: production hardening ---\n\n") + +# --- 7a: Depends packages are attached during a full load --- +cat("\n--- 7a: Depends attachment ---\n\n") + +if ("package:tools" %in% search()) detach("package:tools") +tmp_dep <- tempfile("loadfast_depends_") +write_pkg( + tmp_dep, "dependspkg", + desc_extra = c("Depends:", " R (>= 4.0),", " tools"), + ns_lines = "export(depends_md5)", + r_files = list("a.R" = 'depends_md5 <- function(f) md5sum(f)') +) +ns_dep7 <- load_fast(tmp_dep, helpers = FALSE, attach_testthat = FALSE) + +check("depends: package in Depends is attached to the search path", quote( + "package:tools" %in% search() +)) + +check("depends: package code can call Depends symbols unqualified", quote( + !is.na(get("depends_md5", envir = ns_dep7)(file.path(tmp_dep, "DESCRIPTION"))) +)) + +tmp_dep_missing <- tempfile("loadfast_depends_missing_") +write_pkg( + tmp_dep_missing, "dependsmissing", + desc_extra = "Depends: surelynotarealinstalledpackage", + ns_lines = "export(dm)", + r_files = list("a.R" = 'dm <- function() 1') +) +depends_missing_err <- tryCatch( + load_fast(tmp_dep_missing, helpers = FALSE, attach_testthat = FALSE), + error = function(e) e +) + +check("depends: missing Depends package fails with a clear error", quote( + inherits(depends_missing_err, "error") && + grepl("Depends package 'surelynotarealinstalledpackage'", conditionMessage(depends_missing_err), fixed = TRUE) +)) + +# --- 7b: renaming the package in DESCRIPTION cleans up the old identity --- +cat("\n--- 7b: rename-in-place cleanup ---\n\n") + +tmp_ren <- tempfile("loadfast_rename_") +write_pkg( + tmp_ren, "renameold", + desc_extra = character(0), + ns_lines = "export(ren_fn)", + r_files = list("a.R" = 'ren_fn <- function() 1') +) +invisible(load_fast(tmp_ren, helpers = FALSE, attach_testthat = FALSE)) + +check("rename: original name is loaded", quote( + "renameold" %in% loadedNamespaces() && "package:renameold" %in% search() +)) + +replace_description_field(file.path(tmp_ren, "DESCRIPTION"), "Package", "Package: renamenew") +ren_reload <- capture_messages( + load_fast(tmp_ren, helpers = FALSE, attach_testthat = FALSE) +) + +check("rename: reload reports the identity change", quote( + any(grepl("changed from 'renameold' to 'renamenew'", ren_reload$messages, fixed = TRUE)) +)) + +check("rename: new name is loaded and attached", quote( + "renamenew" %in% loadedNamespaces() && "package:renamenew" %in% search() +)) + +check("rename: old namespace is unloaded", quote( + !("renameold" %in% loadedNamespaces()) +)) + +check("rename: old package env is detached", quote( + !("package:renameold" %in% search()) +)) + +# --- 7c: a failed full load leaves no half-built namespace behind --- +cat("\n--- 7c: failed full load cleanup ---\n\n") + +tmp_failfull <- tempfile("loadfast_failfull_") +write_pkg( + tmp_failfull, "failfull", + desc_extra = character(0), + ns_lines = "export(ff_fn)", + r_files = list("a.R" = 'ff_fn <- function() "ok"') +) +invisible(load_fast(tmp_failfull, helpers = FALSE, attach_testthat = FALSE)) +writeLines('ff_fn <- function() "broken" (', file.path(tmp_failfull, "R", "a.R")) +failfull_err <- tryCatch( + load_fast(tmp_failfull, helpers = FALSE, attach_testthat = FALSE, full = TRUE), + error = function(e) e +) + +check("fail-clean: broken full load errors", quote( + inherits(failfull_err, "error") +)) + +check("fail-clean: no half-built namespace stays registered", quote( + !("failfull" %in% loadedNamespaces()) +)) + +check("fail-clean: no half-built package env stays attached", quote( + !("package:failfull" %in% search()) +)) + +writeLines('ff_fn <- function() "fixed"', file.path(tmp_failfull, "R", "a.R")) +ns_failfull <- load_fast(tmp_failfull, helpers = FALSE, attach_testthat = FALSE) + +check("fail-clean: recovery load works after fixing the file", quote( + get("ff_fn", envir = ns_failfull)() == "fixed" +)) + +# --- 7d: namespace version comes from DESCRIPTION --- +cat("\n--- 7d: namespace version fidelity ---\n\n") + +tmp_ver <- tempfile("loadfast_version_") +write_pkg( + tmp_ver, "versionpkg", + desc_extra = character(0), + ns_lines = "export(v_fn)", + r_files = list("a.R" = 'v_fn <- function() 1') +) +replace_description_field(file.path(tmp_ver, "DESCRIPTION"), "Version", "Version: 2.5.1") +invisible(load_fast(tmp_ver, helpers = FALSE, attach_testthat = FALSE)) + +check("version: getNamespaceVersion() reflects DESCRIPTION", quote( + as.character(getNamespaceVersion("versionpkg")) == "2.5.1" +)) + +# --- 7e: incremental re-sourcing respects Collate order --- +cat("\n--- 7e: Collate order on incremental reload ---\n\n") + +tmp_coll_incr <- tempfile("loadfast_collate_incr_") +write_pkg( + tmp_coll_incr, "collateincr", + desc_extra = c("Collate:", " 'zzz_first.R'", " 'aaa_second.R'"), + ns_lines = "export(collate_order)", + r_files = list( + "zzz_first.R" = c( + ".load_order <- c(get0('.load_order', ifnotfound = NULL), 'first')", + "collate_order <- function() .load_order" + ), + "aaa_second.R" = ".load_order <- c(get0('.load_order', ifnotfound = NULL), 'second')" + ) +) +ns_coll <- load_fast(tmp_coll_incr, helpers = FALSE, attach_testthat = FALSE) + +check("collate-incr: full load follows Collate order", quote( + identical(get("collate_order", envir = ns_coll)(), c("first", "second")) +)) + +# Touch both files so the incremental path re-sources them together; the +# re-source order must still follow Collate, not alphabetical basenames. +writeLines(c( + ".load_order <- c(get0('.load_order', ifnotfound = NULL), 'first')", + "collate_order <- function() .load_order", + "# touched" +), file.path(tmp_coll_incr, "R", "zzz_first.R")) +writeLines(c( + ".load_order <- c(get0('.load_order', ifnotfound = NULL), 'second')", + "# touched" +), file.path(tmp_coll_incr, "R", "aaa_second.R")) + +ns_coll2 <- load_fast(tmp_coll_incr, helpers = FALSE, attach_testthat = FALSE) +coll_order_after <- get("collate_order", envir = ns_coll2)() + +check("collate-incr: incremental re-source follows Collate order", quote( + identical(coll_order_after[c(length(coll_order_after) - 1L, length(coll_order_after))], c("first", "second")) +)) + +# --- 7f: re-entrance guard --- +cat("\n--- 7f: re-entrance guard ---\n\n") + +tmp_reent <- tempfile("loadfast_reentrance_") +write_pkg( + tmp_reent, "reentrance", + desc_extra = character(0), + ns_lines = "export(re_fn)", + r_files = list("a.R" = c( + "re_fn <- function() 1", + "load_fast('.')" + )) +) +reent_err <- tryCatch( + load_fast(tmp_reent, helpers = FALSE, attach_testthat = FALSE), + error = function(e) e +) + +check("re-entrance: sourced file calling load_fast() errors", quote( + inherits(reent_err, "error") +)) + +check("re-entrance: error message mentions re-entrance", quote( + grepl("re-entrance", conditionMessage(reent_err), fixed = TRUE) +)) + +check("re-entrance: loader state recovers for subsequent loads", quote({ + writeLines("re_fn <- function() 1", file.path(tmp_reent, "R", "a.R")) + ns_re <- load_fast(tmp_reent, helpers = FALSE, attach_testthat = FALSE) + get("re_fn", envir = ns_re)() == 1 +})) + +# --- 7g: verbose mode emits per-phase timings --- +cat("\n--- 7g: verbose timing logs ---\n\n") + +tmp_verb <- tempfile("loadfast_verbose_") +write_pkg( + tmp_verb, "verbosepkg", + desc_extra = character(0), + ns_lines = "export(vb_fn)", + r_files = list("a.R" = 'vb_fn <- function() 1') +) +verbose_load <- capture_messages( + load_fast(tmp_verb, helpers = FALSE, attach_testthat = FALSE, verbose = TRUE) +) + +check("verbose: full load emits [load_fast] phase timings", quote( + any(grepl("[load_fast]", verbose_load$messages, fixed = TRUE)) && + any(grepl("TOTAL (full load)", verbose_load$messages, fixed = TRUE)) +)) + +verbose_nochange <- capture_messages( + load_fast(tmp_verb, helpers = FALSE, attach_testthat = FALSE, verbose = TRUE) +) + +check("verbose: no-change load emits timing summary", quote( + any(grepl("TOTAL (no-change)", verbose_nochange$messages, fixed = TRUE)) +)) + # ============================================================================ # Summary # ============================================================================ diff --git a/tests/testthat.R b/tests/testthat.R new file mode 100644 index 0000000..1168284 --- /dev/null +++ b/tests/testthat.R @@ -0,0 +1,4 @@ +library(testthat) +library(loadfast) + +test_check("loadfast") diff --git a/tests/testthat/test-load_fast.R b/tests/testthat/test-load_fast.R new file mode 100644 index 0000000..cf1510b --- /dev/null +++ b/tests/testthat/test-load_fast.R @@ -0,0 +1,147 @@ +# Self-contained checks that run against the *installed* package during +# R CMD check. The exhaustive suite (multi-package fidelity, S3/S4/R6 fixtures, +# incremental edge cases) lives in test_loadfast.R at the repository root. + +write_mini_pkg <- function(name, fn_body = 'mini_fn <- function() "v1"', + ns_lines = "export(mini_fn)", desc_extra = character(0)) { + root <- file.path(tempfile("loadfast_tt_"), name) + dir.create(file.path(root, "R"), recursive = TRUE) + writeLines( + c(paste0("Package: ", name), "Title: t", "Version: 0.0.1", + "Description: d.", "License: MIT + file LICENSE", desc_extra), + file.path(root, "DESCRIPTION") + ) + writeLines(ns_lines, file.path(root, "NAMESPACE")) + writeLines(fn_body, file.path(root, "R", "fns.R")) + root +} + +cleanup_pkg <- function(name, root) { + pkg_env <- paste0("package:", name) + if (pkg_env %in% search()) { + detach(pkg_env, character.only = TRUE, unload = FALSE, force = TRUE) + } + if (name %in% loadedNamespaces()) { + try(unloadNamespace(name), silent = TRUE) + } + unlink(dirname(root), recursive = TRUE) +} + +test_that("load_fast() performs a full load and registers a working namespace", { + root <- write_mini_pkg("ttminia") + on.exit(cleanup_pkg("ttminia", root)) + + ns <- suppressMessages(load_fast(root, helpers = FALSE, attach_testthat = FALSE)) + + expect_true(isNamespace(ns)) + expect_true("ttminia" %in% loadedNamespaces()) + expect_true("package:ttminia" %in% search()) + expect_identical(get("mini_fn", envir = ns)(), "v1") + expect_identical(getExportedValue("ttminia", "mini_fn")(), "v1") +}) + +test_that("load_fast() reloads changed files incrementally", { + root <- write_mini_pkg("ttminib") + on.exit(cleanup_pkg("ttminib", root)) + + ns <- suppressMessages(load_fast(root, helpers = FALSE, attach_testthat = FALSE)) + expect_identical(get("mini_fn", envir = ns)(), "v1") + + writeLines('mini_fn <- function() "v2"', file.path(root, "R", "fns.R")) + expect_message( + ns2 <- load_fast(root, helpers = FALSE, attach_testthat = FALSE), + "Incremental reload" + ) + expect_identical(ns2, ns) + expect_identical(get("mini_fn", envir = ns2)(), "v2") + + expect_message( + load_fast(root, helpers = FALSE, attach_testthat = FALSE), + "No changes" + ) +}) + +test_that("importing packages are flagged for a full reload when a dependency reloads", { + root_a <- write_mini_pkg("ttdepa", fn_body = 'dep_fn <- function() "a1"', + ns_lines = "export(dep_fn)") + root_b <- write_mini_pkg("ttdepb", + fn_body = 'use_dep <- function() paste0("b:", dep_fn())', + ns_lines = c("importFrom(ttdepa, dep_fn)", "export(use_dep)")) + on.exit({ + cleanup_pkg("ttdepb", root_b) + cleanup_pkg("ttdepa", root_a) + }) + + suppressMessages(load_fast(root_a, helpers = FALSE, attach_testthat = FALSE)) + ns_b <- suppressMessages(load_fast(root_b, helpers = FALSE, attach_testthat = FALSE)) + expect_identical(get("use_dep", envir = ns_b)(), "b:a1") + + writeLines('dep_fn <- function() "a2"', file.path(root_a, "R", "fns.R")) + suppressMessages(load_fast(root_a, helpers = FALSE, attach_testthat = FALSE)) + + expect_message( + ns_b2 <- load_fast(root_b, helpers = FALSE, attach_testthat = FALSE), + "Full reload of 'ttdepb'" + ) + expect_identical(get("use_dep", envir = ns_b2)(), "b:a2") +}) + +test_that("load_fast() rejects paths outside a package", { + bare <- tempfile("loadfast_bare_") + dir.create(bare) + on.exit(unlink(bare, recursive = TRUE)) + + expect_error(load_fast(bare), "Could not find package root") + expect_error(load_fast(file.path(bare, "nope")), "No such file|cannot be found|does not exist") +}) + +test_that("load_fast() validates the DESCRIPTION Package field", { + root <- write_mini_pkg("ttminic") + on.exit(unlink(dirname(root), recursive = TRUE)) + writeLines(c("Title: t", "Version: 0.0.1"), file.path(root, "DESCRIPTION")) + + expect_error( + load_fast(root, helpers = FALSE, attach_testthat = FALSE), + "No valid 'Package' field" + ) +}) + +test_that("load_fast_register_reload() validates its inputs", { + expect_error(load_fast_register_reload(files = character(0)), "at least one path") + + root <- write_mini_pkg("ttminid") + on.exit(cleanup_pkg("ttminid", root)) + suppressMessages(load_fast(root, helpers = FALSE, attach_testthat = FALSE)) + + expect_error( + load_fast_register_reload(root, files = file.path(tempdir(), "elsewhere.R")), + "No such file|cannot be found|must be inside" + ) + + expect_message( + result <- load_fast_register_reload(root, files = "fns.R", reason = "test"), + "Registered file" + ) + expect_true(result) + expect_message( + load_fast(root, helpers = FALSE, attach_testthat = FALSE), + "Applying registered reload" + ) +}) + +test_that("a broken source file fails cleanly and recovery works", { + root <- write_mini_pkg("ttminie") + on.exit(cleanup_pkg("ttminie", root)) + + suppressMessages(load_fast(root, helpers = FALSE, attach_testthat = FALSE)) + writeLines('mini_fn <- function() "broken" (', file.path(root, "R", "fns.R")) + expect_error( + suppressMessages(load_fast(root, helpers = FALSE, attach_testthat = FALSE, full = TRUE)), + "Failed to source" + ) + expect_false("ttminie" %in% loadedNamespaces()) + + writeLines('mini_fn <- function() "fixed"', file.path(root, "R", "fns.R")) + ns <- suppressMessages(load_fast(root, helpers = FALSE, attach_testthat = FALSE)) + expect_identical(get("mini_fn", envir = ns)(), "fixed") +}) From e973af6d73268918b2b6e8054bad75e6d19cf8d4 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 6 Jul 2026 13:12:18 +0000 Subject: [PATCH 2/3] Drop rlang dependency: registry access via runtime .Internal calls; fix CI installs rlang::ns_registry_env() is defunct as of rlang 1.2.0 and hard-errors, which broke every R CMD check CI job on fresh installs (the renv.lock pinned an old rlang, masking it locally). Replace it with .loadfast.ns_register() / .loadfast.ns_unregister(), which construct .Internal(registerNamespace(...)) / .Internal(unregisterNamespace(...)) calls at runtime -- the same approach current pkgload uses. loadfast now has no third-party runtime dependencies; rlang moves to Suggests (still imported by the devpackage test fixture, which now imports the stable rlang::abort instead of the defunct symbol). The harness CI jobs failed separately during renv::restore(): the lockfile drags in dev-machine packages (keyring needs libsecret on ubuntu; a package failed to resolve on macos). Switch those jobs to setup-r-dependencies, which installs exactly the DESCRIPTION Imports+Suggests the harness needs and handles system requirements; renv.lock remains for local development. Verified locally: harness 299/299, R CMD check --as-cran clean (2 expected NOTEs), testthat suite passes under check. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01S8v6K52c7AJwuDkyyETMU7 --- .github/workflows/ci.yml | 10 ++++++++-- AGENTS.md | 6 +++--- DESCRIPTION | 2 +- R/loadfast.R | 38 +++++++++++++++++++++++++++++--------- README.md | 2 +- devpackage/NAMESPACE | 2 +- test_loadfast.R | 10 +++++----- 7 files changed, 48 insertions(+), 22 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e843d7e..617db17 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -16,12 +16,18 @@ jobs: matrix: os: [ubuntu-latest, windows-latest, macos-latest] + env: + RENV_CONFIG_AUTOLOADER_ENABLED: "FALSE" + steps: - uses: actions/checkout@v4 - uses: r-lib/actions/setup-r@v2 with: - r-version: "4.5.2" - - uses: r-lib/actions/setup-renv@v2 + r-version: "release" + use-public-rspm: true + # Installs Imports + Suggests from DESCRIPTION (everything the harness + # needs) and handles system requirements; the renv.lock is for local dev. + - uses: r-lib/actions/setup-r-dependencies@v2 - run: Rscript test_loadfast.R check: diff --git a/AGENTS.md b/AGENTS.md index 09fdc0c..1982bcc 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -31,7 +31,7 @@ This `AGENT.md` file is read by every agent session. !!!Keep them high-signal!!! - The package name is **read from the `Package:` field in DESCRIPTION**. No hard-coding required — just set the field in your DESCRIPTION file. - `load_fast(path)` accepts a package root or any path inside a package. It walks upward until it finds a directory containing `DESCRIPTION`, `NAMESPACE`, and `R/`. -- Requires **`rlang`** as a runtime dependency (for `rlang::ns_registry_env()`). The user already has it because they use devtools. +- No third-party runtime dependencies: the namespace registry is accessed via runtime-constructed `.Internal(registerNamespace(...))` / `.Internal(unregisterNamespace(...))` calls (`.loadfast.ns_register()` / `.loadfast.ns_unregister()`), the same approach current pkgload uses. Do NOT use `rlang::ns_registry_env()` -- it is defunct as of rlang 1.2.0 and hard-errors. - DESCRIPTION is parsed only for the `Package:` field. Imports are read from the **NAMESPACE** file. - `load_fast()` accepts `helpers` and `attach_testthat` parameters (mirroring pkgload's `load_all`). When `helpers = TRUE` (the default) and testthat is available, it calls `testthat::source_test_helpers()` to source `helper*.R` files from `tests/testthat/` into the attached package environment — the same behavior as pkgload. - `attach_testthat` defaults to auto-detect: if `tests/testthat/` (or `inst/tests/`) exists and testthat is installed, testthat is attached to the search path via `library("testthat")`. @@ -43,7 +43,7 @@ This `AGENT.md` file is read by every agent session. !!!Keep them high-signal!!! - **`.loadfast.cache`** is a module-level environment (`parent = emptyenv()`) keyed by `normalizePath(path)`. Each entry stores the namespace env, `pkg_name`, file hashes, `renv.lock` hash baseline, the S3 methods matrix, `import_pkgs` (names of packages this package imports), `needs_full` (NULL, or a reason string set by dependency invalidation), any registered reload files, and the pending reload message to emit on the next load. - **Dependency invalidation**: after any load that re-sources files, `.loadfast.invalidate_dependents()` flags every cached package that (transitively) imports the reloaded package by setting `needs_full`. A flagged package's next `load_fast()` is upgraded to a full reload, and the full-load path first recursively reloads flagged *dependencies* (via `.loadfast.load()`, guarded by the `.loadfast.state$stack` path stack), so one call on the top of a chain converges the whole chain. No-change loads do not invalidate. - **Depends**: `.loadfast.parse_depends()` reads `Depends` from DESCRIPTION (minus `R (...)`), and the full-load path attaches each with `library()` if not already attached; a missing Depends package is a clear error. -- **Teardown** (`.loadfast.teardown()`): detach + `unloadNamespace()`. When the namespace is imported by other loaded namespaces, `unloadNamespace()` errors — the fallback runs the old namespace's `.onUnload` best-effort and force-removes it from the registry via `rlang::ns_registry_env()`. A failed full load cleans up after itself (unregisters the half-built ns, detaches) via an `on.exit` guard. +- **Teardown** (`.loadfast.teardown()`): detach + `unloadNamespace()`. When the namespace is imported by other loaded namespaces, `unloadNamespace()` errors — the fallback runs the old namespace's `.onUnload` best-effort and force-removes it from the registry via `.loadfast.ns_unregister()`. A failed full load cleans up after itself (unregisters the half-built ns, detaches) via an `on.exit` guard. - **Package identity**: if the `Package:` field changes in place for a cached path, the old name is detached/unloaded and the cache entry dropped before the full load of the new name. - **Change detection**: `tools::md5sum()` on all `R/*.R` files every call. Compared against cached hashes to classify files as changed or added. - **No per-file symbol tracking**: the incremental path does **not** track which symbols came from which file, and does **not** remove stale symbols when files are deleted or functions are removed. This avoids the O(n²) `ls()` overhead that dominated load time (27.5s of 33.6s in a 475-file project). Stale symbols linger until the user calls `load_fast(path, full = TRUE)`. @@ -80,7 +80,7 @@ S4's `setClass`/`setMethod` rely on `topenv()` and `isNamespace()` internally. A 2. Set up `.__NAMESPACE__.` metadata environment with `spec`, `exports`, `imports`, etc. 3. **Register the env in R's internal namespace registry** so `isNamespace()` returns `TRUE`. -R's `registerNamespace` is an `.Internal` and cannot be called from user code. The workaround is `rlang::ns_registry_env()` which returns the registry as an R environment — you can assign directly into it: `reg[[pkg_name]] <- ns_env`. +R's `registerNamespace`/`unregisterNamespace` wrappers are not exported from base, and `rlang::ns_registry_env()` is defunct as of rlang 1.2.0 (it hard-errors). The working approach -- also used by current pkgload -- is constructing the `.Internal()` call at runtime: `eval(as.call(list(quote(.Internal), as.call(list(quote(registerNamespace), name, env)))))`. See `.loadfast.ns_register()`. ### `makeNamespace` is not exported `makeNamespace` is a **local function defined inside `base::loadNamespace`**. It is not accessible directly. pkgload extracts it at load time via AST manipulation (`extract_lang` + `modify_lang`). In `R/loadfast.R` we instead replicate its logic inline. diff --git a/DESCRIPTION b/DESCRIPTION index a688927..951b235 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -13,11 +13,11 @@ Roxygen: list(markdown = TRUE) RoxygenNote: 7.3.2 Imports: methods, - rlang, tools Suggests: data.table, digest, pkgload, R6, + rlang, testthat diff --git a/R/loadfast.R b/R/loadfast.R index b36a18f..1522467 100644 --- a/R/loadfast.R +++ b/R/loadfast.R @@ -2,6 +2,26 @@ .loadfast.state <- new.env(parent = emptyenv()) .loadfast.state$stack <- character(0) +# R's namespace registry has no public API for dev loaders: the base wrappers +# around registerNamespace/unregisterNamespace are not exported, and rlang's +# ns_registry_env() is defunct as of rlang 1.2.0. Construct the .Internal() +# calls at runtime instead -- the same approach current pkgload uses. +.loadfast.ns_register <- function(name, env) { + eval(as.call(list( + quote(.Internal), + as.call(list(quote(registerNamespace), name, env)) + ))) + invisible(env) +} + +.loadfast.ns_unregister <- function(name) { + eval(as.call(list( + quote(.Internal), + as.call(list(quote(unregisterNamespace), name)) + ))) + invisible(NULL) +} + .onAttach <- function(libname, pkgname) { packageStartupMessage("Incremental reload is available via load_fast().") } @@ -395,8 +415,10 @@ load_fast <- function(path = ".", helpers = TRUE, attach_testthat = NULL, full = ns_env[[".__S3MethodsTable__."]] <- new.env(hash = TRUE, parent = baseenv()) ns_env[[".__DEVTOOLS__"]] <- new.env(parent = ns_env) - reg <- rlang::ns_registry_env() - reg[[pkg_name]] <- ns_env + if (isNamespaceLoaded(pkg_name)) { + .loadfast.ns_unregister(pkg_name) + } + .loadfast.ns_register(pkg_name, ns_env) # If the load fails from here on (bad source file, failed import), do not # leave a half-built namespace registered or attached: the next call would @@ -412,10 +434,9 @@ load_fast <- function(path = ".", helpers = TRUE, attach_testthat = NULL, full = error = function(e) NULL ) } - cleanup_reg <- rlang::ns_registry_env() - if (exists(pkg_name, envir = cleanup_reg, inherits = FALSE) && - identical(cleanup_reg[[pkg_name]], ns_env)) { - rm(list = pkg_name, envir = cleanup_reg) + if (isNamespaceLoaded(pkg_name) && + identical(tryCatch(asNamespace(pkg_name), error = function(e) NULL), ns_env)) { + tryCatch(.loadfast.ns_unregister(pkg_name), error = function(e) NULL) } } }, @@ -822,9 +843,8 @@ load_fast_register_reload <- function(path = ".", files, reason = NULL) { } ) } - reg <- rlang::ns_registry_env() - if (exists(pkg_name, envir = reg, inherits = FALSE)) { - rm(list = pkg_name, envir = reg) + if (isNamespaceLoaded(pkg_name)) { + tryCatch(.loadfast.ns_unregister(pkg_name), error = function(e2) NULL) } }) } diff --git a/README.md b/README.md index 96b49ae..2cfc54c 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,7 @@ For a given package path, `loadfast` performs a full load on the first call, the `loadfast` is intended for the edit-reload-test loop, not as a general replacement for `devtools::load_all()`. The main tradeoff is described in [Important limitation: incremental reload does not clean up stale symbols](#important-limitation-incremental-reload-does-not-clean-up-stale-symbols). -At runtime, `loadfast` does not depend on `pkgload` or `devtools`. Its main runtime dependency is `rlang`. +At runtime, `loadfast` depends only on base R (plus the bundled `methods` and `tools` packages) — not on `pkgload`, `devtools`, or `rlang`. ## Installation diff --git a/devpackage/NAMESPACE b/devpackage/NAMESPACE index 99db7d1..4b51c0f 100644 --- a/devpackage/NAMESPACE +++ b/devpackage/NAMESPACE @@ -1,4 +1,4 @@ -importFrom(rlang, ns_registry_env) +importFrom(rlang, abort) import(methods) importFrom(R6, R6Class) importFrom(data.table,":=") diff --git a/test_loadfast.R b/test_loadfast.R index 26c8b9a..6c41518 100644 --- a/test_loadfast.R +++ b/test_loadfast.R @@ -243,9 +243,9 @@ check("Imports env parent is .BaseNamespaceEnv", quote( identical(parent.env(impenv), .BaseNamespaceEnv) )) -# --- importFrom(rlang, ns_registry_env) --- -check("ns_registry_env is in imports env", quote( - exists("ns_registry_env", envir = impenv, inherits = FALSE) +# --- importFrom(rlang, abort) --- +check("abort (importFrom rlang) is in imports env", quote( + exists("abort", envir = impenv, inherits = FALSE) )) # --- import(methods) --- @@ -1400,7 +1400,7 @@ rename_package(tmp_order_b, "ordbpkg") writeLines(c( "export(add)", - "importFrom(rlang, ns_registry_env)", + "importFrom(rlang, abort)", "import(methods)", "importFrom(R6, R6Class)", "importFrom(data.table,\":=\")", @@ -1412,7 +1412,7 @@ replace_namespace_imports( file.path(tmp_order_b, "NAMESPACE"), c( "importFrom(ordapkg,add)", - "importFrom(rlang, ns_registry_env)", + "importFrom(rlang, abort)", "import(methods)", "importFrom(R6, R6Class)", "importFrom(data.table,\":=\")", From a3f7ed753582d9caeb912787bd6a6ba3dd96ea73 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 6 Jul 2026 13:16:01 +0000 Subject: [PATCH 3/3] Make error-message expectations OS-agnostic in testthat suite normalizePath(mustWork = TRUE) wording differs by OS: "No such file or directory" on unix vs "The system cannot find the file specified" on Windows. Broaden the two expected-error patterns accordingly; this was the only remaining CI failure (Windows R CMD check). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01S8v6K52c7AJwuDkyyETMU7 --- tests/testthat/test-load_fast.R | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/testthat/test-load_fast.R b/tests/testthat/test-load_fast.R index cf1510b..fc7c7d8 100644 --- a/tests/testthat/test-load_fast.R +++ b/tests/testthat/test-load_fast.R @@ -92,7 +92,9 @@ test_that("load_fast() rejects paths outside a package", { on.exit(unlink(bare, recursive = TRUE)) expect_error(load_fast(bare), "Could not find package root") - expect_error(load_fast(file.path(bare, "nope")), "No such file|cannot be found|does not exist") + # normalizePath(mustWork = TRUE) wording differs by OS ("No such file or + # directory" on unix, "The system cannot find the file specified" on Windows). + expect_error(load_fast(file.path(bare, "nope")), "No such file|cannot find|does not exist") }) test_that("load_fast() validates the DESCRIPTION Package field", { @@ -115,7 +117,7 @@ test_that("load_fast_register_reload() validates its inputs", { expect_error( load_fast_register_reload(root, files = file.path(tempdir(), "elsewhere.R")), - "No such file|cannot be found|must be inside" + "No such file|cannot find|must be inside" ) expect_message(