Skip to content

feat: find R packages in well-known subdirectories - #240

Draft
schloerke wants to merge 1 commit into
r-lib:mainfrom
schloerke:feature/package-subdirs
Draft

schloerke wants to merge 1 commit into
r-lib:mainfrom
schloerke:feature/package-subdirs

Conversation

@schloerke

Copy link
Copy Markdown

Summary

find_package_root_file() now also finds an R package that lives in a
subdirectory of a multi-language repository — one holding, say, an R package, a
Python package and a JavaScript library side by side. Today such a repository
can be installed with pak::pkg_install("owner/repo") (r-lib/pkgdepends#466)
but cannot be developed with devtools::load_all() from the repository root,
because pkgload:::pkg_path() calls this function.

Motivating example, posit-dev/shinychat:

shinychat/
├── shinychat.Rproj      BuildType: Package
├── .github/workflows/   R-CMD-check, pkgdown, py-check, quartodoc, ...
├── pkg-r/               DESCRIPTION, NAMESPACE, R/, man/, tests/, vignettes/
├── pkg-py/  js/  docs/

Behavior

For each directory in the hierarchy, in order:

  1. the directory itself, if it contains a DESCRIPTION matching ^Package: ;
  2. the directory named by the PackagePath: field of an RStudio project file
    (.Rproj) in that directory, if it contains such a DESCRIPTION;
  3. the new subdirs argument — by default r_package_subdirs(), i.e.
    pkg-r, r, R — first match wins.

Step 1 always wins, so any repository with a root DESCRIPTION resolves exactly
as before. Because the rules are applied at every level of the upward walk,
load_all() also works from a sibling directory such as shinychat/js/.

Step 3's candidate list matches pkgdepends, so a repository installable with
pak::pkg_install("owner/repo") resolves the same way locally. Step 2 is the
existing explicit escape hatch: PackagePath: is already used in the wild for
directories the convention can't express — PackagePath: rkeops in
getkeops/keops, PackagePath: release/missRanger in mayer79/missRanger.

Two warnings, per review discussion:

  • a PackagePath: field pointing at a directory with no DESCRIPTION warns and
    falls through to step 3;
  • more than one .Rproj file in a directory warns and skips step 2.

Compatibility

  • subdirs = NULL restores the behavior of rprojroot 2.1.1 and earlier exactly.
    subdirs = character() consults .Rproj files only.
  • No criterion changed. is_r_package, criteria, find_root(),
    get_root_desc() and is_r_package$find_file are all untouched, so
    find_root(is_r_package) keeps returning the directory that matched.
  • No new dependencies; warnings use base warning().

Downstream, this makes devtools::load_all(), document(), test() and
check() work in such repositories with no changes to pkgload (verified against
an unmodified pkgload/devtools with this branch installed), and gives
rcmdcheck() the same reach. usethis is the one caller that needs a companion
change: usethis:::is_package() uses this function's error as a predicate, and
r-lib/usethis will be pinned to find_root(is_r_package, path) so that the
active project — which in a monorepo is the repository root — is not mistaken
for a package. That PR is coming separately.

Notes for review

  • root_criterion(subdir =) already exists, and is_testthat uses it to
    descend into tests/testthat. I didn't build on it because get_start_path()
    rewrites only the starting directory (so cwd = repo/js/ wouldn't resolve)
    and descends whenever the directory merely exists (so pkg-r/ could outrank a
    root DESCRIPTION). Happy to revisit if you'd rather extend that mechanism.
  • r and R are the same directory on case-insensitive filesystems, so
    locally they are effectively one candidate; GitHub's trees are case-sensitive,
    so pkgdepends can distinguish them. Worth documenting if this lands.
  • The subdirs argument carries two modes (NULL vs character()). A separate
    flag for step 2 may read better — happy to change.

Test plan

  • 19 tests in tests/testthat/test-package-root.R plus one snapshot: root wins
    over a subdirectory, subdirectory found from the root and from a sibling,
    candidate order, PackagePath: winning over a subdirectory, both warnings and
    their fallbacks, subdirs = NULL / character(), empty DESCRIPTION not
    counting, no package anywhere, absolute-path passthrough.
  • Fixtures mirror real layouts (shinychat's pkg-r, keops' PackagePath).
    Not-found cases use isolated temporary directories so they don't accidentally
    find rprojroot's own DESCRIPTION under R CMD check.
  • R CMD check: Status OK. Full existing suite unchanged.
  • NEWS.md not edited, since it's fledge-managed; the rationale is in the
    feat: commit message.

Filed as a draft for discussion of the API shape.

`find_package_root_file()` now also finds a package that lives in a
subdirectory of a multi-language repository, e.g. a repository holding an
R package, a Python package and a JavaScript library side by side.

For each directory in the hierarchy, the following are consulted in order:

1. the directory itself, if it contains a `DESCRIPTION` file;
2. the directory given by the `PackagePath:` field of an RStudio project
   file (`.Rproj`) in that directory;
3. the new `subdirs` argument, by default `r_package_subdirs()`, i.e.
   `pkg-r`, `r`, and `R`.

Pass `subdirs = NULL` for the previous behavior. Criteria are untouched, so
`find_root(is_r_package)` and `criteria$is_r_package` are unaffected.

The `pkg-r`/`r`/`R` convention matches pkgdepends, so that a repository
installable with `pak::pkg_install("owner/repo")` can also be developed
with `devtools::load_all()` from the repository root.

Warns and falls through when a `PackagePath:` field points to a directory
without a `DESCRIPTION` file, and when a directory holds more than one
`.Rproj` file.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant