From 88f8576eda2880587fb4487ba469d909dbe35a7b Mon Sep 17 00:00:00 2001 From: Bert Wesarg Date: Mon, 27 Jan 2020 08:04:27 +0100 Subject: pull --rebase/remote rename: document and honor single-letter abbreviations rebase types When 46af44b07d (pull --rebase=: allow single-letter abbreviations for the type, 2018-08-04) landed in Git, it had the side effect that not only 'pull --rebase=' accepted the single-letter abbreviations but also the 'pull.rebase' and 'branch..rebase' configurations. However, 'git remote rename' did not honor these single-letter abbreviations when reading the 'branch.*.rebase' configurations. We now document the single-letter abbreviations and both code places share a common function to parse the values of 'git pull --rebase=*', 'pull.rebase', and 'branches.*.rebase'. The only functional change is the handling of the `branch_info::rebase` value. Before it was an unsigned enum, thus the truth value could be checked with `branch_info::rebase != 0`. But `enum rebase_type` is signed, thus the truth value must now be checked with `branch_info::rebase >= REBASE_TRUE` Signed-off-by: Bert Wesarg Signed-off-by: Junio C Hamano --- builtin/pull.c | 29 ++++------------------------- builtin/remote.c | 30 ++++++++++++------------------ 2 files changed, 16 insertions(+), 43 deletions(-) (limited to 'builtin') diff --git a/builtin/pull.c b/builtin/pull.c index d25ff13a60..888181c07c 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -15,6 +15,7 @@ #include "sha1-array.h" #include "remote.h" #include "dir.h" +#include "rebase.h" #include "refs.h" #include "refspec.h" #include "revision.h" @@ -26,15 +27,6 @@ #include "commit-reach.h" #include "sequencer.h" -enum rebase_type { - REBASE_INVALID = -1, - REBASE_FALSE = 0, - REBASE_TRUE, - REBASE_PRESERVE, - REBASE_MERGES, - REBASE_INTERACTIVE -}; - /** * Parses the value of --rebase. If value is a false value, returns * REBASE_FALSE. If value is a true value, returns REBASE_TRUE. If value is @@ -45,22 +37,9 @@ enum rebase_type { static enum rebase_type parse_config_rebase(const char *key, const char *value, int fatal) { - int v = git_parse_maybe_bool(value); - - if (!v) - return REBASE_FALSE; - else if (v > 0) - return REBASE_TRUE; - else if (!strcmp(value, "preserve") || !strcmp(value, "p")) - return REBASE_PRESERVE; - else if (!strcmp(value, "merges") || !strcmp(value, "m")) - return REBASE_MERGES; - else if (!strcmp(value, "interactive") || !strcmp(value, "i")) - return REBASE_INTERACTIVE; - /* - * Please update _git_config() in git-completion.bash when you - * add new rebase modes. - */ + enum rebase_type v = rebase_parse_value(value); + if (v != REBASE_INVALID) + return v; if (fatal) die(_("Invalid value for %s: %s"), key, value); diff --git a/builtin/remote.c b/builtin/remote.c index 96bbe828fe..6802765e73 100644 --- a/builtin/remote.c +++ b/builtin/remote.c @@ -6,6 +6,7 @@ #include "string-list.h" #include "strbuf.h" #include "run-command.h" +#include "rebase.h" #include "refs.h" #include "refspec.h" #include "object-store.h" @@ -248,9 +249,7 @@ static int add(int argc, const char **argv) struct branch_info { char *remote_name; struct string_list merge; - enum { - NO_REBASE, NORMAL_REBASE, INTERACTIVE_REBASE, REBASE_MERGES - } rebase; + enum rebase_type rebase; }; static struct string_list branch_list = STRING_LIST_INIT_NODUP; @@ -305,17 +304,12 @@ static int config_read_branches(const char *key, const char *value, void *cb) space = strchr(value, ' '); } string_list_append(&info->merge, xstrdup(value)); - } else { - int v = git_parse_maybe_bool(value); - if (v >= 0) - info->rebase = v; - else if (!strcmp(value, "preserve")) - info->rebase = NORMAL_REBASE; - else if (!strcmp(value, "merges")) - info->rebase = REBASE_MERGES; - else if (!strcmp(value, "interactive")) - info->rebase = INTERACTIVE_REBASE; - } + } else + /* + * Consider invalid values as false and check the + * truth value with >= REBASE_TRUE. + */ + info->rebase = rebase_parse_value(value); } return 0; } @@ -943,7 +937,7 @@ static int add_local_to_show_info(struct string_list_item *branch_item, void *cb return 0; if ((n = strlen(branch_item->string)) > show_info->width) show_info->width = n; - if (branch_info->rebase) + if (branch_info->rebase >= REBASE_TRUE) show_info->any_rebase = 1; item = string_list_insert(show_info->list, branch_item->string); @@ -960,16 +954,16 @@ static int show_local_info_item(struct string_list_item *item, void *cb_data) int width = show_info->width + 4; int i; - if (branch_info->rebase && branch_info->merge.nr > 1) { + if (branch_info->rebase >= REBASE_TRUE && branch_info->merge.nr > 1) { error(_("invalid branch.%s.merge; cannot rebase onto > 1 branch"), item->string); return 0; } printf(" %-*s ", show_info->width, item->string); - if (branch_info->rebase) { + if (branch_info->rebase >= REBASE_TRUE) { const char *msg; - if (branch_info->rebase == INTERACTIVE_REBASE) + if (branch_info->rebase == REBASE_INTERACTIVE) msg = _("rebases interactively onto remote %s"); else if (branch_info->rebase == REBASE_MERGES) msg = _("rebases interactively (with merges) onto " -- cgit v1.3 From 1a83068c26206d2b45718e0896955eb61bef8353 Mon Sep 17 00:00:00 2001 From: Bert Wesarg Date: Mon, 27 Jan 2020 08:04:28 +0100 Subject: remote: clean-up by returning early to avoid one indentation Signed-off-by: Bert Wesarg Signed-off-by: Junio C Hamano --- builtin/remote.c | 94 +++++++++++++++++++++++++++++--------------------------- 1 file changed, 48 insertions(+), 46 deletions(-) (limited to 'builtin') diff --git a/builtin/remote.c b/builtin/remote.c index 6802765e73..4cf929bfc6 100644 --- a/builtin/remote.c +++ b/builtin/remote.c @@ -263,54 +263,56 @@ static const char *abbrev_ref(const char *name, const char *prefix) static int config_read_branches(const char *key, const char *value, void *cb) { - if (starts_with(key, "branch.")) { - const char *orig_key = key; - char *name; - struct string_list_item *item; - struct branch_info *info; - enum { REMOTE, MERGE, REBASE } type; - size_t key_len; - - key += 7; - if (strip_suffix(key, ".remote", &key_len)) { - name = xmemdupz(key, key_len); - type = REMOTE; - } else if (strip_suffix(key, ".merge", &key_len)) { - name = xmemdupz(key, key_len); - type = MERGE; - } else if (strip_suffix(key, ".rebase", &key_len)) { - name = xmemdupz(key, key_len); - type = REBASE; - } else - return 0; + const char *orig_key = key; + char *name; + struct string_list_item *item; + struct branch_info *info; + enum { REMOTE, MERGE, REBASE } type; + size_t key_len; - item = string_list_insert(&branch_list, name); + if (!starts_with(key, "branch.")) + return 0; + + key += 7; + if (strip_suffix(key, ".remote", &key_len)) { + name = xmemdupz(key, key_len); + type = REMOTE; + } else if (strip_suffix(key, ".merge", &key_len)) { + name = xmemdupz(key, key_len); + type = MERGE; + } else if (strip_suffix(key, ".rebase", &key_len)) { + name = xmemdupz(key, key_len); + type = REBASE; + } else + return 0; + + item = string_list_insert(&branch_list, name); + + if (!item->util) + item->util = xcalloc(1, sizeof(struct branch_info)); + info = item->util; + if (type == REMOTE) { + if (info->remote_name) + warning(_("more than one %s"), orig_key); + info->remote_name = xstrdup(value); + } else if (type == MERGE) { + char *space = strchr(value, ' '); + value = abbrev_branch(value); + while (space) { + char *merge; + merge = xstrndup(value, space - value); + string_list_append(&info->merge, merge); + value = abbrev_branch(space + 1); + space = strchr(value, ' '); + } + string_list_append(&info->merge, xstrdup(value)); + } else + /* + * Consider invalid values as false and check the + * truth value with >= REBASE_TRUE. + */ + info->rebase = rebase_parse_value(value); - if (!item->util) - item->util = xcalloc(1, sizeof(struct branch_info)); - info = item->util; - if (type == REMOTE) { - if (info->remote_name) - warning(_("more than one %s"), orig_key); - info->remote_name = xstrdup(value); - } else if (type == MERGE) { - char *space = strchr(value, ' '); - value = abbrev_branch(value); - while (space) { - char *merge; - merge = xstrndup(value, space - value); - string_list_append(&info->merge, merge); - value = abbrev_branch(space + 1); - space = strchr(value, ' '); - } - string_list_append(&info->merge, xstrdup(value)); - } else - /* - * Consider invalid values as false and check the - * truth value with >= REBASE_TRUE. - */ - info->rebase = rebase_parse_value(value); - } return 0; } -- cgit v1.3 From ceff1a13084bba74b607d738c6c467365aa6a5f7 Mon Sep 17 00:00:00 2001 From: Bert Wesarg Date: Mon, 27 Jan 2020 08:04:29 +0100 Subject: remote: clean-up config callback Some minor clean-ups in function `config_read_branches`: * remove hardcoded length in `key += 7` * call `xmemdupz` only once * use a switch to handle the configuration type and add a `BUG()` Suggested-by: Junio C Hamano Signed-off-by: Bert Wesarg Signed-off-by: Junio C Hamano --- builtin/remote.c | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) (limited to 'builtin') diff --git a/builtin/remote.c b/builtin/remote.c index 4cf929bfc6..9ee44c9f6c 100644 --- a/builtin/remote.c +++ b/builtin/remote.c @@ -273,29 +273,29 @@ static int config_read_branches(const char *key, const char *value, void *cb) if (!starts_with(key, "branch.")) return 0; - key += 7; - if (strip_suffix(key, ".remote", &key_len)) { - name = xmemdupz(key, key_len); + key += strlen("branch."); + if (strip_suffix(key, ".remote", &key_len)) type = REMOTE; - } else if (strip_suffix(key, ".merge", &key_len)) { - name = xmemdupz(key, key_len); + else if (strip_suffix(key, ".merge", &key_len)) type = MERGE; - } else if (strip_suffix(key, ".rebase", &key_len)) { - name = xmemdupz(key, key_len); + else if (strip_suffix(key, ".rebase", &key_len)) type = REBASE; - } else + else return 0; + name = xmemdupz(key, key_len); item = string_list_insert(&branch_list, name); if (!item->util) item->util = xcalloc(1, sizeof(struct branch_info)); info = item->util; - if (type == REMOTE) { + switch (type) { + case REMOTE: if (info->remote_name) warning(_("more than one %s"), orig_key); info->remote_name = xstrdup(value); - } else if (type == MERGE) { + break; + case MERGE: { char *space = strchr(value, ' '); value = abbrev_branch(value); while (space) { @@ -306,12 +306,18 @@ static int config_read_branches(const char *key, const char *value, void *cb) space = strchr(value, ' '); } string_list_append(&info->merge, xstrdup(value)); - } else + break; + } + case REBASE: /* * Consider invalid values as false and check the * truth value with >= REBASE_TRUE. */ info->rebase = rebase_parse_value(value); + break; + default: + BUG("unexpected type=%d", type); + } return 0; } -- cgit v1.3 From 923d4a5ca4f86c19247ef436e6d03d261ebce904 Mon Sep 17 00:00:00 2001 From: Bert Wesarg Date: Mon, 27 Jan 2020 08:04:30 +0100 Subject: remote rename/remove: handle branch..pushRemote config values When renaming or removing a remote with git remote rename X Y git remote remove X Git already renames/removes any config values from branch..remote = X to branch..remote = Y As branch..pushRemote also names a remote, it now also renames or removes these config values from branch..pushRemote = X to branch..pushRemote = Y Signed-off-by: Bert Wesarg Signed-off-by: Junio C Hamano --- builtin/remote.c | 22 +++++++++++++++++++++- t/t5505-remote.sh | 16 +++++++++++++++- 2 files changed, 36 insertions(+), 2 deletions(-) (limited to 'builtin') diff --git a/builtin/remote.c b/builtin/remote.c index 9ee44c9f6c..a2379a14bf 100644 --- a/builtin/remote.c +++ b/builtin/remote.c @@ -250,6 +250,7 @@ struct branch_info { char *remote_name; struct string_list merge; enum rebase_type rebase; + char *push_remote_name; }; static struct string_list branch_list = STRING_LIST_INIT_NODUP; @@ -267,7 +268,7 @@ static int config_read_branches(const char *key, const char *value, void *cb) char *name; struct string_list_item *item; struct branch_info *info; - enum { REMOTE, MERGE, REBASE } type; + enum { REMOTE, MERGE, REBASE, PUSH_REMOTE } type; size_t key_len; if (!starts_with(key, "branch.")) @@ -280,6 +281,8 @@ static int config_read_branches(const char *key, const char *value, void *cb) type = MERGE; else if (strip_suffix(key, ".rebase", &key_len)) type = REBASE; + else if (strip_suffix(key, ".pushremote", &key_len)) + type = PUSH_REMOTE; else return 0; name = xmemdupz(key, key_len); @@ -315,6 +318,11 @@ static int config_read_branches(const char *key, const char *value, void *cb) */ info->rebase = rebase_parse_value(value); break; + case PUSH_REMOTE: + if (info->push_remote_name) + warning(_("more than one %s"), orig_key); + info->push_remote_name = xstrdup(value); + break; default: BUG("unexpected type=%d", type); } @@ -682,6 +690,11 @@ static int mv(int argc, const char **argv) strbuf_addf(&buf, "branch.%s.remote", item->string); git_config_set(buf.buf, rename.new_name); } + if (info->push_remote_name && !strcmp(info->push_remote_name, rename.old_name)) { + strbuf_reset(&buf); + strbuf_addf(&buf, "branch.%s.pushremote", item->string); + git_config_set(buf.buf, rename.new_name); + } } if (!refspec_updated) @@ -783,6 +796,13 @@ static int rm(int argc, const char **argv) die(_("could not unset '%s'"), buf.buf); } } + if (info->push_remote_name && !strcmp(info->push_remote_name, remote->name)) { + strbuf_reset(&buf); + strbuf_addf(&buf, "branch.%s.pushremote", item->string); + result = git_config_set_gently(buf.buf, NULL); + if (result && result != CONFIG_NOTHING_SET) + die(_("could not unset '%s'"), buf.buf); + } } /* diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh index 883b32efa0..082042b05a 100755 --- a/t/t5505-remote.sh +++ b/t/t5505-remote.sh @@ -737,12 +737,14 @@ test_expect_success 'rename a remote' ' git clone one four && ( cd four && + git config branch.master.pushRemote origin && git remote rename origin upstream && test -z "$(git for-each-ref 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 "$(git config branch.master.remote)" = "upstream" && + test "$(git config branch.master.pushRemote)" = "upstream" ) ' @@ -784,6 +786,18 @@ test_expect_success 'rename succeeds with existing remote..prune' ' git -C four.four remote rename origin upstream ' +test_expect_success 'remove a remote' ' + git clone one four.five && + ( + cd four.five && + git config branch.master.pushRemote origin && + git remote remove origin && + test -z "$(git for-each-ref refs/remotes/origin)" && + test_must_fail git config branch.master.remote && + test_must_fail git config branch.master.pushRemote + ) +' + cat >remotes_origin < Date: Sat, 1 Feb 2020 10:34:09 +0100 Subject: remote rename/remove: gently handle remote.pushDefault config When renaming a remote with git remote rename X Y git remote remove X Git already renames or removes any branch..remote and branch..pushRemote configurations if their value is X. However remote.pushDefault needs a more gentle approach, as this may be set in a non-repo configuration file. In such a case only a warning is printed, such as: warning: The global configuration remote.pushDefault in: $HOME/.gitconfig:35 now names the non-existent remote origin It is changed to remote.pushDefault = Y or removed when set in a repo configuration though. Signed-off-by: Bert Wesarg Signed-off-by: Junio C Hamano --- builtin/remote.c | 55 ++++++++++++++++++++++++++++++++++++++++ t/t5505-remote.sh | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 129 insertions(+), 2 deletions(-) (limited to 'builtin') diff --git a/builtin/remote.c b/builtin/remote.c index a2379a14bf..555d4c896c 100644 --- a/builtin/remote.c +++ b/builtin/remote.c @@ -615,6 +615,56 @@ static int migrate_file(struct remote *remote) return 0; } +struct push_default_info +{ + const char *old_name; + enum config_scope scope; + struct strbuf origin; + int linenr; +}; + +static int config_read_push_default(const char *key, const char *value, + void *cb) +{ + struct push_default_info* info = cb; + if (strcmp(key, "remote.pushdefault") || + !value || strcmp(value, info->old_name)) + return 0; + + info->scope = current_config_scope(); + strbuf_reset(&info->origin); + strbuf_addstr(&info->origin, current_config_name()); + info->linenr = current_config_line(); + + return 0; +} + +static void handle_push_default(const char* old_name, const char* new_name) +{ + struct push_default_info push_default = { + old_name, CONFIG_SCOPE_UNKNOWN, STRBUF_INIT, -1 }; + git_config(config_read_push_default, &push_default); + if (push_default.scope >= CONFIG_SCOPE_COMMAND) + ; /* pass */ + else if (push_default.scope >= CONFIG_SCOPE_LOCAL) { + int result = git_config_set_gently("remote.pushDefault", + new_name); + if (new_name && result && result != CONFIG_NOTHING_SET) + die(_("could not set '%s'"), "remote.pushDefault"); + else if (!new_name && result && result != CONFIG_NOTHING_SET) + die(_("could not unset '%s'"), "remote.pushDefault"); + } else if (push_default.scope >= CONFIG_SCOPE_SYSTEM) { + /* warn */ + warning(_("The %s configuration remote.pushDefault in:\n" + "\t%s:%d\n" + "now names the non-existent remote '%s'"), + config_scope_name(push_default.scope), + push_default.origin.buf, push_default.linenr, + old_name); + } +} + + static int mv(int argc, const char **argv) { struct option options[] = { @@ -750,6 +800,9 @@ static int mv(int argc, const char **argv) die(_("creating '%s' failed"), buf.buf); } string_list_clear(&remote_branches, 1); + + handle_push_default(rename.old_name, rename.new_name); + return 0; } @@ -835,6 +888,8 @@ static int rm(int argc, const char **argv) strbuf_addf(&buf, "remote.%s", remote->name); if (git_config_rename_section(buf.buf, NULL) < 1) return error(_("Could not remove config section '%s'"), buf.buf); + + handle_push_default(remote->name, NULL); } return result; diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh index 082042b05a..dda81b7d07 100755 --- a/t/t5505-remote.sh +++ b/t/t5505-remote.sh @@ -734,6 +734,7 @@ test_expect_success 'reject adding remote with an invalid name' ' # the last two ones check if the config is updated. test_expect_success 'rename a remote' ' + test_config_global remote.pushDefault origin && git clone one four && ( cd four && @@ -744,7 +745,42 @@ test_expect_success 'rename a remote' ' 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 "$(git config branch.master.pushRemote)" = "upstream" + test "$(git config branch.master.pushRemote)" = "upstream" && + test "$(git config --global remote.pushDefault)" = "origin" + ) +' + +test_expect_success 'rename a remote renames repo remote.pushDefault' ' + git clone one four.1 && + ( + cd four.1 && + git config remote.pushDefault origin && + git remote rename origin upstream && + test "$(git config --local remote.pushDefault)" = "upstream" + ) +' + +test_expect_success 'rename a remote renames repo remote.pushDefault but ignores global' ' + test_config_global remote.pushDefault other && + git clone one four.2 && + ( + cd four.2 && + git config remote.pushDefault origin && + git remote rename origin upstream && + test "$(git config --global remote.pushDefault)" = "other" && + test "$(git config --local remote.pushDefault)" = "upstream" + ) +' + +test_expect_success 'rename a remote renames repo remote.pushDefault but keeps global' ' + test_config_global remote.pushDefault origin && + git clone one four.3 && + ( + cd four.3 && + git config remote.pushDefault origin && + git remote rename origin upstream && + test "$(git config --global remote.pushDefault)" = "origin" && + test "$(git config --local remote.pushDefault)" = "upstream" ) ' @@ -787,6 +823,7 @@ test_expect_success 'rename succeeds with existing remote..prune' ' ' test_expect_success 'remove a remote' ' + test_config_global remote.pushDefault origin && git clone one four.five && ( cd four.five && @@ -794,7 +831,42 @@ test_expect_success 'remove a remote' ' git remote remove origin && test -z "$(git for-each-ref refs/remotes/origin)" && test_must_fail git config branch.master.remote && - test_must_fail git config branch.master.pushRemote + test_must_fail git config branch.master.pushRemote && + test "$(git config --global remote.pushDefault)" = "origin" + ) +' + +test_expect_success 'remove a remote removes repo remote.pushDefault' ' + git clone one four.five.1 && + ( + cd four.five.1 && + git config remote.pushDefault origin && + git remote remove origin && + test_must_fail git config --local remote.pushDefault + ) +' + +test_expect_success 'remove a remote removes repo remote.pushDefault but ignores global' ' + test_config_global remote.pushDefault other && + git clone one four.five.2 && + ( + cd four.five.2 && + git config remote.pushDefault origin && + git remote remove origin && + test "$(git config --global remote.pushDefault)" = "other" && + test_must_fail git config --local remote.pushDefault + ) +' + +test_expect_success 'remove a remote removes repo remote.pushDefault but keeps global' ' + test_config_global remote.pushDefault origin && + git clone one four.five.3 && + ( + cd four.five.3 && + git config remote.pushDefault origin && + git remote remove origin && + test "$(git config --global remote.pushDefault)" = "origin" && + test_must_fail git config --local remote.pushDefault ) ' -- cgit v1.3