Rebase `committer` should be actual user

Zendesk: https://gitlab.zendesk.com/agent/tickets/31598

When GitLab does a merge operation it specifically sets both the author and committer to the actual user, not the 'GitLab' user. This results in an accurate commit log. However, during a rebase, GitLab shells out and is unable to set the committer. In this case, the author appears to be the actual user but the commiter is the generic GitLab user. See the screenshot for an example - you see the rebase commit has a different committer while the merge commit has the actual user.

• Can we do the rebase in a way to preserve the 'actual' committer?

@vsizov Maybe you can comment since you have done lots with rebase. @DouweM Maybe you can comment on how we could set the 'right' committer either via shell out or if Rugged supports something we can use with rebase.

I think that's a correct behavior actually. If I do rebase for someone else's commit I will be a commiter right? but not the author. That's what GitLab does.

@vsizov Yes, in the case of a rebase I would expect that the committer could be different than the author. However, the committer should still be the actual GitLab user that clicked rebase, not the generic GitLab user. Right now we're ending up with the generic GitLab user.

I meant that GitLab actually does rebase and it's OK to set generic user. But after thinking a bit, I think that maybe you are right and that can be considered as a bug. Any other thoughts?

Sounds good. Thanks @vsizov

I agree this is a bug. It should be easy enough to set the git user name and email before we call git rebase at https://gitlab.com/gitlab-org/gitlab-ee/blob/master/app/services/merge_requests/rebase_service.rb#L44.

Milestone changed to %Backlog

Added ~91797 label

Customer also reported a different reason we need to fix this in https://gitlab.zendesk.com/agent/tickets/42250

When using the push rule to check if the author is a member of the team the rebase will fail with GitLab: Committer 'noreply@gitlab.example.com' is not a member of team. If we set the committer to the person executing the rebase, the problem will be solved.

Another report here: https://gitlab.zendesk.com/agent/tickets/53400 Adding SP3 label so it gets considered in the next scheduling call.

added ~1035183 label

mentioned in issue #1229 (closed)

This is getting lots of attention in a duplicate issue - https://gitlab.com/gitlab-org/gitlab-ee/issues/1229.

added ~1035178 and removed ~1035183 labels

@DouweM Can we fit this for 9.0? It's a fairly long-standing issue that's getting more customer attention.

@smcgivern Can you have a look here? Would this be easy with the new worktree approach?

I don't think the worktree approach has a direct impact here. I think it should be sufficient to set the committer details in the environment variables, as we do when squashing: https://gitlab.com/gitlab-org/gitlab-ee/blob/master/app/services/merge_requests/squash_service.rb#L45

added Deliverable ~874214 labels

changed milestone to %9.0

We'd need to check this, of course, but I know that git-rebase shells out to other git commands (git-apply or git-cherry-pick, depending on the arguments), so this should cover it.

@smcgivern Can we fit this for 9.0?

@dblessing it's deliverable, on the milestone, and - I think - fairly easy, so if it doesn't get done, either I'm wrong, or something has gone wrong

assigned to @jarka

mentioned in merge request !1341 (merged)

closed via merge request !1341 (merged)