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

Fix gitlab url for subgroups #699

Closed
wants to merge 1 commit into from
Closed

Conversation

galachad
Copy link
Contributor

@galachad galachad commented Jan 9, 2023

Another gitlab bug to fix cc: @aronatkins.

This change allow install packages from subgroups.

Example:
https://gitlab.com/main518/child/skeleton

Minimal reproduce example:

# remotes::install_gitlab("main518/child/skeleton")

library(shiny)
library(skeleton)

ui <- fluidPage(

)

server <- function(input, output) {

}

shinyApp(ui = ui, server = server)

The / needs to be replace with %2F for RemoteRepo.

> packageDescription("skeleton")
Package: skeleton
Title: A skeleton R package.
Version: 1.0.1
Author: Kevin Ushey
Description: Doesn't really do anything.
Depends: R (>= 3.0.1)
License: GPL (>= 2)
Encoding: UTF-8
LazyData: true
RemoteType: gitlab
RemoteHost: gitlab.com
RemoteRepo: child/skeleton
RemoteUsername: main518
RemoteRef: HEAD
RemoteSha: 958296dbbbf7f1d82f7f5dd1b121c7558604809f
NeedsCompilation: no
Packaged: 2023-01-09 13:25:41 UTC; forysa
Built: R 4.0.5; ; 2023-01-09 13:25:41 UTC; unix

-- File: /Library/Frameworks/R.framework/Versions/4.0/Resources/library/skeleton/Meta/package.rds 

@aronatkins aronatkins requested a review from toph-allen January 9, 2023 14:36
@@ -71,7 +71,7 @@ gitlabArchiveUrl <- function(pkgRecord) {
archiveUrl <- sprintf(fmt,
pkgRecord$remote_host,
pkgRecord$remote_username,
pkgRecord$remote_repo,
sub("/", "%2F", pkgRecord$remote_repo),
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, @galachad

Note to @toph-allen --

Per https://docs.gitlab.com/ee/api/repositories.html#get-file-archive, the :id needs to be URL-encoded. We may want to use this opportunity to escape other characters, as well.

URLencode(pkgRecord$remote_repo, TRUE)

Additionally, we should update NEWS.

Copy link
Contributor

@toph-allen toph-allen Jan 9, 2023

Choose a reason for hiding this comment

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

Yeah, we should use URLencode here, I think.

I'm also going to check GitHub and Bitbucket to see if we should treat them similar.

We may also want to add this case to the gitlabArchiveUrl test.

Thanks for submitting this, @galachad! I may open a new branch on rstudio/packrat to make the above changes, and if I do that, I'll close this PR with a reference to that.

@aronatkins
Copy link
Contributor

Replaced by #700.

@aronatkins aronatkins closed this Jan 9, 2023
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