Investigate merge driver/merge strategy to reduce merge conflicts on ChangesSinceLastRelease.txt

[elachlan]

To help PRs get merged faster, we should find a way to reduce merge conflicts on the ChangesSinceLastRelease.txt and enums.json.

[elachlan]

@mikebattista I created this issue for fixing the merging issues. I think either a file specific merge driver or a changing in merge strategy will work.

[mikebattista]

Thanks. @sotteson1 any thoughts on this?

[elachlan]

I think the settings in the repo might need changing. Github documents them here: https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/configuring-pull-request-merges/about-merge-methods-on-github

[elachlan]

For a singular file you can do the following: https://stackoverflow.com/questions/11841127/strategy-for-git-and-append-mostly-files

[elachlan]

Gitlab had a similar problem, this is how they solved it: https://about.gitlab.com/blog/2015/02/10/gitlab-reduced-merge-conflicts-by-90-percent-with-changelog-placeholders/

[mikebattista]

Interesting. So we just need to add the changes txt file to .gitattributes as shown in the Gitlab blog?

[elachlan]

That is my understanding from reading it. I am no expert in git though.

[AArnott]

It's worth a shot.

[mikebattista]

I checked in the .gitattributes change. Let's see if it helps.

I started just with the baseline file. If it works, we can try with enums.json as well if that's still complicating merges.

[AArnott]

enums.json is a structured file, so a union merge policy may lead to poor results in some cases.

[elachlan]

#1245 seems to have a merge conflict. Are they on the ChangesSinceLastRelease.txt? Could be worse with this change.

Also apparently Github recently changed the default merge commit strategy. I thought this would be prudent to mention here.
https://github.blog/changelog/2022-09-12-merge-commits-now-created-using-the-merge-ort-strategy/

[mikebattista]

Interestingly I just merged a PR with ChangesSinceLastRelease.txt conflicts through VS Code, and VS Code didn't prompt for any merge conflicts. It looks like the changes from both branches were appended to the file. Maybe the browser merge has issues with this approach but local tooling doesn't? Or maybe the merge I did locally was different from the squash and merge we do in the browser?

[AArnott]

VS Code uses git.exe to merge, which honors the .gitattributes file for merge rules. Git hosting providers usually don't merge with git.exe for security and scaling reasons, so their support for less-common git configurations like this can be spotty. Are you saying that github wouldn't merge the PR until you merged it locally and pushed first?

[mikebattista]

I tried merging locally and pushing but got access issues. I then just merged to main on my machine and synced main (again no merge conflicts were reported), however, I just noticed the changes file wasn't merged properly for some reason and it broke the build since some existing changes were apparently removed in the merge.

I'm not sure if .gitattributes is helping or hurting. It's certainly not helping merges in the browser which is what we were trying to optimize.

[elachlan]

Gihub has its own settings against the repo for merge commits and merge strategy. I don't know if they have it for singular files.

We might need to contact someone at GitHub and see what their guidance on it is.

[AArnott]

@mikebattista I wonder if the access issue you had in pushing was because you were trying to push directly to main. The standard procedure when a PR has merge conflicts is to checkout the source branch of the PR locally, and then merge origin/main (or whatever the PR target branch is) into the source branch, resolve conflicts, commit, then push (to the PR source branch). That way you still get full PR build validation.

As for not helping merges because github still conflicts, and the local git merge policy that we set produces invalid results, that sounds like an argument for removing this file from source control, and instead producing it in the build and capturing as a pipeline artifact, and having a PR build step be to post a comment to the PR with the diff between the merge-base CI build and the PR build result. This was an option we discussed before, but as it's more work to set up, we wanted to go with the .gitattributes solution first to see how well it worked.

[mikebattista]

I did what you suggest. The access issue was pushing changes back to the PR branch, not main. The merge issue seems to be that changes from a previous release were included in the changes file from the PR branch, triggering the below error:

*Items from the known differences file that were not used:*
Windows.Win32.System.Wmi.Apis.WBEM_INFINITE added
Windows.Win32.System.Wmi.Apis.WBEM_NO_WAIT added
Windows.Win32.System.Wmi.WBEM_TIMEOUT_TYPE removed
Windows.Win32.System.Wmi.WBEM_TIMEOUT_TYPE.WBEM_INFINITE removed
Windows.Win32.System.Wmi.WBEM_TIMEOUT_TYPE.WBEM_NO_WAIT removed
Windows.Win32.Networking.BackgroundIntelligentTransferService.IBackgroundCopyJobHttpOptions.GetClientCertificate : ppCertHashBlob : [FreeWith(CoTaskMemFree),NativeArrayInfo(CountConst=20),Out] => [FreeWith(CoTaskMemFree),Out]
Windows.Win32.System.Wmi.IWbemServices.ExecQuery : lFlags...Int32 => WBEM_GENERIC_FLAG_TYPE
Windows.Win32.System.Wmi.WBEM_GENERIC_FLAG_TYPE :  => [Flags]
Windows.Win32.System.Wmi.WBEM_GENERIC_FLAG_TYPE.value__...System.Int32 => System.UInt32
Windows.Win32.Graphics.DirectWrite.Apis.DWriteCreateFactory : factory...IUnknown* => Void**
Windows.Win32.UI.WindowsAndMessaging.Apis.TrackPopupMenu : prcRect : [Const,In,Reserved] => [Const,In,Optional]
Windows.Win32.System.Ole.PICTYPE : [Flags] => 
Windows.Win32.Networking.Ldap.Apis.LdapMapErrorToWin32 : LdapError...UInt32 => LDAP_RETCODE
Windows.Win32.Networking.Ldap.Apis.LdapMapErrorToWin32 : return...UInt32 => WIN32_ERROR

It's possible the PR branch had changes from before the latest metadata release, then the local merge unioned the files (per .gitattributes) and included the older changes that were no longer needed. Hence, the "items from the known differences file were not used" error. Will need to test some more with locally merging other PRs that have conflicts.

.gitattributes for sure hasn't helped with browser merging, though.

[mikebattista]

Closing out for now. Resolving conflicts hasn't been an issue.