From f6f348a6d5d585fb3eda326a20bf2bab9e60ef51 Mon Sep 17 00:00:00 2001 From: Ævar Arnfjörð Bjarmason Date: Tue, 28 Mar 2023 16:04:23 +0200 Subject: versioncmp.c: refactor config reading next commit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refactor the reading of the versionSort.suffix and versionSort.prereleaseSuffix configuration variables to stay within the bounds of our CodingGuidelines when it comes to line length, and to avoid repeating ourselves. Renaming "deprecated_prereleases" to "oldl" doesn't help us to avoid line wrapping now, but it will in a subsequent commit. Let's also split out the names of the config variables into variables of our own, and refactor the nested if/else to avoid indenting it, and the existing bracing style issue. This all helps with the subsequent commit, where we'll need to start checking different git_config_get_value_multi() return value. See c026557a373 (versioncmp: generalize version sort suffix reordering, 2016-12-08) for the original implementation of most of this. Moving the "initialized = 1" assignment allows us to move some of this to the variable declarations in the subsequent commit. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- versioncmp.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) (limited to 'versioncmp.c') diff --git a/versioncmp.c b/versioncmp.c index 069ee94a4d..323f5d35ea 100644 --- a/versioncmp.c +++ b/versioncmp.c @@ -160,15 +160,18 @@ int versioncmp(const char *s1, const char *s2) } if (!initialized) { - const struct string_list *deprecated_prereleases; + const char *const newk = "versionsort.suffix"; + const char *const oldk = "versionsort.prereleasesuffix"; + const struct string_list *oldl; + + prereleases = git_config_get_value_multi(newk); + oldl = git_config_get_value_multi(oldk); + if (prereleases && oldl) + warning("ignoring %s because %s is set", oldk, newk); + else if (!prereleases) + prereleases = oldl; + initialized = 1; - prereleases = git_config_get_value_multi("versionsort.suffix"); - deprecated_prereleases = git_config_get_value_multi("versionsort.prereleasesuffix"); - if (prereleases) { - if (deprecated_prereleases) - warning("ignoring versionsort.prereleasesuffix because versionsort.suffix is set"); - } else - prereleases = deprecated_prereleases; } if (prereleases && swap_prereleases(s1, s2, (const char *) p1 - s1 - 1, &diff)) -- cgit v1.3 From a428619309f42b76b5f750e66a5c1fd2225db3b3 Mon Sep 17 00:00:00 2001 From: Ævar Arnfjörð Bjarmason Date: Tue, 28 Mar 2023 16:04:24 +0200 Subject: config API: have *_multi() return an "int" and take a "dest" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Have the "git_configset_get_value_multi()" function and its siblings return an "int" and populate a "**dest" parameter like every other git_configset_get_*()" in the API. As we'll take advantage of in subsequent commits, this fixes a blind spot in the API where it wasn't possible to tell whether a list was empty from whether a config key existed. For now we don't make use of those new return values, but faithfully convert existing API users. Most of this is straightforward, commentary on cases that stand out: - To ensure that we'll properly use the return values of this function in the future we're using the "RESULT_MUST_BE_USED" macro introduced in [1]. As git_die_config() now has to handle this return value let's have it BUG() if it can't find the config entry. As tested for in a preceding commit we can rely on getting the config list in git_die_config(). - The loops after getting the "list" value in "builtin/gc.c" could also make use of "unsorted_string_list_has_string()" instead of using that loop, but let's leave that for now. - In "versioncmp.c" we now use the return value of the functions, instead of checking if the lists are still non-NULL. 1. 1e8697b5c4e (submodule--helper: check repo{_submodule,}_init() return values, 2022-09-01), Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/for-each-repo.c | 5 +---- builtin/gc.c | 10 ++++------ builtin/log.c | 6 +++--- config.c | 34 ++++++++++++++++++++-------------- config.h | 29 +++++++++++++++++++++-------- pack-bitmap.c | 6 +++++- submodule.c | 3 +-- t/helper/test-config.c | 6 ++---- versioncmp.c | 11 +++++++---- 9 files changed, 64 insertions(+), 46 deletions(-) (limited to 'versioncmp.c') diff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c index 6aeac37148..fd0e7739e6 100644 --- a/builtin/for-each-repo.c +++ b/builtin/for-each-repo.c @@ -45,14 +45,11 @@ int cmd_for_each_repo(int argc, const char **argv, const char *prefix) if (!config_key) die(_("missing --config=")); - values = repo_config_get_value_multi(the_repository, - config_key); - /* * Do nothing on an empty list, which is equivalent to the case * where the config variable does not exist at all. */ - if (!values) + if (repo_config_get_value_multi(the_repository, config_key, &values)) return 0; for (i = 0; !result && i < values->nr; i++) diff --git a/builtin/gc.c b/builtin/gc.c index e38d1783f3..2b3da377d5 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1510,8 +1510,7 @@ static int maintenance_register(int argc, const char **argv, const char *prefix) if (git_config_get("maintenance.strategy")) git_config_set("maintenance.strategy", "incremental"); - list = git_config_get_value_multi(key); - if (list) { + if (!git_config_get_value_multi(key, &list)) { for_each_string_list_item(item, list) { if (!strcmp(maintpath, item->string)) { found = 1; @@ -1577,11 +1576,10 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi if (config_file) { git_configset_init(&cs); git_configset_add_file(&cs, config_file); - list = git_configset_get_value_multi(&cs, key); - } else { - list = git_config_get_value_multi(key); } - if (list) { + if (!(config_file + ? git_configset_get_value_multi(&cs, key, &list) + : git_config_get_value_multi(key, &list))) { for_each_string_list_item(item, list) { if (!strcmp(maintpath, item->string)) { found = 1; diff --git a/builtin/log.c b/builtin/log.c index 5eafcf26b4..cc9d92f95d 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -182,10 +182,10 @@ static void set_default_decoration_filter(struct decoration_filter *decoration_f int i; char *value = NULL; struct string_list *include = decoration_filter->include_ref_pattern; - const struct string_list *config_exclude = - git_config_get_value_multi("log.excludeDecoration"); + const struct string_list *config_exclude; - if (config_exclude) { + if (!git_config_get_value_multi("log.excludeDecoration", + &config_exclude)) { struct string_list_item *item; for_each_string_list_item(item, config_exclude) string_list_append(decoration_filter->exclude_ref_config_pattern, diff --git a/config.c b/config.c index 5d3e84f85a..1f654daf6f 100644 --- a/config.c +++ b/config.c @@ -2404,29 +2404,34 @@ int git_configset_add_file(struct config_set *cs, const char *filename) int git_configset_get_value(struct config_set *cs, const char *key, const char **value) { const struct string_list *values = NULL; + int ret; + /* * Follows "last one wins" semantic, i.e., if there are multiple matches for the * queried key in the files of the configset, the value returned will be the last * value in the value list for that key. */ - values = git_configset_get_value_multi(cs, key); + if ((ret = git_configset_get_value_multi(cs, key, &values))) + return ret; - if (!values) - return 1; assert(values->nr > 0); *value = values->items[values->nr - 1].string; return 0; } -const struct string_list *git_configset_get_value_multi(struct config_set *cs, const char *key) +int git_configset_get_value_multi(struct config_set *cs, const char *key, + const struct string_list **dest) { struct config_set_element *e; + int ret; - if (configset_find_element(cs, key, &e)) - return NULL; + if ((ret = configset_find_element(cs, key, &e))) + return ret; else if (!e) - return NULL; - return &e->value_list; + return 1; + *dest = &e->value_list; + + return 0; } int git_configset_get(struct config_set *cs, const char *key) @@ -2590,11 +2595,11 @@ int repo_config_get_value(struct repository *repo, return git_configset_get_value(repo->config, key, value); } -const struct string_list *repo_config_get_value_multi(struct repository *repo, - const char *key) +int repo_config_get_value_multi(struct repository *repo, const char *key, + const struct string_list **dest) { git_config_check_init(repo); - return git_configset_get_value_multi(repo->config, key); + return git_configset_get_value_multi(repo->config, key, dest); } int repo_config_get_string(struct repository *repo, @@ -2707,9 +2712,9 @@ int git_config_get_value(const char *key, const char **value) return repo_config_get_value(the_repository, key, value); } -const struct string_list *git_config_get_value_multi(const char *key) +int git_config_get_value_multi(const char *key, const struct string_list **dest) { - return repo_config_get_value_multi(the_repository, key); + return repo_config_get_value_multi(the_repository, key, dest); } int git_config_get_string(const char *key, char **dest) @@ -2856,7 +2861,8 @@ void git_die_config(const char *key, const char *err, ...) error_fn(err, params); va_end(params); } - values = git_config_get_value_multi(key); + if (git_config_get_value_multi(key, &values)) + BUG("for key '%s' we must have a value to report on", key); kv_info = values->items[values->nr - 1].util; git_die_config_linenr(key, kv_info->filename, kv_info->linenr); } diff --git a/config.h b/config.h index cbd82e5c43..f228fa9d5d 100644 --- a/config.h +++ b/config.h @@ -459,10 +459,18 @@ int git_configset_add_parameters(struct config_set *cs); /** * Finds and returns the value list, sorted in order of increasing priority * for the configuration variable `key` and config set `cs`. When the - * configuration variable `key` is not found, returns NULL. The caller - * should not free or modify the returned pointer, as it is owned by the cache. + * configuration variable `key` is not found, returns 1 without touching + * `value`. + * + * The key will be parsed for validity with git_config_parse_key(), on + * error a negative value will be returned. + * + * The caller should not free or modify the returned pointer, as it is + * owned by the cache. */ -const struct string_list *git_configset_get_value_multi(struct config_set *cs, const char *key); +RESULT_MUST_BE_USED +int git_configset_get_value_multi(struct config_set *cs, const char *key, + const struct string_list **dest); /** * Clears `config_set` structure, removes all saved variable-value pairs. @@ -511,8 +519,9 @@ RESULT_MUST_BE_USED int repo_config_get(struct repository *repo, const char *key); int repo_config_get_value(struct repository *repo, const char *key, const char **value); -const struct string_list *repo_config_get_value_multi(struct repository *repo, - const char *key); +RESULT_MUST_BE_USED +int repo_config_get_value_multi(struct repository *repo, const char *key, + const struct string_list **dest); int repo_config_get_string(struct repository *repo, const char *key, char **dest); int repo_config_get_string_tmp(struct repository *repo, @@ -566,10 +575,14 @@ int git_config_get_value(const char *key, const char **value); /** * Finds and returns the value list, sorted in order of increasing priority * for the configuration variable `key`. When the configuration variable - * `key` is not found, returns NULL. The caller should not free or modify - * the returned pointer, as it is owned by the cache. + * `key` is not found, returns 1 without touching `value`. + * + * The caller should not free or modify the returned pointer, as it is + * owned by the cache. */ -const struct string_list *git_config_get_value_multi(const char *key); +RESULT_MUST_BE_USED +int git_config_get_value_multi(const char *key, + const struct string_list **dest); /** * Resets and invalidates the config cache. diff --git a/pack-bitmap.c b/pack-bitmap.c index 440407f1be..81f0c0e016 100644 --- a/pack-bitmap.c +++ b/pack-bitmap.c @@ -2301,7 +2301,11 @@ int bitmap_is_midx(struct bitmap_index *bitmap_git) const struct string_list *bitmap_preferred_tips(struct repository *r) { - return repo_config_get_value_multi(r, "pack.preferbitmaptips"); + const struct string_list *dest; + + if (!repo_config_get_value_multi(r, "pack.preferbitmaptips", &dest)) + return dest; + return NULL; } int bitmap_is_preferred_refname(struct repository *r, const char *refname) diff --git a/submodule.c b/submodule.c index 8ac2fca855..e43c4230ba 100644 --- a/submodule.c +++ b/submodule.c @@ -274,8 +274,7 @@ int is_tree_submodule_active(struct repository *repo, free(key); /* submodule.active is set */ - sl = repo_config_get_value_multi(repo, "submodule.active"); - if (sl) { + if (!repo_config_get_value_multi(repo, "submodule.active", &sl)) { struct pathspec ps; struct strvec args = STRVEC_INIT; const struct string_list_item *item; diff --git a/t/helper/test-config.c b/t/helper/test-config.c index cbb33ae1ff..6dc4c37444 100644 --- a/t/helper/test-config.c +++ b/t/helper/test-config.c @@ -97,8 +97,7 @@ int cmd__config(int argc, const char **argv) goto exit1; } } else if (argc == 3 && !strcmp(argv[1], "get_value_multi")) { - strptr = git_config_get_value_multi(argv[2]); - if (strptr) { + if (!git_config_get_value_multi(argv[2], &strptr)) { for (i = 0; i < strptr->nr; i++) { v = strptr->items[i].string; if (!v) @@ -181,8 +180,7 @@ int cmd__config(int argc, const char **argv) goto exit2; } } - strptr = git_configset_get_value_multi(&cs, argv[2]); - if (strptr) { + if (!git_configset_get_value_multi(&cs, argv[2], &strptr)) { for (i = 0; i < strptr->nr; i++) { v = strptr->items[i].string; if (!v) diff --git a/versioncmp.c b/versioncmp.c index 323f5d35ea..60c3a51712 100644 --- a/versioncmp.c +++ b/versioncmp.c @@ -162,13 +162,16 @@ int versioncmp(const char *s1, const char *s2) if (!initialized) { const char *const newk = "versionsort.suffix"; const char *const oldk = "versionsort.prereleasesuffix"; + const struct string_list *newl; const struct string_list *oldl; + int new = git_config_get_value_multi(newk, &newl); + int old = git_config_get_value_multi(oldk, &oldl); - prereleases = git_config_get_value_multi(newk); - oldl = git_config_get_value_multi(oldk); - if (prereleases && oldl) + if (!new && !old) warning("ignoring %s because %s is set", oldk, newk); - else if (!prereleases) + if (!new) + prereleases = newl; + else if (!old) prereleases = oldl; initialized = 1; -- cgit v1.3 From 9e2d884d0fcf0631164c33d458a251bee6053bb1 Mon Sep 17 00:00:00 2001 From: Ævar Arnfjörð Bjarmason Date: Tue, 28 Mar 2023 16:04:27 +0200 Subject: config API: add "string" version of *_value_multi(), fix segfaults MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix numerous and mostly long-standing segfaults in consumers of the *_config_*value_multi() API. As discussed in the preceding commit an empty key in the config syntax yields a "NULL" string, which these users would give to strcmp() (or similar), resulting in segfaults. As this change shows, most users users of the *_config_*value_multi() API didn't really want such an an unsafe and low-level API, let's give them something with the safety of git_config_get_string() instead. This fix is similar to what the *_string() functions and others acquired in[1] and [2]. Namely introducing and using a safer "*_get_string_multi()" variant of the low-level "_*value_multi()" function. This fixes segfaults in code introduced in: - d811c8e17c6 (versionsort: support reorder prerelease suffixes, 2015-02-26) - c026557a373 (versioncmp: generalize version sort suffix reordering, 2016-12-08) - a086f921a72 (submodule: decouple url and submodule interest, 2017-03-17) - a6be5e6764a (log: add log.excludeDecoration config option, 2020-04-16) - 92156291ca8 (log: add default decoration filter, 2022-08-05) - 50a044f1e40 (gc: replace config subprocesses with API calls, 2022-09-27) There are now two users ofthe low-level API: - One in "builtin/for-each-repo.c", which we'll convert in a subsequent commit. - The "t/helper/test-config.c" code added in [3]. As seen in the preceding commit we need to give the "t/helper/test-config.c" caller these "NULL" entries. We could also alter the underlying git_configset_get_value_multi() function to be "string safe", but doing so would leave no room for other variants of "*_get_value_multi()" that coerce to other types. Such coercion can't be built on the string version, since as we've established "NULL" is a true value in the boolean context, but if we coerced it to "" for use in a list of strings it'll be subsequently coerced to "false" as a boolean. The callback pattern being used here will make it easy to introduce e.g. a "multi" variant which coerces its values to "bool", "int", "path" etc. 1. 40ea4ed9032 (Add config_error_nonbool() helper function, 2008-02-11) 2. 6c47d0e8f39 (config.c: guard config parser from value=NULL, 2008-02-11). 3. 4c715ebb96a (test-config: add tests for the config_set API, 2014-07-28) Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/gc.c | 6 +++--- builtin/log.c | 4 ++-- config.c | 32 ++++++++++++++++++++++++++++++++ config.h | 19 +++++++++++++++++++ pack-bitmap.c | 2 +- submodule.c | 2 +- t/t4202-log.sh | 8 ++++++-- t/t5310-pack-bitmaps.sh | 8 ++++++-- t/t7004-tag.sh | 9 +++++++-- t/t7413-submodule-is-active.sh | 8 ++++++-- t/t7900-maintenance.sh | 25 ++++++++++++++++++++----- versioncmp.c | 4 ++-- 12 files changed, 105 insertions(+), 22 deletions(-) (limited to 'versioncmp.c') diff --git a/builtin/gc.c b/builtin/gc.c index 2b3da377d5..9497bdf23e 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1510,7 +1510,7 @@ static int maintenance_register(int argc, const char **argv, const char *prefix) if (git_config_get("maintenance.strategy")) git_config_set("maintenance.strategy", "incremental"); - if (!git_config_get_value_multi(key, &list)) { + if (!git_config_get_string_multi(key, &list)) { for_each_string_list_item(item, list) { if (!strcmp(maintpath, item->string)) { found = 1; @@ -1578,8 +1578,8 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi git_configset_add_file(&cs, config_file); } if (!(config_file - ? git_configset_get_value_multi(&cs, key, &list) - : git_config_get_value_multi(key, &list))) { + ? git_configset_get_string_multi(&cs, key, &list) + : git_config_get_string_multi(key, &list))) { for_each_string_list_item(item, list) { if (!strcmp(maintpath, item->string)) { found = 1; diff --git a/builtin/log.c b/builtin/log.c index cc9d92f95d..cd17f31171 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -184,8 +184,8 @@ static void set_default_decoration_filter(struct decoration_filter *decoration_f struct string_list *include = decoration_filter->include_ref_pattern; const struct string_list *config_exclude; - if (!git_config_get_value_multi("log.excludeDecoration", - &config_exclude)) { + if (!git_config_get_string_multi("log.excludeDecoration", + &config_exclude)) { struct string_list_item *item; for_each_string_list_item(item, config_exclude) string_list_append(decoration_filter->exclude_ref_config_pattern, diff --git a/config.c b/config.c index 1f654daf6f..92d9e7e968 100644 --- a/config.c +++ b/config.c @@ -2434,6 +2434,25 @@ int git_configset_get_value_multi(struct config_set *cs, const char *key, return 0; } +static int check_multi_string(struct string_list_item *item, void *util) +{ + return item->string ? 0 : config_error_nonbool(util); +} + +int git_configset_get_string_multi(struct config_set *cs, const char *key, + const struct string_list **dest) +{ + int ret; + + if ((ret = git_configset_get_value_multi(cs, key, dest))) + return ret; + if ((ret = for_each_string_list((struct string_list *)*dest, + check_multi_string, (void *)key))) + return ret; + + return 0; +} + int git_configset_get(struct config_set *cs, const char *key) { struct config_set_element *e; @@ -2602,6 +2621,13 @@ int repo_config_get_value_multi(struct repository *repo, const char *key, return git_configset_get_value_multi(repo->config, key, dest); } +int repo_config_get_string_multi(struct repository *repo, const char *key, + const struct string_list **dest) +{ + git_config_check_init(repo); + return git_configset_get_string_multi(repo->config, key, dest); +} + int repo_config_get_string(struct repository *repo, const char *key, char **dest) { @@ -2717,6 +2743,12 @@ int git_config_get_value_multi(const char *key, const struct string_list **dest) return repo_config_get_value_multi(the_repository, key, dest); } +int git_config_get_string_multi(const char *key, + const struct string_list **dest) +{ + return repo_config_get_string_multi(the_repository, key, dest); +} + int git_config_get_string(const char *key, char **dest) { return repo_config_get_string(the_repository, key, dest); diff --git a/config.h b/config.h index f228fa9d5d..65e569e739 100644 --- a/config.h +++ b/config.h @@ -472,6 +472,19 @@ RESULT_MUST_BE_USED int git_configset_get_value_multi(struct config_set *cs, const char *key, const struct string_list **dest); +/** + * A validation wrapper for git_configset_get_value_multi() which does + * for it what git_configset_get_string() does for + * git_configset_get_value(). + * + * The configuration syntax allows for "[section] key", which will + * give us a NULL entry in the "struct string_list", as opposed to + * "[section] key =" which is the empty string. Most users of the API + * are not prepared to handle NULL in a "struct string_list". + */ +int git_configset_get_string_multi(struct config_set *cs, const char *key, + const struct string_list **dest); + /** * Clears `config_set` structure, removes all saved variable-value pairs. */ @@ -522,6 +535,9 @@ int repo_config_get_value(struct repository *repo, RESULT_MUST_BE_USED int repo_config_get_value_multi(struct repository *repo, const char *key, const struct string_list **dest); +RESULT_MUST_BE_USED +int repo_config_get_string_multi(struct repository *repo, const char *key, + const struct string_list **dest); int repo_config_get_string(struct repository *repo, const char *key, char **dest); int repo_config_get_string_tmp(struct repository *repo, @@ -583,6 +599,9 @@ int git_config_get_value(const char *key, const char **value); RESULT_MUST_BE_USED int git_config_get_value_multi(const char *key, const struct string_list **dest); +RESULT_MUST_BE_USED +int git_config_get_string_multi(const char *key, + const struct string_list **dest); /** * Resets and invalidates the config cache. diff --git a/pack-bitmap.c b/pack-bitmap.c index 81f0c0e016..dd05ab03ca 100644 --- a/pack-bitmap.c +++ b/pack-bitmap.c @@ -2303,7 +2303,7 @@ const struct string_list *bitmap_preferred_tips(struct repository *r) { const struct string_list *dest; - if (!repo_config_get_value_multi(r, "pack.preferbitmaptips", &dest)) + if (!repo_config_get_string_multi(r, "pack.preferbitmaptips", &dest)) return dest; return NULL; } diff --git a/submodule.c b/submodule.c index e43c4230ba..0f6cf864ed 100644 --- a/submodule.c +++ b/submodule.c @@ -274,7 +274,7 @@ int is_tree_submodule_active(struct repository *repo, free(key); /* submodule.active is set */ - if (!repo_config_get_value_multi(repo, "submodule.active", &sl)) { + if (!repo_config_get_string_multi(repo, "submodule.active", &sl)) { struct pathspec ps; struct strvec args = STRVEC_INIT; const struct string_list_item *item; diff --git a/t/t4202-log.sh b/t/t4202-log.sh index e4f02d8208..ae73aef922 100755 --- a/t/t4202-log.sh +++ b/t/t4202-log.sh @@ -835,7 +835,7 @@ test_expect_success 'log.decorate configuration' ' ' -test_expect_failure 'parse log.excludeDecoration with no value' ' +test_expect_success 'parse log.excludeDecoration with no value' ' cp .git/config .git/config.orig && test_when_finished mv .git/config.orig .git/config && @@ -843,7 +843,11 @@ test_expect_failure 'parse log.excludeDecoration with no value' ' [log] excludeDecoration EOF - git log --decorate=short + cat >expect <<-\EOF && + error: missing value for '\''log.excludeDecoration'\'' + EOF + git log --decorate=short 2>actual && + test_cmp expect actual ' test_expect_success 'decorate-refs with glob' ' diff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh index 2e65c8139c..894c750080 100755 --- a/t/t5310-pack-bitmaps.sh +++ b/t/t5310-pack-bitmaps.sh @@ -404,7 +404,7 @@ test_bitmap_cases () { ) ' - test_expect_failure 'pack.preferBitmapTips' ' + test_expect_success 'pack.preferBitmapTips' ' git init repo && test_when_finished "rm -rf repo" && ( @@ -416,7 +416,11 @@ test_bitmap_cases () { [pack] preferBitmapTips EOF - git repack -adb + cat >expect <<-\EOF && + error: missing value for '\''pack.preferbitmaptips'\'' + EOF + git repack -adb 2>actual && + test_cmp expect actual ) ' diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh index f343551a7d..f4a31ada79 100755 --- a/t/t7004-tag.sh +++ b/t/t7004-tag.sh @@ -1843,7 +1843,7 @@ test_expect_success 'invalid sort parameter in configuratoin' ' test_must_fail git tag -l "foo*" ' -test_expect_failure 'version sort handles empty value for versionsort.{prereleaseSuffix,suffix}' ' +test_expect_success 'version sort handles empty value for versionsort.{prereleaseSuffix,suffix}' ' cp .git/config .git/config.orig && test_when_finished mv .git/config.orig .git/config && @@ -1852,7 +1852,12 @@ test_expect_failure 'version sort handles empty value for versionsort.{prereleas prereleaseSuffix suffix EOF - git tag -l --sort=version:refname + cat >expect <<-\EOF && + error: missing value for '\''versionsort.suffix'\'' + error: missing value for '\''versionsort.prereleasesuffix'\'' + EOF + git tag -l --sort=version:refname 2>actual && + test_cmp expect actual ' test_expect_success 'version sort with prerelease reordering' ' diff --git a/t/t7413-submodule-is-active.sh b/t/t7413-submodule-is-active.sh index bfe27e5073..887d181b72 100755 --- a/t/t7413-submodule-is-active.sh +++ b/t/t7413-submodule-is-active.sh @@ -51,7 +51,7 @@ test_expect_success 'is-active works with submodule..active config' ' test-tool -C super submodule is-active sub1 ' -test_expect_failure 'is-active handles submodule.active config missing a value' ' +test_expect_success 'is-active handles submodule.active config missing a value' ' cp super/.git/config super/.git/config.orig && test_when_finished mv super/.git/config.orig super/.git/config && @@ -60,7 +60,11 @@ test_expect_failure 'is-active handles submodule.active config missing a value' active EOF - test-tool -C super submodule is-active sub1 + cat >expect <<-\EOF && + error: missing value for '\''submodule.active'\'' + EOF + test-tool -C super submodule is-active sub1 2>actual && + test_cmp expect actual ' test_expect_success 'is-active works with basic submodule.active config' ' diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh index d82eac6a47..487e326b3f 100755 --- a/t/t7900-maintenance.sh +++ b/t/t7900-maintenance.sh @@ -524,7 +524,7 @@ test_expect_success 'register and unregister' ' git maintenance unregister --config-file ./other --force ' -test_expect_failure 'register with no value for maintenance.repo' ' +test_expect_success 'register with no value for maintenance.repo' ' cp .git/config .git/config.orig && test_when_finished mv .git/config.orig .git/config && @@ -532,10 +532,15 @@ test_expect_failure 'register with no value for maintenance.repo' ' [maintenance] repo EOF - git maintenance register + cat >expect <<-\EOF && + error: missing value for '\''maintenance.repo'\'' + EOF + git maintenance register 2>actual && + test_cmp expect actual && + git config maintenance.repo ' -test_expect_failure 'unregister with no value for maintenance.repo' ' +test_expect_success 'unregister with no value for maintenance.repo' ' cp .git/config .git/config.orig && test_when_finished mv .git/config.orig .git/config && @@ -543,8 +548,18 @@ test_expect_failure 'unregister with no value for maintenance.repo' ' [maintenance] repo EOF - git maintenance unregister && - git maintenance unregister --force + cat >expect <<-\EOF && + error: missing value for '\''maintenance.repo'\'' + EOF + test_expect_code 128 git maintenance unregister 2>actual.raw && + grep ^error actual.raw >actual && + test_cmp expect actual && + git config maintenance.repo && + + git maintenance unregister --force 2>actual.raw && + grep ^error actual.raw >actual && + test_cmp expect actual && + git config maintenance.repo ' test_expect_success !MINGW 'register and unregister with regex metacharacters' ' diff --git a/versioncmp.c b/versioncmp.c index 60c3a51712..7498da96e0 100644 --- a/versioncmp.c +++ b/versioncmp.c @@ -164,8 +164,8 @@ int versioncmp(const char *s1, const char *s2) const char *const oldk = "versionsort.prereleasesuffix"; const struct string_list *newl; const struct string_list *oldl; - int new = git_config_get_value_multi(newk, &newl); - int old = git_config_get_value_multi(oldk, &oldl); + int new = git_config_get_string_multi(newk, &newl); + int old = git_config_get_string_multi(oldk, &oldl); if (!new && !old) warning("ignoring %s because %s is set", oldk, newk); -- cgit v1.3