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

Introduce Gitlab support #524

Merged
merged 6 commits into from
Jun 27, 2019

Conversation

daddykotex
Copy link
Contributor

The problem GitHub and Gilab solves is similar (if not the same). So
most of the things apply to both. But somethings are different.

In GitHub, requests for changes are called Pull Requests (PR). In Gitlab,
they are called Merge Requests (MR). The rest of this PR will use both
in alternance. I'm used to Gitlab because of work so I use MR a lot :P.

  1. Authentication

This is a minor difference, in one case, GitHub uses Basic authorization
with the username/password. In the other case, GitLab uses a private token.
As I explained in this post,
the token can be generated from your GitLab settings page.
The token needs API permission.

  1. Listing merge requests

The GitHub API exposes a list pull requests endpoint where you can explicitly
filter by owner and branch name with the head and base parameter. In GitLab,
there is source_branch and author_id. Right now this PR sets the source branch
only. I think it needs to be addressed. The problem is that we have the username
(string) but the author_id has to be an integer.

See more here: https://docs.gitlab.com/ee/api/merge_requests.html#list-merge-requests.

I have two solutions in mind:

  • ask the user to provide the id
  • fetch from the user's profile

The first approach is easy but also error prone (provide an id that does
not match the token...).

The second one requires an additional request for some operations but would
look like that:

→ curl -s -H "Private-Token: $TOKEN" https://gitlab.com/api/v4/users?username=daddykotex-scalasteward | jq .
[
  {
    "id": 4014258,
    "name": "David Francoeur",
    "username": "daddykotex-scalasteward",
    "state": "active",
    "avatar_url": "https://secure.gravatar.com/avatar/09e751aadcb536cdd02f50832b3085dd?s=80&d=identicon",
    "web_url": "https://gitlab.com/daddykotex-scalasteward"
  }
]
  1. Creating a pull request

On GitHub, to create a pull request you post on the upstream project. For example,
if you were to create a pull request on scala-steward, you'd do:

curl -XPOST '/repos/fthomas/scala-steward/pulls -d $BODY'

The body contains the base which is the branch in which the changes
will be merged and the head which is where the changes are implemented.
An example would be: base => master and head => scala-steward:update/sbt-site-1.4.0

On Gitlab, it's different. The request has o target the fork instead of
the upstream repository. So if you'd like to open a similar merge request on
Gitlab, your request would look like that:

curl -XPOST '/repos/scala-steward/http4s/merge_requests -d $BODY'

Where the body contains target_project_id, source_branch, target_branch.
The target_project_id is an integer, and just like the user, we don't have it,
so we have to fetch it first via an extra request. The source_branch and target_branch
have explicit names. An example would be: target_project_id => 123,
source_branch => update/sbt-site-1.4.0 and target_branch => master.

  1. Creating a fork

Creating a fork is similar but there are a few differences. On GitHub,
you just POST on the upstream repository /forks endpoint. On Gitlab,
you post to a similar endpoint but you provide a body. The body contains
an id and a namespace, like so:

{ "id": "scala-steward%2Fhttp4s", "namespace": "scala-steward"}

The id the is the owner/repo url encoded and the namespace is the username.

It also appeared (from my testing), the scala-steward tries to create fork
everytime it runs, and GitHub is just kind enough to say 202 Accepted even if
the thing is already existing. On Gitlab, you are rejected with a 409 Conflict.
This is why I added a MonadThrowable constraint so that I could catch the error
and recover from it. An alternative solution would be to check it before hand.

  1. Pull request state

It's a minor difference, but on GitHub you have open and closed where
as Gitlab has opened and closed.


There are probably things I went over and this pull requests is probably
not 100% working but I ran it a few times and I can say that:

  • it can fork a repository
  • it can open a MR
  • it can see when a MR was closed and not reattempt the update

@daddykotex daddykotex changed the base branch from master to topic/gitlab May 26, 2019 16:51
@daddykotex
Copy link
Contributor Author

Note that this is a work in progress, it's not ready to be merged. There are a bunch of tests that I want to add, I just want to discuss things a little bit before doing so to avoid having to rewrite them.

@mihaisoloi
Copy link

@daddykotex I would rename this PR to Introduce Gitlab support this way it's easier for people to understand what it does just from the title.

Great work btw 👍 I'm going to give this a spin this week

@daddykotex daddykotex changed the title Refactor/vcsapialg Introduce Gitlab support May 28, 2019
@fthomas
Copy link
Member

fthomas commented May 28, 2019

