ok/jj
1
0
Fork 0
forked from mirrors/jj
Code Pull requests Activity

git: use thiserror for errors

Browse source 
When you run e.g. `jj st` outside of a repo, it just
crashes. That'll probably give new users a bad impression, so I
was planning to improve error handling a bit. A good place to
start is by fixing the code I recently added (which obviously
should have been using `thiserror` from the beginning). That's
what this commit does.

Also, this is the first commit in this repo created with
Jujube! I've just started dogfooding it myself.
This commit is contained in:
Martin von Zweigbergk 2021-01-01 12:53:07 -08:00
parent 5b8e10394d
commit 14fe58e76a
3 changed files with 45 additions and 68 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

84
lib/src/git.rs
View file

@ -15,18 +15,14 @@
use crate::commit::Commit;
use crate::store::CommitId;
use crate::transaction::Transaction;
use git2::Error;
use thiserror::Error;
#[derive(Debug, PartialEq, Eq)]
#[derive(Error, Debug, PartialEq)]
pub enum GitImportError {
#[error("The repo is not backed by a git repo")]
NotAGitRepo,
InternalGitError(String),
}
impl From<git2::Error> for GitImportError {
fn from(err: Error) -> Self {
GitImportError::InternalGitError(format!("failed to read git refs: {}", err))
}
#[error("Unexpected git error when importing refs: {0}")]
InternalGitError(#[from] git2::Error),
}
// Reflect changes made in the underlying Git repo in the Jujube repo.
@ -49,12 +45,15 @@ pub fn import_refs(tx: &mut Transaction) -> Result<(), GitImportError> {
Ok(())
}
#[derive(Debug, PartialEq, Eq)]
#[derive(Error, Debug, PartialEq)]
pub enum GitFetchError {
#[error("The repo is not backed by a git repo")]
NotAGitRepo,
NoSuchRemote,
#[error("No git remote named '{0}'")]
NoSuchRemote(String),
// TODO: I'm sure there are other errors possible, such as transport-level errors.
InternalGitError(String),
#[error("Unexpected git error when fetching: {0}")]
InternalGitError(#[from] git2::Error),
}
pub fn fetch(tx: &mut Transaction, remote_name: &str) -> Result<(), GitFetchError> {
@ -64,32 +63,34 @@ pub fn fetch(tx: &mut Transaction, remote_name: &str) -> Result<(), GitFetchErro
.find_remote(remote_name)
.map_err(|err| match (err.class(), err.code()) {
(git2::ErrorClass::Config, git2::ErrorCode::NotFound) => {
GitFetchError::NoSuchRemote
GitFetchError::NoSuchRemote(remote_name.to_string())
}
(git2::ErrorClass::Config, git2::ErrorCode::InvalidSpec) => {
GitFetchError::NoSuchRemote
GitFetchError::NoSuchRemote(remote_name.to_string())
}
_ => GitFetchError::InternalGitError(format!("unhandled git error: {:?}", err)),
_ => GitFetchError::InternalGitError(err),
})?;
let refspec: &[&str] = &[];
remote.fetch(refspec, None, None).map_err(|err| {
GitFetchError::InternalGitError(format!("unhandled git error: {:?}", err))
})?;
remote.fetch(refspec, None, None)?;
import_refs(tx).map_err(|err| match err {
GitImportError::NotAGitRepo => panic!("git repo somehow became a non-git repo"),
GitImportError::InternalGitError(err) => GitFetchError::InternalGitError(err),
GitImportError::InternalGitError(source) => GitFetchError::InternalGitError(source),
})?;
Ok(())
}
#[derive(Debug, PartialEq, Eq)]
#[derive(Error, Debug, PartialEq)]
pub enum GitPushError {
#[error("The repo is not backed by a git repo")]
NotAGitRepo,
NoSuchRemote,
#[error("No git remote named '{0}'")]
NoSuchRemote(String),
#[error("Push is not fast-forwardable'")]
NotFastForward,
// TODO: I'm sure there are other errors possible, such as transport-level errors,
// and errors caused by the remote rejecting the push.
InternalGitError(String),
#[error("Unexpected git error when pushing: {0}")]
InternalGitError(#[from] git2::Error),
}
pub fn push_commit(
@ -100,28 +101,23 @@ pub fn push_commit(
let git_repo = commit.store().git_repo().ok_or(GitPushError::NotAGitRepo)?;
// Create a temporary ref to work around https://github.com/libgit2/libgit2/issues/3178
let temp_ref_name = format!("refs/jj/git-push/{}", commit.id().hex());
let mut temp_ref = git_repo
.reference(
&temp_ref_name,
git2::Oid::from_bytes(&commit.id().0).unwrap(),
true,
"temporary reference for git push",
)
.map_err(|err| {
GitPushError::InternalGitError(format!(
"failed to create temporary git ref for push: {}",
err
))
})?;
let mut temp_ref = git_repo.reference(
&temp_ref_name,
git2::Oid::from_bytes(&commit.id().0).unwrap(),
true,
"temporary reference for git push",
)?;
let mut remote =
git_repo
.find_remote(remote_name)
.map_err(|err| match (err.class(), err.code()) {
(git2::ErrorClass::Config, git2::ErrorCode::NotFound) => GitPushError::NoSuchRemote,
(git2::ErrorClass::Config, git2::ErrorCode::InvalidSpec) => {
GitPushError::NoSuchRemote
(git2::ErrorClass::Config, git2::ErrorCode::NotFound) => {
GitPushError::NoSuchRemote(remote_name.to_string())
}
_ => GitPushError::InternalGitError(format!("unhandled git error: {:?}", err)),
(git2::ErrorClass::Config, git2::ErrorCode::InvalidSpec) => {
GitPushError::NoSuchRemote(remote_name.to_string())
}
_ => GitPushError::InternalGitError(err),
})?;
// Need to add "refs/heads/" prefix due to https://github.com/libgit2/libgit2/issues/1125
let refspec = format!("{}:refs/heads/{}", temp_ref_name, remote_branch);
@ -131,12 +127,8 @@ pub fn push_commit(
(git2::ErrorClass::Reference, git2::ErrorCode::NotFastForward) => {
GitPushError::NotFastForward
}
_ => GitPushError::InternalGitError(format!("unhandled git error: {:?}", err)),
_ => GitPushError::InternalGitError(err),
})?;
temp_ref.delete().map_err(|err| {
GitPushError::InternalGitError(format!(
"failed to delete temporary git ref for push: {}",
err
))
})
temp_ref.delete()?;
Ok(())
}

6
lib/tests/test_git.rs
View file

@ -185,7 +185,7 @@ fn test_fetch_no_such_remote() {
let mut tx = jj_repo.start_transaction("test");
let result = git::fetch(&mut tx, "invalid-remote");
assert_eq!(result, Err(GitFetchError::NoSuchRemote));
assert!(matches!(result, Err(GitFetchError::NoSuchRemote(_))));
tx.discard();
}
@ -266,7 +266,7 @@ fn test_push_commit_no_such_remote() {
let temp_dir = tempfile::tempdir().unwrap();
let setup = set_up_push_repos(&settings, &temp_dir);
let result = git::push_commit(&setup.new_commit, "invalid-remote", "main");
assert_eq!(result, Err(GitPushError::NoSuchRemote));
assert!(matches!(result, Err(GitPushError::NoSuchRemote(_))));
}
#[test]
@ -275,7 +275,7 @@ fn test_push_commit_invalid_remote() {
let temp_dir = tempfile::tempdir().unwrap();
let setup = set_up_push_repos(&settings, &temp_dir);
let result = git::push_commit(&setup.new_commit, "http://invalid-remote", "main");
assert_eq!(result, Err(GitPushError::NoSuchRemote));
assert!(matches!(result, Err(GitPushError::NoSuchRemote(_))));
}
#[test]

23
src/commands.rs
View file

@ -1993,12 +1993,7 @@ fn cmd_git_fetch(
GitFetchError::NotAGitRepo => CommandError::UserError(
"git push can only be used in repos backed by a git repo".to_string(),
),
GitFetchError::NoSuchRemote => {
CommandError::UserError(format!("No such git remote: {}", remote_name))
}
GitFetchError::InternalGitError(err) => {
CommandError::UserError(format!("Fetch failed: {:?}", err))
}
err => CommandError::UserError(err.to_string()),
})?;
tx.commit();
Ok(())
@ -2033,7 +2028,7 @@ fn cmd_git_clone(
.unwrap();
let mut tx = repo.start_transaction("fetch from git remote into empty repo");
git::fetch(&mut tx, remote_name).map_err(|err| match err {
GitFetchError::NotAGitRepo | GitFetchError::NoSuchRemote => {
GitFetchError::NotAGitRepo | GitFetchError::NoSuchRemote(_) => {
panic!("should't happen as we just created the repo and the git remote")
}
GitFetchError::InternalGitError(err) => {
@ -2060,15 +2055,7 @@ fn cmd_git_push(
GitPushError::NotAGitRepo => CommandError::UserError(
"git push can only be used in repos backed by a git repo".to_string(),
),
GitPushError::NoSuchRemote => {
CommandError::UserError(format!("No such git remote: {}", remote_name))
}
GitPushError::NotFastForward => {
CommandError::UserError("Push is not fast-forwardable".to_string())
}
GitPushError::InternalGitError(err) => {
CommandError::UserError(format!("Push failed: {:?}", err))
}
err => CommandError::UserError(err.to_string()),
})?;
Ok(())
}
@ -2085,9 +2072,7 @@ fn cmd_git_refresh(
GitImportError::NotAGitRepo => CommandError::UserError(
"git refresh can only be used in repos backed by a git repo".to_string(),
),
GitImportError::InternalGitError(err) => {
CommandError::UserError(format!("import of Git refs failed: {:?}", err))
}
err => CommandError::UserError(err.to_string()),
})?;
tx.commit();
Ok(())