From b6bfd96fdd0cffc37c843cbf0f7c43efdbe96ef9 Mon Sep 17 00:00:00 2001 From: nulltoken Date: Tue, 3 Jul 2012 12:07:33 +0200 Subject: [PATCH 01/19] refs: fix moving of the reflog when renaming a ref --- src/reflog.c | 48 +++++++++++++++++++++++++++++++++------- src/refs.c | 1 + tests-clar/refs/reflog.c | 28 +++++++++++++++++++++-- 3 files changed, 67 insertions(+), 10 deletions(-) diff --git a/src/reflog.c b/src/reflog.c index 3ea073e65..004ba936d 100644 --- a/src/reflog.c +++ b/src/reflog.c @@ -269,18 +269,50 @@ cleanup: int git_reflog_rename(git_reference *ref, const char *new_name) { - int error; + int error = -1, fd; git_buf old_path = GIT_BUF_INIT; git_buf new_path = GIT_BUF_INIT; + git_buf temp_path = GIT_BUF_INIT; - if (!git_buf_join_n(&old_path, '/', 3, ref->owner->path_repository, - GIT_REFLOG_DIR, ref->name) && - !git_buf_join_n(&new_path, '/', 3, ref->owner->path_repository, - GIT_REFLOG_DIR, new_name)) - error = p_rename(git_buf_cstr(&old_path), git_buf_cstr(&new_path)); - else - error = -1; + assert(ref && new_name); + if (git_buf_joinpath(&temp_path, ref->owner->path_repository, GIT_REFLOG_DIR) < 0) + return -1; + + if (git_buf_joinpath(&old_path, git_buf_cstr(&temp_path), ref->name) < 0) + goto cleanup; + + if (git_buf_joinpath(&new_path, git_buf_cstr(&temp_path), new_name) < 0) + goto cleanup; + + /* + * Move the reflog to a temporary place. This two-phase renaming is required + * in order to cope with funny renaming use cases when one tries to move a reference + * to a partially colliding namespace: + * - a/b -> a/b/c + * - a/b/c/d -> a/b/c + */ + if (git_buf_joinpath(&temp_path, git_buf_cstr(&temp_path), "temp_reflog") < 0) + goto cleanup; + + if ((fd = git_futils_mktmp(&temp_path, git_buf_cstr(&temp_path))) < 0) + goto cleanup; + p_close(fd); + + if (p_rename(git_buf_cstr(&old_path), git_buf_cstr(&temp_path)) < 0) + goto cleanup; + + if (git_path_isdir(git_buf_cstr(&new_path)) && + (git_futils_rmdir_r(git_buf_cstr(&new_path), GIT_DIRREMOVAL_ONLY_EMPTY_DIRS) < 0)) + goto cleanup; + + if (git_futils_mkpath2file(git_buf_cstr(&new_path), GIT_REFLOG_DIR_MODE) < 0) + goto cleanup; + + error = p_rename(git_buf_cstr(&temp_path), git_buf_cstr(&new_path)); + +cleanup: + git_buf_free(&temp_path); git_buf_free(&old_path); git_buf_free(&new_path); diff --git a/src/refs.c b/src/refs.c index ee076b3b8..80349b710 100644 --- a/src/refs.c +++ b/src/refs.c @@ -1406,6 +1406,7 @@ int git_reference_rename(git_reference *ref, const char *new_name, int force) /* * Rename the reflog file. */ + git_buf_clear(&aux_path); if (git_buf_join_n(&aux_path, '/', 3, ref->owner->path_repository, GIT_REFLOG_DIR, ref->name) < 0) goto cleanup; diff --git a/tests-clar/refs/reflog.c b/tests-clar/refs/reflog.c index 1bc51b2b8..a945b4789 100644 --- a/tests-clar/refs/reflog.c +++ b/tests-clar/refs/reflog.c @@ -26,7 +26,7 @@ static void assert_signature(git_signature *expected, git_signature *actual) // Fixture setup and teardown void test_refs_reflog__initialize(void) { - g_repo = cl_git_sandbox_init("testrepo"); + g_repo = cl_git_sandbox_init("testrepo.git"); } void test_refs_reflog__cleanup(void) @@ -61,7 +61,7 @@ void test_refs_reflog__write_then_read(void) cl_git_pass(git_reflog_write(ref, &oid, committer, commit_msg)); /* Reopen a new instance of the repository */ - cl_git_pass(git_repository_open(&repo2, "testrepo")); + cl_git_pass(git_repository_open(&repo2, "testrepo.git")); /* Lookup the preivously created branch */ cl_git_pass(git_reference_lookup(&lookedup_ref, repo2, new_ref)); @@ -121,3 +121,27 @@ void test_refs_reflog__dont_write_bad(void) git_reference_free(ref); } + +void test_refs_reflog__renaming_the_reference_moves_the_reflog(void) +{ + git_reference *master; + git_buf master_log_path = GIT_BUF_INIT, moved_log_path = GIT_BUF_INIT; + + git_buf_joinpath(&master_log_path, git_repository_path(g_repo), GIT_REFLOG_DIR); + git_buf_puts(&moved_log_path, git_buf_cstr(&master_log_path)); + git_buf_joinpath(&master_log_path, git_buf_cstr(&master_log_path), "refs/heads/master"); + git_buf_joinpath(&moved_log_path, git_buf_cstr(&moved_log_path), "refs/moved"); + + cl_assert_equal_i(true, git_path_isfile(git_buf_cstr(&master_log_path))); + cl_assert_equal_i(false, git_path_isfile(git_buf_cstr(&moved_log_path))); + + cl_git_pass(git_reference_lookup(&master, g_repo, "refs/heads/master")); + cl_git_pass(git_reference_rename(master, "refs/moved", 0)); + + cl_assert_equal_i(false, git_path_isfile(git_buf_cstr(&master_log_path))); + cl_assert_equal_i(true, git_path_isfile(git_buf_cstr(&moved_log_path))); + + git_reference_free(master); + git_buf_free(&moved_log_path); + git_buf_free(&master_log_path); +} From 75261421ec00b6dc0a72931ed7640743a4998c7d Mon Sep 17 00:00:00 2001 From: nulltoken Date: Wed, 4 Jul 2012 11:58:04 +0200 Subject: [PATCH 02/19] refs: add git_reference_has_log() --- include/git2/refs.h | 10 ++++++++++ src/refs.c | 17 +++++++++++++++++ tests-clar/refs/reflog.c | 17 +++++++++++++++++ 3 files changed, 44 insertions(+) diff --git a/include/git2/refs.h b/include/git2/refs.h index 2aa0ac267..7f6eb0e9b 100644 --- a/include/git2/refs.h +++ b/include/git2/refs.h @@ -353,6 +353,16 @@ GIT_EXTERN(int) git_reference_foreach_glob( void *payload ); +/** + * Check if a reflog exists for the specified reference. + * + * @param ref A git reference + * + * @return 0 when no reflog can be found, 1 when it exists; + * otherwise an error code. + */ +GIT_EXTERN(int) git_reference_has_log(git_reference *ref); + /** @} */ GIT_END_DECL #endif diff --git a/src/refs.c b/src/refs.c index 80349b710..2aba83ef5 100644 --- a/src/refs.c +++ b/src/refs.c @@ -1802,3 +1802,20 @@ int git_reference_foreach_glob( return git_reference_foreach( repo, list_flags, fromglob_cb, &data); } + +int git_reference_has_log( + git_reference *ref) +{ + git_buf path = GIT_BUF_INIT; + int result; + + assert(ref); + + if (git_buf_join_n(&path, '/', 3, ref->owner->path_repository, GIT_REFLOG_DIR, ref->name) < 0) + return -1; + + result = git_path_isfile(git_buf_cstr(&path)); + git_buf_free(&path); + + return result; +} diff --git a/tests-clar/refs/reflog.c b/tests-clar/refs/reflog.c index a945b4789..05f3786bb 100644 --- a/tests-clar/refs/reflog.c +++ b/tests-clar/refs/reflog.c @@ -145,3 +145,20 @@ void test_refs_reflog__renaming_the_reference_moves_the_reflog(void) git_buf_free(&moved_log_path); git_buf_free(&master_log_path); } +static void assert_has_reflog(bool expected_result, const char *name) +{ + git_reference *ref; + + cl_git_pass(git_reference_lookup(&ref, g_repo, name)); + + cl_assert_equal_i(expected_result, git_reference_has_log(ref)); + + git_reference_free(ref); +} + +void test_refs_reflog__reference_has_reflog(void) +{ + assert_has_reflog(true, "HEAD"); + assert_has_reflog(true, "refs/heads/master"); + assert_has_reflog(false, "refs/heads/subtrees"); +} From 33c3370700083b9138b167778814d5af06ccf0b4 Mon Sep 17 00:00:00 2001 From: nulltoken Date: Wed, 4 Jul 2012 12:20:43 +0200 Subject: [PATCH 03/19] refs: deploy git_reference_has_log() --- include/git2/reflog.h | 2 ++ src/refs.c | 12 +++--------- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/include/git2/reflog.h b/include/git2/reflog.h index f490e29de..8acba349b 100644 --- a/include/git2/reflog.h +++ b/include/git2/reflog.h @@ -53,6 +53,8 @@ GIT_EXTERN(int) git_reflog_write(git_reference *ref, const git_oid *oid_old, con /** * Rename the reflog for the given reference * + * The reflog to be renamed is expected to already exist + * * @param ref the reference * @param new_name the new name of the reference * @return 0 or an error code diff --git a/src/refs.c b/src/refs.c index 2aba83ef5..fbbb86689 100644 --- a/src/refs.c +++ b/src/refs.c @@ -1404,18 +1404,12 @@ int git_reference_rename(git_reference *ref, const char *new_name, int force) } /* - * Rename the reflog file. + * Rename the reflog file, if it exists. */ - git_buf_clear(&aux_path); - if (git_buf_join_n(&aux_path, '/', 3, ref->owner->path_repository, GIT_REFLOG_DIR, ref->name) < 0) + if ((git_reference_has_log(ref)) && (git_reflog_rename(ref, new_name) < 0)) goto cleanup; - if (git_path_exists(aux_path.ptr) == true) { - if (git_reflog_rename(ref, new_name) < 0) - goto cleanup; - } else { - giterr_clear(); - } + giterr_clear(); /* * Change the name of the reference given by the user. From 5ffd510dd2f39f674d502853cee38c80ad959756 Mon Sep 17 00:00:00 2001 From: nulltoken Date: Wed, 4 Jul 2012 12:23:03 +0200 Subject: [PATCH 04/19] refs: remove seemingly useless giterr_clear() call --- src/refs.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/refs.c b/src/refs.c index fbbb86689..e8f9fc8dc 100644 --- a/src/refs.c +++ b/src/refs.c @@ -1409,8 +1409,6 @@ int git_reference_rename(git_reference *ref, const char *new_name, int force) if ((git_reference_has_log(ref)) && (git_reflog_rename(ref, new_name) < 0)) goto cleanup; - giterr_clear(); - /* * Change the name of the reference given by the user. */ From d0a920a6fd70aaad9a3cee10ba6465f3b04a7bc5 Mon Sep 17 00:00:00 2001 From: nulltoken Date: Sat, 7 Jul 2012 10:36:35 +0200 Subject: [PATCH 05/19] refs: deep unfound ref returns ENOTFOUND --- src/fileops.c | 2 +- tests-clar/refs/read.c | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/fileops.c b/src/fileops.c index 95a65893c..5849b79b2 100644 --- a/src/fileops.c +++ b/src/fileops.c @@ -94,7 +94,7 @@ int git_futils_open_ro(const char *path) { int fd = p_open(path, O_RDONLY); if (fd < 0) { - if (errno == ENOENT) + if (errno == ENOENT || errno == ENOTDIR) fd = GIT_ENOTFOUND; giterr_set(GITERR_OS, "Failed to open '%s'", path); } diff --git a/tests-clar/refs/read.c b/tests-clar/refs/read.c index d7111b232..6838ead74 100644 --- a/tests-clar/refs/read.c +++ b/tests-clar/refs/read.c @@ -192,3 +192,13 @@ void test_refs_read__loose_first(void) git_reference_free(reference); } + +void test_refs_read__unfound_return_ENOTFOUND(void) +{ + git_reference *reference; + + cl_assert_equal_i(GIT_ENOTFOUND, git_reference_lookup(&reference, g_repo, "test/master")); + cl_assert_equal_i(GIT_ENOTFOUND, git_reference_lookup(&reference, g_repo, "refs/test/master")); + cl_assert_equal_i(GIT_ENOTFOUND, git_reference_lookup(&reference, g_repo, "refs/tags/test/master")); + cl_assert_equal_i(GIT_ENOTFOUND, git_reference_lookup(&reference, g_repo, "refs/tags/test/farther/master")); +} From 34922eeeedaf2067a2fb1326bcbc623fb11a3f6a Mon Sep 17 00:00:00 2001 From: nulltoken Date: Tue, 3 Jul 2012 14:59:14 +0200 Subject: [PATCH 06/19] revparse: readonly tests don't need a sandboxed repo --- tests-clar/refs/revparse.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests-clar/refs/revparse.c b/tests-clar/refs/revparse.c index e282cd710..235d18f9c 100644 --- a/tests-clar/refs/revparse.c +++ b/tests-clar/refs/revparse.c @@ -28,12 +28,13 @@ void test_refs_revparse__initialize(void) if (tz) strcpy(g_orig_tz, tz); cl_setenv("TZ", "UTC"); - g_repo = cl_git_sandbox_init("testrepo.git"); + + cl_git_pass(git_repository_open(&g_repo, cl_fixture("testrepo.git"))); } void test_refs_revparse__cleanup(void) { - cl_git_sandbox_cleanup(); + git_repository_free(g_repo); g_obj = NULL; cl_setenv("TZ", g_orig_tz); } From 1decf88bc12a55261c42db5d3c086500e642a088 Mon Sep 17 00:00:00 2001 From: nulltoken Date: Tue, 3 Jul 2012 15:34:22 +0200 Subject: [PATCH 07/19] revparse: slightly improve readability of tests --- tests-clar/refs/revparse.c | 42 ++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/tests-clar/refs/revparse.c b/tests-clar/refs/revparse.c index 235d18f9c..f36809494 100644 --- a/tests-clar/refs/revparse.c +++ b/tests-clar/refs/revparse.c @@ -12,16 +12,21 @@ static char g_orig_tz[16] = {0}; static void test_object(const char *spec, const char *expected_oid) { char objstr[64] = {0}; + git_object *obj = NULL; + int error; - cl_git_pass(git_revparse_single(&g_obj, g_repo, spec)); - git_oid_fmt(objstr, git_object_id(g_obj)); - cl_assert_equal_s(objstr, expected_oid); + error = git_revparse_single(&obj, g_repo, spec); - git_object_free(g_obj); - g_obj = NULL; + if (expected_oid != NULL) { + cl_assert_equal_i(0, error); + git_oid_fmt(objstr, git_object_id(obj)); + cl_assert_equal_s(objstr, expected_oid); + } else + cl_assert_equal_i(GIT_ENOTFOUND, error); + + git_object_free(obj); } - void test_refs_revparse__initialize(void) { char *tz = cl_getenv("TZ"); @@ -41,7 +46,8 @@ void test_refs_revparse__cleanup(void) void test_refs_revparse__nonexistant_object(void) { - cl_assert_equal_i(GIT_ENOTFOUND, git_revparse_single(&g_obj, g_repo, "this doesn't exist")); + test_object("this doesn't exist", NULL); + cl_git_fail(git_revparse_single(&g_obj, g_repo, "this doesn't exist^1")); cl_git_fail(git_revparse_single(&g_obj, g_repo, "this doesn't exist~2")); } @@ -86,7 +92,7 @@ void test_refs_revparse__nth_parent(void) test_object("be3563a^2^1", "5b5b025afb0b4c913b4c338a42934a3863bf3644"); test_object("be3563a^0", "be3563ae3f795b2b4353bcce3a527ad0a4f7f644"); - cl_assert_equal_i(GIT_ENOTFOUND, git_revparse_single(&g_obj, g_repo, "be3563a^42")); + test_object("be3563a^42", NULL); } void test_refs_revparse__not_tag(void) @@ -129,8 +135,8 @@ void test_refs_revparse__reflog(void) cl_git_fail(git_revparse_single(&g_obj, g_repo, "@{-0}")); cl_git_fail(git_revparse_single(&g_obj, g_repo, "@{1000}")); - cl_assert_equal_i(GIT_ENOTFOUND, git_revparse_single(&g_obj, g_repo, "nope@{0}")); - cl_assert_equal_i(GIT_ENOTFOUND, git_revparse_single(&g_obj, g_repo, "master@{31415}")); + test_object("nope@{0}", NULL); + test_object("master@{31415}", NULL); test_object("@{-2}", "a65fedf39aefe402d3bb6e24df4d4f5fe4547750"); test_object("@{-1}", "a4a7dce85cf63874e984719f4fdd239f5145052f"); @@ -171,7 +177,7 @@ void test_refs_revparse__date(void) * a65fedf HEAD@{1335806603 -0900}: commit: * be3563a HEAD@{1335806563 -0700}: clone: from /Users/ben/src/libgit2/tests/resour */ - cl_assert_equal_i(GIT_ENOTFOUND, git_revparse_single(&g_obj, g_repo, "HEAD@{10 years ago}")); + test_object("HEAD@{10 years ago}", NULL); test_object("HEAD@{1 second}", "a65fedf39aefe402d3bb6e24df4d4f5fe4547750"); test_object("HEAD@{1 second ago}", "a65fedf39aefe402d3bb6e24df4d4f5fe4547750"); @@ -192,8 +198,8 @@ void test_refs_revparse__date(void) * $ git reflog -1 "master@{2012-04-30 17:22:42 +0000}" * warning: Log for 'master' only goes back to Mon, 30 Apr 2012 09:22:43 -0800. */ - cl_assert_equal_i(GIT_ENOTFOUND, git_revparse_single(&g_obj, g_repo, "master@{2012-04-30 17:22:42 +0000}")); - cl_assert_equal_i(GIT_ENOTFOUND, git_revparse_single(&g_obj, g_repo, "master@{2012-04-30 09:22:42 -0800}")); + test_object("master@{2012-04-30 17:22:42 +0000}", NULL); + test_object("master@{2012-04-30 09:22:42 -0800}", NULL); /* * $ git reflog -1 "master@{2012-04-30 17:22:43 +0000}" @@ -230,11 +236,11 @@ void test_refs_revparse__colon(void) cl_git_fail(git_revparse_single(&g_obj, g_repo, ":/")); cl_git_fail(git_revparse_single(&g_obj, g_repo, ":2:README")); - cl_assert_equal_i(GIT_ENOTFOUND, git_revparse_single(&g_obj, g_repo, ":/not found in any commit")); - cl_assert_equal_i(GIT_ENOTFOUND, git_revparse_single(&g_obj, g_repo, "subtrees:ab/42.txt")); - cl_assert_equal_i(GIT_ENOTFOUND, git_revparse_single(&g_obj, g_repo, "subtrees:ab/4.txt/nope")); - cl_assert_equal_i(GIT_ENOTFOUND, git_revparse_single(&g_obj, g_repo, "subtrees:nope")); - cl_assert_equal_i(GIT_ENOTFOUND, git_revparse_single(&g_obj, g_repo, "test/master^1:branch_file.txt")); + test_object(":/not found in any commit", NULL); + test_object("subtrees:ab/42.txt", NULL); + test_object("subtrees:ab/4.txt/nope", NULL); + test_object("subtrees:nope", NULL); + test_object("test/master^1:branch_file.txt", NULL); /* Trees */ test_object("master:", "944c0f6e4dfa41595e6eb3ceecdb14f50fe18162"); From cab65c2b23e1a01969c4586ef123ab153652aa6e Mon Sep 17 00:00:00 2001 From: nulltoken Date: Thu, 5 Jul 2012 22:26:14 +0200 Subject: [PATCH 08/19] revparse: detect incorrect "refname@{-n}" syntax --- src/revparse.c | 29 +++++++++++++---------------- tests-clar/refs/revparse.c | 1 + 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/src/revparse.c b/src/revparse.c index 8c15f46c6..f5cf93ba2 100644 --- a/src/revparse.c +++ b/src/revparse.c @@ -21,9 +21,10 @@ typedef enum { REVPARSE_STATE_DONE, } revparse_state; -static void set_invalid_syntax_err(const char *spec) +static int revspec_error(const char *revspec) { - giterr_set(GITERR_INVALID, "Refspec '%s' is not valid.", spec); + giterr_set(GITERR_INVALID, "Failed to parse revision specifier - Invalid pattern '%s'", revspec); + return -1; } static int revparse_lookup_fully_qualifed_ref(git_object **out, git_repository *repo, const char*spec) @@ -154,21 +155,19 @@ static int walk_ref_history(git_object **out, git_repository *repo, const char * size_t reflogspeclen = strlen(reflogspec); if (git__prefixcmp(reflogspec, "@{") != 0 || - git__suffixcmp(reflogspec, "}") != 0) { - giterr_set(GITERR_INVALID, "Bad reflogspec '%s'", reflogspec); - return GIT_ERROR; - } + git__suffixcmp(reflogspec, "}") != 0) + return revspec_error(reflogspec); /* "@{-N}" form means walk back N checkouts. That means the HEAD log. */ - if (refspeclen == 0 && !git__prefixcmp(reflogspec, "@{-")) { + if (!git__prefixcmp(reflogspec, "@{-")) { regex_t regex; int regex_error; - if (git__strtol32(&n, reflogspec+3, NULL, 0) < 0 || - n < 1) { - giterr_set(GITERR_INVALID, "Invalid reflogspec %s", reflogspec); - return GIT_ERROR; - } + if (refspeclen > 0) + return revspec_error(reflogspec); + + if (git__strtol32(&n, reflogspec+3, NULL, 0) < 0 || n < 1) + return revspec_error(reflogspec); if (!git_reference_lookup(&ref, repo, "HEAD")) { if (!git_reflog_read(&reflog, ref)) { @@ -373,10 +372,8 @@ static int handle_caret_syntax(git_object **out, git_repository *repo, git_objec int n; if (*movement == '{') { - if (movement[movementlen-1] != '}') { - set_invalid_syntax_err(movement); - return GIT_ERROR; - } + if (movement[movementlen-1] != '}') + return revspec_error(movement); /* {} -> Dereference until we reach an object that isn't a tag. */ if (movementlen == 2) { diff --git a/tests-clar/refs/revparse.c b/tests-clar/refs/revparse.c index f36809494..892cb760a 100644 --- a/tests-clar/refs/revparse.c +++ b/tests-clar/refs/revparse.c @@ -133,6 +133,7 @@ void test_refs_revparse__reflog(void) { cl_git_fail(git_revparse_single(&g_obj, g_repo, "@{-xyz}")); cl_git_fail(git_revparse_single(&g_obj, g_repo, "@{-0}")); + cl_git_fail(git_revparse_single(&g_obj, g_repo, "master@{-2}")); cl_git_fail(git_revparse_single(&g_obj, g_repo, "@{1000}")); test_object("nope@{0}", NULL); From 98d6a1fdda5fa0605dee933930b5b7ddff4feb37 Mon Sep 17 00:00:00 2001 From: nulltoken Date: Wed, 4 Jul 2012 16:24:09 +0200 Subject: [PATCH 09/19] util: add git__isdigit() --- src/util.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/util.h b/src/util.h index adc665027..abdd543cc 100644 --- a/src/util.h +++ b/src/util.h @@ -204,6 +204,11 @@ GIT_INLINE(bool) git__isalpha(int c) return ((c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z')); } +GIT_INLINE(bool) git__isdigit(int c) +{ + return (c >= '0' && c <= '9'); +} + GIT_INLINE(bool) git__isspace(int c) { return (c == ' ' || c == '\t' || c == '\n' || c == '\f' || c == '\r' || c == '\v'); From 29f72aa63844967cfd56d2edce804af243ef3ddd Mon Sep 17 00:00:00 2001 From: nulltoken Date: Wed, 4 Jul 2012 22:02:54 +0200 Subject: [PATCH 10/19] revparse: leverage git__isdigit() --- src/revparse.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/revparse.c b/src/revparse.c index f5cf93ba2..3b9b2c903 100644 --- a/src/revparse.c +++ b/src/revparse.c @@ -115,10 +115,11 @@ static int revparse_lookup_object(git_object **out, git_repository *repo, const static int all_chars_are_digits(const char *str, size_t len) { - size_t i=0; - for (i=0; i '9') return 0; - } + size_t i = 0; + + for (i = 0; i < len; i++) + if (!git__isdigit(str[i])) return 0; + return 1; } From 6a5136e5389034b696c9cd0292a760e78e975fd8 Mon Sep 17 00:00:00 2001 From: nulltoken Date: Fri, 6 Jul 2012 12:47:14 +0200 Subject: [PATCH 11/19] revparse: only allow decimal reflog ordinal specs passing 0 to git_strol(32|64) let the implementation guess if it's dealing with an octal number or a decimal one. Let's make it safe and ensure that both 'HEAD@{010}' and 'HEAD@{10}' point at the same commit. --- src/revparse.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/revparse.c b/src/revparse.c index 3b9b2c903..af0b055be 100644 --- a/src/revparse.c +++ b/src/revparse.c @@ -167,7 +167,7 @@ static int walk_ref_history(git_object **out, git_repository *repo, const char * if (refspeclen > 0) return revspec_error(reflogspec); - if (git__strtol32(&n, reflogspec+3, NULL, 0) < 0 || n < 1) + if (git__strtol32(&n, reflogspec+3, NULL, 10) < 0 || n < 1) return revspec_error(reflogspec); if (!git_reference_lookup(&ref, repo, "HEAD")) { @@ -233,7 +233,7 @@ static int walk_ref_history(git_object **out, git_repository *repo, const char * /* @{N} -> Nth prior value for the ref (from reflog) */ else if (all_chars_are_digits(reflogspec+2, reflogspeclen-3) && - !git__strtol32(&n, reflogspec+2, NULL, 0) && + !git__strtol32(&n, reflogspec+2, NULL, 10) && n <= 100000000) { /* Allow integer time */ normalize_maybe_empty_refname(&buf, repo, refspec, refspeclen); From 35bed94fd5fec699faca6a6d7dc7d03994373bb7 Mon Sep 17 00:00:00 2001 From: nulltoken Date: Tue, 3 Jul 2012 18:01:46 +0200 Subject: [PATCH 12/19] revparse: enhance refs/ coverage --- tests-clar/refs/revparse.c | 59 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 57 insertions(+), 2 deletions(-) diff --git a/tests-clar/refs/revparse.c b/tests-clar/refs/revparse.c index 892cb760a..bd50ca3cd 100644 --- a/tests-clar/refs/revparse.c +++ b/tests-clar/refs/revparse.c @@ -1,6 +1,9 @@ #include "clar_libgit2.h" #include "git2/revparse.h" +#include "buffer.h" +#include "refs.h" +#include "path.h" static git_repository *g_repo; static git_object *g_obj; @@ -9,13 +12,13 @@ static char g_orig_tz[16] = {0}; /* Helpers */ -static void test_object(const char *spec, const char *expected_oid) +static void test_object_inrepo(const char *spec, const char *expected_oid, git_repository *repo) { char objstr[64] = {0}; git_object *obj = NULL; int error; - error = git_revparse_single(&obj, g_repo, spec); + error = git_revparse_single(&obj, repo, spec); if (expected_oid != NULL) { cl_assert_equal_i(0, error); @@ -27,6 +30,11 @@ static void test_object(const char *spec, const char *expected_oid) git_object_free(obj); } +static void test_object(const char *spec, const char *expected_oid) +{ + test_object_inrepo(spec, expected_oid, g_repo); +} + void test_refs_revparse__initialize(void) { char *tz = cl_getenv("TZ"); @@ -149,6 +157,53 @@ void test_refs_revparse__reflog(void) test_object("master@{u}", "be3563ae3f795b2b4353bcce3a527ad0a4f7f644"); } +static void create_fake_stash_reference_and_reflog(git_repository *repo) +{ + git_reference *master; + git_buf log_path = GIT_BUF_INIT; + + git_buf_joinpath(&log_path, git_repository_path(repo), "logs/refs/fakestash"); + + cl_assert_equal_i(false, git_path_isfile(git_buf_cstr(&log_path))); + + cl_git_pass(git_reference_lookup(&master, repo, "refs/heads/master")); + cl_git_pass(git_reference_rename(master, "refs/fakestash", 0)); + + cl_assert_equal_i(true, git_path_isfile(git_buf_cstr(&log_path))); + + git_buf_free(&log_path); + git_reference_free(master); +} + +void test_refs_revparse__reflog_of_a_ref_under_refs(void) +{ + git_repository *repo = cl_git_sandbox_init("testrepo.git"); + + test_object_inrepo("refs/fakestash", NULL, repo); + + create_fake_stash_reference_and_reflog(repo); + + /* + * $ git reflog -1 refs/fakestash + * a65fedf refs/fakestash@{0}: commit: checking in + * + * $ git reflog -1 refs/fakestash@{0} + * a65fedf refs/fakestash@{0}: commit: checking in + * + * $ git reflog -1 fakestash + * a65fedf fakestash@{0}: commit: checking in + * + * $ git reflog -1 fakestash@{0} + * a65fedf fakestash@{0}: commit: checking in + */ + test_object_inrepo("refs/fakestash", "a65fedf39aefe402d3bb6e24df4d4f5fe4547750", repo); + test_object_inrepo("refs/fakestash@{0}", "a65fedf39aefe402d3bb6e24df4d4f5fe4547750", repo); + test_object_inrepo("fakestash", "a65fedf39aefe402d3bb6e24df4d4f5fe4547750", repo); + test_object_inrepo("fakestash@{0}", "a65fedf39aefe402d3bb6e24df4d4f5fe4547750", repo); + + cl_git_sandbox_cleanup(); +} + void test_refs_revparse__revwalk(void) { cl_git_fail(git_revparse_single(&g_obj, g_repo, "master^{/not found in any commit}")); From 3cd90893a0b8737e9536d46bb3813c0a8432fdad Mon Sep 17 00:00:00 2001 From: nulltoken Date: Fri, 6 Jul 2012 17:25:31 +0200 Subject: [PATCH 13/19] revparse: enhance upstream reflog test coverage --- tests-clar/refs/revparse.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/tests-clar/refs/revparse.c b/tests-clar/refs/revparse.c index bd50ca3cd..b23ce6788 100644 --- a/tests-clar/refs/revparse.c +++ b/tests-clar/refs/revparse.c @@ -137,6 +137,18 @@ void test_refs_revparse__chaining(void) test_object("master^1^1^1^1^1", "8496071c1b46c854b31185ea97743be6a8774479"); } +void test_refs_revparse__upstream(void) +{ + cl_git_fail(git_revparse_single(&g_obj, g_repo, "e90810b@{u}")); + cl_git_fail(git_revparse_single(&g_obj, g_repo, "refs/tags/e90810b@{u}")); + + test_object("master@{upstream}", "be3563ae3f795b2b4353bcce3a527ad0a4f7f644"); + test_object("@{u}", "be3563ae3f795b2b4353bcce3a527ad0a4f7f644"); + test_object("master@{u}", "be3563ae3f795b2b4353bcce3a527ad0a4f7f644"); + test_object("heads/master@{u}", "be3563ae3f795b2b4353bcce3a527ad0a4f7f644"); + test_object("refs/heads/master@{u}", "be3563ae3f795b2b4353bcce3a527ad0a4f7f644"); +} + void test_refs_revparse__reflog(void) { cl_git_fail(git_revparse_single(&g_obj, g_repo, "@{-xyz}")); @@ -153,8 +165,6 @@ void test_refs_revparse__reflog(void) test_object("master@{1}", "be3563ae3f795b2b4353bcce3a527ad0a4f7f644"); test_object("@{0}", "a65fedf39aefe402d3bb6e24df4d4f5fe4547750"); test_object("@{1}", "be3563ae3f795b2b4353bcce3a527ad0a4f7f644"); - test_object("master@{upstream}", "be3563ae3f795b2b4353bcce3a527ad0a4f7f644"); - test_object("master@{u}", "be3563ae3f795b2b4353bcce3a527ad0a4f7f644"); } static void create_fake_stash_reference_and_reflog(git_repository *repo) From 3d78ab64272ab56fc449f50bbd970d64801aa8bd Mon Sep 17 00:00:00 2001 From: nulltoken Date: Fri, 6 Jul 2012 19:48:57 +0200 Subject: [PATCH 14/19] revparse: split reflog test per feature --- tests-clar/refs/revparse.c | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/tests-clar/refs/revparse.c b/tests-clar/refs/revparse.c index b23ce6788..3f4fcd68b 100644 --- a/tests-clar/refs/revparse.c +++ b/tests-clar/refs/revparse.c @@ -149,22 +149,32 @@ void test_refs_revparse__upstream(void) test_object("refs/heads/master@{u}", "be3563ae3f795b2b4353bcce3a527ad0a4f7f644"); } -void test_refs_revparse__reflog(void) +void test_refs_revparse__ordinal(void) { - cl_git_fail(git_revparse_single(&g_obj, g_repo, "@{-xyz}")); - cl_git_fail(git_revparse_single(&g_obj, g_repo, "@{-0}")); cl_git_fail(git_revparse_single(&g_obj, g_repo, "master@{-2}")); - cl_git_fail(git_revparse_single(&g_obj, g_repo, "@{1000}")); test_object("nope@{0}", NULL); test_object("master@{31415}", NULL); + test_object("@{1000}", NULL); + + test_object("@{0}", "a65fedf39aefe402d3bb6e24df4d4f5fe4547750"); + test_object("@{1}", "be3563ae3f795b2b4353bcce3a527ad0a4f7f644"); + + test_object("master@{0}", "a65fedf39aefe402d3bb6e24df4d4f5fe4547750"); + test_object("master@{1}", "be3563ae3f795b2b4353bcce3a527ad0a4f7f644"); + test_object("heads/master@{1}", "be3563ae3f795b2b4353bcce3a527ad0a4f7f644"); + test_object("refs/heads/master@{1}", "be3563ae3f795b2b4353bcce3a527ad0a4f7f644"); +} + +void test_refs_revparse__previous_head(void) +{ + cl_git_fail(git_revparse_single(&g_obj, g_repo, "@{-xyz}")); + cl_git_fail(git_revparse_single(&g_obj, g_repo, "@{-0}")); + + test_object("@{-42}", NULL); test_object("@{-2}", "a65fedf39aefe402d3bb6e24df4d4f5fe4547750"); test_object("@{-1}", "a4a7dce85cf63874e984719f4fdd239f5145052f"); - test_object("master@{0}", "a65fedf39aefe402d3bb6e24df4d4f5fe4547750"); - test_object("master@{1}", "be3563ae3f795b2b4353bcce3a527ad0a4f7f644"); - test_object("@{0}", "a65fedf39aefe402d3bb6e24df4d4f5fe4547750"); - test_object("@{1}", "be3563ae3f795b2b4353bcce3a527ad0a4f7f644"); } static void create_fake_stash_reference_and_reflog(git_repository *repo) From 805c81594dad1ad5758e78490f1c9d46b42cd41c Mon Sep 17 00:00:00 2001 From: nulltoken Date: Fri, 6 Jul 2012 20:44:17 +0200 Subject: [PATCH 15/19] revparse: unfound previous head return ENOTFOUND --- src/revparse.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/revparse.c b/src/revparse.c index af0b055be..574392f7c 100644 --- a/src/revparse.c +++ b/src/revparse.c @@ -178,6 +178,8 @@ static int walk_ref_history(git_object **out, git_repository *repo, const char * } else { regmatch_t regexmatches[2]; + retcode = GIT_ENOTFOUND; + refloglen = git_reflog_entrycount(reflog); for (i=refloglen-1; i >= 0; i--) { const char *msg; From e727938112a45a3ef9b8751aaef96d4ff7da74b2 Mon Sep 17 00:00:00 2001 From: nulltoken Date: Fri, 6 Jul 2012 21:25:42 +0200 Subject: [PATCH 16/19] revparse: fix disambiguation of refs --- src/revparse.c | 182 ++++++++++++++++++++++++++----------------------- 1 file changed, 98 insertions(+), 84 deletions(-) diff --git a/src/revparse.c b/src/revparse.c index 574392f7c..47437f355 100644 --- a/src/revparse.c +++ b/src/revparse.c @@ -54,13 +54,15 @@ static int spec_looks_like_describe_output(const char *spec) return retcode == 0; } -static int revparse_lookup_object(git_object **out, git_repository *repo, const char *spec) +static int disambiguate_refname(git_reference **out, git_repository *repo, const char *refname) { - size_t speclen = strlen(spec); - git_object *obj = NULL; - git_oid oid; - git_buf refnamebuf = GIT_BUF_INIT; + int error, i; + bool fallbackmode = true; + git_reference *ref; + git_buf refnamebuf = GIT_BUF_INIT, name = GIT_BUF_INIT; + static const char* formatters[] = { + "%s", "refs/%s", "refs/tags/%s", "refs/heads/%s", @@ -68,7 +70,46 @@ static int revparse_lookup_object(git_object **out, git_repository *repo, const "refs/remotes/%s/HEAD", NULL }; - unsigned int i; + + if (*refname) + git_buf_puts(&name, refname); + else { + git_buf_puts(&name, "HEAD"); + fallbackmode = false; + } + + for (i = 0; formatters[i] && (fallbackmode || i == 0); i++) { + + git_buf_clear(&refnamebuf); + + if ((error = git_buf_printf(&refnamebuf, formatters[i], git_buf_cstr(&name))) < 0) + goto cleanup; + + error = git_reference_lookup_resolved(&ref, repo, git_buf_cstr(&refnamebuf), -1); + + if (!error) { + *out = ref; + error = 0; + goto cleanup; + } + + if (error != GIT_ENOTFOUND) + goto cleanup; + } + +cleanup: + git_buf_free(&name); + git_buf_free(&refnamebuf); + return error; +} + +static int revparse_lookup_object(git_object **out, git_repository *repo, const char *spec) +{ + size_t speclen = strlen(spec); + git_object *obj = NULL; + git_oid oid; + int error; + git_reference *ref; const char *substr; /* "git describe" output; snip everything before/including "-g" */ @@ -87,32 +128,20 @@ static int revparse_lookup_object(git_object **out, git_repository *repo, const } } - /* Fully-named ref */ - if (!revparse_lookup_fully_qualifed_ref(&obj, repo, spec)) { - *out = obj; - return 0; + 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 error; } - /* Partially-named ref; match in this order: */ - for (i=0; formatters[i]; i++) { - git_buf_clear(&refnamebuf); - if (git_buf_printf(&refnamebuf, formatters[i], spec) < 0) { - return GIT_ERROR; - } - - if (!revparse_lookup_fully_qualifed_ref(&obj, repo, git_buf_cstr(&refnamebuf))) { - git_buf_free(&refnamebuf); - *out = obj; - return 0; - } - } - git_buf_free(&refnamebuf); + if (error < 0) + return error; giterr_set(GITERR_REFERENCE, "Refspec '%s' not found.", spec); return GIT_ENOTFOUND; } - static int all_chars_are_digits(const char *str, size_t len) { size_t i = 0; @@ -123,30 +152,9 @@ static int all_chars_are_digits(const char *str, size_t len) return 1; } -static void normalize_maybe_empty_refname(git_buf *buf, git_repository *repo, const char *refspec, size_t refspeclen) -{ - git_reference *ref; - - if (!refspeclen) { - /* Empty refspec means current branch (target of HEAD) */ - git_reference_lookup(&ref, repo, "HEAD"); - git_buf_puts(buf, git_reference_target(ref)); - git_reference_free(ref); - } else if (strstr(refspec, "HEAD")) { - /* Explicit head */ - git_buf_puts(buf, refspec); - }else { - if (git__prefixcmp(refspec, "refs/heads/") != 0) { - git_buf_printf(buf, "refs/heads/%s", refspec); - } else { - git_buf_puts(buf, refspec); - } - } -} - static int walk_ref_history(git_object **out, git_repository *repo, const char *refspec, const char *reflogspec) { - git_reference *ref; + git_reference *disambiguated = NULL; git_reflog *reflog = NULL; int n, retcode = GIT_ERROR; int i, refloglen; @@ -170,8 +178,8 @@ static int walk_ref_history(git_object **out, git_repository *repo, const char * if (git__strtol32(&n, reflogspec+3, NULL, 10) < 0 || n < 1) return revspec_error(reflogspec); - if (!git_reference_lookup(&ref, repo, "HEAD")) { - if (!git_reflog_read(&reflog, ref)) { + if (!git_reference_lookup(&disambiguated, repo, "HEAD")) { + if (!git_reflog_read(&reflog, disambiguated)) { regex_error = regcomp(®ex, "checkout: moving from (.*) to .*", REG_EXTENDED); if (regex_error != 0) { giterr_set_regex(®ex, regex_error); @@ -198,29 +206,40 @@ static int walk_ref_history(git_object **out, git_repository *repo, const char * regfree(®ex); } } - git_reference_free(ref); } } else { - int date_error = 0; + int date_error = 0, result; git_time_t timestamp; git_buf datebuf = GIT_BUF_INIT; + result = disambiguate_refname(&disambiguated, repo, refspec); + + if (result < 0) { + retcode = result; + goto cleanup; + } + git_buf_put(&datebuf, reflogspec+2, reflogspeclen-3); date_error = git__date_parse(×tamp, git_buf_cstr(&datebuf)); /* @{u} or @{upstream} -> upstream branch, for a tracking branch. This is stored in the config. */ - if (!strcmp(reflogspec, "@{u}") || !strcmp(reflogspec, "@{upstream}")) { + if (!git__prefixcmp(git_reference_name(disambiguated), GIT_REFS_HEADS_DIR) && + (!strcmp(reflogspec, "@{u}") || !strcmp(reflogspec, "@{upstream}"))) { git_config *cfg; if (!git_repository_config(&cfg, repo)) { /* Is the ref a tracking branch? */ const char *remote; git_buf_clear(&buf); - git_buf_printf(&buf, "branch.%s.remote", refspec); + git_buf_printf(&buf, "branch.%s.remote", + git_reference_name(disambiguated) + strlen(GIT_REFS_HEADS_DIR)); + if (!git_config_get_string(&remote, cfg, git_buf_cstr(&buf))) { /* Yes. Find the first merge target name. */ const char *mergetarget; git_buf_clear(&buf); - git_buf_printf(&buf, "branch.%s.merge", refspec); + git_buf_printf(&buf, "branch.%s.merge", + git_reference_name(disambiguated) + strlen(GIT_REFS_HEADS_DIR)); + if (!git_config_get_string(&mergetarget, cfg, git_buf_cstr(&buf)) && !git__prefixcmp(mergetarget, "refs/heads/")) { /* Success. Look up the target and fetch the object. */ @@ -237,12 +256,12 @@ static int walk_ref_history(git_object **out, git_repository *repo, const char * else if (all_chars_are_digits(reflogspec+2, reflogspeclen-3) && !git__strtol32(&n, reflogspec+2, NULL, 10) && n <= 100000000) { /* Allow integer time */ - normalize_maybe_empty_refname(&buf, repo, refspec, refspeclen); - if (n == 0) { + git_buf_puts(&buf, git_reference_name(disambiguated)); + + if (n == 0) retcode = revparse_lookup_fully_qualifed_ref(out, repo, git_buf_cstr(&buf)); - } else if (!git_reference_lookup(&ref, repo, git_buf_cstr(&buf))) { - if (!git_reflog_read(&reflog, ref)) { + else if (!git_reflog_read(&reflog, disambiguated)) { int numentries = git_reflog_entrycount(reflog); if (numentries < n) { giterr_set(GITERR_REFERENCE, "Reflog for '%s' has only %d entries, asked for %d", @@ -253,48 +272,43 @@ static int walk_ref_history(git_object **out, git_repository *repo, const char * const git_oid *oid = git_reflog_entry_oidold(entry); retcode = git_object_lookup(out, repo, oid, GIT_OBJ_ANY); } - } - git_reference_free(ref); } } else if (!date_error) { /* Ref as it was on a certain date */ - normalize_maybe_empty_refname(&buf, repo, refspec, refspeclen); + git_reflog *reflog; + if (!git_reflog_read(&reflog, disambiguated)) { + /* Keep walking until we find an entry older than the given date */ + int numentries = git_reflog_entrycount(reflog); + int i; - if (!git_reference_lookup(&ref, repo, git_buf_cstr(&buf))) { - git_reflog *reflog; - if (!git_reflog_read(&reflog, ref)) { - /* Keep walking until we find an entry older than the given date */ - int numentries = git_reflog_entrycount(reflog); - int i; - - for (i = numentries - 1; i >= 0; i--) { - const git_reflog_entry *entry = git_reflog_entry_byindex(reflog, i); - git_time commit_time = git_reflog_entry_committer(entry)->when; - if (commit_time.time - timestamp <= 0) { - retcode = git_object_lookup(out, repo, git_reflog_entry_oidnew(entry), GIT_OBJ_ANY); - break; - } + for (i = numentries - 1; i >= 0; i--) { + const git_reflog_entry *entry = git_reflog_entry_byindex(reflog, i); + git_time commit_time = git_reflog_entry_committer(entry)->when; + if (commit_time.time - timestamp <= 0) { + retcode = git_object_lookup(out, repo, git_reflog_entry_oidnew(entry), GIT_OBJ_ANY); + break; } - - if (i == -1) { - /* Didn't find a match */ - retcode = GIT_ENOTFOUND; - } - - git_reflog_free(reflog); } - git_reference_free(ref); + if (i == -1) { + /* Didn't find a match */ + retcode = GIT_ENOTFOUND; + } + + git_reflog_free(reflog); } } git_buf_free(&datebuf); } - if (reflog) git_reflog_free(reflog); +cleanup: + if (reflog) + git_reflog_free(reflog); git_buf_free(&buf); + git_reference_free(disambiguated); return retcode; } From b8460c2015d0954f3317caf60c1e49fb3cd56ace Mon Sep 17 00:00:00 2001 From: nulltoken Date: Fri, 6 Jul 2012 23:37:44 +0200 Subject: [PATCH 17/19] revparse: do not segfault when retrieving the last entry --- src/revparse.c | 2 +- tests-clar/refs/revparse.c | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/revparse.c b/src/revparse.c index 47437f355..3194cc491 100644 --- a/src/revparse.c +++ b/src/revparse.c @@ -263,7 +263,7 @@ static int walk_ref_history(git_object **out, git_repository *repo, const char * retcode = revparse_lookup_fully_qualifed_ref(out, repo, git_buf_cstr(&buf)); else if (!git_reflog_read(&reflog, disambiguated)) { int numentries = git_reflog_entrycount(reflog); - if (numentries < n) { + if (numentries < n + 1) { giterr_set(GITERR_REFERENCE, "Reflog for '%s' has only %d entries, asked for %d", git_buf_cstr(&buf), numentries, n); retcode = GIT_ENOTFOUND; diff --git a/tests-clar/refs/revparse.c b/tests-clar/refs/revparse.c index 3f4fcd68b..f723a1426 100644 --- a/tests-clar/refs/revparse.c +++ b/tests-clar/refs/revparse.c @@ -156,6 +156,7 @@ void test_refs_revparse__ordinal(void) test_object("nope@{0}", NULL); test_object("master@{31415}", NULL); test_object("@{1000}", NULL); + test_object("@{2}", NULL); test_object("@{0}", "a65fedf39aefe402d3bb6e24df4d4f5fe4547750"); test_object("@{1}", "be3563ae3f795b2b4353bcce3a527ad0a4f7f644"); From ce9e8e11cafdbb6895e7fae9b714430de151ba5b Mon Sep 17 00:00:00 2001 From: nulltoken Date: Sat, 7 Jul 2012 07:27:53 +0200 Subject: [PATCH 18/19] revparse: fix invalid test reference name --- tests-clar/refs/revparse.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tests-clar/refs/revparse.c b/tests-clar/refs/revparse.c index f723a1426..31029f726 100644 --- a/tests-clar/refs/revparse.c +++ b/tests-clar/refs/revparse.c @@ -54,10 +54,14 @@ void test_refs_revparse__cleanup(void) void test_refs_revparse__nonexistant_object(void) { - test_object("this doesn't exist", NULL); + test_object("this-does-not-exist", NULL); +} - cl_git_fail(git_revparse_single(&g_obj, g_repo, "this doesn't exist^1")); - cl_git_fail(git_revparse_single(&g_obj, g_repo, "this doesn't exist~2")); +void test_refs_revparse__invalid_reference_name(void) +{ + cl_git_fail(git_revparse_single(&g_obj, g_repo, "this doesn't make sense")); + cl_git_fail(git_revparse_single(&g_obj, g_repo, "this doesn't make sense^1")); + cl_git_fail(git_revparse_single(&g_obj, g_repo, "this doesn't make sense~2")); } void test_refs_revparse__shas(void) From 3e82d6c6f0783a5c74b79d7a849e9ca319f171b5 Mon Sep 17 00:00:00 2001 From: nulltoken Date: Sat, 7 Jul 2012 08:25:39 +0200 Subject: [PATCH 19/19] revparse: unfound reference return ENOTFOUND --- src/revparse.c | 74 +++++++++++++++++++++++++------------- tests-clar/refs/revparse.c | 2 ++ 2 files changed, 52 insertions(+), 24 deletions(-) diff --git a/src/revparse.c b/src/revparse.c index 3194cc491..f9859b18b 100644 --- a/src/revparse.c +++ b/src/revparse.c @@ -47,8 +47,9 @@ static int spec_looks_like_describe_output(const char *spec) regex_error = regcomp(®ex, ".+-[0-9]+-g[0-9a-fA-F]+", REG_EXTENDED); if (regex_error != 0) { giterr_set_regex(®ex, regex_error); - return 1; /* To be safe */ + return regex_error; } + retcode = regexec(®ex, spec, 0, NULL, 0); regfree(®ex); return retcode == 0; @@ -103,39 +104,66 @@ cleanup: return error; } -static int revparse_lookup_object(git_object **out, git_repository *repo, const char *spec) +extern int revparse_lookup_object(git_object **out, git_repository *repo, const char *spec); + +static int maybe_describe(git_object**out, git_repository *repo, const char *spec) { - size_t speclen = strlen(spec); - git_object *obj = NULL; - git_oid oid; - int error; - git_reference *ref; const char *substr; + int match; /* "git describe" output; snip everything before/including "-g" */ substr = strstr(spec, "-g"); - if (substr && - spec_looks_like_describe_output(spec) && - !revparse_lookup_object(out, repo, substr+2)) { - return 0; - } - /* SHA or prefix */ - if (!git_oid_fromstrn(&oid, spec, speclen)) { - if (!git_object_lookup_prefix(&obj, repo, &oid, speclen, GIT_OBJ_ANY)) { - *out = obj; - return 0; - } - } + if (substr == NULL) + return GIT_ENOTFOUND; + + if ((match = spec_looks_like_describe_output(spec)) < 0) + return match; + + if (!match) + return GIT_ENOTFOUND; + + return revparse_lookup_object(out, repo, substr+2); +} + +static int maybe_sha_or_abbrev(git_object**out, git_repository *repo, const char *spec) +{ + git_oid oid; + size_t speclen = strlen(spec); + + if (git_oid_fromstrn(&oid, spec, speclen) < 0) + return GIT_ENOTFOUND; + + return git_object_lookup_prefix(out, repo, &oid, speclen, GIT_OBJ_ANY); +} + +int revparse_lookup_object(git_object **out, git_repository *repo, const char *spec) +{ + int error; + git_reference *ref; + + error = maybe_describe(out, repo, spec); + if (!error) + return 0; + + 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 error; + return 0; } - if (error < 0) + if (error < 0 && error != GIT_ENOTFOUND) return error; giterr_set(GITERR_REFERENCE, "Refspec '%s' not found.", spec); @@ -711,10 +739,8 @@ int git_revparse_single(git_object **out, git_repository *repo, const char *spec if (current_state != next_state && next_state != REVPARSE_STATE_DONE) { /* Leaving INIT state, find the object specified, in case that state needs it */ - if (revparse_lookup_object(&next_obj, repo, git_buf_cstr(&specbuffer)) < 0) { - retcode = GIT_ERROR; + if ((retcode = revparse_lookup_object(&next_obj, repo, git_buf_cstr(&specbuffer))) < 0) next_state = REVPARSE_STATE_DONE; - } } break; diff --git a/tests-clar/refs/revparse.c b/tests-clar/refs/revparse.c index 31029f726..dbc002d38 100644 --- a/tests-clar/refs/revparse.c +++ b/tests-clar/refs/revparse.c @@ -55,6 +55,8 @@ void test_refs_revparse__cleanup(void) void test_refs_revparse__nonexistant_object(void) { test_object("this-does-not-exist", NULL); + test_object("this-does-not-exist^1", NULL); + test_object("this-does-not-exist~2", NULL); } void test_refs_revparse__invalid_reference_name(void)