search

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

Fix get_extra_deps function #711

Closed maksymiuks closed 1 year ago

maksymiuks commented 2 years ago

Recently I've discovered a bug in the get_extra_deps function. I'll describe it using a reproducible example:

setwd(tempdir())
download.file("https://cran.r-project.org/web/packages/htmlwidgets/DESCRIPTION", "DESCRIPTION")
remotes::dev_package_deps(
  dependencies = c("Depends", "Imports", "LinkingTo", "Suggests", "Enhances")
)

it returns an error saying:

Error: Missing commas separating Remotes: 'shiny (>= 1.1)'

inspection has shown that the problem is in those lines

https://github.com/r-lib/remotes/blob/main/R/deps.R#L212-L213

because standardise_dep(TRUE) returns c("Depends", "Imports", "LinkingTo", "Suggests") which does not necessarily reflects the comments in the code which mentions removing all standard dependencies. "Enhances" although not commonly used is still a standard dependency supported by the R language and developers can specify packages of this type (https://cran.r-project.org/doc/manuals/r-release/R-exts.html 1.1.3 Package Dependencies). Because the current implementation does not include Enhances, this field is interpreted as remotes source, and thus any version specification results in an error. An important note is that although Enhances versions are not used now, R manual clearly states they can be specified.

The PR is a fix to the mentioned issues. It ensures Enahnces packages are recognized as standard dependencies.

maksymiuks commented 2 years ago

@gaborcsardi Requesting review

maksymiuks commented 1 year ago

@gaborcsardi any thoughts about it?

maksymiuks commented 1 year ago

Hi

I see the PR got stale for a while. I'd like to ask what should be the next steps. Should I update the PR so it can get reviewed, or create an issue so we can discuss thoroughly why is it an issue? Would appreciate some guidance @gaborcsardi as the bug is a pain in the neck, every time I have to reinstall remotes from the branch.

gaborcsardi commented 1 year ago

Thanks and sorry for the long wait!