* git config --get-urlmatch does not set exit code 1 when no match is found @ 2016-02-28 4:39 Guilherme 2016-02-28 10:45 ` John Keeping 2016-02-29 11:53 ` Jeff King 0 siblings, 2 replies; 11+ messages in thread From: Guilherme @ 2016-02-28 4:39 UTC (permalink / raw) To: git@vger•kernel.org Hello, My current woes are with multi-valued configuration values. More specifically credential.helper The documentation of git config says that when a value is not matched it should return 1. To reproduce make sure that credential.helper is not set. git config --get-urlmatch credential.helper http://somedomain:1234/ echo %ERRORLEVEL% 0 git config --get credential.helper echo %ERRORLEVEL% 1 git config --get credential.http://somedomain:1234/.helper echo %ERRORLEVEL% 1 The documentation says that for credential.helper is not found for a domain it should fall back to credential.helper if it is set. So I think that all those tests above should have returned 0. Am i right? Cheers. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: git config --get-urlmatch does not set exit code 1 when no match is found 2016-02-28 4:39 git config --get-urlmatch does not set exit code 1 when no match is found Guilherme @ 2016-02-28 10:45 ` John Keeping 2016-02-28 11:52 ` John Keeping 2016-02-28 11:54 ` [PATCH 0/3] " John Keeping 2016-02-29 11:53 ` Jeff King 1 sibling, 2 replies; 11+ messages in thread From: John Keeping @ 2016-02-28 10:45 UTC (permalink / raw) To: Guilherme; +Cc: git@vger•kernel.org On Sun, Feb 28, 2016 at 10:09:12AM +0530, Guilherme wrote: > My current woes are with multi-valued configuration values. More > specifically credential.helper > > The documentation of git config says that when a value is not matched > it should return 1. > > To reproduce make sure that credential.helper is not set. > > git config --get-urlmatch credential.helper http://somedomain:1234/ > echo %ERRORLEVEL% > 0 > > git config --get credential.helper > echo %ERRORLEVEL% > 1 > > git config --get credential.http://somedomain:1234/.helper > echo %ERRORLEVEL% > 1 > > The documentation says that for credential.helper is not found for a > domain it should fall back to credential.helper if it is set. So I > think that all those tests above should have returned 0. Am i right? It looks to me like a simple bug that --get-urlmatch doesn't return 1 if the key isn't found, but git-config(1) isn't entirely clear. The overall documentation on exit codes at the end of DESCRIPTION says that exit code 1 means: the section or key is invalid (ret=1) Then the documentation for the --get option says: Returns error code 1 if the key was not found. and --get-all says: Like get, but does not fail if the number of values for the key is not exactly one. although it does return 1 if there are zero values. --get-regexp behaves in the same way. Overall I think that the fact that --get-urlmatch is the outlier here means that it should change to match the other --get* options (ignoring --get-color and --get-colorbool which are very different). Although I wonder if anyone is relying on the current behaviour and will find their workflow broken if we change this. The documentation could also use some clarification since most of the return codes only apply for the "set" options and in some cases this isn't clear from the existing descriptions. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: git config --get-urlmatch does not set exit code 1 when no match is found 2016-02-28 10:45 ` John Keeping @ 2016-02-28 11:52 ` John Keeping 2016-02-28 11:54 ` [PATCH 0/3] " John Keeping 1 sibling, 0 replies; 11+ messages in thread From: John Keeping @ 2016-02-28 11:52 UTC (permalink / raw) To: Guilherme; +Cc: git@vger•kernel.org On Sun, Feb 28, 2016 at 10:45:57AM +0000, John Keeping wrote: > On Sun, Feb 28, 2016 at 10:09:12AM +0530, Guilherme wrote: > > My current woes are with multi-valued configuration values. More > > specifically credential.helper > > > > The documentation of git config says that when a value is not matched > > it should return 1. > > > > To reproduce make sure that credential.helper is not set. > > > > git config --get-urlmatch credential.helper http://somedomain:1234/ > > echo %ERRORLEVEL% > > 0 > > > > git config --get credential.helper > > echo %ERRORLEVEL% > > 1 > > > > git config --get credential.http://somedomain:1234/.helper > > echo %ERRORLEVEL% > > 1 > > > > The documentation says that for credential.helper is not found for a > > domain it should fall back to credential.helper if it is set. So I > > think that all those tests above should have returned 0. Am i right? I misread this as "should have returned 1", which is what the text below agrees with. The "git config" command does not know anything about the semantics of particular config keys. It is purely an interface to parse and query the config file format and it is up to the consumer to know what to do if a key doesn't exist. Both of the "git config --get" examples you give are behaving as documented in git-config(1). > It looks to me like a simple bug that --get-urlmatch doesn't return 1 if > the key isn't found, but git-config(1) isn't entirely clear. The > overall documentation on exit codes at the end of DESCRIPTION says that > exit code 1 means: > > the section or key is invalid (ret=1) > > Then the documentation for the --get option says: > > Returns error code 1 if the key was not found. > > and --get-all says: > > Like get, but does not fail if the number of values for the key > is not exactly one. > > although it does return 1 if there are zero values. --get-regexp > behaves in the same way. > > Overall I think that the fact that --get-urlmatch is the outlier here > means that it should change to match the other --get* options (ignoring > --get-color and --get-colorbool which are very different). Although I > wonder if anyone is relying on the current behaviour and will find their > workflow broken if we change this. > > The documentation could also use some clarification since most of the > return codes only apply for the "set" options and in some cases this > isn't clear from the existing descriptions. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 0/3] Re: git config --get-urlmatch does not set exit code 1 when no match is found 2016-02-28 10:45 ` John Keeping 2016-02-28 11:52 ` John Keeping @ 2016-02-28 11:54 ` John Keeping 2016-02-28 11:54 ` [PATCH 1/3] config: fail if --get-urlmatch finds no value John Keeping ` (3 more replies) 1 sibling, 4 replies; 11+ messages in thread From: John Keeping @ 2016-02-28 11:54 UTC (permalink / raw) To: git; +Cc: John Keeping, Guilherme On Sun, Feb 28, 2016 at 10:45:57AM +0000, John Keeping wrote: > It looks to me like a simple bug that --get-urlmatch doesn't return 1 if > the key isn't found, but git-config(1) isn't entirely clear. The > overall documentation on exit codes at the end of DESCRIPTION says that > exit code 1 means: > > the section or key is invalid (ret=1) > > Then the documentation for the --get option says: > > Returns error code 1 if the key was not found. > > and --get-all says: > > Like get, but does not fail if the number of values for the key > is not exactly one. > > although it does return 1 if there are zero values. --get-regexp > behaves in the same way. > > Overall I think that the fact that --get-urlmatch is the outlier here > means that it should change to match the other --get* options (ignoring > --get-color and --get-colorbool which are very different). Although I > wonder if anyone is relying on the current behaviour and will find their > workflow broken if we change this. > > The documentation could also use some clarification since most of the > return codes only apply for the "set" options and in some cases this > isn't clear from the existing descriptions. Here's a series that changes the behaviour of "git config --get-urlmatch" when no appropriate key is found as well as a couple of improvements to the documentation while we're here. The second two patches are independent of the first and I think they should be picked up even if we decide the change to --get-urlmatch's exit code is not desirable. John Keeping (3): config: fail if --get-urlmatch finds no value Documentation/git-config: use bulleted list for exit codes Documentation/git-config: fix --get-all description Documentation/git-config.txt | 19 +++++++++---------- builtin/config.c | 5 ++++- t/t1300-repo-config.sh | 3 +++ 3 files changed, 16 insertions(+), 11 deletions(-) -- 2.7.1.503.g3cfa3ac ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/3] config: fail if --get-urlmatch finds no value 2016-02-28 11:54 ` [PATCH 0/3] " John Keeping @ 2016-02-28 11:54 ` John Keeping 2016-02-28 11:54 ` [PATCH 2/3] Documentation/git-config: use bulleted list for exit codes John Keeping ` (2 subsequent siblings) 3 siblings, 0 replies; 11+ messages in thread From: John Keeping @ 2016-02-28 11:54 UTC (permalink / raw) To: git; +Cc: John Keeping, Guilherme The --get, --get-all and --get-regexp options to git-config exit with status 1 if the key is not found but --get-urlmatch succeeds in this case. Change --get-urlmatch to behave in the same way as the other --get* options so that all four are consistent. --get-color is a special case because it accepts a default value to return and so should not return an error if the key is not found. Also clarify this behaviour in the documentation. Signed-off-by: John Keeping <john@keeping•me.uk> --- Documentation/git-config.txt | 2 +- builtin/config.c | 5 ++++- t/t1300-repo-config.sh | 3 +++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt index 153b2d8..2a04e87 100644 --- a/Documentation/git-config.txt +++ b/Documentation/git-config.txt @@ -102,7 +102,7 @@ OPTIONS given URL is returned (if no such key exists, the value for section.key is used as a fallback). When given just the section as name, do so for all the keys in the section and - list them. + list them. Returns error code 1 if no value is found. --global:: For writing options: write to global `~/.gitconfig` file diff --git a/builtin/config.c b/builtin/config.c index ca9f834..1d7c6ef 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -417,6 +417,7 @@ static int urlmatch_collect_fn(const char *var, const char *value, void *cb) static int get_urlmatch(const char *var, const char *url) { + int ret; char *section_tail; struct string_list_item *item; struct urlmatch_config config = { STRING_LIST_INIT_DUP }; @@ -443,6 +444,8 @@ static int get_urlmatch(const char *var, const char *url) git_config_with_options(urlmatch_config_entry, &config, &given_config_source, respect_includes); + ret = !values.nr; + for_each_string_list_item(item, &values) { struct urlmatch_current_candidate_value *matched = item->util; struct strbuf buf = STRBUF_INIT; @@ -459,7 +462,7 @@ static int get_urlmatch(const char *var, const char *url) free(config.url.url); free((void *)config.section); - return 0; + return ret; } static char *default_user_config(void) diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh index 8867ce1..89d8c47 100755 --- a/t/t1300-repo-config.sh +++ b/t/t1300-repo-config.sh @@ -1148,6 +1148,9 @@ test_expect_success 'urlmatch' ' cookieFile = /tmp/cookie.txt EOF + test_expect_code 1 git config --bool --get-urlmatch doesnt.exist https://good.example.com >actual && + test_must_be_empty actual && + echo true >expect && git config --bool --get-urlmatch http.SSLverify https://good.example.com >actual && test_cmp expect actual && -- 2.7.1.503.g3cfa3ac ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/3] Documentation/git-config: use bulleted list for exit codes 2016-02-28 11:54 ` [PATCH 0/3] " John Keeping 2016-02-28 11:54 ` [PATCH 1/3] config: fail if --get-urlmatch finds no value John Keeping @ 2016-02-28 11:54 ` John Keeping 2016-02-28 11:54 ` [PATCH 3/3] Documentation/git-config: fix --get-all description John Keeping 2016-02-28 20:01 ` [PATCH 0/3] Re: git config --get-urlmatch does not set exit code 1 when no match is found Junio C Hamano 3 siblings, 0 replies; 11+ messages in thread From: John Keeping @ 2016-02-28 11:54 UTC (permalink / raw) To: git; +Cc: John Keeping, Guilherme Using a numbered list is confusing because the exit codes are not listed in order so the numbers at the start of each line do not match the exit codes described by the following text. Switch to a bulleted list so that the only number appearing on each line is the exit code described. Signed-off-by: John Keeping <john@keeping•me.uk> --- Documentation/git-config.txt | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt index 2a04e87..e9c755f 100644 --- a/Documentation/git-config.txt +++ b/Documentation/git-config.txt @@ -58,13 +58,13 @@ that location (you can say '--local' but that is the default). This command will fail with non-zero status upon error. Some exit codes are: -. The config file is invalid (ret=3), -. can not write to the config file (ret=4), -. no section or name was provided (ret=2), -. the section or key is invalid (ret=1), -. you try to unset an option which does not exist (ret=5), -. you try to unset/set an option for which multiple lines match (ret=5), or -. you try to use an invalid regexp (ret=6). +- The config file is invalid (ret=3), +- can not write to the config file (ret=4), +- no section or name was provided (ret=2), +- the section or key is invalid (ret=1), +- you try to unset an option which does not exist (ret=5), +- you try to unset/set an option for which multiple lines match (ret=5), or +- you try to use an invalid regexp (ret=6). On success, the command returns the exit code 0. -- 2.7.1.503.g3cfa3ac ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/3] Documentation/git-config: fix --get-all description 2016-02-28 11:54 ` [PATCH 0/3] " John Keeping 2016-02-28 11:54 ` [PATCH 1/3] config: fail if --get-urlmatch finds no value John Keeping 2016-02-28 11:54 ` [PATCH 2/3] Documentation/git-config: use bulleted list for exit codes John Keeping @ 2016-02-28 11:54 ` John Keeping 2016-02-28 20:01 ` [PATCH 0/3] Re: git config --get-urlmatch does not set exit code 1 when no match is found Junio C Hamano 3 siblings, 0 replies; 11+ messages in thread From: John Keeping @ 2016-02-28 11:54 UTC (permalink / raw) To: git; +Cc: John Keeping, Guilherme --get does not fail if a key is multi-valued, it returns the last value as described in its documentation. Clarify the description of --get-all to avoid implying that --get does fail in this case. Signed-off-by: John Keeping <john@keeping•me.uk> --- Documentation/git-config.txt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt index e9c755f..6fc08e3 100644 --- a/Documentation/git-config.txt +++ b/Documentation/git-config.txt @@ -86,8 +86,7 @@ OPTIONS found and the last value if multiple key values were found. --get-all:: - Like get, but does not fail if the number of values for the key - is not exactly one. + Like get, but returns all values for a multi-valued key. --get-regexp:: Like --get-all, but interprets the name as a regular expression and -- 2.7.1.503.g3cfa3ac ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 0/3] Re: git config --get-urlmatch does not set exit code 1 when no match is found 2016-02-28 11:54 ` [PATCH 0/3] " John Keeping ` (2 preceding siblings ...) 2016-02-28 11:54 ` [PATCH 3/3] Documentation/git-config: fix --get-all description John Keeping @ 2016-02-28 20:01 ` Junio C Hamano 3 siblings, 0 replies; 11+ messages in thread From: Junio C Hamano @ 2016-02-28 20:01 UTC (permalink / raw) To: John Keeping; +Cc: git, Guilherme John Keeping <john@keeping•me.uk> writes: > Here's a series that changes the behaviour of "git config --get-urlmatch" > when no appropriate key is found as well as a couple of improvements to > the documentation while we're here. Sounds sensible. It does change the behaviour, but it is inevitable that a bugfix has to change existing behaviour, so... > > John Keeping (3): > config: fail if --get-urlmatch finds no value > Documentation/git-config: use bulleted list for exit codes > Documentation/git-config: fix --get-all description > > Documentation/git-config.txt | 19 +++++++++---------- > builtin/config.c | 5 ++++- > t/t1300-repo-config.sh | 3 +++ > 3 files changed, 16 insertions(+), 11 deletions(-) ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: git config --get-urlmatch does not set exit code 1 when no match is found 2016-02-28 4:39 git config --get-urlmatch does not set exit code 1 when no match is found Guilherme 2016-02-28 10:45 ` John Keeping @ 2016-02-29 11:53 ` Jeff King 2016-02-29 13:08 ` Guilherme 1 sibling, 1 reply; 11+ messages in thread From: Jeff King @ 2016-02-29 11:53 UTC (permalink / raw) To: Guilherme; +Cc: git@vger•kernel.org On Sun, Feb 28, 2016 at 10:09:12AM +0530, Guilherme wrote: > My current woes are with multi-valued configuration values. More > specifically credential.helper > > The documentation of git config says that when a value is not matched > it should return 1. > > To reproduce make sure that credential.helper is not set. > > git config --get-urlmatch credential.helper http://somedomain:1234/ > echo %ERRORLEVEL% > 0 This isn't really addressing your question, but I should warn you that internally, the credential code _doesn't_ use the urlmatch infrastructure. It predates the urlmatch code, and was never converted (so basically only http.* uses urlmatch). I think there are some corner cases where the two behave differently. I'm not sure what you're using this for, but you may get surprising results. -Peff ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: git config --get-urlmatch does not set exit code 1 when no match is found 2016-02-29 11:53 ` Jeff King @ 2016-02-29 13:08 ` Guilherme 2016-03-01 15:03 ` Jeff King 0 siblings, 1 reply; 11+ messages in thread From: Guilherme @ 2016-02-29 13:08 UTC (permalink / raw) To: Jeff King; +Cc: git@vger•kernel.org @Peff Thank you for the heads up. I'm trying to find out if there are any credential helpers configured in the system that will be running tests. On the dedicated test machines that is not a problem but the developer machines are. Should I already post a pre-emptive email asking about the corner cases? More importantly for me is if there is a case where get-url would not show a match where git clone would. If git clone skips a configuration that config url-match doesn't then it's not so bad. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: git config --get-urlmatch does not set exit code 1 when no match is found 2016-02-29 13:08 ` Guilherme @ 2016-03-01 15:03 ` Jeff King 0 siblings, 0 replies; 11+ messages in thread From: Jeff King @ 2016-03-01 15:03 UTC (permalink / raw) To: Guilherme; +Cc: git@vger•kernel.org On Mon, Feb 29, 2016 at 06:38:28PM +0530, Guilherme wrote: > @Peff Thank you for the heads up. > > I'm trying to find out if there are any credential helpers configured > in the system that will be running tests. On the dedicated test > machines that is not a problem but the developer machines are. Do you need to do it in an automated way, or just once? If manual is OK, I suspect running: GIT_TRACE=1 git credential </dev/null will give you a list of what gets run. That's awfully hacky, though. Probably a "git credential list" command would be useful. Or just adapting it to use the urlmatch stuff. > Should I already post a pre-emptive email asking about the corner cases? > > More importantly for me is if there is a case where get-url would not > show a match where git clone would. If git clone skips a configuration > that config url-match doesn't then it's not so bad. Sorry, I don't recall the details. I feel like we discussed it a little on the list, but I can't find it now. The closest I could find is: http://article.gmane.org/gmane.comp.version-control.git/267895 I think the main differences would be in ordering, not in what is selected. -Peff ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2016-03-01 15:03 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-02-28 4:39 git config --get-urlmatch does not set exit code 1 when no match is found Guilherme 2016-02-28 10:45 ` John Keeping 2016-02-28 11:52 ` John Keeping 2016-02-28 11:54 ` [PATCH 0/3] " John Keeping 2016-02-28 11:54 ` [PATCH 1/3] config: fail if --get-urlmatch finds no value John Keeping 2016-02-28 11:54 ` [PATCH 2/3] Documentation/git-config: use bulleted list for exit codes John Keeping 2016-02-28 11:54 ` [PATCH 3/3] Documentation/git-config: fix --get-all description John Keeping 2016-02-28 20:01 ` [PATCH 0/3] Re: git config --get-urlmatch does not set exit code 1 when no match is found Junio C Hamano 2016-02-29 11:53 ` Jeff King 2016-02-29 13:08 ` Guilherme 2016-03-01 15:03 ` Jeff King
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox