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

Install Suggested package after package installation #1184

Merged
merged 2 commits into from May 26, 2016

Conversation

Projects
None yet
3 participants
@jimhester
Member

jimhester commented May 10, 2016

This includes packages specified in Remotes:. If build_vignettes is TRUE
they are still installed before the package.

This fixes tidyverse/dplyr#1809

Install Suggested package after package installation
This includes packages specified in Remotes:. If build_vignettes is TRUE
they are still installed before the package.

This fixes tidyverse/dplyr#1809
@jimhester

This comment has been minimized.

Member

jimhester commented May 10, 2016

@krlmlr The main issue with this currently is it queries the remotes twice, but getting rate limited seems to be less of an issue with the embedded PAT now.

@jimhester

This comment has been minimized.

Member

jimhester commented May 11, 2016

@hadley if the small hack used in 7f54aaf (use of the installed environment to store state) is OK I can merge this, I verified it correctly handles the dplyr -> dtplyr issue in both directions.

install_deps(pkg, dependencies = dependencies, upgrade = upgrade_dependencies,
initial_deps <- dependencies[dependencies != "Suggests"]
final_deps <- dependencies[dependencies == "Suggests"]

This comment has been minimized.

@krlmlr

krlmlr May 11, 2016

Member

What about "Enhances"?

This comment has been minimized.

@jimhester

jimhester May 12, 2016

Member

I think Enhances should be fine to installed in the initial installation. Note Enhances: are not installed unless explicitly set by default (see ?install.packages).

logical indicating whether to also install uninstalled
packages which these packages depend on/link
to/import/suggest (and so on recursively). Not used if
‘repos = NULL’. Can also be a character vector, a subset of
‘c("Depends", "Imports", "LinkingTo", "Suggests",
"Enhances")’

Only supported if ‘lib’ is of length one (or missing), so it is
unambiguous where to install the dependent packages. If this is not the case
it is ignored, with a warning.

The default, ‘NA’, means ‘c("Depends", "Imports",
"LinkingTo")’.

‘TRUE’ means to use ‘c("Depends", "Imports", "LinkingTo",
"Suggests")’ for ‘pkgs’ and ‘c("Depends", "Imports",
"LinkingTo")’ for added dependencies: this installs all the
packages needed to run ‘pkgs’, their examples, tests and
vignettes (if the package author specified them correctly).

In all of these, ‘"LinkingTo"’ is omitted for binary
packages.

This comment has been minimized.

@krlmlr

krlmlr May 13, 2016

Member

What happens if there's a dependency chain like A -> (enhances) B -> (imports) A and I try to install(A) or install_deps(A)?

@krlmlr

This comment has been minimized.

Member

krlmlr commented May 11, 2016

Does this also work for install_deps(".", dependencies = TRUE) for dplyr?

@jimhester

This comment has been minimized.

Member

jimhester commented May 12, 2016

Does this also work for install_deps(".", dependencies = TRUE) for dplyr?

Yes, that seems to work fine on my machine

krlmlr pushed a commit to krlmlr/dplyr that referenced this pull request May 13, 2016

Kirill Müller
@krlmlr

This comment has been minimized.

Member

krlmlr commented May 13, 2016

I can confirm that dtplyr -> dplyr works (https://travis-ci.org/krlmlr/dplyr/builds/129929004), but not dplyr -> dtplyr -> dplyr if I do install_deps(dependencies = TRUE) (https://travis-ci.org/krlmlr/dplyr/builds/129933568). I think the "release" build always runs with an empty cache because it's failing.

@jimhester

This comment has been minimized.

Member

jimhester commented May 16, 2016

Looks like you forgot to put dtplyr back in Remotes: on that test https://github.com/krlmlr/dplyr/blob/4b2911fa657df1b0e4cf03ad2ca4b81c95df489b/DESCRIPTION

@krlmlr

This comment has been minimized.

Member

krlmlr commented May 16, 2016

Thanks. Of course I forgot, now it works. My mistake.

That's just awesome! Are you planning a release anytime soon?

@hadley hadley merged commit 9cded43 into r-lib:master May 26, 2016

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment