Default backport merge strategy

[lampajr]

Hi all,

Right now the Backporting mechanism we have in place is using the default cherry-pick merge strategy that the tool itself is setting [1], which is strategy recursive with option theirs.

This default seems to differ with resepct to the actual git-merge default strategy [2], which is ort with find-renames option.

Since this choice will affect how backporting pull requests are generated, I'd like to raise this point in order to choose the proper merge-strategy together based on what is our expectation (especially considering the conflicts resolution perspective).

[1] https://github.com/kiegroup/git-backporting?tab=readme-ov-file#cherry-pick-strategy
[2] https://git-scm.com/docs/git-merge#Documentation/git-merge.txt--sltstrategygt

[willr3]

I have to admit my git knowledge is limited to "I check out code and submit a PR". Does one handle incompatible changes better than the other? My concern with automatic code changes are how it detects when the change form master is not applicable to the current production branch (even though someone added the backport label)

[lampajr]

I have to admit my git knowledge is limited to "I check out code and submit a PR". Does one handle incompatible changes better than the other?

I don't know raw details but based on the git documentation the ort strategy looks like the replacement of the previous recursive one:

The name for this algorithm is an acronym ("Ostensibly Recursive’s Twin") and came from the fact that it was written as a replacement for the previous default algorithm, recursive.

Therefore I am in favor of that switch, maybe I can do that directly in the tool (but that's another story).

My concern with automatic code changes are how it detects when the change form master is not applicable to the current production branch

I think this is the main/crucial aspect to take into account.

Do we want an automatic conflicts resolution? Right now we are using theirs option which means that
This option forces conflicting hunks to be auto-resolved cleanly by favoring their version, i.e., the branch we are merging from (the original pull request) which could lead to bring unwanted changes from the master stream to the stable one.

If we change that to, e.g., the default option which is find-renames, we could have more backports to be managed manually but we reduce the risk of having/backporting unwanted changes due to conflicts resolution.

In conclusion I think this is up to us, based on what will be our most common use case.

[johnaohara]

@lampajr do you think the "theirs" conflict resolution caused the issue with #1593 where unwanted changes were included in the backport PR?

[lampajr]

Yeah I think so, because during the resolution it favored theirs which was the master branch.

Using the default strategy (ort + find-renames) I think the backporting would have failed requiring a manual backporting/cherry-pick

[lampajr]

(aside note with the latest version of backporting I implemented an automated mechanism to send a comment in the original pull request if the backporting failed, so it should be easier to track backporting failures)

[lampajr]

Just to provide more context on the actual change, here the pull request #1611 for this.

I know we did not come up to a decision yet but in that case the PR is ready :)