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
2 changes: 1 addition & 1 deletion LibGit2Sharp.Tests/BlameFixture.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public void CanBlameFromADifferentCommit()
using (var repo = new Repository(path))
{
// File doesn't exist at HEAD
Assert.Throws<LibGit2SharpException>(() => repo.Blame("ancestor-only.txt"));
Assert.Throws<NotFoundException>(() => repo.Blame("ancestor-only.txt"));

var blame = repo.Blame("ancestor-only.txt", new BlameOptions { StartingAt = "9107b30" });
Assert.Equal(1, blame.Count());
Expand Down
8 changes: 4 additions & 4 deletions LibGit2Sharp.Tests/BranchFixture.cs
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ public void CreatingBranchWithUnknownNamedTargetThrows()
string path = SandboxBareTestRepo();
using (var repo = new Repository(path))
{
Assert.Throws<LibGit2SharpException>(() => repo.Branches.Add("my_new_branch", "my_old_branch"));
Assert.Throws<NotFoundException>(() => repo.Branches.Add("my_new_branch", "my_old_branch"));
}
}

Expand All @@ -264,8 +264,8 @@ public void CreatingBranchWithUnknownShaTargetThrows()
string path = SandboxBareTestRepo();
using (var repo = new Repository(path))
{
Assert.Throws<LibGit2SharpException>(() => repo.Branches.Add("my_new_branch", Constants.UnknownSha));
Assert.Throws<LibGit2SharpException>(() => repo.Branches.Add("my_new_branch", Constants.UnknownSha.Substring(0, 7)));
Assert.Throws<NotFoundException>(() => repo.Branches.Add("my_new_branch", Constants.UnknownSha));
Assert.Throws<NotFoundException>(() => repo.Branches.Add("my_new_branch", Constants.UnknownSha.Substring(0, 7)));
}
}

Expand Down Expand Up @@ -729,7 +729,7 @@ public void SetTrackedBranchForUnreasolvableRemoteThrows()

Branch trackedBranch = repo.Branches[trackedBranchName];

Assert.Throws<LibGit2SharpException>(() => repo.Branches.Update(branch,
Assert.Throws<NotFoundException>(() => repo.Branches.Update(branch,
b => b.TrackedBranch = trackedBranch.CanonicalName));
}
}
Expand Down
4 changes: 2 additions & 2 deletions LibGit2Sharp.Tests/CheckoutFixture.cs
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,7 @@ public void CheckingOutANonExistingBranchThrows()
string path = SandboxBareTestRepo();
using (var repo = new Repository(path))
{
Assert.Throws<LibGit2SharpException>(() => repo.Checkout("i-do-not-exist"));
Assert.Throws<NotFoundException>(() => repo.Checkout("i-do-not-exist"));
}
}

Expand Down Expand Up @@ -895,7 +895,7 @@ public void CheckoutLowerCasedHeadThrows()
string path = SandboxStandardTestRepo();
using (var repo = new Repository(path))
{
Assert.Throws<LibGit2SharpException>(() => repo.Checkout("head"));
Assert.Throws<NotFoundException>(() => repo.Checkout("head"));
}
}

Expand Down
8 changes: 4 additions & 4 deletions LibGit2Sharp.Tests/CommitFixture.cs
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,8 @@ public void QueryingTheCommitHistoryWithUnknownShaOrInvalidEntryPointThrows()
string path = SandboxBareTestRepo();
using (var repo = new Repository(path))
{
Assert.Throws<LibGit2SharpException>(() => repo.Commits.QueryBy(new CommitFilter { Since = Constants.UnknownSha }).Count());
Assert.Throws<LibGit2SharpException>(() => repo.Commits.QueryBy(new CommitFilter { Since = "refs/heads/deadbeef" }).Count());
Assert.Throws<NotFoundException>(() => repo.Commits.QueryBy(new CommitFilter { Since = Constants.UnknownSha }).Count());
Assert.Throws<NotFoundException>(() => repo.Commits.QueryBy(new CommitFilter { Since = "refs/heads/deadbeef" }).Count());
Assert.Throws<ArgumentNullException>(() => repo.Commits.QueryBy(new CommitFilter { Since = null }).Count());
}
}
Expand All @@ -121,8 +121,8 @@ public void QueryingTheCommitHistoryFromACorruptedReferenceThrows()
{
CreateCorruptedDeadBeefHead(repo.Info.Path);

Assert.Throws<LibGit2SharpException>(() => repo.Commits.QueryBy(new CommitFilter { Since = repo.Branches["deadbeef"] }).Count());
Assert.Throws<LibGit2SharpException>(() => repo.Commits.QueryBy(new CommitFilter { Since = repo.Refs["refs/heads/deadbeef"] }).Count());
Assert.Throws<NotFoundException>(() => repo.Commits.QueryBy(new CommitFilter { Since = repo.Branches["deadbeef"] }).Count());
Assert.Throws<NotFoundException>(() => repo.Commits.QueryBy(new CommitFilter { Since = repo.Refs["refs/heads/deadbeef"] }).Count());
}
}

