From 1b820a5b5f2228beb6bb1abd2e082c261360145f Mon Sep 17 00:00:00 2001 From: Jameson Miller Date: Wed, 24 Sep 2014 16:47:01 -0400 Subject: [PATCH 1/2] Branch.Remote property should not throw The Branch.Remote property can throw when it attempts to resolve a remote for a remote-tracking branch, and the remote cannot be found or is ambiguous. In those situations, Branch.Remote should return null. If there is a need for the specific exception to be returned, we can look at transitioning to a method for this functionality. --- LibGit2Sharp.Tests/BranchFixture.cs | 58 +++++++++++++++++++++++++++++ LibGit2Sharp/Branch.cs | 18 ++++++--- LibGit2Sharp/BranchUpdater.cs | 2 +- LibGit2Sharp/Core/Proxy.cs | 12 ++++-- 4 files changed, 80 insertions(+), 10 deletions(-) diff --git a/LibGit2Sharp.Tests/BranchFixture.cs b/LibGit2Sharp.Tests/BranchFixture.cs index 0aef1a27f..642b1a8a7 100644 --- a/LibGit2Sharp.Tests/BranchFixture.cs +++ b/LibGit2Sharp.Tests/BranchFixture.cs @@ -391,6 +391,64 @@ public void QueryUpstreamBranchCanonicalNameForLocalTrackingBranch() } } + [Fact] + public void QueryRemoteForRemoteBranch() + { + using (var repo = new Repository(StandardTestRepoPath)) + { + var master = repo.Branches["origin/master"]; + Assert.Equal(repo.Network.Remotes["origin"], master.Remote); + } + } + + [Fact] + public void QueryUnresolvableRemoteForRemoteBranch() + { + var path = CloneStandardTestRepo(); + + var fetchRefSpecs = new string[] { "+refs/heads/notfound/*:refs/remotes/origin/notfound/*" }; + + using (var repo = InitIsolatedRepository(path)) + { + // Update the remote config such that the remote for a + // remote branch cannot be resolved + Remote remote = repo.Network.Remotes["origin"]; + Assert.NotNull(remote); + + repo.Network.Remotes.Update(remote, r => r.FetchRefSpecs = fetchRefSpecs); + + Branch branch = repo.Branches["refs/remotes/origin/master"]; + + Assert.NotNull(branch); + Assert.True(branch.IsRemote); + + Assert.Null(branch.Remote); + } + } + + [Fact] + public void QueryAmbigousRemoteForRemoteBranch() + { + var path = CloneStandardTestRepo(); + + var fetchRefSpec = "+refs/heads/*:refs/remotes/origin/*"; + var url = "https://github.com/libgit2/TestGitRepository"; + + using (var repo = InitIsolatedRepository(path)) + { + // Add a second remote so that it is ambiguous which remote + // the remote-tracking branch tracks. + repo.Network.Remotes.Add("ambiguous", url, fetchRefSpec); + + Branch branch = repo.Branches["refs/remotes/origin/master"]; + + Assert.NotNull(branch); + Assert.True(branch.IsRemote); + + Assert.Null(branch.Remote); + } + } + [Fact] public void CanLookupABranchByItsCanonicalName() { diff --git a/LibGit2Sharp/Branch.cs b/LibGit2Sharp/Branch.cs index 2e0266ec1..0067bebb3 100644 --- a/LibGit2Sharp/Branch.cs +++ b/LibGit2Sharp/Branch.cs @@ -149,7 +149,13 @@ public virtual string UpstreamBranchCanonicalName } /// - /// Gets the configured to fetch from and push to. + /// Get the remote for the branch. + /// + /// If this is a local branch, this will return the configured + /// to fetch from and push to. If this is a + /// remote-tracking branch, this will return the remote containing + /// the tracked branch. + /// /// public virtual Remote Remote { @@ -164,11 +170,11 @@ public virtual Remote Remote else { remoteName = RemoteNameFromLocalBranch(); + } - if (remoteName == null) - { - return null; - } + if (remoteName == null) + { + return null; } return repo.Network.Remotes[remoteName]; @@ -210,7 +216,7 @@ private string RemoteNameFromLocalBranch() private string RemoteNameFromRemoteTrackingBranch() { - return Proxy.git_branch_remote_name(repo.Handle, CanonicalName); + return Proxy.git_branch_remote_name(repo.Handle, CanonicalName, false); } /// diff --git a/LibGit2Sharp/BranchUpdater.cs b/LibGit2Sharp/BranchUpdater.cs index 83a61d34c..f51332eb7 100644 --- a/LibGit2Sharp/BranchUpdater.cs +++ b/LibGit2Sharp/BranchUpdater.cs @@ -181,7 +181,7 @@ private void GetUpstreamInformation(string canonicalName, out string remoteName, } else if (canonicalName.LooksLikeRemoteTrackingBranch()) { - remoteName = Proxy.git_branch_remote_name(repo.Handle, canonicalName); + remoteName = Proxy.git_branch_remote_name(repo.Handle, canonicalName, true); Remote remote = repo.Network.Remotes.RemoteForName(remoteName); mergeBranchName = remote.FetchSpecTransformToSource(canonicalName); diff --git a/LibGit2Sharp/Core/Proxy.cs b/LibGit2Sharp/Core/Proxy.cs index 40b757330..8c987a0ea 100644 --- a/LibGit2Sharp/Core/Proxy.cs +++ b/LibGit2Sharp/Core/Proxy.cs @@ -203,15 +203,21 @@ public static ReferenceSafeHandle git_branch_move(ReferenceSafeHandle reference, } } - public static string git_branch_remote_name(RepositorySafeHandle repo, string canonical_branch_name) + public static string git_branch_remote_name(RepositorySafeHandle repo, string canonical_branch_name, bool shouldThrowIfNotFound) { using (ThreadAffinity()) using (var buf = new GitBuf()) { int res = NativeMethods.git_branch_remote_name(buf, repo, canonical_branch_name); - Ensure.Int32Result(res); - return LaxUtf8Marshaler.FromNative(buf.ptr) ?? string.Empty; + if (!shouldThrowIfNotFound && + (res == (int) GitErrorCode.NotFound || res == (int) GitErrorCode.Ambiguous)) + { + return null; + } + + Ensure.ZeroResult(res); + return LaxUtf8Marshaler.FromNative(buf.ptr); } } From 0e5bfbc346684743a2eae6b43413e989e6ccc749 Mon Sep 17 00:00:00 2001 From: Jameson Miller Date: Wed, 24 Sep 2014 16:55:51 -0400 Subject: [PATCH 2/2] Verify that resolving a remote-tracking branch's remote can throw --- LibGit2Sharp.Tests/BranchFixture.cs | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/LibGit2Sharp.Tests/BranchFixture.cs b/LibGit2Sharp.Tests/BranchFixture.cs index 642b1a8a7..adfaeceb3 100644 --- a/LibGit2Sharp.Tests/BranchFixture.cs +++ b/LibGit2Sharp.Tests/BranchFixture.cs @@ -685,6 +685,33 @@ public void CanSetTrackedBranch() } } + [Fact] + public void SetTrackedBranchForUnreasolvableRemoteThrows() + { + const string testBranchName = "branchToSetUpstreamInfoFor"; + const string trackedBranchName = "refs/remotes/origin/master"; + var fetchRefSpecs = new string[] { "+refs/heads/notfound/*:refs/remotes/origin/notfound/*" }; + + string path = CloneStandardTestRepo(); + using (var repo = new Repository(path)) + { + // Modify the fetch spec so that the remote for the remote-tracking branch + // cannot be resolved. + Remote remote = repo.Network.Remotes["origin"]; + Assert.NotNull(remote); + repo.Network.Remotes.Update(remote, r => r.FetchRefSpecs = fetchRefSpecs); + + // Now attempt to update the tracked branch + Branch branch = repo.CreateBranch(testBranchName); + Assert.False(branch.IsTracking); + + Branch trackedBranch = repo.Branches[trackedBranchName]; + + Assert.Throws(() => repo.Branches.Update(branch, + b => b.TrackedBranch = trackedBranch.CanonicalName)); + } + } + [Fact] public void CanSetUpstreamBranch() {