Skip to content
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
85 changes: 85 additions & 0 deletions LibGit2Sharp.Tests/BranchFixture.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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 = "http://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()
{
Expand Down Expand Up @@ -627,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<LibGit2SharpException>(() => repo.Branches.Update(branch,
b => b.TrackedBranch = trackedBranch.CanonicalName));
}
}

[Fact]
public void CanSetUpstreamBranch()
{
Expand Down
18 changes: 12 additions & 6 deletions LibGit2Sharp/Branch.cs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,13 @@ public virtual string UpstreamBranchCanonicalName
}

/// <summary>
/// Gets the configured <see cref="Remote"/> to fetch from and push to.
/// Get the remote for the branch.
/// <para>
/// If this is a local branch, this will return the configured
/// <see cref="Remote"/> to fetch from and push to. If this is a
/// remote-tracking branch, this will return the remote containing
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about "containing", but can't come with a better word...

@carlosmn @dahlbyk Thoughts?

/// the tracked branch.
/// </para>
/// </summary>
public virtual Remote Remote
{
Expand All @@ -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];
Expand Down Expand Up @@ -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);
}

/// <summary>
Expand Down
2 changes: 1 addition & 1 deletion LibGit2Sharp/BranchUpdater.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you think of a test that could leverage this code path?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a commit that tests this code path. Setting a tracked branch where the remote cannot be resolved (in this case the remote cannot be found) throws an exception.


Remote remote = repo.Network.Remotes.RemoteForName(remoteName);
mergeBranchName = remote.FetchSpecTransformToSource(canonicalName);
Expand Down
12 changes: 9 additions & 3 deletions LibGit2Sharp/Core/Proxy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jamill Can you please share a hint about this change? 😉

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we expect this to be null - it should always be a proper null terminated string? This is not a pattern that we follow consistently - for instance the next method does not do this coalescing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like it's been bring in through 4e43723. But I do not remember why it's been added. Let's get rid of it provided all the tests pass.

}
}

Expand Down