# [PATCH] Implement git remote mv 15 messages from 2008-10-22 to 2008-11-12. Participants: Miklos Vajna, Brandon Casey, Jeff King, Junio C Hamano. Thread: https://gitlist.dev/t/15999 ## Miklos Vajna, 2008-10-22 00:23 Subject: [PATCH] Implement git remote mv Message-ID: <1224634994-1664-1-git-send-email-vmiklos@frugalware.org> URL: https://gitlist.dev/e/1224634994-1664-1-git-send-email-vmiklos%40frugalware.org ``` The new rename subcommand does the followings: 1) Renames the remote.foo configuration section to remote.bar 2) Updates the remote.bar.fetch refspecs 3) Updates the branch.*.remote settings 4) Renames the tracking branches. Signed-off-by: Miklos Vajna --- Documentation/git-remote.txt | 6 +++ builtin-remote.c | 102 ++++++++++++++++++++++++++++++++++++++++++ t/t5505-remote.sh | 14 ++++++ 3 files changed, 122 insertions(+), 0 deletions(-) diff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt index bb99810..4b5542a 100644 --- a/Documentation/git-remote.txt +++ b/Documentation/git-remote.txt @@ -11,6 +11,7 @@ SYNOPSIS [verse] 'git remote' [-v | --verbose] 'git remote add' [-t ] [-m ] [-f] [--mirror] +'git remote mv' 'git remote rm' 'git remote show' [-n] 'git remote prune' [-n | --dry-run] @@ -61,6 +62,11 @@ only makes sense in bare repositories. If a remote uses mirror mode, furthermore, `git push` will always behave as if `\--mirror` was passed. +'mv':: + +Rename the remote named to . All remote tracking branches and +configuration settings for the remote are updated. + 'rm':: Remove the remote named . All remote tracking branches and diff --git a/builtin-remote.c b/builtin-remote.c index 6b3325d..4a23738 100644 --- a/builtin-remote.c +++ b/builtin-remote.c @@ -10,6 +10,7 @@ static const char * const builtin_remote_usage[] = { "git remote", "git remote add ", + "git remote mv ", "git remote rm ", "git remote show ", "git remote prune ", @@ -329,6 +330,105 @@ static int add_branch_for_removal(const char *refname, return 0; } +struct rename_info { + const char *old; + const char *new; + struct string_list *remote_branches; +}; + +static int read_remote_branches(const char *refname, + const unsigned char *sha1, int flags, void *cb_data) +{ + struct rename_info *rename = cb_data; + struct strbuf buf = STRBUF_INIT; + + strbuf_addf(&buf, "refs/remotes/%s", rename->old); + if(!prefixcmp(refname, buf.buf)) + string_list_append(xstrdup(refname), rename->remote_branches); + + return 0; +} + +static int mv(int argc, const char **argv) +{ + struct option options[] = { + OPT_END() + }; + struct remote *oldremote, *newremote; + struct strbuf buf = STRBUF_INIT, buf2 = STRBUF_INIT; + struct string_list remote_branches = { NULL, 0, 0, 0 }; + struct rename_info rename = { argv[1], argv[2], &remote_branches }; + int i; + + if (argc != 3) + usage_with_options(builtin_remote_usage, options); + + oldremote = remote_get(rename.old); + if (!oldremote) + die("No such remote: %s", rename.old); + + newremote = remote_get(rename.new); + if (newremote && (newremote->url_nr > 1 || newremote->fetch_refspec_nr)) + die("remote %s already exists.", rename.new); + + strbuf_addf(&buf, "refs/heads/test:refs/remotes/%s/test", rename.new); + if (!valid_fetch_refspec(buf.buf)) + die("'%s' is not a valid remote name", rename.new); + + strbuf_reset(&buf); + strbuf_addf(&buf, "remote.%s", rename.old); + strbuf_addf(&buf2, "remote.%s", rename.new); + if (git_config_rename_section(buf.buf, buf2.buf) < 1) + return error("Could not rename config section '%s' to '%s'", + buf.buf, buf2.buf); + + strbuf_reset(&buf); + strbuf_addf(&buf, "remote.%s.fetch", rename.new); + if (git_config_set_multivar(buf.buf, NULL, NULL, 1)) + return error("Could not remove config section '%s'", buf.buf); + for (i = 0; i < oldremote->fetch_refspec_nr; i++) { + char *ptr; + + strbuf_reset(&buf2); + strbuf_addstr(&buf2, oldremote->fetch_refspec[i]); + ptr = strstr(buf2.buf, rename.old); + if (ptr) + strbuf_splice(&buf2, ptr-buf2.buf, strlen(rename.old), + rename.new, strlen(rename.new)); + if (git_config_set_multivar(buf.buf, buf2.buf, "^$", 0)) + return error("Could not append '%s'", buf.buf); + } + + read_branches(); + for (i = 0; i < branch_list.nr; i++) { + struct string_list_item *item = branch_list.items + i; + struct branch_info *info = item->util; + if (info->remote && !strcmp(info->remote, rename.old)) { + strbuf_reset(&buf); + strbuf_addf(&buf, "branch.%s.remote", item->string); + if (git_config_set(buf.buf, rename.new)) { + return error("Could not set '%s'", buf.buf); + } + } + } + + for_each_ref(read_remote_branches, &rename); + for (i = 0; i < remote_branches.nr; i++) { + struct string_list_item *item = remote_branches.items + i; + strbuf_reset(&buf); + strbuf_addstr(&buf, item->string); + strbuf_splice(&buf, strlen("refs/remotes/"), strlen(rename.old), + rename.new, strlen(rename.new)); + strbuf_reset(&buf2); + strbuf_addf(&buf2, "remote: renamed %s to %s", + item->string, buf.buf); + if (rename_ref(item->string, buf.buf, buf2.buf)) + die("renaming '%s' failed", item->string); + } + + return 0; +} + static int remove_branches(struct string_list *branches) { int i, result = 0; @@ -696,6 +796,8 @@ int cmd_remote(int argc, const char **argv, const char *prefix) result = show_all(); else if (!strcmp(argv[0], "add")) result = add(argc, argv); + else if (!strcmp(argv[0], "mv")) + result = mv(argc, argv); else if (!strcmp(argv[0], "rm")) result = rm(argc, argv); else if (!strcmp(argv[0], "show")) diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh index c449663..9b11fd3 100755 --- a/t/t5505-remote.sh +++ b/t/t5505-remote.sh @@ -324,4 +324,18 @@ test_expect_success 'reject adding remote with an invalid name' ' ' +# The first three tests if the config is properly updated, the last one +# checks if the branches are renamed. + +test_expect_success 'rename a remote' ' + + git clone one four && + (cd four && + git remote mv origin upstream && + git remote show |grep -q upstream && + git config remote.upstream.fetch |grep -q upstream && + test $(git config branch.master.remote) = "upstream" && + git for-each-ref|grep -q refs/remotes/upstream) + +' test_done -- 1.6.0.2 ``` ## Brandon Casey, 2008-10-22 16:52 Subject: Re: [PATCH] Implement git remote mv Message-ID: URL: https://gitlist.dev/e/JQScKsOjmj-C7kmMPApAxZ3UDmaTZ1j0HkObQuCveJ9n4G-4o6Zxbw%40cipher.nrlssc.navy.mil In-Reply-To: <1224634994-1664-1-git-send-email-vmiklos@frugalware.org> ``` Miklos Vajna wrote: > +static int mv(int argc, const char **argv) > +{ > + struct option options[] = { > + OPT_END() > + }; > + struct remote *oldremote, *newremote; > + struct strbuf buf = STRBUF_INIT, buf2 = STRBUF_INIT; > + struct string_list remote_branches = { NULL, 0, 0, 0 }; > + struct rename_info rename = { argv[1], argv[2], &remote_branches }; I think some non-c99 compilers would have issues with this run-time initialization from function arguments. Plus, what if argv doesn't have 3 elements? I see you have a check for that below... > + int i; > + > + if (argc != 3) > + usage_with_options(builtin_remote_usage, options); -brandon ``` ## Miklos Vajna, 2008-10-23 01:18 Subject: [PATCH] Implement git remote mv Message-ID: <1224724704-12700-1-git-send-email-vmiklos@frugalware.org> URL: https://gitlist.dev/e/1224724704-12700-1-git-send-email-vmiklos%40frugalware.org In-Reply-To: ``` The new rename subcommand does the followings: 1) Renames the remote.foo configuration section to remote.bar 2) Updates the remote.bar.fetch refspecs 3) Updates the branch.*.remote settings 4) Renames the tracking branches. Signed-off-by: Miklos Vajna --- On Wed, Oct 22, 2008 at 11:52:54AM -0500, Brandon Casey wrote: > > + struct rename_info rename = { argv[1], argv[2], &remote_branches > > }; > > I think some non-c99 compilers would have issues with this run-time > initialization from function arguments. Plus, what if argv doesn't > have > 3 elements? I see you have a check for that below... Hm true. Here is a corrected version. Documentation/git-remote.txt | 6 ++ builtin-remote.c | 106 ++++++++++++++++++++++++++++++++++++++++++ t/t5505-remote.sh | 14 ++++++ 3 files changed, 126 insertions(+), 0 deletions(-) diff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt index bb99810..4b5542a 100644 --- a/Documentation/git-remote.txt +++ b/Documentation/git-remote.txt @@ -11,6 +11,7 @@ SYNOPSIS [verse] 'git remote' [-v | --verbose] 'git remote add' [-t ] [-m ] [-f] [--mirror] +'git remote mv' 'git remote rm' 'git remote show' [-n] 'git remote prune' [-n | --dry-run] @@ -61,6 +62,11 @@ only makes sense in bare repositories. If a remote uses mirror mode, furthermore, `git push` will always behave as if `\--mirror` was passed. +'mv':: + +Rename the remote named to . All remote tracking branches and +configuration settings for the remote are updated. + 'rm':: Remove the remote named . All remote tracking branches and diff --git a/builtin-remote.c b/builtin-remote.c index 6b3325d..8a54bca 100644 --- a/builtin-remote.c +++ b/builtin-remote.c @@ -10,6 +10,7 @@ static const char * const builtin_remote_usage[] = { "git remote", "git remote add ", + "git remote mv ", "git remote rm ", "git remote show ", "git remote prune ", @@ -329,6 +330,109 @@ static int add_branch_for_removal(const char *refname, return 0; } +struct rename_info { + const char *old; + const char *new; + struct string_list *remote_branches; +}; + +static int read_remote_branches(const char *refname, + const unsigned char *sha1, int flags, void *cb_data) +{ + struct rename_info *rename = cb_data; + struct strbuf buf = STRBUF_INIT; + + strbuf_addf(&buf, "refs/remotes/%s", rename->old); + if(!prefixcmp(refname, buf.buf)) + string_list_append(xstrdup(refname), rename->remote_branches); + + return 0; +} + +static int mv(int argc, const char **argv) +{ + struct option options[] = { + OPT_END() + }; + struct remote *oldremote, *newremote; + struct strbuf buf = STRBUF_INIT, buf2 = STRBUF_INIT; + struct string_list remote_branches = { NULL, 0, 0, 0 }; + struct rename_info rename; + int i; + + if (argc != 3) + usage_with_options(builtin_remote_usage, options); + + rename.old = argv[1]; + rename.new = argv[2]; + rename.remote_branches = &remote_branches; + + oldremote = remote_get(rename.old); + if (!oldremote) + die("No such remote: %s", rename.old); + + newremote = remote_get(rename.new); + if (newremote && (newremote->url_nr > 1 || newremote->fetch_refspec_nr)) + die("remote %s already exists.", rename.new); + + strbuf_addf(&buf, "refs/heads/test:refs/remotes/%s/test", rename.new); + if (!valid_fetch_refspec(buf.buf)) + die("'%s' is not a valid remote name", rename.new); + + strbuf_reset(&buf); + strbuf_addf(&buf, "remote.%s", rename.old); + strbuf_addf(&buf2, "remote.%s", rename.new); + if (git_config_rename_section(buf.buf, buf2.buf) < 1) + return error("Could not rename config section '%s' to '%s'", + buf.buf, buf2.buf); + + strbuf_reset(&buf); + strbuf_addf(&buf, "remote.%s.fetch", rename.new); + if (git_config_set_multivar(buf.buf, NULL, NULL, 1)) + return error("Could not remove config section '%s'", buf.buf); + for (i = 0; i < oldremote->fetch_refspec_nr; i++) { + char *ptr; + + strbuf_reset(&buf2); + strbuf_addstr(&buf2, oldremote->fetch_refspec[i]); + ptr = strstr(buf2.buf, rename.old); + if (ptr) + strbuf_splice(&buf2, ptr-buf2.buf, strlen(rename.old), + rename.new, strlen(rename.new)); + if (git_config_set_multivar(buf.buf, buf2.buf, "^$", 0)) + return error("Could not append '%s'", buf.buf); + } + + read_branches(); + for (i = 0; i < branch_list.nr; i++) { + struct string_list_item *item = branch_list.items + i; + struct branch_info *info = item->util; + if (info->remote && !strcmp(info->remote, rename.old)) { + strbuf_reset(&buf); + strbuf_addf(&buf, "branch.%s.remote", item->string); + if (git_config_set(buf.buf, rename.new)) { + return error("Could not set '%s'", buf.buf); + } + } + } + + for_each_ref(read_remote_branches, &rename); + for (i = 0; i < remote_branches.nr; i++) { + struct string_list_item *item = remote_branches.items + i; + strbuf_reset(&buf); + strbuf_addstr(&buf, item->string); + strbuf_splice(&buf, strlen("refs/remotes/"), strlen(rename.old), + rename.new, strlen(rename.new)); + strbuf_reset(&buf2); + strbuf_addf(&buf2, "remote: renamed %s to %s", + item->string, buf.buf); + if (rename_ref(item->string, buf.buf, buf2.buf)) + die("renaming '%s' failed", item->string); + } + + return 0; +} + static int remove_branches(struct string_list *branches) { int i, result = 0; @@ -696,6 +800,8 @@ int cmd_remote(int argc, const char **argv, const char *prefix) result = show_all(); else if (!strcmp(argv[0], "add")) result = add(argc, argv); + else if (!strcmp(argv[0], "mv")) + result = mv(argc, argv); else if (!strcmp(argv[0], "rm")) result = rm(argc, argv); else if (!strcmp(argv[0], "show")) diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh index c449663..9b11fd3 100755 --- a/t/t5505-remote.sh +++ b/t/t5505-remote.sh @@ -324,4 +324,18 @@ test_expect_success 'reject adding remote with an invalid name' ' ' +# The first three tests if the config is properly updated, the last one +# checks if the branches are renamed. + +test_expect_success 'rename a remote' ' + + git clone one four && + (cd four && + git remote mv origin upstream && + git remote show |grep -q upstream && + git config remote.upstream.fetch |grep -q upstream && + test $(git config branch.master.remote) = "upstream" && + git for-each-ref|grep -q refs/remotes/upstream) + +' test_done -- 1.6.0.2 ``` ## Jeff King, 2008-10-23 03:52 Subject: Re: [PATCH] Implement git remote mv Message-ID: <20081023035213.GA8396@coredump.intra.peff.net> URL: https://gitlist.dev/e/20081023035213.GA8396%40coredump.intra.peff.net In-Reply-To: <1224724704-12700-1-git-send-email-vmiklos@frugalware.org> ``` On Thu, Oct 23, 2008 at 03:18:24AM +0200, Miklos Vajna wrote: > The new rename subcommand does the followings: > > 1) Renames the remote.foo configuration section to remote.bar > > 2) Updates the remote.bar.fetch refspecs > > 3) Updates the branch.*.remote settings > > 4) Renames the tracking branches. I can't help but notice that the word "rename" appears all over the commit description and in the code, but not in the user interface. Maybe "rename" would be a better name for the command instead of (or in addition to) "mv"? -Peff ``` ## Miklos Vajna, 2008-10-23 12:56 Subject: [PATCH] Implement git remote rename Message-ID: <1224766597-16034-1-git-send-email-vmiklos@frugalware.org> URL: https://gitlist.dev/e/1224766597-16034-1-git-send-email-vmiklos%40frugalware.org In-Reply-To: <20081023035213.GA8396@coredump.intra.peff.net> ``` The new rename subcommand does the followings: 1) Renames the remote.foo configuration section to remote.bar 2) Updates the remote.bar.fetch refspecs 3) Updates the branch.*.remote settings 4) Renames the tracking branches. Signed-off-by: Miklos Vajna --- On Wed, Oct 22, 2008 at 11:52:14PM -0400, Jeff King wrote: > I can't help but notice that the word "rename" appears all over the > commit description and in the code, but not in the user interface. > Maybe > "rename" would be a better name for the command instead of (or in > addition to) "mv"? I called it "mv" because of "rm" (it is not "remove") and git-mv/git-add, but I don't think it's a problem if it's called "rename". Here is an updated patch. The function name is still "mv" because of rename(2). Documentation/git-remote.txt | 6 ++ builtin-remote.c | 106 ++++++++++++++++++++++++++++++++++++++++++ t/t5505-remote.sh | 14 ++++++ 3 files changed, 126 insertions(+), 0 deletions(-) diff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt index bb99810..7b227b3 100644 --- a/Documentation/git-remote.txt +++ b/Documentation/git-remote.txt @@ -11,6 +11,7 @@ SYNOPSIS [verse] 'git remote' [-v | --verbose] 'git remote add' [-t ] [-m ] [-f] [--mirror] +'git remote rename' 'git remote rm' 'git remote show' [-n] 'git remote prune' [-n | --dry-run] @@ -61,6 +62,11 @@ only makes sense in bare repositories. If a remote uses mirror mode, furthermore, `git push` will always behave as if `\--mirror` was passed. +'rename':: + +Rename the remote named to . All remote tracking branches and +configuration settings for the remote are updated. + 'rm':: Remove the remote named . All remote tracking branches and diff --git a/builtin-remote.c b/builtin-remote.c index 6b3325d..106d6f6 100644 --- a/builtin-remote.c +++ b/builtin-remote.c @@ -10,6 +10,7 @@ static const char * const builtin_remote_usage[] = { "git remote", "git remote add ", + "git remote rename ", "git remote rm ", "git remote show ", "git remote prune ", @@ -329,6 +330,109 @@ static int add_branch_for_removal(const char *refname, return 0; } +struct rename_info { + const char *old; + const char *new; + struct string_list *remote_branches; +}; + +static int read_remote_branches(const char *refname, + const unsigned char *sha1, int flags, void *cb_data) +{ + struct rename_info *rename = cb_data; + struct strbuf buf = STRBUF_INIT; + + strbuf_addf(&buf, "refs/remotes/%s", rename->old); + if(!prefixcmp(refname, buf.buf)) + string_list_append(xstrdup(refname), rename->remote_branches); + + return 0; +} + +static int mv(int argc, const char **argv) +{ + struct option options[] = { + OPT_END() + }; + struct remote *oldremote, *newremote; + struct strbuf buf = STRBUF_INIT, buf2 = STRBUF_INIT; + struct string_list remote_branches = { NULL, 0, 0, 0 }; + struct rename_info rename; + int i; + + if (argc != 3) + usage_with_options(builtin_remote_usage, options); + + rename.old = argv[1]; + rename.new = argv[2]; + rename.remote_branches = &remote_branches; + + oldremote = remote_get(rename.old); + if (!oldremote) + die("No such remote: %s", rename.old); + + newremote = remote_get(rename.new); + if (newremote && (newremote->url_nr > 1 || newremote->fetch_refspec_nr)) + die("remote %s already exists.", rename.new); + + strbuf_addf(&buf, "refs/heads/test:refs/remotes/%s/test", rename.new); + if (!valid_fetch_refspec(buf.buf)) + die("'%s' is not a valid remote name", rename.new); + + strbuf_reset(&buf); + strbuf_addf(&buf, "remote.%s", rename.old); + strbuf_addf(&buf2, "remote.%s", rename.new); + if (git_config_rename_section(buf.buf, buf2.buf) < 1) + return error("Could not rename config section '%s' to '%s'", + buf.buf, buf2.buf); + + strbuf_reset(&buf); + strbuf_addf(&buf, "remote.%s.fetch", rename.new); + if (git_config_set_multivar(buf.buf, NULL, NULL, 1)) + return error("Could not remove config section '%s'", buf.buf); + for (i = 0; i < oldremote->fetch_refspec_nr; i++) { + char *ptr; + + strbuf_reset(&buf2); + strbuf_addstr(&buf2, oldremote->fetch_refspec[i]); + ptr = strstr(buf2.buf, rename.old); + if (ptr) + strbuf_splice(&buf2, ptr-buf2.buf, strlen(rename.old), + rename.new, strlen(rename.new)); + if (git_config_set_multivar(buf.buf, buf2.buf, "^$", 0)) + return error("Could not append '%s'", buf.buf); + } + + read_branches(); + for (i = 0; i < branch_list.nr; i++) { + struct string_list_item *item = branch_list.items + i; + struct branch_info *info = item->util; + if (info->remote && !strcmp(info->remote, rename.old)) { + strbuf_reset(&buf); + strbuf_addf(&buf, "branch.%s.remote", item->string); + if (git_config_set(buf.buf, rename.new)) { + return error("Could not set '%s'", buf.buf); + } + } + } + + for_each_ref(read_remote_branches, &rename); + for (i = 0; i < remote_branches.nr; i++) { + struct string_list_item *item = remote_branches.items + i; + strbuf_reset(&buf); + strbuf_addstr(&buf, item->string); + strbuf_splice(&buf, strlen("refs/remotes/"), strlen(rename.old), + rename.new, strlen(rename.new)); + strbuf_reset(&buf2); + strbuf_addf(&buf2, "remote: renamed %s to %s", + item->string, buf.buf); + if (rename_ref(item->string, buf.buf, buf2.buf)) + die("renaming '%s' failed", item->string); + } + + return 0; +} + static int remove_branches(struct string_list *branches) { int i, result = 0; @@ -696,6 +800,8 @@ int cmd_remote(int argc, const char **argv, const char *prefix) result = show_all(); else if (!strcmp(argv[0], "add")) result = add(argc, argv); + else if (!strcmp(argv[0], "rename")) + result = mv(argc, argv); else if (!strcmp(argv[0], "rm")) result = rm(argc, argv); else if (!strcmp(argv[0], "show")) diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh index c449663..58bd7bf 100755 --- a/t/t5505-remote.sh +++ b/t/t5505-remote.sh @@ -324,4 +324,18 @@ test_expect_success 'reject adding remote with an invalid name' ' ' +# The first three tests if the config is properly updated, the last one +# checks if the branches are renamed. + +test_expect_success 'rename a remote' ' + + git clone one four && + (cd four && + git remote rename origin upstream && + git remote show |grep -q upstream && + git config remote.upstream.fetch |grep -q upstream && + test $(git config branch.master.remote) = "upstream" && + git for-each-ref|grep -q refs/remotes/upstream) + +' test_done -- 1.6.0.2 ``` ## Junio C Hamano, 2008-10-24 23:33 Subject: Re: [PATCH] Implement git remote rename Message-ID: <7v63nh1sc7.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7v63nh1sc7.fsf%40gitster.siamese.dyndns.org In-Reply-To: <1224766597-16034-1-git-send-email-vmiklos@frugalware.org> ``` Miklos Vajna writes: > +static int mv(int argc, const char **argv) > +{ > + struct option options[] = { > + OPT_END() > + }; > + struct remote *oldremote, *newremote; > + struct strbuf buf = STRBUF_INIT, buf2 = STRBUF_INIT; > + struct string_list remote_branches = { NULL, 0, 0, 0 }; > + struct rename_info rename; > + int i; > + > + if (argc != 3) > + usage_with_options(builtin_remote_usage, options); > + > + rename.old = argv[1]; > + rename.new = argv[2]; > + rename.remote_branches = &remote_branches; > + > + oldremote = remote_get(rename.old); > + if (!oldremote) > + die("No such remote: %s", rename.old); > + > + newremote = remote_get(rename.new); > + if (newremote && (newremote->url_nr > 1 || newremote->fetch_refspec_nr)) > + die("remote %s already exists.", rename.new); > + > + strbuf_addf(&buf, "refs/heads/test:refs/remotes/%s/test", rename.new); > + if (!valid_fetch_refspec(buf.buf)) > + die("'%s' is not a valid remote name", rename.new); > + > + strbuf_reset(&buf); > + strbuf_addf(&buf, "remote.%s", rename.old); > + strbuf_addf(&buf2, "remote.%s", rename.new); > + if (git_config_rename_section(buf.buf, buf2.buf) < 1) > + return error("Could not rename config section '%s' to '%s'", > + buf.buf, buf2.buf); Hmm, remote_get() can read from all three supported places that you can define remotes. Could you explain what happens if the old remote is read from say $GIT_DIR/remotes/origin and you are renaming it to "upstream" with "git remote rename origin upstream"? I suspect that if you record where you read the configuration from in "struct remote" and add necessary code to remove the original when rename.old is *not* coming from in-config definition, you would make it possible for repositories initialized with older git that has either $GIT_DIR/branches/origin or $GIT_DIR/remotes/origin to be migrated to the in-config format using "git remote rename origin origin". ``` ## Miklos Vajna, 2008-11-03 18:26 Subject: [PATCH] Implement git remote rename Message-ID: <1225736778-12556-1-git-send-email-vmiklos@frugalware.org> URL: https://gitlist.dev/e/1225736778-12556-1-git-send-email-vmiklos%40frugalware.org In-Reply-To: <7v63nh1sc7.fsf@gitster.siamese.dyndns.org> ``` The new rename subcommand does the followings: 1) Renames the remote.foo configuration section to remote.bar 2) Updates the remote.bar.fetch refspecs 3) Updates the branch.*.remote settings 4) Renames the tracking branches: renames the normal refs and rewrites the symrefs to point to the new refs. Signed-off-by: Miklos Vajna --- On Fri, Oct 24, 2008 at 04:33:28PM -0700, Junio C Hamano wrote: > Hmm, remote_get() can read from all three supported places that you > can define remotes. Could you explain what happens if the old remote > is read from say $GIT_DIR/remotes/origin and you are renaming it to > "upstream" with "git remote rename origin upstream"? Currently it dies because it can't rename config section 'remote.origin' to 'remote.upstream'. > I suspect that if you record where you read the configuration from in > "struct remote" and add necessary code to remove the original when > rename.old is *not* coming from in-config definition, you would make > it possible for repositories initialized with older git that has > either $GIT_DIR/branches/origin or $GIT_DIR/remotes/origin to be > migrated to the in-config format using "git remote rename origin > origin". Yes, that's possible. I think the patch is already large enough that it would be better to do it as a separate patch, but I like the idea. One important step is that currently we have to care about the remote.upstream.fetch variable, but once migration is supported, we have to pay attention to remote.upstream.url/push as well. In the meantime here is an updated patch which applies on top of mv/maint-branch-m-symref. However, it is not meant for 'maint', of course. :) The old version did not "rename" (in fact it is not just a rename but it replaces origin with upstream in the contents) symrefs properly, now the testcases are extended to check for this, too. Documentation/git-remote.txt | 6 ++ builtin-remote.c | 153 ++++++++++++++++++++++++++++++++++++++++++ t/t5505-remote.sh | 15 ++++ 3 files changed, 174 insertions(+), 0 deletions(-) diff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt index bb99810..7b227b3 100644 --- a/Documentation/git-remote.txt +++ b/Documentation/git-remote.txt @@ -11,6 +11,7 @@ SYNOPSIS [verse] 'git remote' [-v | --verbose] 'git remote add' [-t ] [-m ] [-f] [--mirror] +'git remote rename' 'git remote rm' 'git remote show' [-n] 'git remote prune' [-n | --dry-run] @@ -61,6 +62,11 @@ only makes sense in bare repositories. If a remote uses mirror mode, furthermore, `git push` will always behave as if `\--mirror` was passed. +'rename':: + +Rename the remote named to . All remote tracking branches and +configuration settings for the remote are updated. + 'rm':: Remove the remote named . All remote tracking branches and diff --git a/builtin-remote.c b/builtin-remote.c index e396a3a..1ca6cdb 100644 --- a/builtin-remote.c +++ b/builtin-remote.c @@ -10,6 +10,7 @@ static const char * const builtin_remote_usage[] = { "git remote", "git remote add ", + "git remote rename ", "git remote rm ", "git remote show ", "git remote prune ", @@ -329,6 +330,156 @@ static int add_branch_for_removal(const char *refname, return 0; } +struct rename_info { + const char *old; + const char *new; + struct string_list *remote_branches; +}; + +static int read_remote_branches(const char *refname, + const unsigned char *sha1, int flags, void *cb_data) +{ + struct rename_info *rename = cb_data; + struct strbuf buf = STRBUF_INIT; + struct string_list_item *item; + int flag; + unsigned char orig_sha1[20]; + const char *symref; + + strbuf_addf(&buf, "refs/remotes/%s", rename->old); + if(!prefixcmp(refname, buf.buf)) { + item = string_list_append(xstrdup(refname), rename->remote_branches); + symref = resolve_ref(refname, orig_sha1, 1, &flag); + if (flag & REF_ISSYMREF) + item->util = xstrdup(symref); + else + item->util = NULL; + } + + return 0; +} + +static int mv(int argc, const char **argv) +{ + struct option options[] = { + OPT_END() + }; + struct remote *oldremote, *newremote; + struct strbuf buf = STRBUF_INIT, buf2 = STRBUF_INIT, buf3 = STRBUF_INIT; + struct string_list remote_branches = { NULL, 0, 0, 0 }; + struct rename_info rename; + int i; + + if (argc != 3) + usage_with_options(builtin_remote_usage, options); + + rename.old = argv[1]; + rename.new = argv[2]; + rename.remote_branches = &remote_branches; + + oldremote = remote_get(rename.old); + if (!oldremote) + die("No such remote: %s", rename.old); + + newremote = remote_get(rename.new); + if (newremote && (newremote->url_nr > 1 || newremote->fetch_refspec_nr)) + die("remote %s already exists.", rename.new); + + strbuf_addf(&buf, "refs/heads/test:refs/remotes/%s/test", rename.new); + if (!valid_fetch_refspec(buf.buf)) + die("'%s' is not a valid remote name", rename.new); + + strbuf_reset(&buf); + strbuf_addf(&buf, "remote.%s", rename.old); + strbuf_addf(&buf2, "remote.%s", rename.new); + if (git_config_rename_section(buf.buf, buf2.buf) < 1) + return error("Could not rename config section '%s' to '%s'", + buf.buf, buf2.buf); + + strbuf_reset(&buf); + strbuf_addf(&buf, "remote.%s.fetch", rename.new); + if (git_config_set_multivar(buf.buf, NULL, NULL, 1)) + return error("Could not remove config section '%s'", buf.buf); + for (i = 0; i < oldremote->fetch_refspec_nr; i++) { + char *ptr; + + strbuf_reset(&buf2); + strbuf_addstr(&buf2, oldremote->fetch_refspec[i]); + ptr = strstr(buf2.buf, rename.old); + if (ptr) + strbuf_splice(&buf2, ptr-buf2.buf, strlen(rename.old), + rename.new, strlen(rename.new)); + if (git_config_set_multivar(buf.buf, buf2.buf, "^$", 0)) + return error("Could not append '%s'", buf.buf); + } + + read_branches(); + for (i = 0; i < branch_list.nr; i++) { + struct string_list_item *item = branch_list.items + i; + struct branch_info *info = item->util; + if (info->remote && !strcmp(info->remote, rename.old)) { + strbuf_reset(&buf); + strbuf_addf(&buf, "branch.%s.remote", item->string); + if (git_config_set(buf.buf, rename.new)) { + return error("Could not set '%s'", buf.buf); + } + } + } + + /* + * First remove symrefs, then rename the rest, finally create + * the new symrefs. + */ + for_each_ref(read_remote_branches, &rename); + for (i = 0; i < remote_branches.nr; i++) { + struct string_list_item *item = remote_branches.items + i; + int flag = 0; + unsigned char sha1[20]; + const char *symref; + + symref = resolve_ref(item->string, sha1, 1, &flag); + if (!(flag & REF_ISSYMREF)) + continue; + if (delete_ref(item->string, NULL, REF_NODEREF)) + die("deleting '%s' failed", item->string); + } + for (i = 0; i < remote_branches.nr; i++) { + struct string_list_item *item = remote_branches.items + i; + + if (item->util) + continue; + strbuf_reset(&buf); + strbuf_addstr(&buf, item->string); + strbuf_splice(&buf, strlen("refs/remotes/"), strlen(rename.old), + rename.new, strlen(rename.new)); + strbuf_reset(&buf2); + strbuf_addf(&buf2, "remote: renamed %s to %s", + item->string, buf.buf); + if (rename_ref(item->string, buf.buf, buf2.buf)) + die("renaming '%s' failed", item->string); + } + for (i = 0; i < remote_branches.nr; i++) { + struct string_list_item *item = remote_branches.items + i; + + if (!item->util) + continue; + strbuf_reset(&buf); + strbuf_addstr(&buf, item->string); + strbuf_splice(&buf, strlen("refs/remotes/"), strlen(rename.old), + rename.new, strlen(rename.new)); + strbuf_reset(&buf2); + strbuf_addstr(&buf2, item->util); + strbuf_splice(&buf2, strlen("refs/remotes/"), strlen(rename.old), + rename.new, strlen(rename.new)); + strbuf_reset(&buf3); + strbuf_addf(&buf3, "remote: renamed %s to %s", + item->string, buf.buf); + if (create_symref(buf.buf, buf2.buf, buf3.buf)) + die("creating '%s' failed", buf.buf); + } + return 0; +} + static int remove_branches(struct string_list *branches) { int i, result = 0; @@ -695,6 +846,8 @@ int cmd_remote(int argc, const char **argv, const char *prefix) result = show_all(); else if (!strcmp(argv[0], "add")) result = add(argc, argv); + else if (!strcmp(argv[0], "rename")) + result = mv(argc, argv); else if (!strcmp(argv[0], "rm")) result = rm(argc, argv); else if (!strcmp(argv[0], "show")) diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh index c4380c7..0c956ba 100755 --- a/t/t5505-remote.sh +++ b/t/t5505-remote.sh @@ -328,4 +328,19 @@ test_expect_success 'reject adding remote with an invalid name' ' ' +# The first three test if the tracking branches are properly renamed, +# the last two ones check if the config is updated. + +test_expect_success 'rename a remote' ' + + git clone one four && + (cd four && + git remote rename origin upstream && + rmdir .git/refs/remotes/origin && + test "$(git symbolic-ref refs/remotes/upstream/HEAD)" = "refs/remotes/upstream/master" && + test "$(git rev-parse upstream/master)" = "$(git rev-parse master)" && + test "$(git config remote.upstream.fetch)" = "+refs/heads/*:refs/remotes/upstream/*" && + test "$(git config branch.master.remote)" = "upstream") + +' test_done -- 1.6.0.2 ``` ## Miklos Vajna, 2008-11-10 20:42 Subject: Re: [PATCH] Implement git remote rename Message-ID: URL: https://gitlist.dev/e/cover.1226349595.git.vmiklos%40frugalware.org In-Reply-To: <7v63nh1sc7.fsf@gitster.siamese.dyndns.org> ``` On Fri, Oct 24, 2008 at 04:33:28PM -0700, Junio C Hamano wrote: > I suspect that if you record where you read the configuration from in > "struct remote" and add necessary code to remove the original when > rename.old is *not* coming from in-config definition, you would make > it possible for repositories initialized with older git that has > either $GIT_DIR/branches/origin or $GIT_DIR/remotes/origin to be > migrated to the in-config format using "git remote rename origin > origin". Here are 4 patches to implement this + add the related testcases/documentation. Miklos Vajna (4): remote: add a new 'origin' variable to the struct git-remote rename: support remotes->config migration git-remote rename: support branches->config migration git-remote: document the migration feature of the rename subcommand Documentation/git-remote.txt | 4 ++++ builtin-remote.c | 35 +++++++++++++++++++++++++++++++++++ remote.c | 3 +++ remote.h | 7 +++++++ t/t5505-remote.sh | 33 +++++++++++++++++++++++++++++++++ 5 files changed, 82 insertions(+), 0 deletions(-) ``` ## Miklos Vajna, 2008-11-10 20:43 Subject: [PATCH 1/4] remote: add a new 'origin' variable to the struct Message-ID: <95e56b46e30b41af31da86789625c93511f1faef.1226349595.git.vmiklos@frugalware.org> URL: https://gitlist.dev/e/95e56b46e30b41af31da86789625c93511f1faef.1226349595.git.vmiklos%40frugalware.org In-Reply-To: ``` This allows one to track where was the remote's original source, so that it's possible to decide if it makes sense to migrate it to the config format or not. Signed-off-by: Miklos Vajna --- remote.c | 3 +++ remote.h | 7 +++++++ 2 files changed, 10 insertions(+), 0 deletions(-) diff --git a/remote.c b/remote.c index e530a21..cbb3e48 100644 --- a/remote.c +++ b/remote.c @@ -201,6 +201,7 @@ static void read_remotes_file(struct remote *remote) if (!f) return; + remote->origin = REMOTE_REMOTES; while (fgets(buffer, BUF_SIZE, f)) { int value_list; char *s, *p; @@ -261,6 +262,7 @@ static void read_branches_file(struct remote *remote) s++; if (!*s) return; + remote->origin = REMOTE_BRANCHES; p = s + strlen(s); while (isspace(p[-1])) *--p = 0; @@ -350,6 +352,7 @@ static int handle_config(const char *key, const char *value, void *cb) if (!subkey) return error("Config with no key for remote %s", name); remote = make_remote(name, subkey - name); + remote->origin = REMOTE_CONFIG; if (!strcmp(subkey, ".mirror")) remote->mirror = git_config_bool(key, value); else if (!strcmp(subkey, ".skipdefaultupdate")) diff --git a/remote.h b/remote.h index d2e170c..a46a5be 100644 --- a/remote.h +++ b/remote.h @@ -1,8 +1,15 @@ #ifndef REMOTE_H #define REMOTE_H +enum { + REMOTE_CONFIG, + REMOTE_REMOTES, + REMOTE_BRANCHES +}; + struct remote { const char *name; + int origin; const char **url; int url_nr; -- 1.6.0.2 ``` ## Miklos Vajna, 2008-11-10 20:43 Subject: [PATCH 2/4] git-remote rename: support remotes->config migration Message-ID: <033bc63195299e494791e4e6d8a41f142d848bba.1226349595.git.vmiklos@frugalware.org> URL: https://gitlist.dev/e/033bc63195299e494791e4e6d8a41f142d848bba.1226349595.git.vmiklos%40frugalware.org In-Reply-To: <95e56b46e30b41af31da86789625c93511f1faef.1226349595.git.vmiklos@frugalware.org> ``` This patch makes it possible to migrate a remote stored in a $GIT_DIR/remotes/nick file to the configuration file format. Signed-off-by: Miklos Vajna --- builtin-remote.c | 33 +++++++++++++++++++++++++++++++++ t/t5505-remote.sh | 21 +++++++++++++++++++++ 2 files changed, 54 insertions(+), 0 deletions(-) diff --git a/builtin-remote.c b/builtin-remote.c index 1ca6cdb..d9d0ba3 100644 --- a/builtin-remote.c +++ b/builtin-remote.c @@ -359,6 +359,36 @@ static int read_remote_branches(const char *refname, return 0; } +static int migrate_file(struct remote *remote) +{ + struct strbuf buf = STRBUF_INIT; + int i; + char *path = NULL; + + strbuf_addf(&buf, "remote.%s.url", remote->name); + for (i = 0; i < remote->url_nr; i++) + if (git_config_set_multivar(buf.buf, remote->url[i], "^$", 0)) + return error("Could not append '%s' to '%s'", + remote->url[i], buf.buf); + strbuf_reset(&buf); + strbuf_addf(&buf, "remote.%s.push", remote->name); + for (i = 0; i < remote->push_refspec_nr; i++) + if (git_config_set_multivar(buf.buf, remote->push_refspec[i], "^$", 0)) + return error("Could not append '%s' to '%s'", + remote->push_refspec[i], buf.buf); + strbuf_reset(&buf); + strbuf_addf(&buf, "remote.%s.fetch", remote->name); + for (i = 0; i < remote->fetch_refspec_nr; i++) + if (git_config_set_multivar(buf.buf, remote->fetch_refspec[i], "^$", 0)) + return error("Could not append '%s' to '%s'", + remote->fetch_refspec[i], buf.buf); + if (remote->origin == REMOTE_REMOTES) + path = git_path("remotes/%s", remote->name); + if (path && unlink(path)) + warning("failed to remove '%s'", path); + return 0; +} + static int mv(int argc, const char **argv) { struct option options[] = { @@ -381,6 +411,9 @@ static int mv(int argc, const char **argv) if (!oldremote) die("No such remote: %s", rename.old); + if (!strcmp(rename.old, rename.new) && oldremote->origin != REMOTE_CONFIG) + return migrate_file(oldremote); + newremote = remote_get(rename.new); if (newremote && (newremote->url_nr > 1 || newremote->fetch_refspec_nr)) die("remote %s already exists.", rename.new); diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh index 0c956ba..1567631 100755 --- a/t/t5505-remote.sh +++ b/t/t5505-remote.sh @@ -343,4 +343,25 @@ test_expect_success 'rename a remote' ' test "$(git config branch.master.remote)" = "upstream") ' + +cat > remotes_origin << EOF +URL: $(pwd)/one +Push: refs/heads/master:refs/heads/upstream +Pull: refs/heads/master:refs/heads/origin +EOF + +test_expect_success 'migrate a remote from named file in $GIT_DIR/remotes' ' + git clone one five && + origin_url=$(pwd)/one && + (cd five && + git remote rm origin && + mkdir -p .git/remotes && + cat ../remotes_origin > .git/remotes/origin && + git remote rename origin origin && + ! test -f .git/remotes/origin && + test "$(git config remote.origin.url)" = "$origin_url" && + test "$(git config remote.origin.push)" = "refs/heads/master:refs/heads/upstream" && + test "$(git config remote.origin.fetch)" = "refs/heads/master:refs/heads/origin") +' + test_done -- 1.6.0.2 ``` ## Miklos Vajna, 2008-11-10 20:43 Subject: [PATCH 3/4] git-remote rename: support branches->config migration Message-ID: URL: https://gitlist.dev/e/b32cf68df41e417079a49dc02e46ffc0c571029b.1226349595.git.vmiklos%40frugalware.org In-Reply-To: <033bc63195299e494791e4e6d8a41f142d848bba.1226349595.git.vmiklos@frugalware.org> ``` This is similar to the remotes->config one, but it makes the branches->config conversion possible. Signed-off-by: Miklos Vajna --- builtin-remote.c | 2 ++ t/t5505-remote.sh | 12 ++++++++++++ 2 files changed, 14 insertions(+), 0 deletions(-) diff --git a/builtin-remote.c b/builtin-remote.c index d9d0ba3..3af1876 100644 --- a/builtin-remote.c +++ b/builtin-remote.c @@ -384,6 +384,8 @@ static int migrate_file(struct remote *remote) remote->fetch_refspec[i], buf.buf); if (remote->origin == REMOTE_REMOTES) path = git_path("remotes/%s", remote->name); + else if (remote->origin == REMOTE_BRANCHES) + path = git_path("branches/%s", remote->name); if (path && unlink(path)) warning("failed to remove '%s'", path); return 0; diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh index 1567631..1f59960 100755 --- a/t/t5505-remote.sh +++ b/t/t5505-remote.sh @@ -364,4 +364,16 @@ test_expect_success 'migrate a remote from named file in $GIT_DIR/remotes' ' test "$(git config remote.origin.fetch)" = "refs/heads/master:refs/heads/origin") ' +test_expect_success 'migrate a remote from named file in $GIT_DIR/branches' ' + git clone one six && + origin_url=$(pwd)/one && + (cd six && + git remote rm origin && + echo "$origin_url" > .git/branches/origin && + git remote rename origin origin && + ! test -f .git/branches/origin && + test "$(git config remote.origin.url)" = "$origin_url" && + test "$(git config remote.origin.fetch)" = "refs/heads/master:refs/heads/origin") +' + test_done -- 1.6.0.2 ``` ## Miklos Vajna, 2008-11-10 20:43 Subject: [PATCH 4/4] git-remote: document the migration feature of the rename subcommand Message-ID: URL: https://gitlist.dev/e/ebcd8165be4bf4444692f6552aeb4d2ae6c32579.1226349595.git.vmiklos%40frugalware.org In-Reply-To: ``` Signed-off-by: Miklos Vajna --- Documentation/git-remote.txt | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt index 7b227b3..fad983e 100644 --- a/Documentation/git-remote.txt +++ b/Documentation/git-remote.txt @@ -66,6 +66,10 @@ was passed. Rename the remote named to . All remote tracking branches and configuration settings for the remote are updated. ++ +In case and are the same, and is a file under +`$GIT_DIR/remotes` or `$GIT_DIR/branches`, the remote is converted to +the configuration file format. 'rm':: -- 1.6.0.2 ``` ## Junio C Hamano, 2008-11-12 00:49 Subject: Re: [PATCH 3/4] git-remote rename: support branches->config migration Message-ID: <7v63mtvkdx.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7v63mtvkdx.fsf%40gitster.siamese.dyndns.org In-Reply-To: ``` Miklos Vajna writes: > This is similar to the remotes->config one, but it makes the > branches->config conversion possible. > > Signed-off-by: Miklos Vajna > --- > builtin-remote.c | 2 ++ > t/t5505-remote.sh | 12 ++++++++++++ > 2 files changed, 14 insertions(+), 0 deletions(-) > > diff --git a/builtin-remote.c b/builtin-remote.c > index d9d0ba3..3af1876 100644 > --- a/builtin-remote.c > +++ b/builtin-remote.c > @@ -384,6 +384,8 @@ static int migrate_file(struct remote *remote) > remote->fetch_refspec[i], buf.buf); > if (remote->origin == REMOTE_REMOTES) > path = git_path("remotes/%s", remote->name); > + else if (remote->origin == REMOTE_BRANCHES) > + path = git_path("branches/%s", remote->name); There is something fishy going on between 2/4 and 3/4. 2/4 was advertised to migrate remotes to config and had a call to migrate_file() for that purpose. Here this one now allows to convert branches but there is no change to the callsite of migrate_file(). Which would mean that 2/4 would convert branches/foo too. And this one is only to remove the leftover branches/foo file. Or am I utterly confused? ``` ## Miklos Vajna, 2008-11-12 02:01 Subject: Re: [PATCH 3/4] git-remote rename: support branches->config migration Message-ID: <20081112020158.GK24201@genesis.frugalware.org> URL: https://gitlist.dev/e/20081112020158.GK24201%40genesis.frugalware.org In-Reply-To: <7v63mtvkdx.fsf@gitster.siamese.dyndns.org> ``` On Tue, Nov 11, 2008 at 04:49:14PM -0800, Junio C Hamano wrote: > There is something fishy going on between 2/4 and 3/4. 2/4 was advertised > to migrate remotes to config and had a call to migrate_file() for that > purpose. Here this one now allows to convert branches but there is no > change to the callsite of migrate_file(). > > Which would mean that 2/4 would convert branches/foo too. And this one is > only to remove the leftover branches/foo file. > > Or am I utterly confused? The trick is that 2/4 already added support for remotes/foo as it uses remote_get() and that detects remotes/foo as well, but that is completely unintentional. If you think it makes sense, just squash 3/4 to 2/4, I made it two separate patches because I think these are logically separate changes. ``` ## Junio C Hamano, 2008-11-12 04:22 Subject: Re: [PATCH 3/4] git-remote rename: support branches->config migration Message-ID: <7viqqtshdd.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7viqqtshdd.fsf%40gitster.siamese.dyndns.org In-Reply-To: <20081112020158.GK24201@genesis.frugalware.org> ``` Miklos Vajna writes: > On Tue, Nov 11, 2008 at 04:49:14PM -0800, Junio C Hamano wrote: >> There is something fishy going on between 2/4 and 3/4. 2/4 was advertised >> to migrate remotes to config and had a call to migrate_file() for that >> purpose. Here this one now allows to convert branches but there is no >> change to the callsite of migrate_file(). >> >> Which would mean that 2/4 would convert branches/foo too. And this one is >> only to remove the leftover branches/foo file. >> >> Or am I utterly confused? > > The trick is that 2/4 already added support for remotes/foo as it uses > remote_get() and that detects remotes/foo as well, but that is > completely unintentional. That is not a trick; it merely is a broken code. The function migrate_file() introduced by [2/4] is called for any remote definition that did not come from config (by definition, it either came from remotes/foo or branches/foo). The function adds the entries for the given remote definition to the config file, and then removes remotes/foo file if the remote definition came from it. So it is a logically consistent change if you only called this function only for remote definitions that came from remotes/foo. But the function is called for a remote definition that originally came from branches/foo as well. It happily adds the definition to the config, even though it *fails to remove* branches/foo file. Do you still think 2/4 is a logically contained good change? If you apply this to 5505 (taken from 3/4, but removing the check for branches/origin file), and look at resulting t/trash*/six/.git/config file, you will see you have already migrated the remote definition to the config. diff --git i/t/t5505-remote.sh w/t/t5505-remote.sh index 1567631..60bb9e5 100755 --- i/t/t5505-remote.sh +++ w/t/t5505-remote.sh @@ -364,4 +364,15 @@ test_expect_success 'migrate a remote from named file in $GIT_DIR/remotes' ' test "$(git config remote.origin.fetch)" = "refs/heads/master:refs/heads/origin") ' -test_done +test_expect_success 'migrate a remote from named file in $GIT_DIR/branches' ' + git clone one six && + origin_url=$(pwd)/one && + (cd six && + git remote rm origin && + echo "$origin_url" > .git/branches/origin && + git remote rename origin origin && + test "$(git config remote.origin.url)" = "$origin_url" && + test "$(git config remote.origin.fetch)" = "refs/heads/master:refs/heads/origin") +' + +: test_done ```