From 7a39741999a5216257b1fbcc847cf0c62c114088 Mon Sep 17 00:00:00 2001 From: Michael J Gruber Date: Tue, 17 May 2011 17:38:52 +0200 Subject: [PATCH 1/2] config: define and document exit codes The return codes of git_config_set() and friends are magic numbers right in the source. #define them in cache.h where the functions are declared, and use the constants in the source. Also, mention the resulting exit codes of "git config" in its man page (and complete the list). Signed-off-by: Michael J Gruber Signed-off-by: Junio C Hamano --- Documentation/git-config.txt | 22 ++++++++++++---------- cache.h | 10 ++++++++++ config.c | 20 ++++++++++---------- 3 files changed, 32 insertions(+), 20 deletions(-) diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt index 8804de327..e7ecf5d80 100644 --- a/Documentation/git-config.txt +++ b/Documentation/git-config.txt @@ -50,16 +50,18 @@ The default is to assume the config file of the current repository, .git/config unless defined otherwise with GIT_DIR and GIT_CONFIG (see <>). -This command will fail if: - -. The config file is invalid, -. Can not write to the config file, -. no section was provided, -. the section or key is invalid, -. you try to unset an option which does not exist, -. you try to unset/set an option for which multiple lines match, or -. you use '--global' option without $HOME being properly set. - +This command will fail (with exit code ret) if: + +. 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), +. you try to use an invalid regexp (ret=6), or +. you use '--global' option without $HOME being properly set (ret=128). + +On success, the command returns the exit code 0. OPTIONS ------- diff --git a/cache.h b/cache.h index 4e9123b77..6b967cc6d 100644 --- a/cache.h +++ b/cache.h @@ -1012,6 +1012,16 @@ extern const char *packed_object_info_detail(struct packed_git *, off_t, unsigne /* Dumb servers support */ extern int update_server_info(int); +/* git_config_parse_key() returns these negated: */ +#define CONFIG_INVALID_KEY 1 +#define CONFIG_NO_SECTION_OR_NAME 2 +/* git_config_set(), git_config_set_multivar() return the above or these: */ +#define CONFIG_NO_LOCK -1 +#define CONFIG_INVALID_FILE 3 +#define CONFIG_NO_WRITE 4 +#define CONFIG_NOTHING_SET 5 +#define CONFIG_INVALID_PATTERN 6 + typedef int (*config_fn_t)(const char *, const char *, void *); extern int git_default_config(const char *, const char *, void *); extern int git_config_from_file(config_fn_t fn, const char *, void *); diff --git a/config.c b/config.c index 671c8df2c..9d3684888 100644 --- a/config.c +++ b/config.c @@ -1123,12 +1123,12 @@ int git_config_parse_key(const char *key, char **store_key, int *baselen_) if (last_dot == NULL || last_dot == key) { error("key does not contain a section: %s", key); - return -2; + return -CONFIG_NO_SECTION_OR_NAME; } if (!last_dot[1]) { error("key does not contain variable name: %s", key); - return -2; + return -CONFIG_NO_SECTION_OR_NAME; } baselen = last_dot - key; @@ -1165,7 +1165,7 @@ int git_config_parse_key(const char *key, char **store_key, int *baselen_) out_free_ret_1: free(*store_key); - return -1; + return -CONFIG_INVALID_KEY; } /* @@ -1221,7 +1221,7 @@ int git_config_set_multivar(const char *key, const char *value, if (fd < 0) { error("could not lock config file %s: %s", config_filename, strerror(errno)); free(store.key); - ret = -1; + ret = CONFIG_NO_LOCK; goto out_free; } @@ -1235,12 +1235,12 @@ int git_config_set_multivar(const char *key, const char *value, if ( ENOENT != errno ) { error("opening %s: %s", config_filename, strerror(errno)); - ret = 3; /* same as "invalid config file" */ + ret = CONFIG_INVALID_FILE; /* same as "invalid config file" */ goto out_free; } /* if nothing to unset, error out */ if (value == NULL) { - ret = 5; + ret = CONFIG_NOTHING_SET; goto out_free; } @@ -1268,7 +1268,7 @@ int git_config_set_multivar(const char *key, const char *value, REG_EXTENDED)) { error("invalid pattern: %s", value_regex); free(store.value_regex); - ret = 6; + ret = CONFIG_INVALID_PATTERN; goto out_free; } } @@ -1290,7 +1290,7 @@ int git_config_set_multivar(const char *key, const char *value, regfree(store.value_regex); free(store.value_regex); } - ret = 3; + ret = CONFIG_INVALID_FILE; goto out_free; } @@ -1303,7 +1303,7 @@ int git_config_set_multivar(const char *key, const char *value, /* if nothing to unset, or too many matches, error out */ if ((store.seen == 0 && value == NULL) || (store.seen > 1 && multi_replace == 0)) { - ret = 5; + ret = CONFIG_NOTHING_SET; goto out_free; } @@ -1364,7 +1364,7 @@ int git_config_set_multivar(const char *key, const char *value, if (commit_lock_file(lock) < 0) { error("could not commit config file %s", config_filename); - ret = 4; + ret = CONFIG_NO_WRITE; goto out_free; } From 5a2df368c26e4a993db9d09f221af1063d0fe9a8 Mon Sep 17 00:00:00 2001 From: Michael J Gruber Date: Tue, 17 May 2011 17:38:53 +0200 Subject: [PATCH 2/2] config: Give error message when not changing a multivar When trying to set a multivar with "git config var value", "git config" issues warning: remote.repoor.push has multiple values leaving the user under the impression that the operation succeeded, unless one checks the return value. Instead, make it warning: remote.repoor.push has multiple values error: cannot overwrite multiple values with a single value Use a regexp, --add or --set-all to change remote.repoor.push. to be clear and helpful. Note: The "warning" is raised through other code paths also so that it needs to remain a warning for these (which do not raise the error). Only the caller can determine how to go on from that. Signed-off-by: Michael J Gruber Signed-off-by: Junio C Hamano --- builtin/config.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/builtin/config.c b/builtin/config.c index 3e3c52849..211e118d5 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -436,9 +436,14 @@ int cmd_config(int argc, const char **argv, const char *prefix) NULL, NULL); } else if (actions == ACTION_SET) { + int ret; check_argc(argc, 2, 2); value = normalize_value(argv[0], argv[1]); - return git_config_set(argv[0], value); + ret = git_config_set(argv[0], value); + if (ret == CONFIG_NOTHING_SET) + error("cannot overwrite multiple values with a single value\n" + " Use a regexp, --add or --set-all to change %s.", argv[0]); + return ret; } else if (actions == ACTION_SET_ALL) { check_argc(argc, 2, 3);