Expand Down
4 changes: 2 additions & 2 deletions LibGit2Sharp.Tests/ReferenceFixture.cs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public void CreatingADirectReferenceWithARevparseSpecPointingAtAnUnknownObjectFa
string path = SandboxBareTestRepo();
using (var repo = new Repository(path))
{
Assert.Throws<LibGit2SharpException>(() => repo.Refs.Add(name, "master^42"));
Assert.Throws<NotFoundException>(() => repo.Refs.Add(name, "master^42"));
}
}

Expand Down Expand Up @@ -610,7 +610,7 @@ public void UpdatingADirectReferenceTargetWithARevparsePointingAtAnUnknownObject
string path = SandboxBareTestRepo();
using (var repo = new Repository(path))
{
Assert.Throws<LibGit2SharpException>(() => repo.Refs.UpdateTarget(repo.Refs["refs/heads/master"], "refs/heads/nope"));
Assert.Throws<NotFoundException>(() => repo.Refs.UpdateTarget(repo.Refs["refs/heads/master"], "refs/heads/nope"));
}
}

Expand Down
15 changes: 15 additions & 0 deletions LibGit2Sharp.Tests/RepositoryFixture.cs
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,21 @@ public void LookingUpWithATooShortShaThrows()
}
}

[Fact]
public void LookingUpByAWrongRevParseExpressionThrows()
{
string path = SandboxBareTestRepo();
using (var repo = new Repository(path))
{
Assert.Throws<InvalidSpecificationException>(() => repo.Lookup("tags/point_to_blob^{tree}"));
Assert.Throws<InvalidSpecificationException>(() => repo.Lookup("tags/point_to_blob^{commit}"));
Assert.Throws<InvalidSpecificationException>(() => repo.Lookup<Commit>("tags/point_to_blob^{commit}"));
Assert.Throws<InvalidSpecificationException>(() => repo.Lookup("master^{tree}^{blob}"));
Assert.Throws<InvalidSpecificationException>(() => repo.Lookup<Blob>("master^{blob}"));
Assert.Throws<PeelException>(() => repo.Lookup<Blob>("tags/e90810b^{blob}"));
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 still feel a little bothered that those two above throw different exceptions.

FWIW:

  • master^{commit}^{blob} throws InvalidSpecificationException
  • tags/e90810b^{commit}^{blob} throws InvalidSpecificationException

/cc @carlosmn @jamill Thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

Seems fine to me. If an app cares about the difference between the errors, it can tell the user what went wrong more specifically.

Copy link
Member

Choose a reason for hiding this comment

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

I don't know if the ability to discern between these two cases is actually worth the complexity of having two exceptions that are pretty similar (they both mean I can peel the object as requested). However, everything is working as designed in libgit2 - I wouldn't argue to change the current design unless we get feedback that this is confusing.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fits me

}
}

[Fact]
public void LookingUpAGitLinkThrows()
{
Expand Down
2 changes: 1 addition & 1 deletion LibGit2Sharp.Tests/ResetHeadFixture.cs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public void ResettingWithBadParamsThrows()
Assert.Throws<ArgumentNullException>(() => repo.Reset(ResetMode.Soft, (string)null));
Assert.Throws<ArgumentNullException>(() => repo.Reset(ResetMode.Soft, (Commit)null));
Assert.Throws<ArgumentException>(() => repo.Reset(ResetMode.Soft, ""));
Assert.Throws<LibGit2SharpException>(() => repo.Reset(ResetMode.Soft, Constants.UnknownSha));
Assert.Throws<NotFoundException>(() => repo.Reset(ResetMode.Soft, Constants.UnknownSha));
Assert.Throws<InvalidSpecificationException>(() => repo.Reset(ResetMode.Soft, repo.Head.Tip.Tree.Sha));
}
}
Expand Down
8 changes: 4 additions & 4 deletions LibGit2Sharp.Tests/TagFixture.cs
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ public void CreatingATagForAnUnknowReferenceThrows()
string path = SandboxBareTestRepo();
using (var repo = new Repository(path))
{
Assert.Throws<LibGit2SharpException>(() => repo.ApplyTag("mytagnorev", "aaaaaaaaaaa"));
Assert.Throws<NotFoundException>(() => repo.ApplyTag("mytagnorev", "aaaaaaaaaaa"));
}
}

Expand All @@ -269,7 +269,7 @@ public void CreatingATagForAnUnknowObjectIdThrows()
string path = SandboxBareTestRepo();
using (var repo = new Repository(path))
{
Assert.Throws<LibGit2SharpException>(() => repo.ApplyTag("mytagnorev", Constants.UnknownSha));
Assert.Throws<NotFoundException>(() => repo.ApplyTag("mytagnorev", Constants.UnknownSha));
}
}

