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

Subdirectory support results in error for GitLab projects #181

Closed
cicdguy opened this issue Aug 2, 2023 · 1 comment · Fixed by #182
Closed

Subdirectory support results in error for GitLab projects #181

cicdguy opened this issue Aug 2, 2023 · 1 comment · Fixed by #182

Comments

@cicdguy
Copy link
Collaborator

cicdguy commented Aug 2, 2023

Subdirectory support works great for GitHub repos as seen here, since GitHub follows a standard convention of having no hierarchy for its repositories and one can reference a package in a subdirectory using the $organization/repository/subdirectory$ pattern.

However, this imposes a problem for GitLab projects. GitLab supports a hierarchical structure for its repositories (projects).

So if we have a configuration that looks like:

current_repo:
  repo: parent-group/sub-group/r.package
  host: https://gitlab.example.com
  host_type: gitlab

This results in the following error:

Copying local dir /builds/parent-group/sub-group/r.package to cache dir ~/.staged.dependencies/packages_cache/local_parent-group_sub-group_r.package_1234567commit89123456sha
Error in check_dir_exists(repo_dir, "deps_info: ") : 
  deps_info: Directory ~/.staged.dependencies/packages_cache/local_parent-group_sub-group_r.package_1234567commit89123456sha/r.package does not exist.
Calls: <Anonymous> ... rec_checkout_internal_deps -> lapply -> get_yaml_deps_info -> check_dir_exists

Proposed fix

Add support for an additional optional field in the staged_dependencies.yaml configuration that defines a new directive/key called subdir so that one can differentiate between a nested project in GitLab vs an actual subdirectory where the project is hosted. The default value of subdir should be "." and it should be an optional field.

Example:

current_repo:
  repo: organization/awesome-project
  host: https://github.com
  host_type: github
  subdir: awesomePackage
@pawelru
Copy link
Collaborator

pawelru commented Aug 2, 2023

This is a problem that already have a decent solution implemented elsewhere - https://r-lib.github.io/pkgdepends/reference/pkg_refs.html.

pkgdepends::parse_pkg_ref("aa/bb")
#> $package
#> [1] "bb"
#> 
#> $username
#> [1] "aa"
#> 
#> $repo
#> [1] "bb"
#> 
#> $subdir
#> [1] ""
#> 
#> $commitish
#> [1] ""
#> 
#> $pull
#> [1] ""
#> 
#> $release
#> [1] ""
#> 
#> $ref
#> [1] "aa/bb"
#> 
#> $type
#> [1] "github"
#> 
#> $params
#> character(0)
#> 
#> attr(,"class")
#> [1] "remote_ref_github" "remote_ref"        "list"
pkgdepends::parse_pkg_ref("dd=aa/bb/cc/dd")
#> $package
#> [1] "dd"
#> 
#> $username
#> [1] "aa"
#> 
#> $repo
#> [1] "bb"
#> 
#> $subdir
#> [1] "cc/dd"
#> 
#> $commitish
#> [1] ""
#> 
#> $pull
#> [1] ""
#> 
#> $release
#> [1] ""
#> 
#> $ref
#> [1] "dd=aa/bb/cc/dd"
#> 
#> $type
#> [1] "github"
#> 
#> $params
#> character(0)
#> 
#> attr(,"class")
#> [1] "remote_ref_github" "remote_ref"        "list"
pkgdepends::parse_pkg_ref("dd=git::https://gitlab.example.com/aa/bb/cc/dd")
#> $package
#> [1] "dd"
#> 
#> $protocol
#> [1] "https"
#> 
#> $host
#> [1] "gitlab.example.com"
#> 
#> $path
#> [1] "/aa/bb/cc/"
#> 
#> $repo
#> [1] "dd"
#> 
#> $commitish
#> [1] "HEAD"
#> 
#> $ref
#> [1] "dd=git::https://gitlab.example.com/aa/bb/cc/dd"
#> 
#> $type
#> [1] "git"
#> 
#> $dotgit
#> [1] ""
#> 
#> $url
#> [1] "https://gitlab.example.com/aa/bb/cc/dd"
#> 
#> $params
#> character(0)
#> 
#> attr(,"class")
#> [1] "remote_ref_git" "remote_ref"     "list"

Created on 2023-08-02 with reprex v2.0.2

Everything we need can be achieved with an input as a simple string rather than a list which is then processed by regular expressions. Honestly speaking, I was thinking about that during my recent feature development but I resigned because this is going to be a breaking change that would be quite painful to process in all of our repos. But the truth is that either way would be a breaking change.

Let me have a deeper look at the implementation again.

@pawelru pawelru linked a pull request Aug 3, 2023 that will close this issue
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 a pull request may close this issue.

2 participants