Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prompt user to install missing and outdated dependencies #184

Merged
merged 3 commits into from
Dec 16, 2021

Conversation

lionel-
Copy link
Member

@lionel- lionel- commented Dec 16, 2021

Depends on r-lib/rlang#1332.

Uses rlang::check_installed() to prompt users about missing dependencies. The custom action callback uses pak if installed, or remotes if installed, and otherwise install.packages().

For outdated packages, the prompt is only triggered if the versions do not match. This way we avoid the large overhead of the dependency functions of pak and remotes. Also this avoids having to reinstall from github when pak or remotes does not recognise that a local or dev install is sufficient.

Copy link
Member

@hadley hadley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good in theory — I'll install the dev version when you merge this and see how it feels in practice.

Copy link
Member

@gaborcsardi gaborcsardi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess YMMV, but this seems a bit too aggressive to me. How about just printing out the outdated packages instead, once a session?

if (is_installed("pak")) {
deps <- pak::pkg_deps(path, upgrade = FALSE)
deps <- deps[deps$package %in% pkg, ]
pak::pak(deps$ref, ask = FALSE)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does two rounds of dependency lookups, which is not ideal.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have a fix to suggest? This is getting the refs of the package names in pkg which are potentially Remotes refs, and then installing them. Is there a way to do these two steps in one go?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just call

pkg_install("deps::.")

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid having to reinstall dependencies from github when pak does not recognise that a local or dev install is sufficient.

@lionel-
Copy link
Member Author

lionel- commented Dec 16, 2021

I guess YMMV, but this seems a bit too aggressive to me. How about just printing out the outdated packages instead, once a session?

For my usage at least it's very rare that I don't want to install the minimal requirements for Imports and Depends packages. Perhaps we could add an option to control this behaviour if it turns out too annoying? We could first try it out to see how it works in practice.

@lionel-
Copy link
Member Author

lionel- commented Dec 16, 2021

Merging now so we can test it more easily.

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.

3 participants