Couple updating MR merge ref and determining MR mergeability
The MR merge ref (refs/merge-requests/:iid/merge
) introduced in https://gitlab.com/gitlab-org/gitlab-ce/issues/47110 can be updated based on whatever is currently in the source and target branches using the MergeRequests::MergeToRefService
, but this does not currently happen automatically when the source or target branch are updated, and there is no way to know whether the current merge ref is "up to date", meaning it represents the merge of the current HEADs of the source and target branches.
This means that the merge ref can currently only be relied on if you always call MergeRequests::MergeToRefService
to recompute the merge just before using it, while this may not actually have been necessary at all.
I think we can reuse the MergeRequest merge_status
state machine to represent both the current status of the merge ref, and to enable automatic lazy recomputation whenever the source or target branches are updated.
The possible states of this state machine are unchecked
, cannot_be_merged_recheck
, can_be_merged
, and cannot_be_merged
.
Every MR starts out unchecked
, and will transition to can_be_merged
or cannot_be_merged
(depending on the result of repository.can_be_merged?(source_branch, target_branch)
) when #check_if_can_be_merged
is called from #mergeable?
and from Projects::MergeRequestsController#show
whenever the MR is viewed in the browser.
The state is transitioned back to unchecked
(when it was can_be_merged
before) or cannot_be_merged_recheck
(when it was cannot_be_merged
before) when #mark_as_unchecked
is called from MergeRequest::RefreshService
whenever the MR source or target branch is updated.
We can couple the merge status and merge ref concepts by extending #check_if_can_be_merged
to also update the merge ref whenever the result of repository.can_be_merged?(source_branch, target_branch)
is true
(which means we can remove the merge_reqeust.mergeable_to_ref?
check in MergeToRefService
which effectively checks the same thing) and by modifying MergeToRefService
to only recompute the merge if merge_status
is unchecked
or cannot_be_merged_recheck
, which indicates that the source or target branch was updated since the last time the merge ref was updated, and that the merge ref needs to be updated now.
Since MergeToRefService
should always succeed if repository.can_be_merged?(source_branch, target_branch)
(which is not currently the case, but will be after https://gitlab.com/gitlab-org/gitlab-ce/issues/58393), we can trust that the merge ref will always be up to date when merge_status
== can_be_merged
, and to be outdated (or non-existent) otherwise, because it simply may not have been updated yet after the latest source/target branch update (and merge_status
will be unchecked
or cannot_be_merged_recheck
) or because a merge may not have been possible at all (and merge_status
will be cannot_be_merged
).
All of this merge status and merge ref update and check behavior could be moved into a new MergeRequests::UpdateMergeRefService
that can return true
if the ref is up to date (because we just updated it, or because it was up to date already) or false
, if a merge was not possible (because we just attempted to recalculate it, or because it failed before already). Users of the merge ref, like the ~Verify team in https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/9622, would call this service (or an equivalent API action) and only read the merge ref if the result is true
.
@oswaldo What do you think?
If we can trust the merge ref to always be up to date if merge_status
== can_be_merged
, and if we do https://gitlab.com/gitlab-org/gitlab-ce/issues/58226 to make sure the merge ref HEAD will be identical to the target branch HEAD if we actually merged right now, we should be able to reimplement the MergeRequests::MergeService
as to use the merge ref: https://gitlab.com/gitlab-org/gitlab-ce/issues/58496.
Automatically updating the merge ref when the source/target branches are updated (lazily) should also help us with &854 (closed): after updating the merge ref, we could also automatically update the "dynamic" diff version that shows the difference between the target branch HEAD and the newly updated merge ref HEAD.
cc @dosuken123