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_github does not skip package installation with same SHA in case "ref" is defined #1624

Closed
mrustl opened this Issue Oct 17, 2017 · 6 comments

Comments

Projects
None yet
3 participants
@mrustl

mrustl commented Oct 17, 2017

Using the latest version of devtools on CRAN (1.3.3) the following code downloads the package even if the SHA number is the same:

1) Skipping does not work if "ref" is supplied

First execution installs the package

devtools::install_github("hadley/multidplyr", ref = "9cf344c9c63a7ebfa25f166800d67ef8a23c0ba9")

But also second execution installs the package again

devtools::install_github("hadley/multidplyr", ref = "9cf344c9c63a7ebfa25f166800d67ef8a23c0ba9")

2) Skipping works as expected if master branch is used

First execution installs the package

devtools::install_github("hadley/multidplyr")

Second execution skips package installation

devtools::install_github("hadley/multidplyr")

Skipping install of 'multidplyr' from a github remote, the SHA1 (0085ded4) has not changed since last install.
Use force = TRUE to force installation

My system

sessionInfo()
R version 3.4.1 (2017-06-30)
Platform: x86_64-w64-mingw32/x64 (64-bit)
Running under: Windows 7 x64 (build 7601) Service Pack 1

Matrix products: default

locale:
[1] LC_COLLATE=German_Germany.1252 LC_CTYPE=German_Germany.1252 LC_MONETARY=German_Germany.1252 LC_NUMERIC=C
[5] LC_TIME=German_Germany.1252

attached base packages:
[1] stats graphics grDevices utils datasets methods base

other attached packages:
[1] devtools_1.13.3

loaded via a namespace (and not attached):
[1] httr_1.3.1 compiler_3.4.1 R6_2.2.2 tools_3.4.1 withr_2.0.0 curl_2.8.1 memoise_1.1.0 knitr_1.17
[9] git2r_0.19.0 digest_0.6.12

@jimhester jimhester added the bug label Feb 13, 2018

@dracodoc

This comment has been minimized.

dracodoc commented Feb 27, 2018

I had similar issue but it's with current CRAN version 1.13.5, and I'm not using ref.

The example below is simple and didn't create much problems, however when I try to install httpuv it always fail to compile because of fopenmp support in Mac. In every try it will compile promise and later first then start to compile httpuv. I think promise and later should at least be skipped because they can be compiled properly.

> devtools::install_github("r-lib/crayon")
Downloading GitHub repo r-lib/crayon@master
from URL https://api.github.com/repos/r-lib/crayon/zipball/master
Installing crayon
'/Library/Frameworks/R.framework/Resources/bin/R' --no-site-file --no-environ --no-save --no-restore  \
  --quiet CMD INSTALL  \
  '/private/var/folders/f3/6cdx4zf94qz95r6d89s_4xcsrcsjqm/T/RtmpWhp3NA/devtools183212980998b/r-lib-crayon-95b3eae'  \
  --library='/Library/Frameworks/R.framework/Versions/3.4/Resources/library' --install-tests 

* installing *source* packagecrayon...
** R
** inst
** tests
** preparing package for lazy loading
** help
*** installing help indices
** building package indices
** testing if installed package can be loaded
* DONE (crayon)
> devtools::install_github("r-lib/crayon")
Downloading GitHub repo r-lib/crayon@master
from URL https://api.github.com/repos/r-lib/crayon/zipball/master
Installing crayon
'/Library/Frameworks/R.framework/Resources/bin/R' --no-site-file --no-environ --no-save --no-restore  \
  --quiet CMD INSTALL  \
  '/private/var/folders/f3/6cdx4zf94qz95r6d89s_4xcsrcsjqm/T/RtmpWhp3NA/devtools1832116e48be2/r-lib-crayon-95b3eae'  \
  --library='/Library/Frameworks/R.framework/Versions/3.4/Resources/library' --install-tests 

