Skip to content

Fix gitlab url for subgroups#699

Closed
galachad wants to merge 1 commit intorstudio:mainfrom
galachad:fix-gitlab-url
Closed

Fix gitlab url for subgroups#699
galachad wants to merge 1 commit intorstudio:mainfrom
galachad:fix-gitlab-url

Conversation

@galachad
Copy link
Copy Markdown
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
pkgRecord$remote_host,
pkgRecord$remote_username,
pkgRecord$remote_repo,
sub("/", "%2F", pkgRecord$remote_repo),
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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