Skip to content

Fix get_extra_deps function#711

Merged
gaborcsardi merged 5 commits intor-lib:mainfrom
maksymiuks:main
Feb 4, 2023
Merged

Fix get_extra_deps function#711
gaborcsardi merged 5 commits intor-lib:mainfrom
maksymiuks:main

Conversation

@maksymiuks
Copy link
Contributor

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
Copy link
Contributor Author

@gaborcsardi Requesting review

@maksymiuks
Copy link
Contributor Author

@gaborcsardi any thoughts about it?

@maksymiuks
Copy link
Contributor Author

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
Copy link
Member

gaborcsardi commented Feb 4, 2023

Thanks and sorry for the long wait!

@gaborcsardi gaborcsardi merged commit 88fdc4e into r-lib:main Feb 4, 2023
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.

2 participants

Comments