* installing *source* packagecrayon...
** R
** inst
** tests
** preparing package for lazy loading
** help
*** installing help indices
** building package indices
** testing if installed package can be loaded
* DONE (crayon)
> sessionInfo()
R version 3.4.3 (2017-11-30)
Platform: x86_64-apple-darwin15.6.0 (64-bit)
Running under: macOS Sierra 10.12.2

Matrix products: default
BLAS: /System/Library/Frameworks/Accelerate.framework/Versions/A/Frameworks/vecLib.framework/Versions/A/libBLAS.dylib
LAPACK: /Library/Frameworks/R.framework/Versions/3.4/Resources/lib/libRlapack.dylib

locale:
[1] en_US.UTF-8/en_US.UTF-8/en_US.UTF-8/C/en_US.UTF-8/en_US.UTF-8

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

loaded via a namespace (and not attached):
 [1] httr_1.3.1      compiler_3.4.3  R6_2.2.2        tools_3.4.3     withr_2.1.1     curl_3.1       
 [7] yaml_2.1.16     memoise_1.1.0   git2r_0.21.0    digest_0.6.15   devtools_1.13.5
@dracodoc

This comment has been minimized.

dracodoc commented Mar 13, 2018

@jimhester I think I found the problem.

I clone the package, added browser() in various place to track down the problem.

This line remote_sha <- remote_sha(remote) didn't return a valid sha value from remote.

Browse[1]> remote
$host
[1] "https://api.github.com"

$repo
[1] "crayon"

$subdir
NULL

$username
[1] "r-lib"

$ref
[1] "master"

$sha
NULL

$auth_token
NULL

attr(,"class")
[1] "github_remote" "remote"      

Browse[1]> remote_sha(remote)
[1] NA
Browse[1]> remote_sha.github_remote(remote)
[1] NA

It's because this line has error

