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

Updating GitLab url parsing; fix authed list_refs #353

Merged
merged 9 commits into from
Apr 4, 2024

Conversation

dgkf
Copy link
Contributor

@dgkf dgkf commented Mar 2, 2024

This PR is intended to fix #328 with a few other related fixes

  • Updates parsing for GitLab remotes, which currently is forgetting a / between the host and "username". Unlike GitHub, GitLab allows arbitrary nesting of groups:

    Service Style
    GitHub username/repo
    GitLab group/subgroup/subsubgroup/project

    This throws a wrench in the subdir parsing. To allow a subdir, I added a /-/ separator:
    group/subgroup/project/-/sub/dir@ref
    /-/ is a reserved name used for other server paths, so there's no risk of conflict with project paths.

  • Calls to /info/refs?service=git-upload-pack require authentication, which is currently ignored. I added this authentication to async_git_list_refs_v2 and async_git_send_message_v2, necessary for solving package dependencies.

Let me know if I'm on the right track, or if you think this is better solved some other way.

@dgkf dgkf changed the title WIP: Updating GitLab url parsing; fix authed ls-remotes WIP: Updating GitLab url parsing; fix authed list_refs Mar 2, 2024
@dgkf
Copy link
Contributor Author

dgkf commented Mar 4, 2024

Since GitLab can have arbitrarily nested projects and some alternative syntax would be needed to differentiate user/repo/sub/dir from /group/subgroup/subgroup2/subgroup3, would an alternative syntax more akin to Authors@R be up for consideration?

Remotes:
  list(name = "pkg", type = "gitlab", url = "https://acme.co/a/b/c.git", subdir = "x/y/z", ref = "dev")

I appreciate that the current string style is quite brief, but maybe something more explicit would be a nice fallback?

@dgkf dgkf changed the title WIP: Updating GitLab url parsing; fix authed list_refs Updating GitLab url parsing; fix authed list_refs Mar 6, 2024
@gaborcsardi
Copy link
Member

Thanks! I changed this a bit to add auth to all git HTTP queries. There is two things to do still:

  • cache unsuccessful gitcreds queries, because they take about 0.2s now, and could take even longer with different git credential helpers,
  • write tests.

@gaborcsardi
Copy link
Member

would an alternative syntax more akin to Authors@R be up for consideration?

That is not great, because other tools will interpret the Remotes field differently. It also does not play well with other, non-R tools that try to use the DESCRIPTION file.

This throws a wrench in the subdir parsing. To allow a subdir, I added a /-/ separator: group/subgroup/project/-/sub/dir@ref

Yes, this could work.

@gaborcsardi
Copy link
Member

Actually, some the git and gitlab tests are already "live", so we can update those later, and I am going to merge this now, after updating the docs.

@gaborcsardi gaborcsardi merged commit 316d7fb into r-lib:main Apr 4, 2024
12 checks passed
@gaborcsardi
Copy link
Member

Thanks you again!

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Oct 19, 2024
# pak 0.8.0

* `pkg_deps()` now accepts a vector of package names.

* The metadata cache now does not use source URLs for packages in `Archive`
  on Posit Package Manager repositories. This URLs may serve a different
  package, even a source package when the main URL for the same package
  serves a binary package. The alternative URLs are not needed on PPM,
  anyway, because PPM is in a consistent state w.r.t. metadata and
  package files (#623).

* pak now supports `gitlab::` package sources better, by adding
  explicit syntax to specify subdirectories (r-lib/pkgdepends#353, @dgkf).

* `gitlab::` and `git::` package sources now support git submodules if
  the `git-submodules` configuration option is set to `TRUE`. See
  `?"pak-config"` (r-lib/pkgdepends#354).

* The new `?ignore-unavailable` parameter makes it easy to ignore soft
  dependencies that are unavailable (#606).

* pak now automatically ignores soft dependencies that have an
  incompatible OS type (`OS_type` entry in `DESCRIPTION`) when installing
  packages.

* `repo_add()` and the `ppm_*()` functions, e.g. `ppm_snapshots()`, now
  work again after the PPM API changes
  (r-lib/pkgcache#110,
   r-lib/pkgcache#115).

# pak 0.7.2

* pak now supports using parameters for all packages with the
  `*=?<param>` form. E.g. `*=?source` installs all packages from source.

* pak now supports R 4.4.0 again, and also Rtools44.
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.

Incorrect parse result of gitlab ref
2 participants