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

Support for GitLab subgroups #308

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rozwad
Copy link

@rozwad rozwad commented Mar 10, 2019

A quick and dirty prototype workaround to address issue #259.

As the install_gitlab seems to not support (yet) subdirectories, the workaround is based on the subdir variable. However in case a repo is inside a subgroup there is a mismatch – subdir corresponds to a repository, and repo to a subgroup (result of the general parsing rules in parse_repo_spec). How about fixing that mismatch with a new dedicated parse_repo_gitlab function?

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

if (!quiet) {
message("Downloading GitLab repo ", x$username, "/", x$repo, "@", x$ref,
"\nfrom URL ", src)
message("Downloading GitLab repo ", gsub("/$", "", x$username, "/", x$repo, "/", x$subdir),
Copy link
Member

Choose a reason for hiding this comment

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

Are you actually getting a trailing slash in the subdirectory or just being cautious? In general I would consider specs with a trailing slash to be invalid input, and actually it seems parse_git_repo() strips them anyway, so you can remove the gsub here and below I think.

> parse_git_repo("foo/bar/baz/")
$username
[1] "foo"

$repo
[1] "bar"

$subdir
[1] "baz"

Copy link
Member

Choose a reason for hiding this comment

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

Oh now that I look at this again this handles the case when subdir is empty. I think a better approach is to use paste0(c(x$username, x$repo, x$subdir), collapse = "/")

@robertdj
Copy link
Contributor

robertdj commented Jul 4, 2019

I am also missing this functionality of installing packages from a path with three levels (not sure if it is a subgroup or subdirectory).

Having tried out this branch I still run into problems.

As far as I can see install_gitlab makes three calls to get the repository in path/to/repo:

The first two works for me, but the last one fails.
According to GitLab's documentation the last call should also be different -- using the same form as the second call:
https://gitlab.foo.com/api/v4/projects/path%2Fto%2Frepo/repository/archive.tar.gz?ref=master&private_token=

With this change it works for me.
@rozwad : Does this also work for you?

@kevinushey
Copy link
Contributor

IMHO we should avoid overloading the meaning of subdir (since a subgroup and folder within a repository are different things) and instead consider how to handle sub-directories and sub-groups.

I think it would be better to augment the parser so that there's a way to parse sub-directories and sub-groups separately (either globally or just a GitLab-only parser).

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.

4 participants