Merge pull request #2407 from libgit2/cmn/remote-rename-more
More remote rename fixes
diff --git a/include/git2/remote.h b/include/git2/remote.h
index 28771ac..cba57c4 100644
--- a/include/git2/remote.h
+++ b/include/git2/remote.h
@@ -572,6 +572,9 @@
*
* A temporary in-memory remote cannot be given a name with this method.
*
+ * @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 new_name the new name the remote should bear
* @param callback Optional callback to notify the consumer of fetch refspecs
@@ -580,10 +583,9 @@
* @return 0, GIT_EINVALIDSPEC, GIT_EEXISTS or an error code
*/
GIT_EXTERN(int) git_remote_rename(
+ git_strarray *problems,
git_remote *remote,
- const char *new_name,
- git_remote_rename_problem_cb callback,
- void *payload);
+ const char *new_name);
/**
* Retrieve the update FETCH_HEAD setting.
@@ -616,8 +618,6 @@
* All remote-tracking branches and configuration settings
* for the remote will be removed.
*
-* once deleted, the passed remote object will be freed and invalidated.
-*
* @param remote A valid remote
* @return 0 on success, or an error code.
*/
diff --git a/src/remote.c b/src/remote.c
index 0d1a88e..47b61b1 100644
--- a/src/remote.c
+++ b/src/remote.c
@@ -1359,19 +1359,24 @@
}
static int rename_one_remote_reference(
- git_reference *reference,
+ git_reference *reference_in,
const char *old_remote_name,
const char *new_remote_name)
{
int error;
+ git_reference *ref = NULL, *dummy = NULL;
+ git_buf namespace = GIT_BUF_INIT, old_namespace = GIT_BUF_INIT;
git_buf new_name = GIT_BUF_INIT;
git_buf log_message = GIT_BUF_INIT;
+ size_t pfx_len;
+ const char *target;
- if ((error = git_buf_printf(
- &new_name,
- GIT_REFS_REMOTES_DIR "%s%s",
- new_remote_name,
- reference->name + strlen(GIT_REFS_REMOTES_DIR) + strlen(old_remote_name))) < 0)
+ if ((error = git_buf_printf(&namespace, GIT_REFS_REMOTES_DIR "%s/", new_remote_name)) < 0)
+ return error;
+
+ pfx_len = strlen(GIT_REFS_REMOTES_DIR) + strlen(old_remote_name) + 1;
+ git_buf_puts(&new_name, namespace.ptr);
+ if ((error = git_buf_puts(&new_name, git_reference_name(reference_in) + pfx_len)) < 0)
goto cleanup;
if ((error = git_buf_printf(&log_message,
@@ -1379,12 +1384,36 @@
old_remote_name, new_remote_name)) < 0)
goto cleanup;
- error = git_reference_rename(
- NULL, reference, git_buf_cstr(&new_name), 1,
- NULL, git_buf_cstr(&log_message));
- git_reference_free(reference);
+ if ((error = git_reference_rename(&ref, reference_in, git_buf_cstr(&new_name), 1,
+ NULL, git_buf_cstr(&log_message))) < 0)
+ goto cleanup;
+
+ if (git_reference_type(ref) != GIT_REF_SYMBOLIC)
+ goto cleanup;
+
+ /* Handle refs like origin/HEAD -> origin/master */
+ target = git_reference_symbolic_target(ref);
+ if ((error = git_buf_printf(&old_namespace, GIT_REFS_REMOTES_DIR "%s/", old_remote_name)) < 0)
+ goto cleanup;
+
+ if (git__prefixcmp(target, old_namespace.ptr))
+ goto cleanup;
+
+ git_buf_clear(&new_name);
+ git_buf_puts(&new_name, namespace.ptr);
+ if ((error = git_buf_puts(&new_name, target + pfx_len)) < 0)
+ goto cleanup;
+
+ error = git_reference_symbolic_set_target(&dummy, ref, git_buf_cstr(&new_name),
+ NULL, git_buf_cstr(&log_message));
+
+ git_reference_free(dummy);
cleanup:
+ git_reference_free(reference_in);
+ git_reference_free(ref);
+ git_buf_free(&namespace);
+ git_buf_free(&old_namespace);
git_buf_free(&new_name);
git_buf_free(&log_message);
return error;
@@ -1419,11 +1448,7 @@
return (error == GIT_ITEROVER) ? 0 : error;
}
-static int rename_fetch_refspecs(
- git_remote *remote,
- const char *new_name,
- int (*callback)(const char *problematic_refspec, void *payload),
- void *payload)
+static int rename_fetch_refspecs(git_vector *problems, git_remote *remote, const char *new_name)
{
git_config *config;
git_buf base = GIT_BUF_INIT, var = GIT_BUF_INIT, val = GIT_BUF_INIT;
@@ -1434,6 +1459,9 @@
if ((error = git_repository_config__weakptr(&config, remote->repo)) < 0)
return error;
+ if ((error = git_vector_init(problems, 1, NULL)) < 0)
+ return error;
+
if ((error = git_buf_printf(
&base, "+refs/heads/*:refs/remotes/%s/*", remote->name)) < 0)
return error;
@@ -1442,15 +1470,15 @@
if (spec->push)
continue;
- /* Every refspec is a problem refspec for an anonymous remote, OR */
/* Does the dst part of the refspec follow the expected format? */
- if (!remote->name ||
- strcmp(git_buf_cstr(&base), spec->string)) {
+ if (strcmp(git_buf_cstr(&base), spec->string)) {
+ char *dup;
- if ((error = callback(spec->string, payload)) != 0) {
- giterr_set_after_callback(error);
+ dup = git__strdup(spec->string);
+ GITERR_CHECK_ALLOC(dup);
+
+ if ((error = git_vector_insert(problems, dup)) < 0)
break;
- }
continue;
}
@@ -1476,18 +1504,25 @@
git_buf_free(&base);
git_buf_free(&var);
git_buf_free(&val);
+
+ if (error < 0) {
+ char *str;
+ git_vector_foreach(problems, i, str)
+ git__free(str);
+
+ git_vector_free(problems);
+ }
+
return error;
}
-int git_remote_rename(
- git_remote *remote,
- const char *new_name,
- git_remote_rename_problem_cb callback,
- void *payload)
+int git_remote_rename(git_strarray *out, git_remote *remote, const char *new_name)
{
int error;
+ git_vector problem_refspecs;
+ char *tmp, *dup;
- assert(remote && new_name);
+ assert(out && remote && new_name);
if (!remote->name) {
giterr_set(GITERR_INVALID, "Can't rename an anonymous remote.");
@@ -1497,54 +1532,30 @@
if ((error = ensure_remote_name_is_valid(new_name)) < 0)
return error;
- if (remote->repo) {
- if ((error = ensure_remote_doesnot_exist(remote->repo, new_name)) < 0)
- return error;
+ if ((error = ensure_remote_doesnot_exist(remote->repo, new_name)) < 0)
+ return error;
- if (!remote->name) {
- if ((error = rename_fetch_refspecs(
- remote,
- new_name,
- callback,
- payload)) < 0)
- return error;
+ if ((error = rename_remote_config_section(remote->repo, remote->name, new_name)) < 0)
+ return error;
- remote->name = git__strdup(new_name);
- GITERR_CHECK_ALLOC(remote->name);
+ if ((error = update_branch_remote_config_entry(remote->repo, remote->name, new_name)) < 0)
+ return error;
- return git_remote_save(remote);
- }
+ if ((error = rename_remote_references(remote->repo, remote->name, new_name)) < 0)
+ return error;
- if ((error = rename_remote_config_section(
- remote->repo,
- remote->name,
- new_name)) < 0)
- return error;
+ if ((error = rename_fetch_refspecs(&problem_refspecs, remote, new_name)) < 0)
+ return error;
- if ((error = update_branch_remote_config_entry(
- remote->repo,
- remote->name,
- new_name)) < 0)
- return error;
+ out->count = problem_refspecs.length;
+ out->strings = (char **) problem_refspecs.contents;
- if ((error = rename_remote_references(
- remote->repo,
- remote->name,
- new_name)) < 0)
- return error;
+ dup = git__strdup(new_name);
+ GITERR_CHECK_ALLOC(dup);
- if ((error = rename_fetch_refspecs(
- remote,
- new_name,
- callback,
- payload)) < 0)
- return error;
- }
-
- git__free(remote->name);
-
- remote->name = git__strdup(new_name);
- GITERR_CHECK_ALLOC(remote->name);
+ tmp = remote->name;
+ remote->name = dup;
+ git__free(tmp);
return 0;
}
@@ -1910,8 +1921,6 @@
repo, git_remote_name(remote), NULL)) < 0)
return error;
- git_remote_free(remote);
-
return 0;
}
diff --git a/tests/network/remote/delete.c b/tests/network/remote/delete.c
index db55b07..664f47a 100644
--- a/tests/network/remote/delete.c
+++ b/tests/network/remote/delete.c
@@ -15,6 +15,7 @@
void test_network_remote_delete__cleanup(void)
{
+ git_remote_free(_remote);
cl_git_sandbox_cleanup();
}
@@ -27,7 +28,6 @@
cl_git_fail(git_remote_delete(remote));
git_remote_free(remote);
- git_remote_free(_remote);
}
void test_network_remote_delete__remove_remote_tracking_branches(void)
diff --git a/tests/network/remote/rename.c b/tests/network/remote/rename.c
index b7ec447..1b819a4 100644
--- a/tests/network/remote/rename.c
+++ b/tests/network/remote/rename.c
@@ -33,10 +33,14 @@
void test_network_remote_rename__renaming_a_remote_moves_related_configuration_section(void)
{
+ git_strarray problems = {0};
+
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(_remote, "just/renamed", dont_call_me_cb, NULL));
+ cl_git_pass(git_remote_rename(&problems, _remote, "just/renamed"));
+ cl_assert_equal_i(0, problems.count);
+ git_strarray_free(&problems);
assert_config_entry_existence(_repo, "remote.test.fetch", false);
assert_config_entry_existence(_repo, "remote.just/renamed.fetch", true);
@@ -44,16 +48,24 @@
void test_network_remote_rename__renaming_a_remote_updates_branch_related_configuration_entries(void)
{
+ git_strarray problems = {0};
+
assert_config_entry_value(_repo, "branch.master.remote", "test");
- cl_git_pass(git_remote_rename(_remote, "just/renamed", dont_call_me_cb, NULL));
+ cl_git_pass(git_remote_rename(&problems, _remote, "just/renamed"));
+ cl_assert_equal_i(0, problems.count);
+ git_strarray_free(&problems);
assert_config_entry_value(_repo, "branch.master.remote", "just/renamed");
}
void test_network_remote_rename__renaming_a_remote_updates_default_fetchrefspec(void)
{
- cl_git_pass(git_remote_rename(_remote, "just/renamed", dont_call_me_cb, NULL));
+ git_strarray problems = {0};
+
+ cl_git_pass(git_remote_rename(&problems, _remote, "just/renamed"));
+ cl_assert_equal_i(0, problems.count);
+ git_strarray_free(&problems);
assert_config_entry_value(_repo, "remote.just/renamed.fetch", "+refs/heads/*:refs/remotes/just/renamed/*");
}
@@ -61,6 +73,7 @@
void test_network_remote_rename__renaming_a_remote_without_a_fetchrefspec_doesnt_create_one(void)
{
git_config *config;
+ git_strarray problems = {0};
git_remote_free(_remote);
cl_git_pass(git_repository_config__weakptr(&config, _repo));
@@ -70,70 +83,64 @@
assert_config_entry_existence(_repo, "remote.test.fetch", false);
- cl_git_pass(git_remote_rename(_remote, "just/renamed", dont_call_me_cb, NULL));
+ cl_git_pass(git_remote_rename(&problems, _remote, "just/renamed"));
+ cl_assert_equal_i(0, problems.count);
+ git_strarray_free(&problems);
assert_config_entry_existence(_repo, "remote.just/renamed.fetch", false);
}
-static int ensure_refspecs(const char* refspec_name, void *payload)
-{
- int i = 0;
- bool found = false;
- const char ** exp = (const char **)payload;
-
- while (exp[i]) {
- if (strcmp(exp[i++], refspec_name))
- continue;
-
- found = true;
- break;
- }
-
- cl_assert(found);
-
- return 0;
-}
-
void test_network_remote_rename__renaming_a_remote_notifies_of_non_default_fetchrefspec(void)
{
git_config *config;
- char *expected_refspecs[] = {
- "+refs/*:refs/*",
- NULL
- };
+ 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_rename(_remote, "just/renamed", ensure_refspecs, &expected_refspecs));
+ cl_git_pass(git_remote_rename(&problems, _remote, "just/renamed"));
+ cl_assert_equal_i(1, problems.count);
+ cl_assert_equal_s("+refs/*:refs/*", problems.strings[0]);
+ git_strarray_free(&problems);
assert_config_entry_value(_repo, "remote.just/renamed.fetch", "+refs/*:refs/*");
+
+ git_strarray_free(&problems);
}
void test_network_remote_rename__new_name_can_contain_dots(void)
{
- cl_git_pass(git_remote_rename(_remote, "just.renamed", dont_call_me_cb, NULL));
+ git_strarray problems = {0};
+
+ cl_git_pass(git_remote_rename(&problems, _remote, "just.renamed"));
+ cl_assert_equal_i(0, problems.count);
+ git_strarray_free(&problems);
cl_assert_equal_s("just.renamed", git_remote_name(_remote));
}
void test_network_remote_rename__new_name_must_conform_to_reference_naming_conventions(void)
{
+ git_strarray problems = {0};
+
cl_assert_equal_i(
GIT_EINVALIDSPEC,
- git_remote_rename(_remote, "new@{name", dont_call_me_cb, NULL));
+ git_remote_rename(&problems, _remote, "new@{name"));
}
void test_network_remote_rename__renamed_name_is_persisted(void)
{
git_remote *renamed;
git_repository *another_repo;
+ git_strarray problems = {0};
cl_git_fail(git_remote_load(&renamed, _repo, "just/renamed"));
- cl_git_pass(git_remote_rename(_remote, "just/renamed", dont_call_me_cb, NULL));
+ cl_git_pass(git_remote_rename(&problems, _remote, "just/renamed"));
+ cl_assert_equal_i(0, problems.count);
+ git_strarray_free(&problems);
cl_git_pass(git_repository_open(&another_repo, "testrepo.git"));
cl_git_pass(git_remote_load(&renamed, _repo, "just/renamed"));
@@ -144,19 +151,24 @@
void test_network_remote_rename__cannot_overwrite_an_existing_remote(void)
{
- cl_assert_equal_i(GIT_EEXISTS, git_remote_rename(_remote, "test", dont_call_me_cb, NULL));
- cl_assert_equal_i(GIT_EEXISTS, git_remote_rename(_remote, "test_with_pushurl", dont_call_me_cb, NULL));
+ 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"));
}
void test_network_remote_rename__renaming_a_remote_moves_the_underlying_reference(void)
{
git_reference *underlying;
+ git_strarray problems = {0};
cl_assert_equal_i(GIT_ENOTFOUND, git_reference_lookup(&underlying, _repo, "refs/remotes/just/renamed"));
cl_git_pass(git_reference_lookup(&underlying, _repo, "refs/remotes/test/master"));
git_reference_free(underlying);
- cl_git_pass(git_remote_rename(_remote, "just/renamed", dont_call_me_cb, NULL));
+ cl_git_pass(git_remote_rename(&problems, _remote, "just/renamed"));
+ cl_assert_equal_i(0, problems.count);
+ git_strarray_free(&problems);
cl_assert_equal_i(GIT_ENOTFOUND, git_reference_lookup(&underlying, _repo, "refs/remotes/test/master"));
cl_git_pass(git_reference_lookup(&underlying, _repo, "refs/remotes/just/renamed/master"));
@@ -166,10 +178,12 @@
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(remote, "newname", NULL, NULL));
+ cl_git_fail(git_remote_rename(&problems, remote, "newname"));
+ git_strarray_free(&problems);
git_remote_free(remote);
}
@@ -181,15 +195,17 @@
git_reference *ref;
git_branch_t btype;
git_branch_iterator *iter;
+ git_strarray problems = {0};
cl_git_pass(git_oid_fromstr(&id, "a65fedf39aefe402d3bb6e24df4d4f5fe4547750"));
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(remote, "renamed", dont_call_me_cb, NULL));
+ cl_git_pass(git_remote_rename(&problems, remote, "renamed"));
git_remote_free(remote);
-
+ cl_assert_equal_i(0, problems.count);
+ git_strarray_free(&problems);
/* make sure there's only one remote-tracking branch */
cl_git_pass(git_branch_iterator_new(&iter, _repo, GIT_BRANCH_REMOTE));
@@ -202,3 +218,51 @@
cl_git_fail_with(GIT_ITEROVER, git_branch_next(&ref, &btype, iter));
git_branch_iterator_free(iter);
}
+
+void test_network_remote_rename__symref_head(void)
+{
+ int error;
+ git_remote *remote;
+ git_reference *ref;
+ git_branch_t btype;
+ git_branch_iterator *iter;
+ git_strarray problems = {0};
+ char idstr[GIT_OID_HEXSZ + 1] = {0};
+ git_vector refs;
+
+ 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_assert_equal_i(0, problems.count);
+ git_strarray_free(&problems);
+
+ cl_git_pass(git_vector_init(&refs, 2, (git_vector_cmp) git_reference_cmp));
+ cl_git_pass(git_branch_iterator_new(&iter, _repo, GIT_BRANCH_REMOTE));
+
+ while ((error = git_branch_next(&ref, &btype, iter)) == 0) {
+ cl_git_pass(git_vector_insert(&refs, ref));
+ }
+ cl_assert_equal_i(GIT_ITEROVER, error);
+ git_vector_sort(&refs);
+
+ cl_assert_equal_i(2, refs.length);
+
+ ref = git_vector_get(&refs, 0);
+ cl_assert_equal_s("refs/remotes/renamed/HEAD", git_reference_name(ref));
+ cl_assert_equal_s("refs/remotes/renamed/master", git_reference_symbolic_target(ref));
+ git_reference_free(ref);
+
+ ref = git_vector_get(&refs, 1);
+ cl_assert_equal_s("refs/remotes/renamed/master", git_reference_name(ref));
+ git_oid_fmt(idstr, git_reference_target(ref));
+ cl_assert_equal_s("be3563ae3f795b2b4353bcce3a527ad0a4f7f644", idstr);
+ git_reference_free(ref);
+
+ git_vector_free(&refs);
+
+ cl_git_fail_with(GIT_ITEROVER, git_branch_next(&ref, &btype, iter));
+ git_branch_iterator_free(iter);
+}
diff --git a/tests/submodule/add.c b/tests/submodule/add.c
index af81713..9fdc7cc 100644
--- a/tests/submodule/add.c
+++ b/tests/submodule/add.c
@@ -68,13 +68,16 @@
{
git_submodule *sm;
git_remote *remote;
+ git_strarray problems = {0};
/* default remote url is https://github.com/libgit2/false.git */
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(remote, "test_remote", NULL, NULL));
+ cl_git_pass(git_remote_rename(&problems, remote, "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);
