remote rename/remove: improve handling of configuration values
Subsystems (2)
v3 cover remote rename/remove: improve handling of configuration values
While fixing that 'git remote rename X Y' does not rename the values for 'branch.*.pushRemote', it opened the possibility to more improvements in this area:
- 'remote rename' did not accept single-letter abbreviations for 'branch.*.rebase' like 'pull --rebase' does
- minor clean-ups the config callback
- patch 5 will be replaced by/rebased on Matthew's work in 'config: allow user to know scope of config options', once 'config_scope_name' is available
- gently handling the rename of 'remote.pushDefault'
Changes since v3:
* rebasd onto mr/show-config-scope (feaf5eea41)
Changes since v1:
* avoid mixed declarations and statements
* 'git remote remove' learned similar treatment
Bert Wesarg (6):
pull --rebase/remote rename: document and honor single-letter
abbreviations rebase types
remote: clean-up by returning early to avoid one indentation
remote: clean-up config callback
remote rename/remove: handle branch.<name>.pushRemote config values
config: provide access to the current line number
remote rename/remove: gently handle remote.pushDefault config
Documentation/config/branch.txt | 7 +- Documentation/config/pull.txt | 7 +- Makefile | 1 + builtin/pull.c | 29 +---- builtin/remote.c | 188 ++++++++++++++++++++++---------- config.c | 8 ++ config.h | 1 + rebase.c | 35 ++++++ rebase.h | 15 +++ t/helper/test-config.c | 1 + t/t1308-config-set.sh | 14 ++- t/t5505-remote.sh | 88 ++++++++++++++- 12 files changed, 304 insertions(+), 90 deletions(-) create mode 100644 rebase.c create mode 100644 rebase.h
Signature · 1 line
2.24.1.497.g9abd7b20b4.dirty
PATCH 1/6 pull --rebase/remote rename: document and honor single-letter abbreviations rebase types
When 46af44b07d (pull --rebase=<type>: allow single-letter abbreviations for the type, 2018-08-04) landed in Git, it had the side effect that not only 'pull --rebase=<type>' accepted the single-letter abbreviations but also the 'pull.rebase' and 'branch.<name>.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
---
Cc: Johannes Schindelin <[email protected]>
Cc: Junio C Hamano <[email protected]>
changes since v1:
* Add comment that 'git remote rename' considers unknown `.rebase' values as false
--- Documentation/config/branch.txt | 7 ++++--- Documentation/config/pull.txt | 7 ++++--- Makefile | 1 + builtin/pull.c | 29 ++++----------------------- builtin/remote.c | 30 +++++++++++----------------- rebase.c | 35 +++++++++++++++++++++++++++++++++ rebase.h | 15 ++++++++++++++ 7 files changed, 75 insertions(+), 49 deletions(-) create mode 100644 rebase.c create mode 100644 rebase.h
Documentation/config/branch.txt +4 -3
@@ -81,15 +81,16 @@ branch.<name>.rebase:: "git pull" is run. See "pull.rebase" for doing this in a non branch-specific manner. + -When `merges`, pass the `--rebase-merges` option to 'git rebase' +When `merges` (or just 'm'), pass the `--rebase-merges` option to 'git rebase' so that the local merge commits are included in the rebase (see linkgit:git-rebase[1] for details). + -When `preserve` (deprecated in favor of `merges`), also pass +When `preserve` (or just 'p', deprecated in favor of `merges`), also pass `--preserve-merges` along to 'git rebase' so that locally committed merge commits will not be flattened by running 'git pull'. + -When the value is `interactive`, the rebase is run in interactive mode. +When the value is `interactive` (or just 'i'), the rebase is run in interactive +mode. + *NOTE*: this is a possibly dangerous operation; do *not* use it unless you understand the implications (see linkgit:git-rebase[1]
Documentation/config/pull.txt +4 -3
@@ -14,15 +14,16 @@ pull.rebase:: pull" is run. See "branch.<name>.rebase" for setting this on a per-branch basis. + -When `merges`, pass the `--rebase-merges` option to 'git rebase' +When `merges` (or just 'm'), pass the `--rebase-merges` option to 'git rebase' so that the local merge commits are included in the rebase (see linkgit:git-rebase[1] for details). + -When `preserve` (deprecated in favor of `merges`), also pass +When `preserve` (or just 'p', deprecated in favor of `merges`), also pass `--preserve-merges` along to 'git rebase' so that locally committed merge commits will not be flattened by running 'git pull'. + -When the value is `interactive`, the rebase is run in interactive mode. +When the value is `interactive` (or just 'i'), the rebase is run in interactive +mode. + *NOTE*: this is a possibly dangerous operation; do *not* use it unless you understand the implications (see linkgit:git-rebase[1]
Makefile +1 -0
@@ -954,6 +954,7 @@ LIB_OBJS += quote.o LIB_OBJS += range-diff.o LIB_OBJS += reachable.o LIB_OBJS += read-cache.o +LIB_OBJS += rebase.o LIB_OBJS += rebase-interactive.o LIB_OBJS += reflog-walk.o LIB_OBJS += refs.o
builtin/pull.c +4 -25
@@ -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);
builtin/remote.c +12 -18
@@ -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 *cbreturn 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 "
rebase.c new file +35 -0
@@ -0,0 +1,35 @@ +#include "rebase.h" +#include "config.h" + +/* +* Parses textual value for pull.rebase, branch.<name>.rebase, etc. +* Unrecognised value yields REBASE_INVALID, which traditionally is +* treated the same way as REBASE_FALSE. +* +* The callers that care if (any) rebase is requested should say +* if (REBASE_TRUE <= rebase_parse_value(string)) +* +* The callers that want to differenciate an unrecognised value and +* false can do so by treating _INVALID and _FALSE differently. +*/ +enum rebase_type rebase_parse_value (const char *value) +{ +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. +*/ + +return REBASE_INVALID ; +}
rebase.h new file +15 -0
@@ -0,0 +1,15 @@ +#ifndef REBASE_H +#define REBASE_H + +enum rebase_type { +REBASE_INVALID =-1 , +REBASE_FALSE =0 , +REBASE_TRUE , +REBASE_PRESERVE , +REBASE_MERGES , +REBASE_INTERACTIVE +}; + +enum rebase_type rebase_parse_value (const char *value); + +#endif /* REBASE */
Signature · 1 line
2.24.1.497.g9abd7b20b4.dirty
PATCH v2 2/6 remote: clean-up by returning early to avoid one indentation
In reply to Bert Wesarg---
Cc: Junio C Hamano <[email protected]>
Changes since v1:
* avoid mixed declarations and code statements
--- builtin/remote.c | 94 ++++++++++++++++++++++++------------------------ 1 file changed, 48 insertions(+), 46 deletions(-)
builtin/remote.c +48 -46
@@ -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 ; }
Signature · 1 line
2.24.1.497.g9abd7b20b4.dirty
PATCH 3/6 remote: clean-up config callback
In reply to Bert WesargSome 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()
---
Cc: Johannes Schindelin <[email protected]>
--- builtin/remote.c | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-)
builtin/remote.c +17 -11
@@ -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 ; }
Signature · 1 line
2.24.1.497.g9abd7b20b4.dirty
PATCH v4 4/6 remote rename/remove: handle branch.<name>.pushRemote config values
In reply to Bert WesargWhen renaming or removing a remote with
git remote rename X Y
git remote remove X
Git already renames/removes any config values from
branch.<name>.remote = X
to
branch.<name>.remote = Y
As branch.<name>.pushRemote also names a remote, it now also renames or removes these config values from
branch.<name>.pushRemote = X
to
branch.<name>.pushRemote = Y
---
Changes since v3:
* handle also 'git remote remove'
--- builtin/remote.c | 22 +++++++++++++++++++++- t/t5505-remote.sh | 16 +++++++++++++++- 2 files changed, 36 insertions(+), 2 deletions(-)
builtin/remote.c +21 -1
@@ -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); + } }/*
t/t5505-remote.sh +15 -1
@@ -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.<target>.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 <<EOF URL: $(pwd)/one Push: refs/heads/master:refs/heads/upstream
Signature · 1 line
2.24.1.497.g9abd7b20b4.dirty
PATCH 5/6 config: provide access to the current line number
In reply to Bert WesargUsers are nowadays trained to see message from CLI tools in the form
<file>:<lno>: …
To be able to give such messages when notifying the user about configurations in any config file, it is currently only possible to get the file name (if the value originates from a file to begin with) via current_config_name(). Now it is also possible to query the current line number for the configuration.
--- config.c | 8 ++++++++ config.h | 1 + t/helper/test-config.c | 1 + t/t1308-config-set.sh | 14 ++++++++++++-- 4 files changed, 22 insertions(+), 2 deletions(-)
config.c +8 -0
@@ -3338,6 +3338,14 @@ enum config_scope current_config_scope(void)return current_parsing_scope; } +int current_config_line (void ) +{ +if (current_config_kvi) +return current_config_kvi->linenr; +else +return cf->linenr; +} +int lookup_config (const char **mapping,int nr_mapping,const char *var) {int i;
config.h +1 -0
@@ -309,6 +309,7 @@ int git_config_parse_parameter(const char *, config_fn_t fn, void *data);enum config_scope current_config_scope (void );const char *current_config_origin_type (void );const char *current_config_name (void ); +int current_config_line (void );/** * Include Directives
t/helper/test-config.c +1 -0
@@ -48,6 +48,7 @@ static int iterate_cb(const char *var, const char *value, void *data)printf ("value=%s\n" , value ? value :"(null)" );printf ("origin=%s\n" ,current_config_origin_type ());printf ("name=%s\n" ,current_config_name ()); +printf ("lno=%d\n" ,current_config_line ());printf ("scope=%s\n" ,config_scope_name (current_config_scope ()));return 0 ;
t/t1308-config-set.sh +12 -2
@@ -238,8 +238,8 @@ test_expect_success 'error on modifying repo config without repo' ' cmdline_config="'foo.bar=from-cmdline'" test_expect_success 'iteration shows correct origins' ' - echo "[foo]bar = from-repo" >.git/config && - echo "[foo]bar = from-home" >.gitconfig && + printf "[ignore]\n\tthis = please\n[foo]bar = from-repo\n" >.git/config && + printf "[foo]\n\tbar = from-home\n" >.gitconfig && if test_have_prereq MINGW then # Use Windows path (i.e. *not* $HOME)
@@ -253,18 +253,28 @@ test_expect_success 'iteration shows correct origins' ' value=from-home origin=file name=$HOME_GITCONFIG + lno=2 scope=global + key=ignore.this + value=please + origin=file + name=.git/config + lno=2 + scope=local + key=foo.bar value=from-repo origin=file name=.git/config + lno=3 scope=local key=foo.bar value=from-cmdline origin=command line name= + lno=-1 scope=command EOF GIT_CONFIG_PARAMETERS=$cmdline_config test-tool config iterate >actual &&
Signature · 1 line
2.24.1.497.g9abd7b20b4.dirty
PATCH v2 6/6 remote rename/remove: gently handle remote.pushDefault config
In reply to Bert WesargWhen renaming a remote with
git remote rename X Y
git remote remove X
Git already renames or removes any branch.<name>.remote and branch.<name>.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.
---
Matthew, you are in Cc because of your current work 'config: allow user to know scope of config options'. I think I'm correct to assuming an ordering of the enum config_scope.
Changes since v1:
* handle also 'git remote remove'
--- builtin/remote.c | 54 +++++++++++++++++++++++++++++++++ t/t5505-remote.sh | 76 +++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 128 insertions(+), 2 deletions(-)
builtin/remote.c +54 -0
@@ -615,6 +615,55 @@ 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" ) ||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_CMDLINE ) + ;/* pass */ +else if (push_default.scope >=CONFIG_SCOPE_REPO ) { +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 +799,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 +887,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;
t/t5505-remote.sh +74 -2
98 lines not on this page · raw message
Signature · 1 line
2.24.1.497.g9abd7b20b4.dirty
PATCH v3 6/6 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.<name>.remote and branch.<name>.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.
---
Sorry, <029dd071038db1daf590d5224076c089cfd060bc.1580108477.git.bert.wesarg@googlemail.com> missed the ammend to let it work with mr/show-config-scope. Please drop that in favor of this one.
Changes since v1:
* handle also 'git remote remove'
--- builtin/remote.c | 54 +++++++++++++++++++++++++++++++++ t/t5505-remote.sh | 76 +++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 128 insertions(+), 2 deletions(-)
builtin/remote.c +54 -0
@@ -615,6 +615,55 @@ 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" ) ||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 +799,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 +887,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;
t/t5505-remote.sh +74 -2
98 lines not on this page · raw message
Signature · 1 line
2.24.1.497.g9abd7b20b4.dirty
Re: [PATCH v3 6/6] remote rename/remove: gently handle remote.pushDefault config
t/t5505-remote.sh test_expect_success 'rename a remote' ' · line 743
Quoted 12 lines · Bert Wesarg
t/t5505-remote.sh +1
-0@@ -737,6 +737,7 @@ test_expect_success 'rename a remote' ' git clone one four && ( cd four && + test_config_global remote.pushDefault origin && git config branch.master.pushRemote origin && git remote rename origin upstream && test -z "$(git for-each-ref refs/remotes/origin)" &&
You cannot use test_config and test_config_global inside a subshell, as they rely on test_when_finished to rewind their effect, which cannot be used inside a subshell. As you are doing "global" config, there is no reason to make "git config --global" call in a particular repository anyway, so just do this upfront as the first thing in the test sequence. There are a few others in this file.
Tentatively I applied the following fix-up on top of the series to unblock tonight's integration cycle.
Thanks.
t/t5505-remote.sh | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
t/t5505-remote.sh +6 -6
@@ -734,10 +734,10 @@ 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 && - test_config_global remote.pushDefault origin && git config branch.master.pushRemote origin && git remote rename origin upstream && test -z "$(git for-each-ref refs/remotes/origin)" &&
@@ -761,10 +761,10 @@ test_expect_success 'rename a remote renames repo remote.pushDefault' ' ' 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 && - test_config_global remote.pushDefault other && git config remote.pushDefault origin && git remote rename origin upstream && test "$(git config --global remote.pushDefault)" = "other" &&
@@ -773,10 +773,10 @@ test_expect_success 'rename a remote renames repo remote.pushDefault but ignores ' 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 && - test_config_global remote.pushDefault origin && git config remote.pushDefault origin && git remote rename origin upstream && test "$(git config --global remote.pushDefault)" = "origin" &&
@@ -823,10 +823,10 @@ test_expect_success 'rename succeeds with existing remote.<target>.prune' ' ' test_expect_success 'remove a remote' ' + test_config_global remote.pushDefault origin && git clone one four.five && ( cd four.five && - test_config_global remote.pushDefault origin && git config branch.master.pushRemote origin && git remote remove origin && test -z "$(git for-each-ref refs/remotes/origin)" &&
@@ -847,10 +847,10 @@ test_expect_success 'remove a remote removes repo 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 && - test_config_global remote.pushDefault other && git config remote.pushDefault origin && git remote remove origin && test "$(git config --global remote.pushDefault)" = "other" &&
@@ -859,10 +859,10 @@ test_expect_success 'remove a remote removes repo remote.pushDefault but ignores ' 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 && - test_config_global remote.pushDefault origin && git config remote.pushDefault origin && git remote remove origin && test "$(git config --global remote.pushDefault)" = "origin" &&
Re: [PATCH v3 6/6] remote rename/remove: gently handle remote.pushDefault config
Quoted 23 lines · Junio C Hamano
t/t5505-remote.sh test_expect_success 'rename a remote' ' · line 743
Quoted 12 lines · Bert Wesarg
t/t5505-remote.sh +1
-0@@ -737,6 +737,7 @@ test_expect_success 'rename a remote' ' git clone one four && ( cd four && + test_config_global remote.pushDefault origin && git config branch.master.pushRemote origin && git remote rename origin upstream && test -z "$(git for-each-ref refs/remotes/origin)" &&You cannot use
test_configandtest_config_globalinside a subshell, as they rely ontest_when_finishedto rewind their effect, which cannot be used inside a subshell. As you are doing "global" config, there is no reason to make "git config --global" call in a particular repository anyway, so just do this upfront as the first thing in the test sequence. There are a few others in this file.
Quoted 4 lines
Tentatively I applied the following fix-up on top of the series to unblock tonight's integration cycle.
Thanks.
Thanks. Will squash them and re-roll.
Bert
Quoted 80 lines
t/t5505-remote.sh | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)t/t5505-remote.sh +6
-6@@ -734,10 +734,10 @@ 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 && - test_config_global remote.pushDefault origin && git config branch.master.pushRemote origin && git remote rename origin upstream && test -z "$(git for-each-ref refs/remotes/origin)" &&@@ -761,10 +761,10 @@ test_expect_success 'rename a remote renames repo remote.pushDefault' ' ' 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 && - test_config_global remote.pushDefault other && git config remote.pushDefault origin && git remote rename origin upstream && test "$(git config --global remote.pushDefault)" = "other" &&@@ -773,10 +773,10 @@ test_expect_success 'rename a remote renames repo remote.pushDefault but ignores ' 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 && - test_config_global remote.pushDefault origin && git config remote.pushDefault origin && git remote rename origin upstream && test "$(git config --global remote.pushDefault)" = "origin" &&@@ -823,10 +823,10 @@ test_expect_success 'rename succeeds with existing remote.<target>.prune' ' ' test_expect_success 'remove a remote' ' + test_config_global remote.pushDefault origin && git clone one four.five && ( cd four.five && - test_config_global remote.pushDefault origin && git config branch.master.pushRemote origin && git remote remove origin && test -z "$(git for-each-ref refs/remotes/origin)" &&@@ -847,10 +847,10 @@ test_expect_success 'remove a remote removes repo 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 && - test_config_global remote.pushDefault other && git config remote.pushDefault origin && git remote remove origin && test "$(git config --global remote.pushDefault)" = "other" &&@@ -859,10 +859,10 @@ test_expect_success 'remove a remote removes repo remote.pushDefault but ignores ' 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 && - test_config_global remote.pushDefault origin && git config remote.pushDefault origin && git remote remove origin && test "$(git config --global remote.pushDefault)" = "origin" &&