From 48810561fb02cdb290849f314cd1839ff68fc4fd Mon Sep 17 00:00:00 2001 From: Ryan Fu Date: Mon, 7 Mar 2022 17:06:43 -0800 Subject: [PATCH 1/4] update forked branch prior to submission --- src/WingetCreateCore/Common/GitHub.cs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/WingetCreateCore/Common/GitHub.cs b/src/WingetCreateCore/Common/GitHub.cs index 2bd66225..c5e1664e 100644 --- a/src/WingetCreateCore/Common/GitHub.cs +++ b/src/WingetCreateCore/Common/GitHub.cs @@ -291,6 +291,8 @@ private async Task SubmitPRAsync(string packageId, string version, repo = await this.github.Repository.Forks.Create(this.wingetRepoOwner, this.wingetRepo, new NewRepositoryFork()); createdRepo = true; } + + await this.UpdateForkedRepoWithUpstreamCommits(repo); } else { @@ -306,6 +308,7 @@ private async Task SubmitPRAsync(string packageId, string version, var upstreamMasterSha = upstreamMaster.Object.Sha; Reference newBranch = null; + try { var retryPolicy = Policy.Handle().WaitAndRetryAsync(3, i => TimeSpan.FromSeconds(i)); @@ -366,6 +369,24 @@ await retryPolicy.ExecuteAsync(async () => } } + /// + /// Checks if the provided forked repository is behind on upstream commits and updates the default branch with the fetched commits. Update can only be a fast-forward update. + /// + /// Forked repository to be updated. + /// A representing the asynchronous operation. + private async Task UpdateForkedRepoWithUpstreamCommits(Repository forkedRepo) + { + var upstream = forkedRepo.Parent; + var compareResult = await this.github.Repository.Commit.Compare(upstream.Id, upstream.DefaultBranch, $"{forkedRepo.Owner.Login}:{forkedRepo.DefaultBranch}"); + + // Check to ensure that the update is only a fast-forward update. + if (compareResult.BehindBy > 0 && compareResult.AheadBy == 0) + { + var upstreamBranchReference = await this.github.Git.Reference.Get(upstream.Id, $"heads/{upstream.DefaultBranch}"); + await this.github.Git.Reference.Update(forkedRepo.Id, $"heads/{forkedRepo.DefaultBranch}", new ReferenceUpdate(upstreamBranchReference.Object.Sha)); + } + } + private async Task DeletePullRequestBranch(int pullRequestId) { // Delete branch if it's not on a forked repo. From 15dd774d8e14c5f2a43768adc87f6a7a72e8c217 Mon Sep 17 00:00:00 2001 From: Ryan Fu Date: Wed, 9 Mar 2022 13:58:14 -0800 Subject: [PATCH 2/4] create nonfastforwardexception --- src/WingetCreateCLI/Commands/BaseCommand.cs | 7 ++++- .../Properties/Resources.Designer.cs | 9 +++++++ .../Exceptions/NonFastForwardException.cs | 27 +++++++++++++++++++ src/WingetCreateCore/Common/GitHub.cs | 9 ++++++- 4 files changed, 50 insertions(+), 2 deletions(-) create mode 100644 src/WingetCreateCore/Common/Exceptions/NonFastForwardException.cs diff --git a/src/WingetCreateCLI/Commands/BaseCommand.cs b/src/WingetCreateCLI/Commands/BaseCommand.cs index 6e3ae3cc..0edb85cd 100644 --- a/src/WingetCreateCLI/Commands/BaseCommand.cs +++ b/src/WingetCreateCLI/Commands/BaseCommand.cs @@ -504,7 +504,12 @@ protected async Task GitHubSubmitManifests(Manifests manifests) { Logger.ErrorLocalized(nameof(Resources.Error_Prefix), e.Message); return true; - } + } + else if (e is NonFastForwardException nonFastForwardException) + { + Logger.ErrorLocalized(nameof(Resources.FastForwardUpdateFailed_Message), nonFastForwardException.CommitsAheadBy); + return true; + } else { return false; diff --git a/src/WingetCreateCLI/Properties/Resources.Designer.cs b/src/WingetCreateCLI/Properties/Resources.Designer.cs index e6da16d4..012facb0 100644 --- a/src/WingetCreateCLI/Properties/Resources.Designer.cs +++ b/src/WingetCreateCLI/Properties/Resources.Designer.cs @@ -708,6 +708,15 @@ public static string ExternalDependencies_KeywordDescription { } } + /// + /// Looks up a localized string similar to Failed to update the fork's default branch because the branch is ahead by {0} commit(s). Manually update the fork's default branch before resubmitting.. + /// + public static string FastForwardUpdateFailed_Message { + get { + return ResourceManager.GetString("FastForwardUpdateFailed_Message", resourceCulture); + } + } + /// /// Looks up a localized string similar to [{0}] value is. /// diff --git a/src/WingetCreateCore/Common/Exceptions/NonFastForwardException.cs b/src/WingetCreateCore/Common/Exceptions/NonFastForwardException.cs new file mode 100644 index 00000000..75e8bebf --- /dev/null +++ b/src/WingetCreateCore/Common/Exceptions/NonFastForwardException.cs @@ -0,0 +1,27 @@ +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. + +namespace Microsoft.WingetCreateCore.Common.Exceptions +{ + using System; + + /// + /// The exception that is thrown when attemping a non-fast forward update to a GitHub repository branch. + /// + public class NonFastForwardException : Exception + { + /// + /// Initializes a new instance of the class. + /// + /// The number of commits the branch is ahead by. + public NonFastForwardException(int commitsAheadBy) + { + this.CommitsAheadBy = commitsAheadBy; + } + + /// + /// Gets the number of commits the branch is ahead by. + /// + public int CommitsAheadBy { get; private set; } + } +} diff --git a/src/WingetCreateCore/Common/GitHub.cs b/src/WingetCreateCore/Common/GitHub.cs index c5e1664e..6e0c958f 100644 --- a/src/WingetCreateCore/Common/GitHub.cs +++ b/src/WingetCreateCore/Common/GitHub.cs @@ -10,6 +10,7 @@ namespace Microsoft.WingetCreateCore.Common using System.Security.Cryptography; using System.Threading.Tasks; using Jose; + using Microsoft.WingetCreateCore.Common.Exceptions; using Microsoft.WingetCreateCore.Models; using Octokit; using Polly; @@ -380,8 +381,14 @@ private async Task UpdateForkedRepoWithUpstreamCommits(Repository forkedRepo) var compareResult = await this.github.Repository.Commit.Compare(upstream.Id, upstream.DefaultBranch, $"{forkedRepo.Owner.Login}:{forkedRepo.DefaultBranch}"); // Check to ensure that the update is only a fast-forward update. - if (compareResult.BehindBy > 0 && compareResult.AheadBy == 0) + if (compareResult.BehindBy > 0) { + int commitsAheadBy = compareResult.AheadBy; + if (commitsAheadBy > 0) + { + throw new NonFastForwardException(commitsAheadBy); + } + var upstreamBranchReference = await this.github.Git.Reference.Get(upstream.Id, $"heads/{upstream.DefaultBranch}"); await this.github.Git.Reference.Update(forkedRepo.Id, $"heads/{forkedRepo.DefaultBranch}", new ReferenceUpdate(upstreamBranchReference.Object.Sha)); } From 3e72f936715f3ba6c18cf0232f4e60de85034546 Mon Sep 17 00:00:00 2001 From: Ryan Fu Date: Fri, 11 Mar 2022 18:30:31 -0800 Subject: [PATCH 3/4] add exception --- src/WingetCreateCLI/Commands/BaseCommand.cs | 53 +++++++++---------- .../Properties/Resources.Designer.cs | 4 +- src/WingetCreateCLI/Properties/Resources.resx | 4 ++ src/WingetCreateCore/Common/GitHub.cs | 16 ++++-- 4 files changed, 43 insertions(+), 34 deletions(-) diff --git a/src/WingetCreateCLI/Commands/BaseCommand.cs b/src/WingetCreateCLI/Commands/BaseCommand.cs index 0edb85cd..3792eeff 100644 --- a/src/WingetCreateCLI/Commands/BaseCommand.cs +++ b/src/WingetCreateCLI/Commands/BaseCommand.cs @@ -487,34 +487,31 @@ protected async Task GitHubSubmitManifests(Manifests manifests) Logger.InfoLocalized(nameof(Resources.PullRequestURI_Message), pullRequest.HtmlUrl); Console.WriteLine(); - } - catch (AggregateException ae) - { - ae.Handle((e) => - { - TelemetryManager.Log.WriteEvent(new PullRequestEvent - { - IsSuccessful = false, - ErrorMessage = e.Message, - ExceptionType = e.GetType().ToString(), - StackTrace = e.StackTrace, - }); - - if (e is Octokit.ForbiddenException) - { - Logger.ErrorLocalized(nameof(Resources.Error_Prefix), e.Message); - return true; - } - else if (e is NonFastForwardException nonFastForwardException) - { - Logger.ErrorLocalized(nameof(Resources.FastForwardUpdateFailed_Message), nonFastForwardException.CommitsAheadBy); - return true; - } - else - { - return false; - } - }); + } + catch (Exception e) + { + TelemetryManager.Log.WriteEvent(new PullRequestEvent + { + IsSuccessful = false, + ErrorMessage = e.Message, + ExceptionType = e.GetType().ToString(), + StackTrace = e.StackTrace, + }); + + if (e is Octokit.ForbiddenException) + { + Logger.ErrorLocalized(nameof(Resources.Error_Prefix), e.Message); + return true; + } + else if (e is NonFastForwardException nonFastForwardException) + { + Logger.ErrorLocalized(nameof(Resources.FastForwardUpdateFailed_Message), nonFastForwardException.CommitsAheadBy); + return true; + } + else + { + return false; + } } return true; diff --git a/src/WingetCreateCLI/Properties/Resources.Designer.cs b/src/WingetCreateCLI/Properties/Resources.Designer.cs index 012facb0..4515bacf 100644 --- a/src/WingetCreateCLI/Properties/Resources.Designer.cs +++ b/src/WingetCreateCLI/Properties/Resources.Designer.cs @@ -19,7 +19,7 @@ namespace Microsoft.WingetCreateCLI.Properties { // class via a tool like ResGen or Visual Studio. // To add or remove a member, edit your .ResX file then rerun ResGen // with the /str option, or rebuild your VS project. - [global::System.CodeDom.Compiler.GeneratedCodeAttribute("System.Resources.Tools.StronglyTypedResourceBuilder", "16.0.0.0")] + [global::System.CodeDom.Compiler.GeneratedCodeAttribute("System.Resources.Tools.StronglyTypedResourceBuilder", "17.0.0.0")] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Runtime.CompilerServices.CompilerGeneratedAttribute()] public class Resources { @@ -709,7 +709,7 @@ public static string ExternalDependencies_KeywordDescription { } /// - /// Looks up a localized string similar to Failed to update the fork's default branch because the branch is ahead by {0} commit(s). Manually update the fork's default branch before resubmitting.. + /// 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 { diff --git a/src/WingetCreateCLI/Properties/Resources.resx b/src/WingetCreateCLI/Properties/Resources.resx index d5fe8558..00b8aafb 100644 --- a/src/WingetCreateCLI/Properties/Resources.resx +++ b/src/WingetCreateCLI/Properties/Resources.resx @@ -890,4 +890,8 @@ 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 + \ No newline at end of file diff --git a/src/WingetCreateCore/Common/GitHub.cs b/src/WingetCreateCore/Common/GitHub.cs index 6e0c958f..b53fac67 100644 --- a/src/WingetCreateCore/Common/GitHub.cs +++ b/src/WingetCreateCore/Common/GitHub.cs @@ -278,8 +278,8 @@ private async Task FindPackageIdRecursive(string[] packageId, string pat private async Task SubmitPRAsync(string packageId, string version, Dictionary contents, bool submitToFork) { bool createdRepo = false; - Repository repo; + if (submitToFork) { try @@ -292,8 +292,6 @@ private async Task SubmitPRAsync(string packageId, string version, repo = await this.github.Repository.Forks.Create(this.wingetRepoOwner, this.wingetRepo, new NewRepositoryFork()); createdRepo = true; } - - await this.UpdateForkedRepoWithUpstreamCommits(repo); } else { @@ -315,7 +313,17 @@ private async Task SubmitPRAsync(string packageId, string version, var retryPolicy = Policy.Handle().WaitAndRetryAsync(3, i => TimeSpan.FromSeconds(i)); await retryPolicy.ExecuteAsync(async () => { - await this.github.Git.Reference.Create(repo.Id, new NewReference($"refs/{newBranchNameHeads}", upstreamMasterSha)); + try + { + Reference reference = 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. + await this.UpdateForkedRepoWithUpstreamCommits(repo); + throw; + } }); // Update from upstream branch master From 991c7148c213425ea9d8f8a76de245ffc34af4f0 Mon Sep 17 00:00:00 2001 From: Ryan Fu Date: Tue, 15 Mar 2022 18:03:57 -0700 Subject: [PATCH 4/4] remove unused local variable --- src/WingetCreateCore/Common/GitHub.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/WingetCreateCore/Common/GitHub.cs b/src/WingetCreateCore/Common/GitHub.cs index b53fac67..5cadb25c 100644 --- a/src/WingetCreateCore/Common/GitHub.cs +++ b/src/WingetCreateCore/Common/GitHub.cs @@ -315,8 +315,7 @@ await retryPolicy.ExecuteAsync(async () => { try { - Reference reference = await this.github.Git.Reference.Create(repo.Id, new NewReference($"refs/{newBranchNameHeads}", upstreamMasterSha)); - + await this.github.Git.Reference.Create(repo.Id, new NewReference($"refs/{newBranchNameHeads}", upstreamMasterSha)); } catch (Octokit.NotFoundException) {