From 041f40395ada4a89da20e7f9323b5c79266a25ab Mon Sep 17 00:00:00 2001 From: Ryan Fu Date: Fri, 22 Jul 2022 11:52:49 -0700 Subject: [PATCH 1/2] sync for prior to creating reference --- src/WingetCreateCLI/Commands/BaseCommand.cs | 6 +++-- .../Properties/Resources.Designer.cs | 18 +++++++------- src/WingetCreateCLI/Properties/Resources.resx | 7 +++--- src/WingetCreateCore/Common/GitHub.cs | 24 ++++++++++++------- 4 files changed, 32 insertions(+), 23 deletions(-) diff --git a/src/WingetCreateCLI/Commands/BaseCommand.cs b/src/WingetCreateCLI/Commands/BaseCommand.cs index 6dc9b2aa..1e547cac 100644 --- a/src/WingetCreateCLI/Commands/BaseCommand.cs +++ b/src/WingetCreateCLI/Commands/BaseCommand.cs @@ -503,9 +503,11 @@ protected async Task GitHubSubmitManifests(Manifests manifests) Logger.ErrorLocalized(nameof(Resources.Error_Prefix), e.Message); return false; } - else if (e is NonFastForwardException nonFastForwardException) + else if (e is Octokit.NotFoundException) { - Logger.ErrorLocalized(nameof(Resources.FastForwardUpdateFailed_Message), nonFastForwardException.CommitsAheadBy); + // This exception can occur if the client is unable to create a reference due to being behind by too many commits. + // The user will need to manually update their master branch of their winget-pkgs fork. + Logger.ErrorLocalized(nameof(Resources.SyncForkWithUpstream_Message)); return false; } else diff --git a/src/WingetCreateCLI/Properties/Resources.Designer.cs b/src/WingetCreateCLI/Properties/Resources.Designer.cs index facb9e64..48315012 100644 --- a/src/WingetCreateCLI/Properties/Resources.Designer.cs +++ b/src/WingetCreateCLI/Properties/Resources.Designer.cs @@ -744,15 +744,6 @@ public static string ExternalDependencies_KeywordDescription { } } - /// - /// Looks up a localized string similar to Failed to update fork because the default branch is ahead by {0} commit(s). . - /// - public static string FastForwardUpdateFailed_Message { - get { - return ResourceManager.GetString("FastForwardUpdateFailed_Message", resourceCulture); - } - } - /// /// Looks up a localized string similar to [{0}] value is. /// @@ -2031,6 +2022,15 @@ public static string Switches_KeywordDescription { } } + /// + /// Looks up a localized string similar to Unable to create a reference to the forked repository. This can be caused when the forked repository is behind by too many commits. Sync your fork and try again.. + /// + public static string SyncForkWithUpstream_Message { + get { + return ResourceManager.GetString("SyncForkWithUpstream_Message", resourceCulture); + } + } + /// /// Looks up a localized string similar to List of additional package search terms. /// diff --git a/src/WingetCreateCLI/Properties/Resources.resx b/src/WingetCreateCLI/Properties/Resources.resx index 9543ca13..dc1ae034 100644 --- a/src/WingetCreateCLI/Properties/Resources.resx +++ b/src/WingetCreateCLI/Properties/Resources.resx @@ -890,10 +890,6 @@ UpgradeCode used for correlation of packages across sources - - Failed to update fork because the default branch is ahead by {0} commit(s). - {0} - represents the number of commits the default branch is ahead by - Indicates whether winget should display a warning message if the install or upgrade is known to interfere with running applications @@ -919,4 +915,7 @@ List of winget arguments the installer does not support "winget" is the proper name of the tool + + Unable to create a reference to the forked repository. This can be caused when the forked repository is behind by too many commits. Sync your fork and try again. + \ No newline at end of file diff --git a/src/WingetCreateCore/Common/GitHub.cs b/src/WingetCreateCore/Common/GitHub.cs index 5cadb25c..91dbc1f5 100644 --- a/src/WingetCreateCore/Common/GitHub.cs +++ b/src/WingetCreateCore/Common/GitHub.cs @@ -307,22 +307,30 @@ private async Task SubmitPRAsync(string packageId, string version, var upstreamMasterSha = upstreamMaster.Object.Sha; Reference newBranch = null; + bool forkSyncAttempted = false; try { - var retryPolicy = Policy.Handle().WaitAndRetryAsync(3, i => TimeSpan.FromSeconds(i)); + var retryPolicy = Policy + .Handle() + .Or() + .WaitAndRetryAsync(3, i => TimeSpan.FromSeconds(i)); + await retryPolicy.ExecuteAsync(async () => { - try + // Related issue: https://github.com/microsoft/winget-create/issues/282 + // There is a known issue where a reference is unable to be created if the fork is behind by too many commits. + // Always attempt to sync fork during first execution in order to mitigate the possibility of this scenario occurring. + // If the fork is behind by too many commits, syncing will also fail with a NotFoundException. + // Updating the fork can fail if it is a non-fast forward update, but this should not be blocking as pull request submission can still proceed. + // If creating a reference fails, that means syncing the fork also failed, therefore the user will need to manually sync their repo regardless. + if (!forkSyncAttempted) { - await this.github.Git.Reference.Create(repo.Id, new NewReference($"refs/{newBranchNameHeads}", upstreamMasterSha)); - } - catch (Octokit.NotFoundException) - { - // Creating a reference can sometimes fail with a NotFoundException if the fork is not up to date with the upstream repository. + forkSyncAttempted = true; await this.UpdateForkedRepoWithUpstreamCommits(repo); - throw; } + + await this.github.Git.Reference.Create(repo.Id, new NewReference($"refs/{newBranchNameHeads}", upstreamMasterSha)); }); // Update from upstream branch master From aff82bdc18bf011be8b4030d5983bba2b4b7d025 Mon Sep 17 00:00:00 2001 From: Ryan Fu Date: Fri, 22 Jul 2022 12:17:22 -0700 Subject: [PATCH 2/2] check if fork submission --- src/WingetCreateCore/Common/GitHub.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/WingetCreateCore/Common/GitHub.cs b/src/WingetCreateCore/Common/GitHub.cs index 91dbc1f5..d2c229fd 100644 --- a/src/WingetCreateCore/Common/GitHub.cs +++ b/src/WingetCreateCore/Common/GitHub.cs @@ -324,7 +324,7 @@ await retryPolicy.ExecuteAsync(async () => // If the fork is behind by too many commits, syncing will also fail with a NotFoundException. // Updating the fork can fail if it is a non-fast forward update, but this should not be blocking as pull request submission can still proceed. // If creating a reference fails, that means syncing the fork also failed, therefore the user will need to manually sync their repo regardless. - if (!forkSyncAttempted) + if (!forkSyncAttempted && submitToFork) { forkSyncAttempted = true; await this.UpdateForkedRepoWithUpstreamCommits(repo);