TypeError is raised when using fast-forward merge

[aidan-gallagher]

Describe the bug
Does not work on self hosted Gitlab instance. TypeError is raised.

To Reproduce

$ git-backporting -tb 202411 -pr https://privategitlab.org/org/cf-sonic-net/cf-sonic-buildimage/-/merge_requests/111 -a ************
[DEBUG] Setting up gitlab client: apiUrl=https://privategitlab.org/api/v4, token=****
[DEBUG] Parsing configs..
[DEBUG] Fetching pull request /projects/org%2Fcf-sonic-net%2Fcf-sonic-buildimage/merge_requests/111
[ERROR] TypeError: Cannot read properties of null (reading 'slice')
[INFO ] Process failed

Expected behavior
I expected it to not fail.

Additional context
If I take the URL it is trying to reach https://privategitlab.org/api/v4/projects/org%2Fcf-sonic-net%2Fcf-sonic-buildimage/merge_requests/111 and paste it into my browser I see Gitlab JSON.

I can also get the same data via the CLI by running curl -H "PRIVATE-TOKEN: *********" "https://privategitlab.org/api/v4/projects/org%2Fcf-sonic-net%2Fcf-sonic-buildimage/merge_requests/111"

[lampajr]

Hi @aidan-gallagher, thanks for creating the issue!

If you don't mind, I need a couple of infos to understand what could be wrong:

• Which version of the tool are you using?
• Is that something that was working before but now it is not anymore?

[lampajr]

Can you somehow share the result you would get from https://privategitlab.org/api/v4/projects/org%2Fcf-sonic-net%2Fcf-sonic-buildimage/merge_requests/111 hiding anything private or that you cannot share?

[aidan-gallagher]

Hi @aidan-gallagher, thanks for creating the issue!

If you don't mind, I need a couple of infos to understand what could be wrong:

• Which version of the tool are you using?
• Is that something that was working before but now it is not anymore?

I am using version 4.8.6 and this is the first time I am using this tool.

$ git-backporting --version
4.8.6

I will email you the other information.

[lampajr]

Ok thanks 🙏🏻

I was not able to reproduce it yet, but looking at the code my suspect is that for some reason the list of commits contains some undefined value:

git-backporting/src/service/configs/pullrequest/pr-configs-parser.ts

Line 142 in 1a106f1

const concatenatedCommits: string = originalPullRequest.commits!.map(c => c.slice(0, 7)).join("-");

which seems the only place where .slice is being used and that could explain the error.

The commits are fetched with an additional request, if squash is undefined:

git-backporting/src/service/git/gitlab/gitlab-client.ts

Line 63 in 1a106f1

const { data } = await this.client.get(`/projects/${projectId}/merge_requests/${mrNumber}/commits`);

, i.e., a call to https://privategitlab.org/api/v4/projects/org%2Fcf-sonic-net%2Fcf-sonic-buildimage/merge_requests/111/commits which is another request for which it would be nice inspect the output

[aidan-gallagher]

(Moving conversation back to Github issue)

I believe "merge_commit_sha" and "squash_commit_sha" are both set to null because we use fast forward merging (see screenshot).

When I use --auto-no-squash it nows works as expected.

---

The --folder option was useful to get round the long clone time due to the repository being so large.

---

I guess a good addition for the tool would be for it to automatically select --auto-no-squash if merge_commit_sha and squash_commit_sha are null?

[lampajr]

Hi @aidan-gallagher, glad to hear that as suggested, using --auto-no-squash, fixed your problem!!

I guess a good addition for the tool would be for it to automatically select --auto-no-squash if merge_commit_sha and squash_commit_sha are null?

Yeah, I totally agree that we should manage that use case. If both are null we should likely fallback to the "no-squash" scenario (with a proper warning so that you know you are falling back to that - if not explicitly set by the user), and try to cherry-pick all the original commits.

I will keep this issue open until that use case is managed, thanks again for raising this issue 🙏🏻

[lampajr]

For the records, I created this branch https://github.com/lampajr/git-backporting/tree/test_slice_error which is reproducing the error (at the moment it is simply trowing a more meaningful error and not handling the use case as discussed in the previous comments)

[aidan-gallagher]

Thanks @lampajr for your quick help getting to the root of the issue.

I've updated the issue name to better reflect the problem.

[lampajr]

I created this #177.

I thought a bit more on the possible solution and I preferred to throw a more meaningful error when the process tries to backport a single squashed/merged commit (squash set to true which is the default) and that commit is undefined. With this approach I don't make any assumption since I cannot recognize whether the squash is set to true because of the default or explicitly set by the user.. therefore hitting the error might be expected or not depending on the use case.

The proper solution, if you expect to have both merge and squash commit undefined (as in your case), is to disable the squash by either setting --auto-no-squash or --no-squash.