diff --git a/include/git2/remote.h b/include/git2/remote.h index 5aea777c1..19b2fdf13 100644 --- a/include/git2/remote.h +++ b/include/git2/remote.h @@ -153,26 +153,30 @@ GIT_EXTERN(const char *) git_remote_url(const git_remote *remote); GIT_EXTERN(const char *) git_remote_pushurl(const git_remote *remote); /** - * Set the remote's url + * Set the remote's url in the configuration * - * Existing connections will not be updated. + * Remote objects already in memory will not be affected. This assumes + * the common case of a single-url remote and will otherwise return an error. * - * @param remote the remote + * @param repo the repository in which to perform the change + * @param remote the remote's name * @param url the url to set * @return 0 or an error value */ -GIT_EXTERN(int) git_remote_set_url(git_remote *remote, const char* url); +GIT_EXTERN(int) git_remote_set_url(git_repository *repo, const char *remote, const char* url); /** - * Set the remote's url for pushing + * Set the remote's url for pushing in the configuration. * - * Existing connections will not be updated. + * Remote objects already in memory will not be affected. This assumes + * the common case of a single-url remote and will otherwise return an error. * - * @param remote the remote - * @param url the url to set or NULL to clear the pushurl - * @return 0 or an error value + * + * @param repo the repository in which to perform the change + * @param remote the remote's name + * @param url the url to set */ -GIT_EXTERN(int) git_remote_set_pushurl(git_remote *remote, const char* url); +GIT_EXTERN(int) git_remote_set_pushurl(git_repository *repo, const char *remote, const char* url); /** * Add a fetch refspec to the remote diff --git a/src/remote.c b/src/remote.c index 85da2dc1b..9c044f7ad 100644 --- a/src/remote.c +++ b/src/remote.c @@ -20,6 +20,10 @@ #include "fetchhead.h" #include "push.h" +#define CONFIG_URL_FMT "remote.%s.url" +#define CONFIG_PUSHURL_FMT "remote.%s.pushurl" +#define CONFIG_FETCH_FMT "remote.%s.fetch" + static int dwim_refspecs(git_vector *out, git_vector *refspecs, git_vector *refs); static int lookup_remote_prune_config(git_remote *remote, git_config *config, const char *name); @@ -141,12 +145,16 @@ static int create_internal(git_remote **out, git_repository *repo, const char *n { git_remote *remote; git_config *config = NULL; - git_buf canonical_url = GIT_BUF_INIT, fetchbuf = GIT_BUF_INIT; + git_buf canonical_url = GIT_BUF_INIT; + git_buf var = GIT_BUF_INIT; int error = -1; /* name is optional */ assert(out && repo && url); + if ((error = git_repository_config__weakptr(&config, repo)) < 0) + return error; + remote = git__calloc(1, sizeof(git_remote)); GITERR_CHECK_ALLOC(remote); @@ -162,6 +170,12 @@ static int create_internal(git_remote **out, git_repository *repo, const char *n if (name != NULL) { remote->name = git__strdup(name); GITERR_CHECK_ALLOC(remote->name); + + if ((error = git_buf_printf(&var, CONFIG_URL_FMT, name)) < 0) + goto on_error; + + if ((error = git_config_set_string(config, var.ptr, remote->url)) < 0) + goto on_error; } if (fetch != NULL) { @@ -183,6 +197,8 @@ static int create_internal(git_remote **out, git_repository *repo, const char *n /* A remote without a name doesn't download tags */ remote->download_tags = GIT_REMOTE_DOWNLOAD_TAGS_NONE; + git_buf_free(&var); + *out = remote; error = 0; @@ -191,8 +207,8 @@ on_error: git_remote_free(remote); git_config_free(config); - git_buf_free(&fetchbuf); git_buf_free(&canonical_url); + git_buf_free(&var); return error; } @@ -564,23 +580,8 @@ int git_remote_save(const git_remote *remote) if ((error = git_repository_config__weakptr(&cfg, remote->repo)) < 0) return error; - if ((error = git_buf_printf(&buf, "remote.%s.url", remote->name)) < 0) - return error; - /* after this point, buffer is allocated so end with cleanup */ - if ((error = git_config_set_string( - cfg, git_buf_cstr(&buf), remote->url)) < 0) - goto cleanup; - - git_buf_clear(&buf); - if ((error = git_buf_printf(&buf, "remote.%s.pushurl", remote->name)) < 0) - goto cleanup; - - if ((error = git_config__update_entry( - cfg, git_buf_cstr(&buf), remote->pushurl, true, false)) < 0) - goto cleanup; - if ((error = update_config_refspec(remote, cfg, GIT_DIRECTION_FETCH)) < 0) goto cleanup; @@ -642,16 +643,42 @@ const char *git_remote_url(const git_remote *remote) return remote->url; } -int git_remote_set_url(git_remote *remote, const char* url) +static int set_url(git_repository *repo, const char *remote, const char *pattern, const char *url) { - assert(remote); - assert(url); + git_config *cfg; + git_buf buf = GIT_BUF_INIT, canonical_url = GIT_BUF_INIT; + int error; - git__free(remote->url); - remote->url = git__strdup(url); - GITERR_CHECK_ALLOC(remote->url); + assert(repo && remote); - return 0; + if ((error = ensure_remote_name_is_valid(remote)) < 0) + return error; + + if ((error = git_repository_config__weakptr(&cfg, repo)) < 0) + return error; + + if ((error = git_buf_printf(&buf, pattern, remote)) < 0) + return error; + + if (url) { + if ((error = canonicalize_url(&canonical_url, url)) < 0) + goto cleanup; + + error = git_config_set_string(cfg, buf.ptr, url); + } else { + error = git_config_delete_entry(cfg, buf.ptr); + } + +cleanup: + git_buf_free(&canonical_url); + git_buf_free(&buf); + + return error; +} + +int git_remote_set_url(git_repository *repo, const char *remote, const char *url) +{ + return set_url(repo, remote, CONFIG_URL_FMT, url); } const char *git_remote_pushurl(const git_remote *remote) @@ -660,18 +687,9 @@ const char *git_remote_pushurl(const git_remote *remote) return remote->pushurl; } -int git_remote_set_pushurl(git_remote *remote, const char* url) +int git_remote_set_pushurl(git_repository *repo, const char *remote, const char* url) { - assert(remote); - - git__free(remote->pushurl); - if (url) { - remote->pushurl = git__strdup(url); - GITERR_CHECK_ALLOC(remote->pushurl); - } else { - remote->pushurl = NULL; - } - return 0; + return set_url(repo, remote, CONFIG_PUSHURL_FMT, url); } const char* git_remote__urlfordirection(git_remote *remote, int direction) diff --git a/tests/fetchhead/nonetwork.c b/tests/fetchhead/nonetwork.c index 489481826..24e87a618 100644 --- a/tests/fetchhead/nonetwork.c +++ b/tests/fetchhead/nonetwork.c @@ -331,9 +331,8 @@ void test_fetchhead_nonetwork__unborn_with_upstream(void) cl_git_pass(git_clone(&repo, "./test1", "./repowithunborn", NULL)); /* Simulate someone pushing to it by changing to one that has stuff */ + cl_git_pass(git_remote_set_url(repo, "origin", cl_fixture("testrepo.git"))); cl_git_pass(git_remote_lookup(&remote, repo, "origin")); - cl_git_pass(git_remote_set_url(remote, cl_fixture("testrepo.git"))); - cl_git_pass(git_remote_save(remote)); cl_git_pass(git_remote_fetch(remote, NULL, NULL, NULL)); git_remote_free(remote); diff --git a/tests/network/fetchlocal.c b/tests/network/fetchlocal.c index a191b7b6b..73ccec031 100644 --- a/tests/network/fetchlocal.c +++ b/tests/network/fetchlocal.c @@ -400,16 +400,16 @@ void test_network_fetchlocal__multi_remotes(void) cl_set_cleanup(&cleanup_sandbox, NULL); options.callbacks.transfer_progress = transfer_cb; + cl_git_pass(git_remote_set_url(repo, "test", cl_git_fixture_url("testrepo.git"))); cl_git_pass(git_remote_lookup(&test, repo, "test")); - cl_git_pass(git_remote_set_url(test, cl_git_fixture_url("testrepo.git"))); cl_git_pass(git_remote_fetch(test, NULL, &options, NULL)); cl_git_pass(git_reference_list(&refnames, repo)); cl_assert_equal_i(32, (int)refnames.count); git_strarray_free(&refnames); + cl_git_pass(git_remote_set_url(repo, "test_with_pushurl", cl_git_fixture_url("testrepo.git"))); cl_git_pass(git_remote_lookup(&test2, repo, "test_with_pushurl")); - cl_git_pass(git_remote_set_url(test2, cl_git_fixture_url("testrepo.git"))); cl_git_pass(git_remote_fetch(test2, NULL, &options, NULL)); cl_git_pass(git_reference_list(&refnames, repo)); diff --git a/tests/network/remote/remotes.c b/tests/network/remote/remotes.c index 456fb39fe..8dbd68f2d 100644 --- a/tests/network/remote/remotes.c +++ b/tests/network/remote/remotes.c @@ -54,11 +54,18 @@ void test_network_remote_remotes__parsing(void) void test_network_remote_remotes__pushurl(void) { - cl_git_pass(git_remote_set_pushurl(_remote, "git://github.com/libgit2/notlibgit2")); - cl_assert_equal_s(git_remote_pushurl(_remote), "git://github.com/libgit2/notlibgit2"); + const char *name = git_remote_name(_remote); + git_remote *mod; - cl_git_pass(git_remote_set_pushurl(_remote, NULL)); - cl_assert(git_remote_pushurl(_remote) == NULL); + cl_git_pass(git_remote_set_pushurl(_repo, name, "git://github.com/libgit2/notlibgit2")); + cl_git_pass(git_remote_lookup(&mod, _repo, name)); + cl_assert_equal_s(git_remote_pushurl(mod), "git://github.com/libgit2/notlibgit2"); + git_remote_free(mod); + + cl_git_pass(git_remote_set_pushurl(_repo, name, NULL)); + cl_git_pass(git_remote_lookup(&mod, _repo, name)); + cl_assert(git_remote_pushurl(mod) == NULL); + git_remote_free(mod); } void test_network_remote_remotes__error_when_not_found(void) @@ -174,13 +181,16 @@ void test_network_remote_remotes__save(void) /* Set up the remote and save it to config */ cl_git_pass(git_remote_create(&_remote, _repo, "upstream", "git://github.com/libgit2/libgit2")); + git_remote_free(_remote); + cl_git_pass(git_remote_set_pushurl(_repo, "upstream", "git://github.com/libgit2/libgit2_push")); + cl_git_pass(git_remote_lookup(&_remote, _repo, "upstream")); + git_remote_clear_refspecs(_remote); cl_git_pass(git_remote_add_fetch(_remote, fetch_refspec1)); cl_git_pass(git_remote_add_fetch(_remote, fetch_refspec2)); cl_git_pass(git_remote_add_push(_remote, push_refspec1)); cl_git_pass(git_remote_add_push(_remote, push_refspec2)); - cl_git_pass(git_remote_set_pushurl(_remote, "git://github.com/libgit2/libgit2_push")); cl_git_pass(git_remote_save(_remote)); git_remote_free(_remote); _remote = NULL; @@ -203,12 +213,11 @@ void test_network_remote_remotes__save(void) cl_assert_equal_s(git_remote_url(_remote), "git://github.com/libgit2/libgit2"); cl_assert_equal_s(git_remote_pushurl(_remote), "git://github.com/libgit2/libgit2_push"); - /* remove the pushurl again and see if we can save that too */ - cl_git_pass(git_remote_set_pushurl(_remote, NULL)); - cl_git_pass(git_remote_save(_remote)); git_remote_free(_remote); _remote = NULL; + /* remove the pushurl again and see if we can save that too */ + cl_git_pass(git_remote_set_pushurl(_repo, "upstream", NULL)); cl_git_pass(git_remote_lookup(&_remote, _repo, "upstream")); cl_assert(git_remote_pushurl(_remote) == NULL); }