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

Enable install_version to install current version of package on Windows/macOS #1730

Merged
merged 2 commits into from Mar 13, 2018

Conversation

Projects
None yet
3 participants
@jdblischak
Contributor

jdblischak commented Mar 7, 2018

Motivation and summary

Because of the impending API change in the package git2r, I want to write documentation that advises users to install version 0.21.0 of git2r, which is still the current version of CRAN. This doesn't work with the current version of devtools::install_version on Windows or macOS. This PR fixes it.

The Diaspora

I read about the diaspora. However, I still decided to send the PR here because unlike packages like pkgbuild and usethis, remotes is not yet a dependency for devtools. Also, remotes::install_version doesn't have this issue. Please feel free to close it.

Edit: I investigated more. The implementation of remotes::install_version has substantially changed from the one in devtools, i.e. it doesn't use install.packages to determine the download URL. Instead it constructs the URL itself, always specifying the extension .tar.gz. Thus this works when installing from source, but breaks if passed type = "binary".

Demonstration

On macOS (tested on 10.10.5 and 10.10.13) and Windows (tested on 10), install_version results in an error when trying to install the current version. Specifying type = "binary" fixes it.

# Using devtools master branch on macOS
> packageVersion("devtools")
[1] ‘1.13.5.9000’
> devtools::install_version("git2r", "0.21.0")
downloaded 0 bytes

Error in download.file(url, destfile, method, mode = "wb", ...) : 
  cannot download all files
In addition: Warning message:
In download.file(url, destfile, method, mode = "wb", ...) :
  URL 'https://cran.rstudio.com/src/contrib/git2r_0.21.0.tgz': status was '404 Not Found'
Warning in download.packages(pkgs, destdir = tmpd, available = available,  :
  download of package ‘git2r’ failed
> devtools::install_version("git2r", "0.21.0", type = "binary")
trying URL 'https://cran.rstudio.com/bin/macosx/mavericks/contrib/3.3/git2r_0.21.0.tgz'
Content type 'application/x-gzip' length 2212047 bytes (2.1 MB)
==================================================
downloaded 2.1 MB


The downloaded binary packages are in
	/var/folders/5b/hbjnbkkd0t957rs24vxgvw200000gn/T//RtmpVPdqFD/downloaded_packages

Proposed solution

When installing the current version, install_version passes everything on to install.packages. According to the install.packages man page, you are not supposed to set both contriburl and type = "both":

Overrides argument repos. Incompatible with type = "both".

On the Windows and macOS machines I tested, getOptions("pkgType") was "both". I was able to fix the problem by removing the manual setting of contriburl by install_version. This was unnecessary anyways because it was using the same default as install.packages does: contrib.url(repos, type).

I confirmed that this solution works on macOS, Windows, and Linux.

Potentially related issues: #1724 (comment), r-hub/rhub#123

If this PR is useful, I can add a bullet point to NEWS.md.

@jimhester

This comment has been minimized.

Member

jimhester commented Mar 9, 2018

Thanks! LGTM, please add a news entry and I can merge it.

@jdblischak

This comment has been minimized.

Contributor

jdblischak commented Mar 9, 2018

@jimhester Great. I added a news entry. Thanks!

@jdblischak

This comment has been minimized.

Contributor

jdblischak commented Mar 9, 2018

@jimhester Also, regarding my observation that remotes::install_version cannot install the binary version of the packages, is this desired behavior or should I open an Issue on remotes? Copy-pasting the relevant section from above:

Edit: I investigated more. The implementation of remotes::install_version has substantially changed from the one in devtools, i.e. it doesn't use install.packages to determine the download URL. Instead it constructs the URL itself, always specifying the extension .tar.gz. Thus this works when installing from source, but breaks if passed type = "binary".

I wasn't able to find an easy fix like I did for here, and there has been recent development about how to handle binary packages (e.g. the most recent commit). Thus I wasn't sure how best to proceed. I think it could be solved by checking contriburl for the string "bin", and then switching the file extension to .tgz. However, I don't know how robust that solution is, and I could only get it to work when manually specifying type = "binary". I think a better default, especially for Windows users that may not have compilers installed, would be to try and install the binary if it is available.

@jimhester

This comment has been minimized.

Member

jimhester commented Mar 9, 2018

Not sure what the best solution is; the current plan is to eventually switch devtools to use pkgman, which uses a different API and method for package installation, so devtools will likely never use remotes as-is.

However you are still welcome to open an issue there if you want to try and fix binary installation; in general you are somewhat limited in what binary versions are available, as CRAN only builds the most recent version of a package for a given version of R.

@gaborcsardi

This comment has been minimized.

Member

gaborcsardi commented Mar 9, 2018

@jdblischak I am not entirely sure in which situations it makes sense to install old binary packages. Most of the time these are not available for R-release, anyway. The old binary builds available are for older R versions.

@jdblischak

This comment has been minimized.

Contributor

jdblischak commented Mar 9, 2018

Not sure what the best solution is; the current plan is to eventually switch devtools to use pkgman, which uses a different API and method for package installation, so devtools will likely never use remotes as-is.

@jimhester Good to know! I didn't realize that was the plan.

I am not entirely sure in which situations it makes sense to install old binary packages. Most of the time these are not available for R-release, anyway. The old binary builds available are for older R versions.

@gaborcsardi Thanks for the advice! I agree this is an edge case. This PR to devtools solves the main problem I was having, so I am fine with remotes keeping its current behavior of always installing from source.

Instead of changing the behavior of remotes::install_version, what about changing the documentation to indicate that binary downloads are not supported? The documentation for type is currently:

character, indicating the type of package to download and install. Will be "source" except on Windows and some macOS builds: see the section on ‘Binary packages’ for those.

If a user specifies type = "binary" with the current CRAN release, it breaks:

> install_version("git2r", "0.21.0", type = "binary")
Downloading package from url: https://cran.rstudio.com/bin/windows/contrib/3.4/git2r_0.21.0.tar.gz
 Show Traceback
 
 Rerun with Debug
 Error in utils::download.file(url, path, method = download_method(), quiet = quiet,  : 
  cannot open URL 'https://cran.rstudio.com/bin/windows/contrib/3.4/git2r_0.21.0.tar.gz' 

If a user specifies type = "binary" with a previous CRAN release, it installs from source:

> install_version("git2r", "0.19.0", type = "binary")
Trying https://cran.rstudio.com/
Downloading package from url: https://cran.rstudio.com//src/contrib/Archive/git2r/git2r_0.19.0.tar.gz
Installing package into ‘C:/Users/john/Documents/R/win-library/3.4’
(as ‘lib’ is unspecified)
* installing *source* package 'git2r' ...

If you're interested, I could send a PR that 1) notes in the documentation that type = "binary" is not accepted by install_version, and 2) has install_version throw an error early if type == "binary". If you're not interested in these changes, I'm fine leaving it as is.

@jdblischak

This comment has been minimized.

Contributor

jdblischak commented Mar 13, 2018

@jimhester Is my NEWS entry ok? If yes, then this PR is ready to merge.

@jimhester jimhester merged commit badeb4d into r-lib:master Mar 13, 2018

@jimhester

This comment has been minimized.

Member

jimhester commented Mar 13, 2018

Looks good, thanks!

@jdblischak jdblischak deleted the jdblischak:install-current branch Mar 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment