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

Fix merge base commit for fast-forwarded GitLab PRs #27825

Merged
merged 2 commits into from
Oct 29, 2023

Conversation

invliD
Copy link
Contributor

@invliD invliD commented Oct 29, 2023

Due to a bug in the GitLab API, the diff_refs field is populated in the response when fetching an individual merge request, but not when fetching a list of them. That field is used to populate the merge base commit SHA.
While there is detection for the merge base even when not populated by the downloader, that detection is not flawless. Specifically, when a GitLab merge request has a single commit, and gets merged with the squash strategy, the base branch will be fast-forwarded instead of a separate squash or merge commit being created. The merge base detection attempts to find the last commit on the base branch that is also on the PR branch, but in the fast-forward case that is the PR's only commit. Assuming the head commit is also the merge base results in the import of a PR with 0 commits and no diff.

This PR uses the individual merge request endpoint to fetch merge request data with the diff_refs field. With its data, the base merge commit can be properly set, which—by not relying on the detection mentioned above—correctly imports PRs that were "merged" by fast-forwarding the base branch.

ref: https://gitlab.com/gitlab-org/gitlab/-/issues/29620

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Oct 29, 2023
@pull-request-size pull-request-size bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Oct 29, 2023
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Oct 29, 2023
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Oct 29, 2023
@KN4CK3R KN4CK3R added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Oct 29, 2023
@lunny lunny merged commit 02dae3f into go-gitea:main Oct 29, 2023
25 checks passed
@GiteaBot GiteaBot added this to the 1.22.0 milestone Oct 29, 2023
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Oct 29, 2023
@invliD invliD deleted the sb/base-sha branch October 29, 2023 23:44
zjjhot added a commit to zjjhot/gitea that referenced this pull request Oct 30, 2023
* giteaofficial/main:
  List all Debian package versions in `Packages` (go-gitea#27786)
  Fix merge base commit for fast-forwarded GitLab PRs (go-gitea#27825)
  Fix bad method call when deleting user secrets via API (go-gitea#27829)
  Change katex limits (go-gitea#27823)
  Dockerfile small refactor (go-gitea#27757)
  Use GitLab's squash_commit_sha when available (go-gitea#27824)
  [skip ci] Updated translations via Crowdin
  Upgrade xorm to 1.3.4 (go-gitea#27807)
fuxiaohei pushed a commit to fuxiaohei/gitea that referenced this pull request Jan 17, 2024
Due to a bug in the GitLab API, the diff_refs field is populated in the
response when fetching an individual merge request, but not when
fetching a list of them. That field is used to populate the merge base
commit SHA.
While there is detection for the merge base even when not populated by
the downloader, that detection is not flawless. Specifically, when a
GitLab merge request has a single commit, and gets merged with the
squash strategy, the base branch will be fast-forwarded instead of a
separate squash or merge commit being created. The merge base detection
attempts to find the last commit on the base branch that is also on the
PR branch, but in the fast-forward case that is the PR's only commit.
Assuming the head commit is also the merge base results in the import of
a PR with 0 commits and no diff.

This PR uses the individual merge request endpoint to fetch merge
request data with the diff_refs field. With its data, the base merge
commit can be properly set, which—by not relying on the detection
mentioned above—correctly imports PRs that were "merged" by
fast-forwarding the base branch.

ref: https://gitlab.com/gitlab-org/gitlab/-/issues/29620
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Jan 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants