Fix remote_download url to use GitLab API

[aornugent]

install_gitlab uses the GitLab API to get commit IDs but not for downloading the repository. This causes it to fail on private repositories, see this issue on the Rstudio Discourse.

[dpprdan]

Nice! And such an easy fix.
However, the ref parameter is ignored at the moment, i.e.

install_gitlab("user/repo@non-default-branch")

will download and install from the default branch (usually master), so the @non-default-branch will be ignored.

According to the Repository API docs, the branch (or tag or SHA) has to be specified with the sha parameter.

Long story short, line 74

src <- paste0(src_root, "/repository/archive.tar.gz?ref=", utils::URLencode(x$ref, reserved = TRUE))

probably ought to be

src <- paste0(src_root, "/repository/archive.tar.gz?sha=", utils::URLencode(x$ref, reserved = TRUE))

Installing from a subdir like e.g.

install_gitlab("dpprdan/rpostgresql/RPostgreSQL")

works BTW, even though it apparently downloads the whole repo and not only the subdir (Update: yes, that's what source_pkg does).

[aornugent]

Thanks for the feedback. If the API is after a commit ID is there any reason to give it ref over sha? It gets fetched anyway and I can't see any obvious consequences of adding it to the remote, but I am only across the GitLab section of this package.

[dpprdan]

Sorry, I don't follow.

If the API is after a commit ID is there any reason to give it ref over sha?

Again, not sure if you meant this, but FWIW, I find Gitlab's API parameters in this particular context a bit confusing. For the /projects/:id/repository/archive endpoint, the sha parameter "[a] tag, branch reference, or [commit] SHA can be used." For the "list repository tree" (the /projects/:id/repository/tree endpoint) the ref parameter specifies the same things (it only mentions tag or branch name in the docs, but a commit SHA works as well).

For the ...archive endpoint ref is not defined and effectively ignored, so "This defaults to the tip of the default branch if not specified."

It gets fetched anyway and I can't see any obvious consequences of adding it to the remote, but I am only across the GitLab section of this package.

Well, if you want to install a branch other than the default (usually master) then you would have to specify this as @other_branch in the remote argument of install_gitlab. install_github also has a ref argument, where you can specify the branch, tag or commit as well (but apparently also in repo).

[aornugent]

Maybe I over complicated things sorry. I understood what you were suggesting, but thought that using the commit SHA made for a better looking API call than:

>install_gitlab("jimhester/covr@child_expressions")
Downloading GitLab repo jimhester/covr@child_expressions
from URL https://gitlab.com/api/v4/projects/jimhester%2Fcovr/repository/archive.tar.gz?sha=child_expressions

But that's not an important metric to maximise. I've reverted the commit and adopted your suggestions.
Thanks for your help.

[jimhester]

Thanks for working on this!

@dpprdan Do the current changes look good to you?

@aornugent
Can you please add a bullet to NEWS? It should briefly describe the change and end with (@yourname, #issuenumber).

[dpprdan]

@jimhester Looking good so far.

However, the installation is not skipped, when local_sha is equal to remote_sha. The root of this is that remote_package_name.gitlab_remote() does not return a package name for non-public packages, because it does not use the Gitlab API (as was the case with remote_download.gitlab_remote(), originally).

The consequence of this is that there is no package_name (also because tryCatch() does not fall back on remote$repo) -> there is no local_sha -> Installation cannot be skipped, because local_sha cannot be equal to remote_sha.

See this commit for the necessary changes. I can open a PR but you can also just copy it from there if you like, @aornugent.

Other than that, everything looks fine to me, including installing from private repos with @ref or /subdir.

[dpprdan]

@jimhester Two OT suggestions:

1. Since the example is install_gitlab("jimhester/covr"), would you consider mirroring the github repo to gitlab?
2. One thing I'd like to see on your Youtube series: How to debug a function that makes use of *apply/purrr::map*, e.g. via debugonce(). 😄

[aornugent]

See this commit for the necessary changes. I can open a PR but you can also just copy it from there if you like, @aornugent.

Let's get it done in one.

[jimhester]

Ok one last thing, can you run make in the package root so that the install-github.R script is updated? This should also fix the failing tests on the CI.

[jimhester]

Thank you to @aornugent for the PR and for @dpprdan for improvements and reviewing the changes!

Great work, this package is now better thanks to you!

[aornugent]

Oh no, we missed a bug here: #355 (comment)

This line

remotes/R/install-gitlab.R

Line 122 in 92f3064

"/raw?ref=", remote$ref, "&private_token=", remote$auth_token)

doesn't need the access token at the end as it is added in the download call on line 125. I must've forgotten to test on a private repo.

What the best way to remedy this? A second PR or revert the merge?

[aornugent]

Actually, I'm not sure that the double token is the cause of the bug, but I don't have a failing example to test on.

[trafficonese]

In my case the double token is not the problem. I also get the "Not Found" error if I remove the second part. I dont know why, but our gitlab is not able to translate user/repo to project ID, with which it would work.

[dpprdan]

Sorry, the double token is my bad. But it does not cause the error, I've tested it successfully with a private repo.

I would fix it myself, but cannot get make to run successfully on Windows. @aornugent could you do that, please? IMO a second PR would be easier.

[jimhester]

Yes, please open a new PR with the fix

[aornugent]

I would fix it myself, but cannot get make to run successfully on Windows.

I ended up installing WSL + Ubuntu + R to get it running. See: https://itsfoss.com/install-bash-on-windows/

[dpprdan]

@aornugent Thanks! I do have make installed (part ot git for windows?), but I get an error while trying to run it.

Details

# First, what versions do I have?
$ make --version
GNU Make 4.2.1
Built for i686-pc-msys
Copyright (C) 1988-2016 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

Daniel@Notebook-DP  ~/Documents/R/github/remotes (master)
$ R --version
R version 3.5.3 (2019-03-11) -- "Great Truth"
Copyright (C) 2019 The R Foundation for Statistical Computing
Platform: x86_64-w64-mingw32/x64 (64-bit)

R is free software and comes with ABSOLUTELY NO WARRANTY.
You are welcome to redistribute it under the terms of the
GNU General Public License versions 2 or 3.
For more information about these matters see
http://www.gnu.org/licenses/.

# make cannot find the brew package
Daniel@Notebook-DP  ~/Documents/R/github/remotes (master)
$ make
Rscript -e 'brew::brew("inst/install-github.Rin", "inst/install-github.R")'
Error: object 'brew' not found

# But brew is installed?!
Daniel@Notebook-DP  ~/Documents/R/github/remotes (master)
$ Rscript -e 'library(brew);brew(system.file("example1.brew",package="brew"),envir=new.env())'
Title: brew test
current time: 2019-05-09 12:39:38
value of foo: bar
 i is 1
from example2.brew, foo is: bar1
 i is 2
from example2.brew, foo is: bar12
 i is 3
from example2.brew, foo is: bar123
 i is 4
from example2.brew, foo is: bar1234
 i is 5
from example2.brew, foo is: bar12345
 i is 6
from example2.brew, foo is: bar123456
 i is 7
from example2.brew, foo is: bar1234567
 i is 8
from example2.brew, foo is: bar12345678
 i is 9
from example2.brew, foo is: bar123456789
 i is 10
from example2.brew, foo is: bar12345678910