I didn't had the time yet to take a close look at all changes but the first thing I noticed is that working with GitLab or GitHub is configured at start-up of a scala-steward instance. Have you considered making this choice on a per-repo basis so that one instance can work with GitHub- and GitLab-hosted repos at the same time? For people like me who run a public instance it would be easier to support both this way.

I'm not suggesting that you change your PR, but would like to hear your opinion if this would be feasible and if it would be small or big change compared to this PR.

@daddykotex
Copy link
Contributor Author

Have you considered making this choice on a per-repo basis so that one instance can work with GitHub- and GitLab-hosted repos at the same time?

I did not think about it but it's a good suggestion. From the top of my head, the problem that would occur would be: you need different configuration for the API (user/pass/baseurl) when you are targeting different VCS.

The most obvious solution to me is to transform the repos.md into a repos.json that has more configuration. Something like:

{
  "github-public": {
    "user": "scala-steward",
    "pass": "/home/user/.github/askpass.sh",
    "url": "https://api.github.com",
    "repos": [
      "tpolecat/skunk",
      "travisbrown/catbird",
      "travisbrown/iteratee",
      "travisbrown/iteratee-twitter",
      "twilio/guardrail",
      "typelevel/cats",
      "typelevel/cats-collections",
      "typelevel/ca"
    ]
  },
  "gitlab-public": {
    "user": "scala-steward",
    "pass": "/home/user/.gitlab/askpass.sh",
    "url": "https://gitlab.com/api/v4",
    "repos": [
      "other/random-repo"
    ]
  }
}

But I think it's content for another PR 😄

@fthomas
Copy link
Member

fthomas commented Jun 1, 2019

But I think it's content for another PR

Agreed! Just wanted to hear your opinion, so thanks for that. :-)

@daddykotex
Copy link
Contributor Author

Also, with Gitlab I'm used to take comments into account by updating my commits. Gitlab UI is good enough to figure this out and show things correctly. I'm not sure if GitHub does the same but if that's a problem let me know, I'll just add commits on top of the other instead of rewriting.

@daddykotex
Copy link
Contributor Author

Since I don't have many comments on the changes I propose, I'll add more tests. I did not want to do it in the first place to avoid rewriting that.

@fthomas
Copy link
Member

fthomas commented Jun 23, 2019

