Merge pull request #815 from nulltoken/topic/revparse-refac

More revparse <3

src/revparse.c

@@ -147,23 +147,23 @@ static int revparse_lookup_object(git_object **out, git_repository *repo, const
 	if (error < 0 && error != GIT_ENOTFOUND)
 		return error;
 
-	error = maybe_sha_or_abbrev(out, repo, spec);
-	if (!error)
-		return 0;
-
-	if (error < 0 && error != GIT_ENOTFOUND)
-		return error;
-
 	error = disambiguate_refname(&ref, repo, spec);
 	if (!error) {
 		error = git_object_lookup(out, repo, git_reference_oid(ref), GIT_OBJ_ANY);
 		git_reference_free(ref);
-		return 0;
+		return error;
 	}
 
 	if (error < 0 && error != GIT_ENOTFOUND)
 		return error;
 
+	error = maybe_sha_or_abbrev(out, repo, spec);
+	if (!error)
+		return 0;
+
+	if (error < 0 && error != GIT_ENOTFOUND)
+		return error;
+
 	giterr_set(GITERR_REFERENCE, "Refspec '%s' not found.", spec);
 	return GIT_ENOTFOUND;
 }
@@ -482,7 +482,7 @@ static int handle_caret_syntax(git_object **out, git_repository *repo, git_objec
 	if (movementlen == 0) {
 		n = 1;
 	} else {
-		git__strtol32(&n, movement, NULL, 0);
+		git__strtol32(&n, movement, NULL, 10);
 	}
 	commit = (git_commit*)obj;
 
@@ -513,95 +513,44 @@ static int handle_linear_syntax(git_object **out, git_object *obj, const char *m
 	/* "~" is the same as "~1" */
 	if (*movement == '\0') {
 		n = 1;
-	} else if (git__strtol32(&n, movement, NULL, 0) < 0) {
+	} else if (git__strtol32(&n, movement, NULL, 10) < 0) {
 		return GIT_ERROR;
 	}
 
 	return git_commit_nth_gen_ancestor((git_commit **)out, (git_commit*)obj, n);
 }
 
-static int oid_for_tree_path(git_oid *out, git_tree *tree, git_repository *repo, const char *path)
-{
-	char *str, *tok;
-	void *alloc;
-	git_tree *tree2 = tree;
-	const git_tree_entry *entry = NULL;
-	git_otype type;
-
-	if (*path == '\0') {
-		git_oid_cpy(out, git_object_id((git_object *)tree));
-		return 0;
-	}
-
-	alloc = str = git__strdup(path);
-
-	while ((tok = git__strtok(&str, "/\\")) != NULL) {
-		entry = git_tree_entry_byname(tree2, tok);
-		if (tree2 != tree) git_tree_free(tree2);
-
-		if (entry == NULL)
-			break;
-
-		type = git_tree_entry_type(entry);
-
-		switch (type) {
-		case GIT_OBJ_TREE:
-			if (*str == '\0')
-				break;
-			if (git_tree_lookup(&tree2, repo, &entry->oid) < 0) {
-				git__free(alloc);
-				return GIT_ERROR;
-			}
-			break;
-		case GIT_OBJ_BLOB:
-			if (*str != '\0') {
-				entry = NULL;
-				goto out;
-			}
-			break;
-		default:
-			/* TODO: support submodules? */
-			giterr_set(GITERR_INVALID, "Unimplemented");
-			git__free(alloc);
-			return GIT_ERROR;
-		}
-	}
-
-out:
-	if (!entry) {
-		giterr_set(GITERR_INVALID, "Invalid tree path '%s'", path);
-		git__free(alloc);
-		return GIT_ENOTFOUND;
-	}
-
-	git_oid_cpy(out, git_tree_entry_id(entry));
-	git__free(alloc);
-	return 0;
-}
-
 static int handle_colon_syntax(git_object **out,
 	git_repository *repo,
 	git_object *obj,
 	const char *path)
 {
-	git_tree *tree;
-	git_oid oid;
-	int error;
+	git_object *tree = obj;
+	int error = -1;
+	git_tree_entry *entry = NULL;
 
 	/* Dereference until we reach a tree. */
-	if (dereference_to_type(&obj, obj, GIT_OBJ_TREE) < 0) {
+	if (dereference_to_type(&tree, obj, GIT_OBJ_TREE) < 0)
 		return GIT_ERROR;
-	}
-	tree = (git_tree*)obj;
 
-	/* Find the blob or tree at the given path. */
-	error = oid_for_tree_path(&oid, tree, repo, path);
-	git_tree_free(tree);
+	if (*path == '\0')
+		return git_object_lookup(out, repo, git_object_id(tree), GIT_OBJ_TREE);
 
-	if (error < 0)
-		return error;
+	/*
+	 * TODO: Handle the relative path syntax
+	 * (:./relative/path and :../relative/path)
+	 */
+	if ((error = git_tree_entry_bypath(&entry, (git_tree *)tree, path)) < 0)
+		goto cleanup;
+
+	error = git_tree_entry_to_object(out, repo, entry);
 
-	return git_object_lookup(out, repo, &oid, GIT_OBJ_ANY);
+cleanup:
+	git_tree_entry_free(entry);
+	if (tree != obj)
+		git_object_free(tree);
+
+	return error;
 }
 
 static int revparse_global_grep(git_object **out, git_repository *repo, const char *pattern)

src/tree.c

@@ -140,6 +140,9 @@ static int tree_key_search(git_vector *entries, const char *filename, size_t fil
 
 void git_tree_entry_free(git_tree_entry *entry)
 {
+	if (entry == NULL)
+		return;
+
 	git__free(entry);
 }
 
@@ -727,7 +730,7 @@ int git_tree_entry_bypath(
 		if (!git_tree_entry__is_tree(entry)) {
 			giterr_set(GITERR_TREE,
 				"The path '%s' does not exist in the given tree", path);
-			return -1;
+			return GIT_ENOTFOUND;
 		}
 
 		/* If there's only a slash left in the path, we 

tests-clar/object/tree/frompath.c

@@ -46,12 +46,12 @@ void test_object_tree_frompath__retrieve_tree_from_path_to_treeentry(void)
 	assert_tree_from_path(tree, "ab/", "ab");
 	assert_tree_from_path(tree, "ab/de/", "de");
 
-	cl_must_fail(git_tree_entry_bypath(&e, tree, "i-do-not-exist.txt"));
-	cl_must_fail(git_tree_entry_bypath(&e, tree, "README/"));
-	cl_must_fail(git_tree_entry_bypath(&e, tree, "ab/de/fgh/i-do-not-exist.txt"));
-	cl_must_fail(git_tree_entry_bypath(&e, tree, "nope/de/fgh/1.txt"));
-	cl_must_fail(git_tree_entry_bypath(&e, tree, "ab/me-neither/fgh/2.txt"));
-	cl_must_fail(git_tree_entry_bypath(&e, tree, "ab/me-neither/fgh/2.txt/"));
+	cl_assert_equal_i(GIT_ENOTFOUND, git_tree_entry_bypath(&e, tree, "i-do-not-exist.txt"));
+	cl_assert_equal_i(GIT_ENOTFOUND, git_tree_entry_bypath(&e, tree, "README/"));
+	cl_assert_equal_i(GIT_ENOTFOUND, git_tree_entry_bypath(&e, tree, "ab/de/fgh/i-do-not-exist.txt"));
+	cl_assert_equal_i(GIT_ENOTFOUND, git_tree_entry_bypath(&e, tree, "nope/de/fgh/1.txt"));
+	cl_assert_equal_i(GIT_ENOTFOUND, git_tree_entry_bypath(&e, tree, "ab/me-neither/fgh/2.txt"));
+	cl_assert_equal_i(GIT_ENOTFOUND, git_tree_entry_bypath(&e, tree, "ab/me-neither/fgh/2.txt/"));
 }
 
 void test_object_tree_frompath__fail_when_processing_an_invalid_path(void)

tests-clar/refs/revparse.c

@@ -325,12 +325,26 @@ void test_refs_revparse__colon(void)
 	test_object("subtrees:nope", NULL);
 	test_object("test/master^1:branch_file.txt", NULL);
 
-	/* Trees */
+	/* From tags */
+	test_object("test:readme.txt", "0266163a49e280c4f5ed1e08facd36a2bd716bcf");
+	test_object("tags/test:readme.txt", "0266163a49e280c4f5ed1e08facd36a2bd716bcf");
+	test_object("e90810b:readme.txt", "0266163a49e280c4f5ed1e08facd36a2bd716bcf");
+	test_object("tags/e90810b:readme.txt", "0266163a49e280c4f5ed1e08facd36a2bd716bcf");
+
+	/* From commits */
+	test_object("a65f:branch_file.txt", "3697d64be941a53d4ae8f6a271e4e3fa56b022cc");
+
+	/* From trees */
+	test_object("a65f^{tree}:branch_file.txt", "3697d64be941a53d4ae8f6a271e4e3fa56b022cc");
+	test_object("944c:branch_file.txt", "3697d64be941a53d4ae8f6a271e4e3fa56b022cc");
+
+	/* Retrieving trees */
 	test_object("master:", "944c0f6e4dfa41595e6eb3ceecdb14f50fe18162");
 	test_object("subtrees:", "ae90f12eea699729ed24555e40b9fd669da12a12");
 	test_object("subtrees:ab", "f1425cef211cc08caa31e7b545ffb232acb098c3");
+	test_object("subtrees:ab/", "f1425cef211cc08caa31e7b545ffb232acb098c3");
 
-	/* Blobs */
+	/* Retrieving blobs */
 	test_object("subtrees:ab/4.txt", "d6c93164c249c8000205dd4ec5cbca1b516d487f");
 	test_object("subtrees:ab/de/fgh/1.txt", "1f67fc4386b2d171e0d21be1c447e12660561f9b");
 	test_object("master:README", "a8233120f6ad708f843d861ce2b7228ec4e3dec6");
@@ -340,3 +354,52 @@ void test_refs_revparse__colon(void)
 	test_object(":/packed commit t", "41bc8c69075bbdb46c5c6f0566cc8cc5b46e8bd9");
 	test_object("test/master^2:branch_file.txt", "45b983be36b73c0788dc9cbcb76cbb80fc7bb057");
 }
+
+void test_refs_revparse__disambiguation(void)
+{
+	/*
+	 * $ git show e90810b
+	 * tag e90810b
+	 * Tagger: Vicent Marti <tanoku@gmail.com>
+	 * Date:   Thu Aug 12 03:59:17 2010 +0200
+	 *
+	 * This is a very simple tag.
+	 *
+	 * commit e90810b8df3e80c413d903f631643c716887138d
+	 * Author: Vicent Marti <tanoku@gmail.com>
+	 * Date:   Thu Aug 5 18:42:20 2010 +0200
+	 *
+	 *     Test commit 2
+	 *
+	 * diff --git a/readme.txt b/readme.txt
+	 * index 6336846..0266163 100644
+	 * --- a/readme.txt
+	 * +++ b/readme.txt
+	 * @@ -1 +1,2 @@
+	 *  Testing a readme.txt
+	 * +Now we add a single line here
+	 *
+	 * $ git show-ref e90810b
+	 * 7b4384978d2493e851f9cca7858815fac9b10980 refs/tags/e90810b
+	 *
+	 */
+	test_object("e90810b", "7b4384978d2493e851f9cca7858815fac9b10980");
+
+	/*
+	 * $ git show e90810
+	 * commit e90810b8df3e80c413d903f631643c716887138d
+	 * Author: Vicent Marti <tanoku@gmail.com>
+	 * Date:   Thu Aug 5 18:42:20 2010 +0200
+	 *
+	 *     Test commit 2
+	 *
+	 * diff --git a/readme.txt b/readme.txt
+	 * index 6336846..0266163 100644
+	 * --- a/readme.txt
+	 * +++ b/readme.txt
+	 * @@ -1 +1,2 @@
+	 *  Testing a readme.txt
+	 * +Now we add a single line here
+	 */
+	test_object("e90810", "e90810b8df3e80c413d903f631643c716887138d");
+}