Expand Down Expand Up @@ -507,7 +507,7 @@ public void AddTagWithNotExistingTargetThrows()
string path = SandboxBareTestRepo();
using (var repo = new Repository(path))
{
Assert.Throws<LibGit2SharpException>(() => repo.Tags.Add("test_tag", Constants.UnknownSha, signatureTim, "message"));
Assert.Throws<NotFoundException>(() => repo.Tags.Add("test_tag", Constants.UnknownSha, signatureTim, "message"));
}
}

Expand Down Expand Up @@ -623,7 +623,7 @@ public void RemovingAnUnknownTagShouldFail()
string path = SandboxBareTestRepo();
using (var repo = new Repository(path))
{
Assert.Throws<LibGit2SharpException>(() => repo.Tags.Remove("unknown-tag"));
Assert.Throws<NotFoundException>(() => repo.Tags.Remove("unknown-tag"));
}
}

Expand Down
4 changes: 3 additions & 1 deletion LibGit2Sharp/Core/Ensure.cs
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@ private static readonly Dictionary<GitErrorCode, Func<string, GitErrorCode, GitE
{ GitErrorCode.NonFastForward, (m, r, c) => new NonFastForwardException(m, r, c) },
{ GitErrorCode.MergeConflict, (m, r, c) => new MergeConflictException(m, r, c) },
{ GitErrorCode.LockedFile, (m, r, c) => new LockedFileException(m, r, c) },
{ GitErrorCode.NotFound, (m, r, c) => new NotFoundException(m, r, c) },
{ GitErrorCode.Peel, (m, r, c) => new PeelException(m, r, c) },
};

private static void HandleError(int result)
Expand Down Expand Up @@ -215,7 +217,7 @@ public static void GitObjectIsNotNull(GitObject gitObject, string identifier)
}
else
{
exceptionBuilder = m => new LibGit2SharpException(m);
exceptionBuilder = m => new NotFoundException(m);
}

GitObjectIsNotNull(gitObject, identifier, exceptionBuilder);
Expand Down
5 changes: 4 additions & 1 deletion LibGit2Sharp/InvalidSpecificationException.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@
namespace LibGit2Sharp
{
/// <summary>
/// The exception that is thrown when the provided specification is syntactically incorrect.
/// The exception that is thrown when a provided specification is bad. This
/// can happen if the provided specification is syntactically incorrect, or
/// if the spec refers to an object of an incorrect type (e.g. asking to
/// create a branch from a blob, or peeling a blob to a commit).
Copy link
Member Author

Choose a reason for hiding this comment

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

👍

/// </summary>
[Serializable]
public class InvalidSpecificationException : LibGit2SharpException
Expand Down
1 change: 1 addition & 0 deletions LibGit2Sharp/LibGit2Sharp.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@
<Compile Include="GitObjectMetadata.cs" />
<Compile Include="PatchEntryChanges.cs" />
<Compile Include="PatchStats.cs" />
<Compile Include="PeelException.cs" />
<Compile Include="PullOptions.cs" />
<Compile Include="RefSpec.cs" />
<Compile Include="RefSpecCollection.cs" />
Expand Down
55 changes: 55 additions & 0 deletions LibGit2Sharp/PeelException.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
using System;
using System.Runtime.Serialization;
using LibGit2Sharp.Core;

namespace LibGit2Sharp
{
/// <summary>
/// The exception that is thrown when a tag cannot be peeled to the
/// target type due to the object model.
/// </summary>
[Serializable]
public class PeelException : LibGit2SharpException
{
/// <summary>
/// Initializes a new instance of the <see cref="PeelException"/> class.
/// </summary>
public PeelException()
{
}

/// <summary>
/// Initializes a new instance of the <see cref="PeelException"/> class with a specified error message.
/// </summary>
/// <param name="message">A message that describes the error.</param>
public PeelException(string message)
: base(message)
{
}

/// <summary>
/// Initializes a new instance of the <see cref="PeelException"/> class with a specified error message and a reference to the inner exception that is the cause of this exception.
/// </summary>
/// <param name="message">The error message that explains the reason for the exception.</param>
/// <param name="innerException">The exception that is the cause of the current exception. If the <paramref name="innerException"/> parameter is not a null reference, the current exception is raised in a catch block that handles the inner exception.</param>
public PeelException(string message, Exception innerException)
: base(message, innerException)
{
}

/// <summary>
/// Initializes a new instance of the <see cref="PeelException"/> class with a serialized data.
/// </summary>
/// <param name="info">The <see cref="SerializationInfo"/> that holds the serialized object data about the exception being thrown.</param>
/// <param name="context">The <see cref="StreamingContext"/> that contains contextual information about the source or destination.</param>
protected PeelException(SerializationInfo info, StreamingContext context)
: base(info, context)
{
}

internal PeelException(string message, GitErrorCode code, GitErrorCategory category)
: base(message, code, category)
{
}
}
}