@daddykotex I just merged master into the topic/gitlab branch because there has been an important bugfix recently (#552) which changed code that is also changed here. Unfortunately there also a few other conflicts that need to be resolved. Once all conflicts are resolved, I'll try to merge it in a timely manner.

This refactoring is breaking for users:
* args change from --github-api-host to --vcs-api-host
* args change from --github-login to --vcs-login
@codecov
Copy link

codecov bot commented Jun 26, 2019

Codecov Report

Merging #524 into topic/gitlab will decrease coverage by 8.44%.
The diff coverage is 21.87%.

Impacted file tree graph

@@               Coverage Diff                @@
##           topic/gitlab     #524      +/-   ##
================================================
- Coverage         60.11%   51.66%   -8.45%     
================================================
  Files                67       71       +4     
  Lines               875      871       -4     
  Branches             23       43      +20     
================================================
- Hits                526      450      -76     
- Misses              349      421      +72
Impacted Files Coverage Δ
...teward/core/github/http4s/Http4sGitHubApiAlg.scala 66.66% <ø> (ø) ⬆️
...calasteward/core/vcs/data/NewPullRequestData.scala 84.61% <ø> (-1.1%) ⬇️
.../scala/org/scalasteward/core/application/Cli.scala 81.81% <ø> (-18.19%) ⬇️
...ala/org/scalasteward/core/application/Config.scala 0% <0%> (ø) ⬆️
...org/scalasteward/core/github/GitHubSpecifics.scala 0% <0%> (ø)
...la/org/scalasteward/core/application/Context.scala 0% <0%> (ø) ⬆️
.../main/scala/org/scalasteward/core/gitlab/Url.scala 0% <0%> (ø)
...alasteward/core/dependency/DependencyService.scala 0% <0%> (ø) ⬆️
...org/scalasteward/core/gitlab/GitlabSpecifics.scala 0% <0%> (ø)
...ala/org/scalasteward/core/nurture/NurtureAlg.scala 0% <0%> (ø) ⬆️
... and 41 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a7a5a57...f72e56a. Read the comment docs.

@codecov
Copy link

codecov bot commented Jun 26, 2019

Codecov Report

Merging #524 into topic/gitlab will decrease coverage by 8.44%.
The diff coverage is 21.87%.

Impacted file tree graph

@@               Coverage Diff                @@
##           topic/gitlab     #524      +/-   ##
================================================
- Coverage         60.11%   51.66%   -8.45%     
================================================
  Files                67       71       +4     
  Lines               875      871       -4     
  Branches             23       43      +20     
================================================
- Hits                526      450      -76     
- Misses              349      421      +72
Impacted Files Coverage Δ
...teward/core/github/http4s/Http4sGitHubApiAlg.scala 66.66% <ø> (ø) ⬆️
...calasteward/core/vcs/data/NewPullRequestData.scala 84.61% <ø> (-1.1%) ⬇️
.../scala/org/scalasteward/core/application/Cli.scala 81.81% <ø> (-18.19%) ⬇️
...ala/org/scalasteward/core/application/Config.scala 0% <0%> (ø) ⬆️
...org/scalasteward/core/github/GitHubSpecifics.scala 0% <0%> (ø)
...la/org/scalasteward/core/application/Context.scala 0% <0%> (ø) ⬆️
.../main/scala/org/scalasteward/core/gitlab/Url.scala 0% <0%> (ø)
...alasteward/core/dependency/DependencyService.scala 0% <0%> (ø) ⬆️
...org/scalasteward/core/gitlab/GitlabSpecifics.scala 0% <0%> (ø)
...ala/org/scalasteward/core/nurture/NurtureAlg.scala 0% <0%> (ø) ⬆️
... and 41 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a7a5a57...83e9755. Read the comment docs.

This is a work in progress. There are a lot of things to discuss:

* the custom encoder/decoder to convert between GitHub json responses
and Gitlab json responses
* the Monad/MonadThrowabled requirements added because some operations
in Gitlab are multi-step where as their original Github conterpart
are a single step
* the specific vcs thingy
@fthomas fthomas merged commit 3024e0e into scala-steward-org:topic/gitlab Jun 27, 2019
@daddykotex
Copy link
Contributor Author

Thanks

@sbrunk
Copy link

sbrunk commented Jun 28, 2019

That's awesome! I'll test it with our internal GitLab very soon.

@fthomas
Copy link
Member

fthomas commented Jun 28, 2019

Note that the changes in this PR are not yet on master.

@fthomas fthomas mentioned this pull request Jun 29, 2019
@kubukoz
Copy link
Contributor

kubukoz commented Jul 1, 2019

I just checked out the topic/gitlab branch and tried this with my team's internal gitlab, but I was getting 404s (logged in Check dependencies of...)... I'll try to investigate further given some more time...

@fthomas
Copy link
Member

fthomas commented Jul 1, 2019

I've opened #645 to track remaining issues before GitLab support can be merged into master.

@daddykotex
Copy link
Contributor Author

daddykotex commented Jul 2, 2019

@kubukoz

Hey,

Thanks for the feedback. I've got a few questions:

  • what version of GitLab are you running?
  • can you run with the debug flag and share the output with us? (maybe hide internal information)
  • can you share the arguments you passed to main?

Thanks, I must admit that I did not retry it after the last rebase, maybe I did a mistake.

@daddykotex
Copy link
Contributor Author

To help you (so you don't have to look for it), here are the arguments I use when I run this on our internal GitLab installation:

    -d \
    --workspace "/data/workspace/" \
    --repos-file "/data/repos.md" \
    --git-author-name "Scala Steward" \
    --git-author-email "dfrancoeur+steward@mnubo.com" \
    --vcs-type "gitlab" \
    --vcs-api-host "https://gitlab.company.com/api/v4/" \
    --vcs-login "scala-steward" \
    --git-ask-pass "/data/pass.sh"
  • -d for debug
  • --workspace is a directory in which scala-steward checks out the code and run it's things
  • --vcs-type "gitlab" to choose gitlab over github
  • --git-ask-pass "/data/pass.sh" points to an executable that print the password to stdout
  • --vcs-api-host "https://gitlab.com" is the host of the API, if you use gitlab.company.com to access the webpage, don't forget to add the /api/v4 at the end of it

@fthomas fthomas added the enhancement New feature or request label Jul 4, 2019
@fthomas fthomas added this to the 0.3.0 milestone Jul 4, 2019
@fthomas
Copy link
Member

fthomas commented Jul 4, 2019

Note that GitLab support has now landed in master (via #645).

Many thanks again to @daddykotex!

@fthomas fthomas mentioned this pull request Jul 4, 2019
@fthomas
Copy link
Member

fthomas commented Jul 9, 2019

I just tried this with our GitLab server at work and it works like a charm! This is so cool, @daddykotex! :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants