remote rename/remove: improve handling of configuration values
Subsystems (2)
v2 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 v1:
* avoid mixed declarations and statements
* 'git remote remove' learned similar treatment
Bert Wesarg (7):
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 [RFC] config: make scope_name global as config_scope_name
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 | 24 ++++ config.h | 2 + rebase.c | 35 ++++++ rebase.h | 15 +++ t/helper/test-config.c | 18 +-- t/t1308-config-set.sh | 14 ++- t/t5505-remote.sh | 88 ++++++++++++++- 12 files changed, 322 insertions(+), 106 deletions(-) create mode 100644 rebase.c create mode 100644 rebase.h
Signature · 1 line
2.24.1.497.g9abd7b20b4.dirty
PATCH v2 1/7 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
---
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/7 remote: clean-up by returning early to avoid one indentation
In reply to Bert Wesarg---
Changes since v1:
* avoid mixed declarations and 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 v2 3/7 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
RFC PATCH v2 5/7 config: make `scope_name` global as `config_scope_name`
In reply to Bert Wesarg---
Will be replaced by Matthew Rogers.
--- config.c | 16 ++++++++++++++++ config.h | 1 + t/helper/test-config.c | 17 +---------------- 3 files changed, 18 insertions(+), 16 deletions(-)
config.c +16 -0
@@ -3317,6 +3317,22 @@ enum config_scope current_config_scope(void)return current_parsing_scope; } +const char *config_scope_name (enum config_scope scope) +{ +switch (scope) { +case CONFIG_SCOPE_SYSTEM : +return "system" ; +case CONFIG_SCOPE_GLOBAL : +return "global" ; +case CONFIG_SCOPE_REPO : +return "repo" ; +case CONFIG_SCOPE_CMDLINE : +return "cmdline" ; +default : +return "unknown" ; + } +} +int lookup_config (const char **mapping,int nr_mapping,const char *var) {int i;
config.h +1 -0
@@ -301,6 +301,7 @@ enum config_scope {CONFIG_SCOPE_REPO , CONFIG_SCOPE_CMDLINE, }; +const char *config_scope_name (enum config_scope scope);enum config_scope current_config_scope (void );const char *current_config_origin_type (void );
t/helper/test-config.c +1 -16
@@ -37,21 +37,6 @@* */ -static const char *scope_name (enum config_scope scope) -{ -switch (scope) { -case CONFIG_SCOPE_SYSTEM : -return "system" ; -case CONFIG_SCOPE_GLOBAL : -return "global" ; -case CONFIG_SCOPE_REPO : -return "repo" ; -case CONFIG_SCOPE_CMDLINE : -return "cmdline" ; -default : -return "unknown" ; - } -}static int iterate_cb (const char *var,const char *value,void *data) {static int nr;
@@ -63,7 +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 ("scope=%s\n" ,scope_name (current_config_scope ())); +printf ("scope=%s\n" ,config_scope_name (current_config_scope ()));return 0 ; }
Signature · 1 line
2.24.1.497.g9abd7b20b4.dirty
PATCH v4 4/7 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
Re: [PATCH v4 4/7] remote rename/remove: handle branch.<name>.pushRemote config values
Hi Bert,
Quoted 18 lines · Bert Wesarg
When renaming or removing a remote with
git remote rename X Y
git remote remove XGit 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
Quoted 7 lines
to
branch.<name>.pushRemote = Y
Signed-off-by Bert Wesarg <[email protected]>---
This commit seems to cause a failure in t5505:
https://dev.azure.com/gitgitgadget/git/_build/results?buildId=27833&view=ms.vss-test-web.build-test-results-tab
Here is the excerpt of the log:
-- snip --
[...]
expecting success of 5505.15 'show':
( cd test && git config --add remote.origin.fetch refs/heads/master:refs/heads/upstream && git fetch && git checkout -b ahead origin/master && echo 1 >>file && test_tick && git commit -m update file && git checkout master && git branch --track octopus origin/master && git branch --track rebase origin/master && git branch -d -r origin/master && git config --add remote.two.url ../two && git config --add remote.two.pushurl ../three && git config branch.rebase.rebase true && git config branch.octopus.merge "topic-a topic-b topic-c" && ( cd ../one && echo 1 >file && test_tick && git commit -m update file ) && git config --add remote.origin.push : && git config --add remote.origin.push refs/heads/master:refs/heads/upstream && git config --add remote.origin.push +refs/tags/lastbackup && git config --add remote.two.push +refs/heads/ahead:refs/heads/master && git config --add remote.two.push refs/heads/master:refs/heads/another && git remote show origin two >output && git branch -d rebase octopus && test_i18ncmp expect output )
+ cd test
+ git config --add remote.origin.fetch refs/heads/master:refs/heads/upstream
+ git fetch
From /home/virtualbox/git/t/trash directory.t5505-remote/one
* [new branch] master -> upstream
+ git checkout -b ahead origin/master
Switched to a new branch 'ahead'
Branch 'ahead' set up to track remote branch 'master' from 'origin'.
+ echo 1
+ test_tick
+ test -z
+ test_tick=1112911993
+ GIT_COMMITTER_DATE=1112911993 -0700
+ GIT_AUTHOR_DATE=1112911993 -0700
+ export GIT_COMMITTER_DATE GIT_AUTHOR_DATE
+ git commit -m update file
[ahead 847549e] update
Author: A U Thor <[email protected]>
1 file changed, 1 insertion(+)
+ git checkout master
Switched to branch 'master'
Your branch is up to date with 'origin/master'.
+ git branch --track octopus origin/master
Branch 'octopus' set up to track remote branch 'master' from 'origin'.
+ git branch --track rebase origin/master
Branch 'rebase' set up to track remote branch 'master' from 'origin'.
+ git branch -d -r origin/master
Deleted remote-tracking branch origin/master (was 9d34b14).
+ git config --add remote.two.url ../two
+ git config --add remote.two.pushurl ../three
+ git config branch.rebase.rebase true
+ git config branch.octopus.merge topic-a topic-b topic-c
+ cd ../one
+ echo 1
+ test_tick
+ test -z set
+ test_tick=1112912053
+ GIT_COMMITTER_DATE=1112912053 -0700
+ GIT_AUTHOR_DATE=1112912053 -0700
+ export GIT_COMMITTER_DATE GIT_AUTHOR_DATE
+ git commit -m update file
[master 6329a3c] update
Author: A U Thor <[email protected]>
1 file changed, 1 insertion(+)
+ git config --add remote.origin.push :
+ git config --add remote.origin.push refs/heads/master:refs/heads/upstream
+ git config --add remote.origin.push +refs/tags/lastbackup
+ git config --add remote.two.push +refs/heads/ahead:refs/heads/master
+ git config --add remote.two.push refs/heads/master:refs/heads/another
+ git remote show origin two
error: src refspec refs/tags/lastbackup does not match any
+ git branch -d rebase octopus
Deleted branch rebase (was 9d34b14).
Deleted branch octopus (was 9d34b14).
+ test_i18ncmp expect output
+ test_have_prereq C_LOCALE_OUTPUT
+ save_IFS=
+ IFS=,
+ set -- C_LOCALE_OUTPUT
+ IFS=
+ total_prereq=0
+ ok_prereq=0
+ missing_prereq=
+ negative_prereq=
+ total_prereq=1
+ satisfied_this_prereq=t
+ ok_prereq=1
+ test 1 = 1
+ test_cmp expect output
+ diff -u expect output
output renamed from expect +0 -7
@@ -5,13 +5,6 @@ Remote branches: master new (next fetch will store in remotes/origin) side tracked - Local branches configured for 'git pull': - ahead merges with remote master - master merges with remote master - octopus merges with remote topic-a - and with remote topic-b - and with remote topic-c - rebase rebases onto remote master Local refs configured for 'git push': master pushes to master (local out of date) master pushes to upstream (create)
error: last command exited with $?=1 not ok 15 - show
#
# (
# cd test &&
# git config --add remote.origin.fetch refs/heads/master:refs/heads/upstream &&
# git fetch &&
# git checkout -b ahead origin/master &&
# echo 1 >>file &&
# test_tick &&
# git commit -m update file &&
# git checkout master &&
# git branch --track octopus origin/master &&
# git branch --track rebase origin/master &&
# git branch -d -r origin/master &&
# git config --add remote.two.url ../two &&
# git config --add remote.two.pushurl ../three &&
# git config branch.rebase.rebase true &&
# git config branch.octopus.merge "topic-a topic-b topic-c" &&
# (
# cd ../one &&
# echo 1 >file &&
# test_tick &&
# git commit -m update file
# ) &&
# git config --add remote.origin.push : &&
# git config --add remote.origin.push refs/heads/master:refs/heads/upstream &&
# git config --add remote.origin.push +refs/tags/lastbackup &&
# git config --add remote.two.push +refs/heads/ahead:refs/heads/master &&
# git config --add remote.two.push refs/heads/master:refs/heads/another &&
# git remote show origin two >output &&
# git branch -d rebase octopus &&
# test_i18ncmp expect output
# )
#
-- snap --
Could you have a look to see whether the code or the test need to be adjusted?
Thanks,
Dscho
Quoted 123 lines
Changes since v3:
* handle also 'git remote remove'
Cc Junio C Hamano <[email protected]>Cc Johannes Schindelin <[email protected]>--- 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
-2@@ -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 --2.24.1.497.g9abd7b20b4.dirty
Re: [PATCH v4 4/7] remote rename/remove: handle branch.<name>.pushRemote config values
Dear Dscho,
Quoted 195 lines · Johannes Schindelin
Hi Bert,
Quoted 18 lines · Bert Wesarg
When renaming or removing a remote with
git remote rename X Y
git remote remove XGit 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
Quoted 7 lines
to
branch.<name>.pushRemote = Y
Signed-off-by Bert Wesarg <[email protected]>---
This commit seems to cause a failure in t5505:
https://dev.azure.com/gitgitgadget/git/_build/results?buildId=27833&view=ms.vss-test-web.build-test-results-tabHere is the excerpt of the log:
-- snip --
[...]
expecting success of 5505.15 'show':( cd test && git config --add remote.origin.fetch refs/heads/master:refs/heads/upstream && git fetch && git checkout -b ahead origin/master && echo 1 >>file && test_tick && git commit -m update file && git checkout master && git branch --track octopus origin/master && git branch --track rebase origin/master && git branch -d -r origin/master && git config --add remote.two.url ../two && git config --add remote.two.pushurl ../three && git config branch.rebase.rebase true && git config branch.octopus.merge "topic-a topic-b topic-c" && ( cd ../one && echo 1 >file && test_tick && git commit -m update file ) && git config --add remote.origin.push : && git config --add remote.origin.push refs/heads/master:refs/heads/upstream && git config --add remote.origin.push +refs/tags/lastbackup && git config --add remote.two.push +refs/heads/ahead:refs/heads/master && git config --add remote.two.push refs/heads/master:refs/heads/another && git remote show origin two >output && git branch -d rebase octopus && test_i18ncmp expect output )+ cd test
+ git config --add remote.origin.fetch refs/heads/master:refs/heads/upstream
+ git fetch
From/home/virtualbox/git/t/trashdirectory.t5505-remote/one
* [new branch] master -> upstream
+ git checkout -b ahead origin/master
Switched to a new branch 'ahead'
Branch 'ahead' set up to track remote branch 'master' from 'origin'.
+ echo 1
+test_tick
+ test -z
+ test_tick=1112911993
+ GIT_COMMITTER_DATE=1112911993 -0700
+ GIT_AUTHOR_DATE=1112911993 -0700
+ exportGIT_COMMITTER_DATEGIT_AUTHOR_DATE
+ git commit -m update file
[ahead 847549e] update
Author: A U Thor <[email protected]>1 file changed, 1 insertion(+)+ git checkout master
Switched to branch 'master'
Your branch is up to date with 'origin/master'.
+ git branch --track octopus origin/master
Branch 'octopus' set up to track remote branch 'master' from 'origin'.
+ git branch --track rebase origin/master
Branch 'rebase' set up to track remote branch 'master' from 'origin'.
+ git branch -d -r origin/master
Deleted remote-tracking branch origin/master (was 9d34b14).
+ git config --add remote.two.url ../two
+ git config --add remote.two.pushurl ../three
+ git config branch.rebase.rebase true
+ git config branch.octopus.merge topic-a topic-b topic-c
+ cd ../one
+ echo 1
+test_tick
+ test -z set
+ test_tick=1112912053
+ GIT_COMMITTER_DATE=1112912053 -0700
+ GIT_AUTHOR_DATE=1112912053 -0700
+ exportGIT_COMMITTER_DATEGIT_AUTHOR_DATE
+ git commit -m update file
[master 6329a3c] update
Author: A U Thor <[email protected]>1 file changed, 1 insertion(+)+ git config --add remote.origin.push :
+ git config --add remote.origin.push refs/heads/master:refs/heads/upstream
+ git config --add remote.origin.push +refs/tags/lastbackup
+ git config --add remote.two.push +refs/heads/ahead:refs/heads/master
+ git config --add remote.two.push refs/heads/master:refs/heads/another
+ git remote show origin two
error: src refspec refs/tags/lastbackup does not match any
+ git branch -d rebase octopus
Deleted branch rebase (was 9d34b14).
Deleted branch octopus (was 9d34b14).
+test_i18ncmpexpect output
+test_have_prereqC_LOCALE_OUTPUT
+ save_IFS=+ IFS=,
+ set --C_LOCALE_OUTPUT
+ IFS=+ total_prereq=0
+ ok_prereq=0
+ missing_prereq=
+ negative_prereq=
+ total_prereq=1
+ satisfied_this_prereq=t
+ ok_prereq=1
+ test 1 = 1
+test_cmpexpect output
+ diff -u expect outputoutput renamed from expect +0
-7@@ -5,13 +5,6 @@ Remote branches: master new (next fetch will store in remotes/origin) side tracked - Local branches configured for 'git pull': - ahead merges with remote master - master merges with remote master - octopus merges with remote topic-a - and with remote topic-b - and with remote topic-c - rebase rebases onto remote master Local refs configured for 'git push': master pushes to master (local out of date) master pushes to upstream (create)error: last command exited with $?=1 not ok 15 - show
#
# (
# cd test &&
# git config --add remote.origin.fetch refs/heads/master:refs/heads/upstream &&
# git fetch &&
# git checkout -b ahead origin/master &&
# echo 1 >>file &&
#test_tick&&
# git commit -m update file &&
# git checkout master &&
# git branch --track octopus origin/master &&
# git branch --track rebase origin/master &&
# git branch -d -r origin/master &&
# git config --add remote.two.url ../two &&
# git config --add remote.two.pushurl ../three &&
# git config branch.rebase.rebase true &&
# git config branch.octopus.merge "topic-a topic-b topic-c" &&
# (
# cd ../one &&
# echo 1 >file &&
#test_tick&&
# git commit -m update file
# ) &&
# git config --add remote.origin.push : &&
# git config --add remote.origin.push refs/heads/master:refs/heads/upstream &&
# git config --add remote.origin.push +refs/tags/lastbackup &&
# git config --add remote.two.push +refs/heads/ahead:refs/heads/master &&
# git config --add remote.two.push refs/heads/master:refs/heads/another &&
# git remote show origin two >output &&
# git branch -d rebase octopus &&
#test_i18ncmpexpect output
# )
#
-- snap --
Quoted 2 lines
Could you have a look to see whether the code or the test need to be adjusted?
please ensure that you have v4 of this patch. What you see was a bug in v3.
Thanks.
Bert
Quoted 4 lines
Thanks,
Dscho
Re: [PATCH v4 4/7] remote rename/remove: handle branch.<name>.pushRemote config values
Hi Bert,
Quoted 34 lines · Bert Wesarg
Quoted 33 lines · Johannes Schindelin
Hi Bert,
Quoted 18 lines · Bert Wesarg
When renaming or removing a remote with
git remote rename X Y
git remote remove XGit 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
Quoted 7 lines
to
branch.<name>.pushRemote = Y
Signed-off-by Bert Wesarg <[email protected]>---
This commit seems to cause a failure in t5505:
https://dev.azure.com/gitgitgadget/git/_build/results?buildId=27833&view=ms.vss-test-web.build-test-results-tab
Quoted 8 lines
Quoted 6 lines
Here is the excerpt of the log:
[...]
Could you have a look to see whether the code or the test need to be adjusted?
please ensure that you have v4 of this patch. What you see was a bug in v3.
I was talking about the current state of what was merged into pu:
https://github.com/gitgitgadget/git/commit/6f032056fd7534b8efd712994c02531d83ada957
(note the red X indicating the build failure).
If you already fixed this, and Junio merely has to pick it up, all the better.
Thanks,
Dscho
PATCH v2 6/7 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
@@ -3333,6 +3333,14 @@ const char *config_scope_name(enum config_scope 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
@@ -306,6 +306,7 @@ const char *config_scope_name(enum config_scope scope);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=repo + key=foo.bar value=from-repo origin=file name=.git/config + lno=3 scope=repo key=foo.bar value=from-cmdline origin=command line name= + lno=-1 scope=cmdline EOF GIT_CONFIG_PARAMETERS=$cmdline_config test-tool config iterate >actual &&
Signature · 1 line
2.24.1.497.g9abd7b20b4.dirty
PATCH v2 7/7 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
All steps looked quite sensibly done.
Quoted 2 lines
- 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
I expect that Matthew's topic would become solid enough after one more reroll to name the function back to config_scope_name(); after that, let's drop the step and instead fork this topic off of Matthew's topic to queue the remaining patches on top.
Thanks.
Yeah, I just resubmitted so you should be good to go
Quoted 12 lines · Junio C Hamano
All steps looked quite sensibly done.
Quoted 2 lines
- 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 availableI expect that Matthew's topic would become solid enough after one more reroll to name the function back to
config_scope_name(); after that, let's drop the step and instead fork this topic off of Matthew's topic to queue the remaining patches on top.Thanks.
Signature · 1 line
Matthew Rogers
Junio,
Quoted 2 lines · Matt Rogers
Yeah, I just resubmitted so you should be good to go
I can resend the rebased topic at any time. And as I don't see that you picked up by latest re-roll (cover in [email protected]) in into bw/remote-rename-update-config yet, I think it makes sense to do this now.
Best,
Bert
Quoted 19 lines · Junio C Hamano
Quoted 6 lines
All steps looked quite sensibly done.
Quoted 2 lines
- 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 availableQuoted 6 lines
I expect that Matthew's topic would become solid enough after one more reroll to name the function back to
config_scope_name(); after that, let's drop the step and instead fork this topic off of Matthew's topic to queue the remaining patches on top.Thanks.
--
Matthew Rogers