diff --git a/CHANGELOG.md b/CHANGELOG.md index 90b550240..11a1bfff0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,11 @@ v0.21 + 1 path of the programs to execute for receive-pack and upload-pack on the server, git_transport_ssh_with_paths. +* git_remote_rename() now takes the repository and the remote's + current name. Accepting a remote indicates we want to change it, + which we only did partially. It is much clearer if we accept a name + and no loaded objects are changed. + * git_remote_delete() now accepts the repository and the remote's name instead of a loaded remote. diff --git a/include/git2/remote.h b/include/git2/remote.h index 977e9b809..600b8ef05 100644 --- a/include/git2/remote.h +++ b/include/git2/remote.h @@ -557,18 +557,21 @@ GIT_EXTERN(void) git_remote_set_autotag( * The new name will be checked for validity. * See `git_tag_create()` for rules about valid names. * - * A temporary in-memory remote cannot be given a name with this method. + * No loaded instances of a the remote with the old name will change + * their name or their list of refspecs. * * @param problems non-default refspecs cannot be renamed and will be * stored here for further processing by the caller. Always free this * strarray on succesful return. - * @param remote the remote to rename + * @param repo the repository in which to rename + * @param name the current name of the reamote * @param new_name the new name the remote should bear * @return 0, GIT_EINVALIDSPEC, GIT_EEXISTS or an error code */ GIT_EXTERN(int) git_remote_rename( git_strarray *problems, - git_remote *remote, + git_repository *repo, + const char *name, const char *new_name); /** diff --git a/src/remote.c b/src/remote.c index ea510c7d4..362c226c4 100644 --- a/src/remote.c +++ b/src/remote.c @@ -1686,48 +1686,44 @@ static int rename_fetch_refspecs(git_vector *problems, git_remote *remote, const return error; } -int git_remote_rename(git_strarray *out, git_remote *remote, const char *new_name) +int git_remote_rename(git_strarray *out, git_repository *repo, const char *name, const char *new_name) { int error; - git_vector problem_refspecs; - char *tmp, *dup; + git_vector problem_refspecs = GIT_VECTOR_INIT; + git_remote *remote; - assert(out && remote && new_name); + assert(out && repo && name && new_name); - if (!remote->name) { - giterr_set(GITERR_INVALID, "Can't rename an anonymous remote."); - return GIT_EINVALIDSPEC; - } + if ((error = git_remote_load(&remote, repo, name)) < 0) + return -1; if ((error = ensure_remote_name_is_valid(new_name)) < 0) - return error; + goto cleanup; - if ((error = ensure_remote_doesnot_exist(remote->repo, new_name)) < 0) - return error; + if ((error = ensure_remote_doesnot_exist(repo, new_name)) < 0) + goto cleanup; - if ((error = rename_remote_config_section(remote->repo, remote->name, new_name)) < 0) - return error; + if ((error = rename_remote_config_section(repo, name, new_name)) < 0) + goto cleanup; - if ((error = update_branch_remote_config_entry(remote->repo, remote->name, new_name)) < 0) - return error; + if ((error = update_branch_remote_config_entry(repo, name, new_name)) < 0) + goto cleanup; - if ((error = rename_remote_references(remote->repo, remote->name, new_name)) < 0) - return error; + if ((error = rename_remote_references(repo, name, new_name)) < 0) + goto cleanup; if ((error = rename_fetch_refspecs(&problem_refspecs, remote, new_name)) < 0) - return error; + goto cleanup; out->count = problem_refspecs.length; out->strings = (char **) problem_refspecs.contents; - dup = git__strdup(new_name); - GITERR_CHECK_ALLOC(dup); +cleanup: + if (error < 0) + git_vector_free(&problem_refspecs); - tmp = remote->name; - remote->name = dup; - git__free(tmp); - - return 0; + git_remote_free(remote); + return error; } int git_remote_update_fetchhead(git_remote *remote) diff --git a/tests/network/remote/rename.c b/tests/network/remote/rename.c index 1b819a445..e1aea0297 100644 --- a/tests/network/remote/rename.c +++ b/tests/network/remote/rename.c @@ -3,21 +3,16 @@ #include "repository.h" -static git_remote *_remote; static git_repository *_repo; +static const char *_remote_name = "test"; void test_network_remote_rename__initialize(void) { _repo = cl_git_sandbox_init("testrepo.git"); - - cl_git_pass(git_remote_load(&_remote, _repo, "test")); } void test_network_remote_rename__cleanup(void) { - git_remote_free(_remote); - _remote = NULL; - cl_git_sandbox_cleanup(); } @@ -38,7 +33,7 @@ void test_network_remote_rename__renaming_a_remote_moves_related_configuration_s assert_config_entry_existence(_repo, "remote.test.fetch", true); assert_config_entry_existence(_repo, "remote.just/renamed.fetch", false); - cl_git_pass(git_remote_rename(&problems, _remote, "just/renamed")); + cl_git_pass(git_remote_rename(&problems, _repo, _remote_name, "just/renamed")); cl_assert_equal_i(0, problems.count); git_strarray_free(&problems); @@ -52,7 +47,7 @@ void test_network_remote_rename__renaming_a_remote_updates_branch_related_config assert_config_entry_value(_repo, "branch.master.remote", "test"); - cl_git_pass(git_remote_rename(&problems, _remote, "just/renamed")); + cl_git_pass(git_remote_rename(&problems, _repo, _remote_name, "just/renamed")); cl_assert_equal_i(0, problems.count); git_strarray_free(&problems); @@ -63,7 +58,7 @@ void test_network_remote_rename__renaming_a_remote_updates_default_fetchrefspec( { git_strarray problems = {0}; - cl_git_pass(git_remote_rename(&problems, _remote, "just/renamed")); + cl_git_pass(git_remote_rename(&problems, _repo, _remote_name, "just/renamed")); cl_assert_equal_i(0, problems.count); git_strarray_free(&problems); @@ -73,17 +68,18 @@ void test_network_remote_rename__renaming_a_remote_updates_default_fetchrefspec( void test_network_remote_rename__renaming_a_remote_without_a_fetchrefspec_doesnt_create_one(void) { git_config *config; + git_remote *remote; git_strarray problems = {0}; - git_remote_free(_remote); cl_git_pass(git_repository_config__weakptr(&config, _repo)); cl_git_pass(git_config_delete_entry(config, "remote.test.fetch")); - cl_git_pass(git_remote_load(&_remote, _repo, "test")); + cl_git_pass(git_remote_load(&remote, _repo, "test")); + git_remote_free(remote); assert_config_entry_existence(_repo, "remote.test.fetch", false); - cl_git_pass(git_remote_rename(&problems, _remote, "just/renamed")); + cl_git_pass(git_remote_rename(&problems, _repo, _remote_name, "just/renamed")); cl_assert_equal_i(0, problems.count); git_strarray_free(&problems); @@ -93,15 +89,15 @@ void test_network_remote_rename__renaming_a_remote_without_a_fetchrefspec_doesnt void test_network_remote_rename__renaming_a_remote_notifies_of_non_default_fetchrefspec(void) { git_config *config; - + git_remote *remote; git_strarray problems = {0}; - git_remote_free(_remote); cl_git_pass(git_repository_config__weakptr(&config, _repo)); cl_git_pass(git_config_set_string(config, "remote.test.fetch", "+refs/*:refs/*")); - cl_git_pass(git_remote_load(&_remote, _repo, "test")); + cl_git_pass(git_remote_load(&remote, _repo, "test")); + git_remote_free(remote); - cl_git_pass(git_remote_rename(&problems, _remote, "just/renamed")); + cl_git_pass(git_remote_rename(&problems, _repo, _remote_name, "just/renamed")); cl_assert_equal_i(1, problems.count); cl_assert_equal_s("+refs/*:refs/*", problems.strings[0]); git_strarray_free(&problems); @@ -115,10 +111,10 @@ void test_network_remote_rename__new_name_can_contain_dots(void) { git_strarray problems = {0}; - cl_git_pass(git_remote_rename(&problems, _remote, "just.renamed")); + cl_git_pass(git_remote_rename(&problems, _repo, _remote_name, "just.renamed")); cl_assert_equal_i(0, problems.count); git_strarray_free(&problems); - cl_assert_equal_s("just.renamed", git_remote_name(_remote)); + assert_config_entry_existence(_repo, "remote.just.renamed.fetch", true); } void test_network_remote_rename__new_name_must_conform_to_reference_naming_conventions(void) @@ -127,7 +123,7 @@ void test_network_remote_rename__new_name_must_conform_to_reference_naming_conve cl_assert_equal_i( GIT_EINVALIDSPEC, - git_remote_rename(&problems, _remote, "new@{name")); + git_remote_rename(&problems, _repo, _remote_name, "new@{name")); } void test_network_remote_rename__renamed_name_is_persisted(void) @@ -138,7 +134,7 @@ void test_network_remote_rename__renamed_name_is_persisted(void) cl_git_fail(git_remote_load(&renamed, _repo, "just/renamed")); - cl_git_pass(git_remote_rename(&problems, _remote, "just/renamed")); + cl_git_pass(git_remote_rename(&problems, _repo, _remote_name, "just/renamed")); cl_assert_equal_i(0, problems.count); git_strarray_free(&problems); @@ -153,8 +149,8 @@ void test_network_remote_rename__cannot_overwrite_an_existing_remote(void) { git_strarray problems = {0}; - cl_assert_equal_i(GIT_EEXISTS, git_remote_rename(&problems, _remote, "test")); - cl_assert_equal_i(GIT_EEXISTS, git_remote_rename(&problems, _remote, "test_with_pushurl")); + cl_assert_equal_i(GIT_EEXISTS, git_remote_rename(&problems, _repo, _remote_name, "test")); + cl_assert_equal_i(GIT_EEXISTS, git_remote_rename(&problems, _repo, _remote_name, "test_with_pushurl")); } void test_network_remote_rename__renaming_a_remote_moves_the_underlying_reference(void) @@ -166,7 +162,7 @@ void test_network_remote_rename__renaming_a_remote_moves_the_underlying_referenc cl_git_pass(git_reference_lookup(&underlying, _repo, "refs/remotes/test/master")); git_reference_free(underlying); - cl_git_pass(git_remote_rename(&problems, _remote, "just/renamed")); + cl_git_pass(git_remote_rename(&problems, _repo, _remote_name, "just/renamed")); cl_assert_equal_i(0, problems.count); git_strarray_free(&problems); @@ -175,23 +171,10 @@ void test_network_remote_rename__renaming_a_remote_moves_the_underlying_referenc git_reference_free(underlying); } -void test_network_remote_rename__cannot_rename_an_inmemory_remote(void) -{ - git_remote *remote; - git_strarray problems = {0}; - - cl_git_pass(git_remote_create_anonymous(&remote, _repo, "file:///blah", NULL)); - cl_git_fail(git_remote_rename(&problems, remote, "newname")); - - git_strarray_free(&problems); - git_remote_free(remote); -} - void test_network_remote_rename__overwrite_ref_in_target(void) { git_oid id; char idstr[GIT_OID_HEXSZ + 1] = {0}; - git_remote *remote; git_reference *ref; git_branch_t btype; git_branch_iterator *iter; @@ -201,9 +184,7 @@ void test_network_remote_rename__overwrite_ref_in_target(void) cl_git_pass(git_reference_create(&ref, _repo, "refs/remotes/renamed/master", &id, 1, NULL, NULL)); git_reference_free(ref); - cl_git_pass(git_remote_load(&remote, _repo, "test")); - cl_git_pass(git_remote_rename(&problems, remote, "renamed")); - git_remote_free(remote); + cl_git_pass(git_remote_rename(&problems, _repo, _remote_name, "renamed")); cl_assert_equal_i(0, problems.count); git_strarray_free(&problems); @@ -222,7 +203,6 @@ void test_network_remote_rename__overwrite_ref_in_target(void) void test_network_remote_rename__symref_head(void) { int error; - git_remote *remote; git_reference *ref; git_branch_t btype; git_branch_iterator *iter; @@ -233,9 +213,7 @@ void test_network_remote_rename__symref_head(void) cl_git_pass(git_reference_symbolic_create(&ref, _repo, "refs/remotes/test/HEAD", "refs/remotes/test/master", 0, NULL, NULL)); git_reference_free(ref); - cl_git_pass(git_remote_load(&remote, _repo, "test")); - cl_git_pass(git_remote_rename(&problems, remote, "renamed")); - git_remote_free(remote); + cl_git_pass(git_remote_rename(&problems, _repo, _remote_name, "renamed")); cl_assert_equal_i(0, problems.count); git_strarray_free(&problems); diff --git a/tests/submodule/add.c b/tests/submodule/add.c index 10717809e..a3e3502b7 100644 --- a/tests/submodule/add.c +++ b/tests/submodule/add.c @@ -94,12 +94,10 @@ void test_submodule_add__url_relative(void) g_repo = cl_git_sandbox_init("testrepo2"); /* make sure we don't default to origin - rename origin -> test_remote */ - cl_git_pass(git_remote_load(&remote, g_repo, "origin")); - cl_git_pass(git_remote_rename(&problems, remote, "test_remote")); + cl_git_pass(git_remote_rename(&problems, g_repo, "origin", "test_remote")); cl_assert_equal_i(0, problems.count); git_strarray_free(&problems); cl_git_fail(git_remote_load(&remote, g_repo, "origin")); - git_remote_free(remote); cl_git_pass( git_submodule_add_setup(&sm, g_repo, "../TestGitRepository", "TestGitRepository", 1)