r-lib / remotes

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

add option `remotes.download.headers` #685

Closed AshesITR closed 1 year ago

AshesITR commented 2 years ago

this allows passing e.g. headers to both, utils::install.packages() and utils::available.packages()

If you don't like passing ellipsis twice, I could instead make headers a named argument in all functions and pass that down to both functions while keeping ellipsis only for the call to install.packages(). The downside to this is that the argument might be surprising given that no other networking-related arguments are exposed in the same way.

fixes #682

gaborcsardi commented 2 years ago

Passing the ... twice is not great because available.packages() and install.packages() take different arguments. Instead top level functions could have an available_packages_args argument, which is a list of arguments that we pass to available.packages().

Alternatively, we could have an option to set the headers that are passed to available.packages() and/or install.packages().

AshesITR commented 2 years ago

I like the option alternative. It is similar to renv, which uses renv.download.headers described here. Will rework this.

Should I add the headers argument to top-level functions so e.g. remotes::install_deps(headers = ...) also works as expected?

gaborcsardi commented 2 years ago

I think if we have an option, then we don't need to add new arguments. You'll need to work out what to do if the user already supplies the headers argument in ..., though. Maybe merge them with the argument taking precedent?

AshesITR commented 2 years ago

@gaborcsardi could you take another look? I've implemented it via the character option remotes.download.headers. If headers was manually provided to any install command, a message is generated pointing to the feature (I'm afraid it's not trivially possible to capture the headers argument in all the right places without capturing it too often). If we wanted to implement the function interface renv uses, we'd have to also re-implement utils::available.packages like renv does, so I didn't go that route.

Thanks!