Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apply fast-forward update to default branch of forked repo prior to submission #235

Merged
merged 6 commits into from
Mar 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 25 additions & 23 deletions src/WingetCreateCLI/Commands/BaseCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -487,29 +487,31 @@ protected async Task<bool> 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
{
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;
Expand Down
11 changes: 10 additions & 1 deletion src/WingetCreateCLI/Properties/Resources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions src/WingetCreateCLI/Properties/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -890,4 +890,8 @@
<data name="UpgradeCode_KeywordDescription" xml:space="preserve">
<value>UpgradeCode used for correlation of packages across sources</value>
</data>
<data name="FastForwardUpdateFailed_Message" xml:space="preserve">
<value>Failed to update fork because the default branch is ahead by {0} commit(s). </value>
<comment>{0} - represents the number of commits the default branch is ahead by</comment>
</data>
</root>
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license.

namespace Microsoft.WingetCreateCore.Common.Exceptions
{
using System;

/// <summary>
/// The exception that is thrown when attemping a non-fast forward update to a GitHub repository branch.
/// </summary>
public class NonFastForwardException : Exception
{
/// <summary>
/// Initializes a new instance of the <see cref="NonFastForwardException"/> class.
/// </summary>
/// <param name="commitsAheadBy">The number of commits the branch is ahead by.</param>
public NonFastForwardException(int commitsAheadBy)
{
this.CommitsAheadBy = commitsAheadBy;
}

/// <summary>
/// Gets the number of commits the branch is ahead by.
/// </summary>
public int CommitsAheadBy { get; private set; }
}
}
39 changes: 37 additions & 2 deletions src/WingetCreateCore/Common/GitHub.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -277,8 +278,8 @@ private async Task<string> FindPackageIdRecursive(string[] packageId, string pat
private async Task<PullRequest> SubmitPRAsync(string packageId, string version, Dictionary<string, string> contents, bool submitToFork)
{
bool createdRepo = false;

Repository repo;

if (submitToFork)
{
try
Expand Down Expand Up @@ -306,12 +307,22 @@ private async Task<PullRequest> SubmitPRAsync(string packageId, string version,
var upstreamMasterSha = upstreamMaster.Object.Sha;

Reference newBranch = null;

try
{
var retryPolicy = Policy.Handle<ApiException>().WaitAndRetryAsync(3, i => TimeSpan.FromSeconds(i));
await retryPolicy.ExecuteAsync(async () =>
{
await this.github.Git.Reference.Create(repo.Id, new NewReference($"refs/{newBranchNameHeads}", upstreamMasterSha));
try
{
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
Expand Down Expand Up @@ -366,6 +377,30 @@ await retryPolicy.ExecuteAsync(async () =>
}
}

/// <summary>
/// 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.
/// </summary>
/// <param name="forkedRepo"><see cref="Repository"/>Forked repository to be updated.</param>
/// <returns>A <see cref="Task"/> representing the asynchronous operation.</returns>
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.
ryfu-msft marked this conversation as resolved.
Show resolved Hide resolved
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));
}
}

private async Task DeletePullRequestBranch(int pullRequestId)
{
// Delete branch if it's not on a forked repo.
Expand Down