• Have you signed the Contributor License Agreement?
• Are you working against an Issue?
  • Targets Exception of type 'Microsoft.WingetCreateCore.Common.Exceptions.NonFastForwardException' was thrown. #614
  • Resolves "submit" fails in 1.10.2.0 #617

Change

Remove code that checks for 'commits ahead by' in a fork. This is because the HTTP API we use can sync fork even with additional commits - i.e., can perform a non-fast forward merge (provided they don't conflict)

Validation

Manually validated by adding extra commits in the default branch of my fork. Verified that the sync fork code succeeds and pulls the latest upstream commits in my fork.

History

The initial code for handling the sync fork step (added in PR #235) made use of a this.github.Git.Reference.Update call for syncing fork. As the PR description states, this call would fail if a fast-forward update was not possible, meaning the forked repo was ahead of / had additional commits that were not present in upstream. In this case, we would throw a NonFastForwardException guarding against this scenario since we knew the Update() call would fail. This exception however, didn't block PR creation in old code since the sync fork step was previously done only during first execution in a retry loop and if it threw a NonFastForwardException, we would still try creating a reference / branch & then a PR. The old code still had flaws in the sense that if the fork was behind way too many commits, the reference creation & PR creation would both fail, but we would never be able to even attempt the sync fork to mitigate it because of some additional commits in fork making us throw a NonFastForwardException

We switched to making a direct HTTP call to the GitHub API for syncing fork in PR #581. The benefit here is that the HTTP API call does not fail if the fork is "ahead of" the upstream. It can successfully sync the fork provided that the additional commits do not conflict with the changes in upstream. If they conflict, it throws a 409 error code which is already handled in the code. Also, when switching to the HTTP API, the logic was changed in PR #597 such that we do not attempt to sync fork only during first execution, but we retry the API multiple times as well. A consequence of it, or a regression one may say, is that the old code did allow cases where even if the sync fork was not attempted due to NonFastForwardException, PR creation may still succeed in some cases provided the fork wasn't wayyy too behind. But now since we check sync fork in every execution after PR #597 and every execution would just throw a NonFastForwardException, we never make it to PR creation step. Now, this regression should also be handled in this PR where we just remove the NonFastForwardException check, since the HTTP API can sync fork even if it is a non-fast forward update

Microsoft Reviewers: Open in CodeFlow