Merge pull request #1736 from github/fixes/1735-remote-config-detritus

Minimize .git/config detritus when fetching

Author: grokys

src/GitHub.App/Services/GitClient.cs

@@ -96,20 +96,30 @@ public Task Fetch(IRepository repository, string remoteName)
 
         public Task Fetch(IRepository repo, UriString cloneUrl, params string[] refspecs)
         {
-            var httpsUrl = UriString.ToUriString(cloneUrl.ToRepositoryUrl());
+            var httpsString = cloneUrl.ToRepositoryUrl().ToString();
 
-            var originRemote = repo.Network.Remotes[defaultOriginName];
-            if (originRemote != null && originRemote.Url == httpsUrl)
+            foreach (var remote in repo.Network.Remotes)
             {
-                return Fetch(repo, defaultOriginName, refspecs);
+                var remoteUrl = new UriString(remote.Url);
+                if (!remoteUrl.IsHypertextTransferProtocol)
+                {
+                    // Only match http urls
+                    continue;
+                }
+
+                var remoteHttpsString = remoteUrl.ToRepositoryUrl().ToString();
+                if (remoteHttpsString.Equals(httpsString, StringComparison.OrdinalIgnoreCase))
+                {
+                    return Fetch(repo, defaultOriginName, refspecs);
+                }
             }
 
             return Task.Factory.StartNew(() =>
             {
                 try
                 {
                     var tempRemoteName = cloneUrl.Owner + "-" + Guid.NewGuid();
-                    var remote = repo.Network.Remotes.Add(tempRemoteName, httpsUrl);
+                    var remote = repo.Network.Remotes.Add(tempRemoteName, httpsString);
                     try
                     {
 #pragma warning disable 0618 // TODO: Replace `Network.Fetch` with `Commands.Fetch`.

test/UnitTests/GitHub.App/Services/GitClientTests.cs

@@ -208,15 +208,15 @@ public async Task FetchUsingHttps(string repoUrl, string expectFetchUrl)
             repo.Network.Remotes.Received(1).Add(Arg.Any<string>(), expectFetchUrl);
         }
 
-        [TestCase("https://github.com/owner/repo", "https://github.com/owner/repo", null)]
-        [TestCase("https://github.com/fetch/repo", "https://github.com/origin/repo", "https://github.com/fetch/repo")]
-        [TestCase("[email protected]:owner/repo", "[email protected]:owner/repo", "https://github.com/owner/repo")]
-        public async Task UseOriginWhenPossible(string fetchUrl, string originUrl, string addUrl = null)
+        [TestCase("https://github.com/owner/repo", "origin", "https://github.com/owner/repo", null)]
+        [TestCase("https://github.com/fetch/repo", "origin", "https://github.com/origin/repo", "https://github.com/fetch/repo")]
+        [TestCase("[email protected]:owner/repo", "origin", "[email protected]:owner/repo", "https://github.com/owner/repo", Description = "Only use http style urls")]
+        [TestCase("https://github.com/jcansdale/repo", "jcansdale", "https://github.com/jcansdale/repo", null, Description = "Use existing remote")]
+        [TestCase("https://github.com/jcansdale/repo.git", "jcansdale", "https://github.com/jcansdale/repo", null, Description = "Ignore trailing .git")]
+        [TestCase("https://github.com/JCANSDALE/REPO", "jcansdale", "https://github.com/jcansdale/repo", null, Description = "Ignore different case")]
+        public async Task UseExistingRemoteWhenPossible(string fetchUrl, string remoteName, string remoteUrl, string addUrl = null)
         {
-            var remote = Substitute.For<Remote>();
-            remote.Url.Returns(originUrl);
-            var repo = Substitute.For<IRepository>();
-            repo.Network.Remotes["origin"].Returns(remote);
+            var repo = CreateRepository(remoteName, remoteUrl);
             var fetchUri = new UriString(fetchUrl);
             var refSpec = "refSpec";
             var gitClient = CreateGitClient();
@@ -232,6 +232,18 @@ public async Task UseOriginWhenPossible(string fetchUrl, string originUrl, strin
                 repo.Network.Remotes.DidNotReceiveWithAnyArgs().Add(null, null);
             }
         }
+
+        static IRepository CreateRepository(string remoteName, string remoteUrl)
+        {
+            var remote = Substitute.For<Remote>();
+            remote.Name.Returns(remoteName);
+            remote.Url.Returns(remoteUrl);
+            var remotes = new List<Remote> { remote };
+            var repo = Substitute.For<IRepository>();
+            repo.Network.Remotes[remoteName].Returns(remote);
+            repo.Network.Remotes.GetEnumerator().Returns(_ => remotes.GetEnumerator());
+            return repo;
+        }
     }
 
     public class TheGetPullRequestMergeBaseMethod : TestBaseClass