From fd21c6f67f45831cf3f89ff43bdd4ccf66f138c8 Mon Sep 17 00:00:00 2001 From: schu Date: Sun, 29 May 2011 22:36:26 +0200 Subject: [PATCH 1/3] Add test case checking we do not corrupt the repository when renaming Signed-off-by: schu --- tests/t10-refs.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tests/t10-refs.c b/tests/t10-refs.c index 4b34146ed..3c9df434c 100644 --- a/tests/t10-refs.c +++ b/tests/t10-refs.c @@ -654,6 +654,38 @@ BEGIN_TEST(rename5, "can force-rename a reference with the name of an existing r close_temp_repo(repo); END_TEST +static const char *ref_one_name = "refs/heads/one/branch"; +static const char *ref_one_name_new = "refs/heads/two/branch"; +static const char *ref_two_name = "refs/heads/two"; + +BEGIN_TEST(rename6, "can not overwrite name of existing reference") + git_reference *ref, *ref_one, *ref_one_new, *ref_two; + git_repository *repo; + git_oid id; + + must_pass(open_temp_repo(&repo, REPOSITORY_FOLDER)); + + must_pass(git_reference_lookup(&ref, repo, ref_master_name)); + must_be_true(ref->type & GIT_REF_OID); + + git_oid_cpy(&id, git_reference_oid(ref)); + + /* Create loose references */ + must_pass(git_reference_create_oid(&ref_one, repo, ref_one_name, &id)); + must_pass(git_reference_create_oid(&ref_two, repo, ref_two_name, &id)); + + /* Pack everything */ + must_pass(git_reference_packall(repo)); + + /* Attempt to create illegal reference */ + must_fail(git_reference_create_oid(&ref_one_new, repo, ref_one_name_new, &id)); + + /* Illegal reference couldn't be created so this is supposed to fail */ + must_fail(git_reference_lookup(&ref_one_new, repo, ref_one_name_new)); + + close_temp_repo(repo); +END_TEST + BEGIN_TEST(delete0, "deleting a ref which is both packed and loose should remove both tracks in the filesystem") git_reference *looked_up_ref, *another_looked_up_ref; git_repository *repo; @@ -935,6 +967,7 @@ BEGIN_SUITE(refs) ADD_TEST(rename3); ADD_TEST(rename4); ADD_TEST(rename5); + ADD_TEST(rename6); ADD_TEST(delete0); ADD_TEST(list0); From 1b6d8163ce405b27cb58627712e658e59bd785c5 Mon Sep 17 00:00:00 2001 From: schu Date: Sun, 5 Jun 2011 19:22:32 +0200 Subject: [PATCH 2/3] Teach reference_rename() to really respect other references Add a new function reference_available() to check if a reference name actually is free and can be used. Signed-off-by: schu --- src/refs.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/src/refs.c b/src/refs.c index c21c9583d..83f299466 100644 --- a/src/refs.c +++ b/src/refs.c @@ -83,6 +83,7 @@ static int packed_write(git_repository *repo); static int reference_create_symbolic(git_reference **ref_out, git_repository *repo, const char *name, const char *target, int force); static int reference_create_oid(git_reference **ref_out, git_repository *repo, const char *name, const git_oid *id, int force); static int reference_rename(git_reference *ref, const char *new_name, int force); +static int reference_available(git_repository *repo, const char *ref, const char *old_ref); /* name normalization */ static int check_valid_ref_char(char ch); @@ -1003,6 +1004,9 @@ static int reference_create_oid(git_reference **ref_out, git_repository *repo, c if(git_reference_lookup(&ref, repo, name) == GIT_SUCCESS && !force) return git__throw(GIT_EEXISTS, "Failed to create reference OID. Reference already exists"); + if ((error = reference_available(repo, name, NULL)) < GIT_SUCCESS) + return git__rethrow(error, "Failed to create reference"); + /* * If they old ref was of the same type, then we can just update * it (once we've checked that the target is valid). Otherwise we @@ -1042,6 +1046,47 @@ cleanup: return error == GIT_SUCCESS ? GIT_SUCCESS : git__rethrow(error, "Failed to create reference OID"); } +static int _reference_available_cb(const char *ref, void *data) +{ + assert(ref && data); + + git_vector *refs = (git_vector *)data; + + const char *new = (const char *)git_vector_get(refs, 0); + const char *old = (const char *)git_vector_get(refs, 1); + + if (!old || strcmp(old, ref)) { + int reflen = strlen(ref); + int newlen = strlen(new); + int cmplen = reflen < newlen ? reflen : newlen; + const char *lead = reflen < newlen ? new : ref; + + if (!strncmp(new, ref, cmplen) && + lead[cmplen] == '/') + return GIT_EEXISTS; + } + + return GIT_SUCCESS; +} + +static int reference_available(git_repository *repo, const char *ref, const char* old_ref) +{ + int error; + git_vector refs; + + if (git_vector_init(&refs, 2, NULL) < GIT_SUCCESS) + return GIT_ENOMEM; + + git_vector_insert(&refs, (void *)ref); + git_vector_insert(&refs, (void *)old_ref); + + error = git_reference_listcb(repo, GIT_REF_LISTALL, _reference_available_cb, (void *)&refs); + + git_vector_free(&refs); + + return error == GIT_SUCCESS ? GIT_SUCCESS : git__rethrow(error, "Reference `%s` already exists", ref); +} + /* * Rename a reference * @@ -1079,6 +1124,8 @@ static int reference_rename(git_reference *ref, const char *new_name, int force) error != GIT_ENOTFOUND) return git__rethrow(error, "Failed to rename reference"); + if ((error == reference_available(ref->owner, new_name, ref->name)) < GIT_SUCCESS) + return error == GIT_SUCCESS ? GIT_SUCCESS : git__rethrow(error, "Failed to rename reference. Reference already exists"); old_name = ref->name; ref->name = git__strdup(new_name); From 76b15cb18a0c153639bf156482e0a0ad57c0c8c8 Mon Sep 17 00:00:00 2001 From: schu Date: Sun, 5 Jun 2011 20:47:30 +0200 Subject: [PATCH 3/3] Raise GIT_EEXISTS in case of conflicting ref names instead of passing the error returned by the subsystem; clarify error message. Fix tiny typo. Signed-off-by: schu --- src/refs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/refs.c b/src/refs.c index 83f299466..a533d6e53 100644 --- a/src/refs.c +++ b/src/refs.c @@ -1084,7 +1084,7 @@ static int reference_available(git_repository *repo, const char *ref, const char git_vector_free(&refs); - return error == GIT_SUCCESS ? GIT_SUCCESS : git__rethrow(error, "Reference `%s` already exists", ref); + return error == GIT_SUCCESS ? GIT_SUCCESS : git__throw(GIT_EEXISTS, "Reference name `%s` conflicts with existing reference", ref); } /* @@ -1124,7 +1124,7 @@ static int reference_rename(git_reference *ref, const char *new_name, int force) error != GIT_ENOTFOUND) return git__rethrow(error, "Failed to rename reference"); - if ((error == reference_available(ref->owner, new_name, ref->name)) < GIT_SUCCESS) + if ((error = reference_available(ref->owner, new_name, ref->name)) < GIT_SUCCESS) return error == GIT_SUCCESS ? GIT_SUCCESS : git__rethrow(error, "Failed to rename reference. Reference already exists"); old_name = ref->name;