Commit bc05f30c470f7d147ff85b30ad5719bbce70a4a3

nulltoken 2012-11-19T18:49:25

object: refine git_object_peel() error report

diff --git a/include/git2/object.h b/include/git2/object.h
index fcc56cb..e5ca17e 100644
--- a/include/git2/object.h
+++ b/include/git2/object.h
@@ -179,8 +179,9 @@ GIT_EXTERN(size_t) git_object__size(git_otype type);
  *
  * @param peeled Pointer to the peeled git_object
  * @param object The object to be processed
- * @param target_type The type of the requested object
- * @return 0 or an error code
+ * @param target_type The type of the requested object (GIT_OBJ_COMMIT,
+ * GIT_OBJ_TAG, GIT_OBJ_TREE, GIT_OBJ_BLOB or GIT_OBJ_ANY).
+ * @return 0 on success, GIT_EAMBIGUOUS, GIT_ENOTFOUND or an error code
  */
 GIT_EXTERN(int) git_object_peel(
 	git_object **peeled,
diff --git a/include/git2/refs.h b/include/git2/refs.h
index 7d047ca..f308210 100644
--- a/include/git2/refs.h
+++ b/include/git2/refs.h
@@ -479,8 +479,9 @@ GIT_EXTERN(int) git_reference_normalize_name(
  *
  * @param peeled Pointer to the peeled git_object
  * @param ref The reference to be processed
- * @param target_type The type of the requested object
- * @return 0 or an error code
+ * @param target_type The type of the requested object (GIT_OBJ_COMMIT,
+ * GIT_OBJ_TAG, GIT_OBJ_TREE, GIT_OBJ_BLOB or GIT_OBJ_ANY).
+ * @return 0 on success, GIT_EAMBIGUOUS, GIT_ENOTFOUND or an error code
  */
 GIT_EXTERN(int) git_reference_peel(
 	git_object **out,
diff --git a/src/object.c b/src/object.c
index f88c2ba..d57b646 100644
--- a/src/object.c
+++ b/src/object.c
@@ -304,12 +304,6 @@ size_t git_object__size(git_otype type)
 	return git_objects_table[type].size;
 }
 
-static int peel_error(int error, const char* msg)
-{
-	giterr_set(GITERR_INVALID, "The given object cannot be peeled - %s", msg);
-	return error;
-}
-
 static int dereference_object(git_object **dereferenced, git_object *obj)
 {
 	git_otype type = git_object_type(obj);
@@ -322,22 +316,46 @@ static int dereference_object(git_object **dereferenced, git_object *obj)
 		return git_tag_target(dereferenced, (git_tag*)obj);
 
 	case GIT_OBJ_BLOB:
-		return peel_error(GIT_ERROR, "cannot dereference blob");
+		return GIT_ENOTFOUND;
 
 	case GIT_OBJ_TREE:
-		return peel_error(GIT_ERROR, "cannot dereference tree");
+		return GIT_EAMBIGUOUS;
 
 	default:
-		return peel_error(GIT_ENOTFOUND, "unexpected object type encountered");
+		return GIT_EINVALIDSPEC;
 	}
 }
 
+static int peel_error(int error, const git_oid *oid, git_otype type)
+{
+	const char *type_name;
+	char hex_oid[GIT_OID_HEXSZ + 1];
+
+	type_name = git_object_type2string(type);
+
+	git_oid_fmt(hex_oid, oid);
+	hex_oid[GIT_OID_HEXSZ] = '\0';
+
+	giterr_set(GITERR_OBJECT, "The git_object of id '%s' can not be "
+		"successfully peeled into a %s (git_otype=%i).", hex_oid, type_name, type);
+
+	return error;
+}
+
 int git_object_peel(
 	git_object **peeled,
 	const git_object *object,
 	git_otype target_type)
 {
 	git_object *source, *deref = NULL;
+	int error;
+
+	if (target_type != GIT_OBJ_TAG && 
+		target_type != GIT_OBJ_COMMIT && 
+		target_type != GIT_OBJ_TREE && 
+		target_type != GIT_OBJ_BLOB && 
+		target_type != GIT_OBJ_ANY)
+			return GIT_EINVALIDSPEC;
 
 	assert(object && peeled);
 
@@ -346,7 +364,7 @@ int git_object_peel(
 
 	source = (git_object *)object;
 
-	while (!dereference_object(&deref, source)) {
+	while (!(error = dereference_object(&deref, source))) {
 
 		if (source != object)
 			git_object_free(source);
@@ -371,6 +389,10 @@ int git_object_peel(
 		git_object_free(source);
 
 	git_object_free(deref);
-	return -1;
+
+	if (error)
+		error = peel_error(error, git_object_id(object), target_type);
+
+	return error;
 }
 
diff --git a/tests-clar/object/peel.c b/tests-clar/object/peel.c
index a197728..bb0bbd0 100644
--- a/tests-clar/object/peel.c
+++ b/tests-clar/object/peel.c
@@ -79,12 +79,12 @@ void test_object_peel__can_peel_a_commit(void)
 
 void test_object_peel__cannot_peel_a_tree(void)
 {
-	assert_peel_error(GIT_ERROR, "53fc32d17276939fc79ed05badaef2db09990016", GIT_OBJ_BLOB);
+	assert_peel_error(GIT_EAMBIGUOUS, "53fc32d17276939fc79ed05badaef2db09990016", GIT_OBJ_BLOB);
 }
 
 void test_object_peel__cannot_peel_a_blob(void)
 {
-	assert_peel_error(GIT_ERROR, "0266163a49e280c4f5ed1e08facd36a2bd716bcf", GIT_OBJ_COMMIT);
+	assert_peel_error(GIT_ENOTFOUND, "0266163a49e280c4f5ed1e08facd36a2bd716bcf", GIT_OBJ_COMMIT);
 }
 
 void test_object_peel__target_any_object_for_type_change(void)
@@ -98,8 +98,13 @@ void test_object_peel__target_any_object_for_type_change(void)
 		"53fc32d17276939fc79ed05badaef2db09990016", GIT_OBJ_TREE);
 
 	/* fail to peel tree */
-	assert_peel_error(GIT_ERROR, "53fc32d17276939fc79ed05badaef2db09990016", GIT_OBJ_ANY);
+	assert_peel_error(GIT_EAMBIGUOUS, "53fc32d17276939fc79ed05badaef2db09990016", GIT_OBJ_ANY);
 
 	/* fail to peel blob */
-	assert_peel_error(GIT_ERROR, "0266163a49e280c4f5ed1e08facd36a2bd716bcf", GIT_OBJ_ANY);
+	assert_peel_error(GIT_ENOTFOUND, "0266163a49e280c4f5ed1e08facd36a2bd716bcf", GIT_OBJ_ANY);
+}
+
+void test_object_peel__should_use_a_well_known_type(void)
+{
+	assert_peel_error(GIT_EINVALIDSPEC, "7b4384978d2493e851f9cca7858815fac9b10980", GIT_OBJ__EXT2);
 }
diff --git a/tests-clar/refs/peel.c b/tests-clar/refs/peel.c
index 6fa6009..34bd02c 100644
--- a/tests-clar/refs/peel.c
+++ b/tests-clar/refs/peel.c
@@ -78,7 +78,7 @@ void test_refs_peel__can_peel_a_symbolic_reference(void)
 
 void test_refs_peel__cannot_peel_into_a_non_existing_target(void)
 {
-	assert_peel_error(GIT_ERROR, "refs/tags/point_to_blob", GIT_OBJ_TAG);
+	assert_peel_error(GIT_ENOTFOUND, "refs/tags/point_to_blob", GIT_OBJ_TAG);
 }
 
 void test_refs_peel__can_peel_into_any_non_tag_object(void)