Fix subscript out of bounds when repos include an R-universe repository - #107
Open
ddsjoberg wants to merge 1 commit into
Open
Fix subscript out of bounds when repos include an R-universe repository#107ddsjoberg wants to merge 1 commit into
subscript out of bounds when repos include an R-universe repository#107ddsjoberg wants to merge 1 commit into
Conversation
R-universe repositories report the `Repository` field of `available.packages()` as a full, per-package tarball url (`.../src/contrib/<pkg>_<ver>.tar.gz?sha256=...&file=`) rather than the plain contrib url that CRAN-like repositories report. Comparing that field to `contrib.url(repo)` with `==` therefore never matched, so `strip_src_contrib()` returned `character(0)`, `pkg_origin_repo()` stored an empty `repos`, and `get_package_source()` failed at check time with `subscript out of bounds`. Match by prefix instead, so a per-package tarball url still resolves back to its originating repository. `contrib.url()` is left at its default `type`, matching the type `available_packages()` uses to produce the `Repository` field, so binary repositories keep working. No change is needed in `get_package_source()`: R-universe crafts the `Repository` field so that appending `/<pkg>_<ver>.tar.gz` (what `utils::download.packages()` does, and what `get_package_source()` already did) yields a valid download url. Fixes Genentech#106 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #106.
What changed
strip_src_contrib()(R/utils-pkg-source.R) now matches a package'sRepositoryfield against each repository's contrib url by prefix instead of equality.Why
For CRAN-like repositories,
available.packages()reportsRepositoryas exactly the contrib url, socontrib.url(repo) == Repositoryholds. R-universe instead reports it as the full, per-package tarball url:The equality check never matched, so
strip_src_contrib()returnedcharacter(0),pkg_origin_repo()stored an emptyrepos, and at check timeget_package_source()indexed an emptyavailable.packages()database:Notes for the reviewer
contrib.url()type. The issue suggestedutils::contrib.url(r, type = "source"). I deliberately left the defaulttypein place:pkg_origin_repo()readsRepositoryout ofavailable_packages(repos = repos)at the session's defaultpkgType, socontrib.url()must use the same type or matching would break for genuine binary repositories (e.g.pkgType = "win.binary", whereRepositoryis.../bin/windows/contrib/4.5). WithpkgType = "both"both sides resolve tosrc/contrib, which is the case in the reported scenario.get_package_source()needs no change. The issue also suggested rebuilding the archive url or switching toutils::download.packages(). That turns out to be unnecessary — R-universe crafts theRepositoryfield to end in&file=precisely so that appending/<pkg>_<ver>.tar.gz(exactly whatdownload.packages()does internally, and whatget_package_source()already did) produces a valid url:I verified this downloads (1,981,048 bytes) and untars cleanly.
basename()of that url is also a cleangtsummary_2.6.0.9003.tar.gz, sofetch_package_source()'s destfile is correct too. Keeping the existing code avoids a behavioural change to thedestdir = NULLbranch.Tests
New
tests/testthat/test-pkg-source.R:Repositorystyles, the no-match case, and archive url construction for CRAN-style and R-universe-styleRepositoryfields;skip_on_cran(), skips if the repos are unreachable) resolvinggtsummaryend-to-end throughpkg_origin_repo()→check_path()againsthttps://ddsjoberg.r-universe.dev.All 7 pass. The rest of the suite is unaffected by this change; the only failures I see locally are pre-existing environment issues (
visNetworknot installed, and a NOTE-count difference intest-check.R/test-results-utils.R).🤖 Generated with Claude Code