From 66d842c3ea9be97a2f3799c7c5be2bf95db53604 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Thu, 5 Oct 2017 17:30:14 +0100 Subject: [PATCH 1/2] Fix race when fetching from temporary remote --- src/GitHub.App/Services/GitClient.cs | 35 ++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/src/GitHub.App/Services/GitClient.cs b/src/GitHub.App/Services/GitClient.cs index cb0d259141..d00a929076 100644 --- a/src/GitHub.App/Services/GitClient.cs +++ b/src/GitHub.App/Services/GitClient.cs @@ -433,21 +433,36 @@ public Task IsHeadPushed(IRepository repo) public Task Fetch(IRepository repo, UriString cloneUrl, params string[] refspecs) { var httpsUrl = UriString.ToUriString(cloneUrl.ToRepositoryUrl()); - if (repo.Network.Remotes[defaultOriginName]?.Url == httpsUrl) + + var originRemote = repo.Network.Remotes[defaultOriginName]; + if (originRemote != null && originRemote.Url == httpsUrl) { return Fetch(repo, defaultOriginName, refspecs); } - var tempRemoteName = cloneUrl.Owner + "-" + Guid.NewGuid(); - repo.Network.Remotes.Add(tempRemoteName, httpsUrl); - try - { - return Fetch(repo, tempRemoteName, refspecs); - } - finally + return Task.Factory.StartNew(() => { - repo.Network.Remotes.Remove(tempRemoteName); - } + try + { + var tempRemoteName = cloneUrl.Owner + "-" + Guid.NewGuid(); + var remote = repo.Network.Remotes.Add(tempRemoteName, httpsUrl); + try + { + repo.Network.Fetch(remote, refspecs, fetchOptions); + } + finally + { + repo.Network.Remotes.Remove(tempRemoteName); + } + } + catch (Exception ex) + { + log.Error("Failed to fetch", ex); +#if DEBUG + throw; +#endif + } + }); } static bool IsCanonical(string s) From 5f1f3d8a8bf8e910590d5e6a81f13ca633558807 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Thu, 5 Oct 2017 17:36:20 +0100 Subject: [PATCH 2/2] Move `Fetch` (from Uri) next to its friends --- src/GitHub.App/Services/GitClient.cs | 70 ++++++++++++++-------------- 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/src/GitHub.App/Services/GitClient.cs b/src/GitHub.App/Services/GitClient.cs index d00a929076..b203df0cb7 100644 --- a/src/GitHub.App/Services/GitClient.cs +++ b/src/GitHub.App/Services/GitClient.cs @@ -86,6 +86,41 @@ public Task Fetch(IRepository repository, string remoteName) }); } + public Task Fetch(IRepository repo, UriString cloneUrl, params string[] refspecs) + { + var httpsUrl = UriString.ToUriString(cloneUrl.ToRepositoryUrl()); + + var originRemote = repo.Network.Remotes[defaultOriginName]; + if (originRemote != null && originRemote.Url == httpsUrl) + { + return Fetch(repo, defaultOriginName, refspecs); + } + + return Task.Factory.StartNew(() => + { + try + { + var tempRemoteName = cloneUrl.Owner + "-" + Guid.NewGuid(); + var remote = repo.Network.Remotes.Add(tempRemoteName, httpsUrl); + try + { + repo.Network.Fetch(remote, refspecs, fetchOptions); + } + finally + { + repo.Network.Remotes.Remove(tempRemoteName); + } + } + catch (Exception ex) + { + log.Error("Failed to fetch", ex); +#if DEBUG + throw; +#endif + } + }); + } + public Task Fetch(IRepository repository, string remoteName, params string[] refspecs) { Guard.ArgumentNotNull(repository, nameof(repository)); @@ -430,41 +465,6 @@ public Task IsHeadPushed(IRepository repo) }); } - public Task Fetch(IRepository repo, UriString cloneUrl, params string[] refspecs) - { - var httpsUrl = UriString.ToUriString(cloneUrl.ToRepositoryUrl()); - - var originRemote = repo.Network.Remotes[defaultOriginName]; - if (originRemote != null && originRemote.Url == httpsUrl) - { - return Fetch(repo, defaultOriginName, refspecs); - } - - return Task.Factory.StartNew(() => - { - try - { - var tempRemoteName = cloneUrl.Owner + "-" + Guid.NewGuid(); - var remote = repo.Network.Remotes.Add(tempRemoteName, httpsUrl); - try - { - repo.Network.Fetch(remote, refspecs, fetchOptions); - } - finally - { - repo.Network.Remotes.Remove(tempRemoteName); - } - } - catch (Exception ex) - { - log.Error("Failed to fetch", ex); -#if DEBUG - throw; -#endif - } - }); - } - static bool IsCanonical(string s) { Guard.ArgumentNotEmptyString(s, nameof(s));