r-lib / remotes

Install R packages from GitHub, GitLab, Bitbucket, git, svn repositories, URLs
https://remotes.r-lib.org/
Other
330 stars 152 forks source link

Repos does not carry forward while looking for Enhances dependencies #704

Closed maksymiuks closed 1 year ago

maksymiuks commented 2 years ago

Hi,

Thank you for your work under this great package, I really appreciate that.

I would like to report an incosistnecy/a bug that is really a thorn in the eye for my usecase.

For the reference, I'll use as an example the base64enc's DESCRIPTION I paste below

Package: base64enc
Version: 0.1-3
Title: Tools for base64 encoding
Author: Simon Urbanek <Simon.Urbanek@r-project.org>
Maintainer: Simon Urbanek <Simon.Urbanek@r-project.org>
Depends: R (>= 2.9.0)
Enhances: png
Description: This package provides tools for handling base64 encoding. It is more flexible than the orphaned base64 package.
License: GPL-2 | GPL-3
URL: http://www.rforge.net/base64enc
NeedsCompilation: yes
Packaged: 2015-02-04 20:31:00 UTC; svnuser
Repository: CRAN
Date/Publication: 2015-07-28 08:03:37

It's oriented around the behavior of the remotes::dev_package_deps. My use-case aims to take arbitrary packages and extract their Enhances dependencies. Dependency like any other, especially that lack of it throws R CMD Check NOTE. When I try to use remotes::dev_package_deps with parameters dependencies = "Enhances" it completely neglects what I'm passing to the repos parameter and is using getOption("repos") instead. It is due to the fact, that this line: https://github.com/r-lib/remotes/blob/main/R/deps.R#L145 calls a get_extra_deps that has hardcoded standardise_dep(TRUE) in the line https://github.com/r-lib/remotes/blob/main/R/deps.R#L213 meaning it excludes "Enhances" dependencies from the "standard" one although those are only I'm interested in that particular case. That causes "Enhances" field to be treated as a remote field, and in the example above, we are looking for the remote sources for the png package. Therefore extra deps function is called and I particular that line https://github.com/r-lib/remotes/blob/main/R/deps.R#L589 and function parse_one_extra for the png package. It properly resolves png to be a CRAN-like package in the line https://github.com/r-lib/remotes/blob/main/R/deps.R#L524 but then in the line https://github.com/r-lib/remotes/blob/main/R/deps.R#L524 it calls cran_remote function but with default parameters, so repos = getOption("repos") rather than using repos that we've set at the very beginning. A quick skimming of the code tells me that it is to due not passing an ellipsis to particular lapply's but I didn't check that. All of that results in a package presumably being unavailable, because it used different repos than I asked. And because Enhances was treated as a remote - this line https://github.com/r-lib/remotes/blob/main/R/deps.R#L141 overrides the value found in this line https://github.com/r-lib/remotes/blob/main/R/deps.R#L141 that actually found a proper dependency and its version.

To summarize I've identified two independent problems that together make this issue particularly annoying

  1. dev_package_deps function with dependencies = "Enhances first treats Enhances packages as regular CRAN dependency and finds appropriate packages and versions in this line: https://github.com/r-lib/remotes/blob/main/R/deps.R#L141 only to two lines later treat it as a remote dependency and override previous result https://github.com/r-lib/remotes/blob/main/R/deps.R#L145
  2. When CRAN package is find in the remotes section, getOptions("repos") (default parameter for remotes:::cran_remote) is used, rather than actual "repos" passed to the dev_package_deps

I hope I didn't explain it in a too complicated way. If so, I'm happy to clrafiy everything if its only necessary

gaborcsardi commented 2 years ago

To make sure that we are on the same page:

maksymiuks commented 2 years ago

Thanks for the prompt response, below is the code:

options(repos = "address_of_a_cranlike_repo")
remotes::dev_package_deps(
  repos = c(getOption("repos"), "https://cran.rstudio.com/"),
  dependencies = "Enhances")

and the output

Needs update -----------------------------
 package installed available is_cran remote
 png     NA        NA        TRUE    CRAN  

and as str()

Classes ‘package_deps’ and 'data.frame':    1 obs. of  6 variables:
 $ package  : chr "png"
 $ installed: chr NA
 $ available: chr NA
 $ diff     : int -2
 $ is_cran  : logi TRUE
 $ remote   :List of 1
  ..$ :List of 3
  .. ..$ name    : chr "png"
  .. ..$ repos   : chr "address_of_a_cranlike_repo"
  .. ..$ pkg_type: chr "both"
  .. ..- attr(*, "class")= chr [1:2] "cran_remote" "remote"

I'd expect the output to has 0.1-7 in the available columns as I've passed cran as one of the repos and this is the version available there.

A small disclaimer I used "address_of_a_cranlike_repo" as I cannot share address to the actual repository I'm using, but it is a standard R CRAN-like repository without png package.

A second not is that it also breaks another way. If we set up options(repos =) to CRAN but pass an empty string as repos to the. remotes::dev_package_deps, it will still find the CRAN version although it shouldn't

maksymiuks commented 2 years ago

I forgot to add, I'm using unzipped cran source of the base64enc package as a source to the remotes::dev_package_deps. I'm sorry @gaborcsardi

gaborcsardi commented 2 years ago

Thanks, reproducible example:

setwd(tempdir())
download.file("https://cran.rstudio.com/web/packages/base64enc/DESCRIPTION", "DESCRIPTION")
options(repos = "address_of_a_cranlike_repo")
remotes::dev_package_deps(
  repos = c(getOption("repos"), CRAN = "https://cran.rstudio.com"), 
  dependencies = "Enhances"
)
Needs update -----------------------------
 package installed available is_cran remote
 png     0.1-7     NA        TRUE    CRAN
maksymiuks commented 1 year ago

@gaborcsardi Kind reminder regarding the bug. Are there plans to prepare a solution? If not, will my PR get accepted if I prepare one? I could help with the fix

gaborcsardi commented 1 year ago

In general, if rlib/pak covers your use case, then it is a better option than remotes. remotes does not get much development time.

maksymiuks commented 1 year ago

Thanks, @gaborcsardi for a prompt answer. I will take a look! The only issue I have with rlib/pak now though, is the fact it does not support arbitrary git:: entry as the remote source, only GitHub.

gaborcsardi commented 1 year ago

If your goal is only to extract dependencies from a package, then desc::desc_get_deps() is another option.

maksymiuks commented 1 year ago

Thank you :) The issue can be closed then