Browse[1]> git2r::remote_ls(
+       paste0(url, "/", remote$username, "/", remote$repo, ".git"))
Error in git2r::remote_ls(paste0(url, "/", remote$username, "/", remote$repo,  : 
  Error in 'git2r_remote_ls': SSL error: error:1407742E:SSL routines:SSL23_GET_SERVER_HELLO:tlsv1 alert protocol version

This could be platform dependent, so it only generate error for some users, like @mrustl , myself, and another issue #1731 created today by @carlganz .

This remote_sha.github_remote function only appeared in devtools, not in remotes. I used the code in remotes to replace this line (remotes don't have this problem for me)

  # remote_sha <- remote_sha(remote)
  remote_sha <- github_commit(remote$username, remote$repo, remote$ref)$sha

and the problem is fixed for me.

> devtools::install_github("r-lib/crayon")
Skipping install of 'crayon' from a github remote, the SHA1 (95b3eae3) has not changed since last install.
  Use `force = TRUE` to force installation

Of course the github_commit is only used to prove the problem cause is here, I'm not sure if it's the best solution. However to check sha from remote, this seemed to be the only alternative other than git2r, and git2r may have more dependency issues especially with SSL error.

By the way, I noticed there were quite some difference between devtools and remotes in this section now

  • remote_metadata.github_remote in remote is using different sha fetching methods
  • remotes don't has the remote_sha.github_remote function
  • and the commit history in two packages in this aspect are in quite different directions.

I think devtools is trying to diaspora this part to remotes, so this could be a difficult task given the differences now.

@dracodoc

This comment has been minimized.

dracodoc commented Mar 13, 2018

It's also possible the cause I found is not the same cause for @mrustl and @carlganz , because they only have problem in a special case, but I'm having the problem in general case. However I think the places I found should be the most possible location.

Because of this, and the complexity of remotes and devtools difference, I didn't create a PR.

I did create a branch in my local fork, so @mrustl and @carlganz can test with that to see if it will solve your problem. Just install with

> devtools::install_github("dracodoc/devtools", ref = "sha-checking")
Downloading GitHub repo dracodoc/devtools@sha-checking
from URL https://api.github.com/repos/dracodoc/devtools/zipball/sha-checking
Installing devtools
Skipping install of 'crayon' from a github remote, the SHA1 (95b3eae3) has not changed since last install.
  Use `force = TRUE` to force installation
Skipping install of 'enc' from a github remote, the SHA1 (6b0171d9) has not changed since last install.
  Use `force = TRUE` to force installation
Skipping install of 'memoise' from a github remote, the SHA1 (611cfadb) has not changed since last install.
  Use `force = TRUE` to force installation
....

The installation process already proved the problem is fixed for me.

@dracodoc

This comment has been minimized.

dracodoc commented Mar 13, 2018

@jimhester I further tracked to your original PR for this feature #903 when I was reading a similar issue in remotes .

My hack above is not optimal because of the github api limit concern mentioned by you. So I checked again. Maybe we should use SSH url instead of HTTPS?

remote_sha.github_remote <- function(remote, url = "git@github.com:", ...)

And this worked for me

> devtools::install_github("r-lib/crayon")
Called from: remote_sha.github_remote(remote)
Browse[1]> url = "https://github.com"
Browse[1]> git2r::remote_ls(
+       paste0(url, "/", remote$username, "/", remote$repo, ".git"))
Error in git2r::remote_ls(paste0(url, "/", remote$username, "/", remote$repo,  : 
  Error in 'git2r_remote_ls': SSL error: error:1407742E:SSL routines:SSL23_GET_SERVER_HELLO:tlsv1 alert protocol version
Browse[1]> url = "git@github.com:"
Browse[1]> git2r::remote_ls(
+       paste0(url, "/", remote$username, "/", remote$repo, ".git"))
                                      HEAD             refs/heads/fix/data-env-travis 
"95b3eae38cdb199fa9fe0db8810e03f45bca0746" "5bce6bad742a569606260652baa41ab45e1ece3e" 
                         refs/heads/master                          refs/heads/travis 

I'm not sure if this is a git2r problem (it could be because many people seemed to start have this problem recently in various cases), and if the SSH hack is a good solution.

Maybe we can try both methods if one failed, and give some error message when there is error, so it'll be easy to locate the problem.

@dracodoc

This comment has been minimized.

dracodoc commented Mar 13, 2018

I attempted with this, it's kind of ugly because I nested trycatch for 3 times. I don't know if there is a better method to do this.

Basically I try HTTPS url first, then SSH (the order may be switched if SSH is more reliable, I used this order because I knew HTTPS will fail for me so I can test the fallback), then github api. Any one failed will fall to next method. If everything failed there will be a warning.

This is not complete because url parameter is not used, and install_bitbucket need a similar function. I can modify the url parameter to make it work on both github and bitbucket if this solution is approved.

@dracodoc

This comment has been minimized.

dracodoc commented Mar 13, 2018

When I run the tests with check in rstudio (test package doesn't seem to run all tests, I didn't find out how to run them separately yet, because the functions are mostly not exported), these tests failed

     ══ testthat results  ═══════════
     OK: 116 SKIPPED: 5 FAILED: 7
     1. Error: SHA for regular repository (@test-git.R#15) 
     2. Error: SHA for detached head (@test-git.R#24) 
     3. Error: git (non-)usage is detected, diagnosed, and can be added (@test-github-connections.R#6) 
     4. Error: GitHub non-usage is handled (@test-github-connections.R#24) 
     5. Error: github info and links can be queried and manipulated (@test-github-connections.R#48) 
     6. Error: github_info() prefers, but doesn't require, remote named 'origin' (@test-github-connections.R#91) 
     7. Error: install on packages adds metadata (@test-remote-metadata.R#15) 

I think that's because some git related functions don't work in my machine, which could be a git2r problem.

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