* Report GLib errors
@ 2023-09-15 12:50 David Bremner
2023-09-15 12:50 ` [PATCH 1/4] test: add known broken test for bad utf8 in config David Bremner
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: David Bremner @ 2023-09-15 12:50 UTC (permalink / raw)
To: notmuch
This series obsoletes the WIP series [0]. I think it's probably worth
doing a point release with this fix as it is not too intrusive, and
upcoming GLib releases will likely cause more users to experience the
problems Eric found [1].
[0]: id:20230913005636.135665-1-david@tethera.net
[1]: id:5a7paaqa2dvdo5lmnxvaeacfwhdytfnkr4gfh6mtlotdviki2s@ro4gz4m2aqsw
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 1/4] test: add known broken test for bad utf8 in config 2023-09-15 12:50 Report GLib errors David Bremner @ 2023-09-15 12:50 ` David Bremner 2023-09-15 12:50 ` [PATCH 2/4] CLI: exit with error when load_config returns an error David Bremner ` (3 subsequent siblings) 4 siblings, 0 replies; 6+ messages in thread From: David Bremner @ 2023-09-15 12:50 UTC (permalink / raw) To: notmuch We should ideally print an informative error message, but at the very least we should not exit with success. --- test/T030-config.sh | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/T030-config.sh b/test/T030-config.sh index ea0b4012..6ef759a4 100755 --- a/test/T030-config.sh +++ b/test/T030-config.sh @@ -3,6 +3,8 @@ test_description='"notmuch config"' . $(dirname "$0")/test-lib.sh || exit 1 +cp notmuch-config initial-config + test_begin_subtest "Get string value" test_expect_equal "$(notmuch config get user.name)" "Notmuch Test Suite" @@ -193,4 +195,11 @@ test_begin_subtest "get built_with.nonexistent prints false" output=$(notmuch config get built_with.nonexistent) test_expect_equal "$output" "false" +test_begin_subtest "Bad utf8 reported as error" +test_subtest_known_broken +cp initial-config bad-config +printf '[query]\nq3=from:\xff\n' >>bad-config +test_expect_code 1 "notmuch --config=./bad-config config list" + test_done + -- 2.40.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/4] CLI: exit with error when load_config returns an error. 2023-09-15 12:50 Report GLib errors David Bremner 2023-09-15 12:50 ` [PATCH 1/4] test: add known broken test for bad utf8 in config David Bremner @ 2023-09-15 12:50 ` David Bremner 2023-09-15 12:50 ` [PATCH 3/4] test: add known broken subtest for the bad config error message David Bremner ` (2 subsequent siblings) 4 siblings, 0 replies; 6+ messages in thread From: David Bremner @ 2023-09-15 12:50 UTC (permalink / raw) To: notmuch For now print a generic error message and exit with error on any non-success code. Previously we were exiting, but with exit code zero, leading users / scripts to thing the command had succeeded. --- notmuch.c | 2 ++ test/T030-config.sh | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/notmuch.c b/notmuch.c index 37286b8f..69a18131 100644 --- a/notmuch.c +++ b/notmuch.c @@ -584,6 +584,8 @@ main (int argc, char *argv[]) case NOTMUCH_STATUS_SUCCESS: break; default: + fputs ("Error: unable to load config file.\n", stderr); + ret = 1; goto DONE; } diff --git a/test/T030-config.sh b/test/T030-config.sh index 6ef759a4..8b3ef436 100755 --- a/test/T030-config.sh +++ b/test/T030-config.sh @@ -196,7 +196,6 @@ output=$(notmuch config get built_with.nonexistent) test_expect_equal "$output" "false" test_begin_subtest "Bad utf8 reported as error" -test_subtest_known_broken cp initial-config bad-config printf '[query]\nq3=from:\xff\n' >>bad-config test_expect_code 1 "notmuch --config=./bad-config config list" -- 2.40.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/4] test: add known broken subtest for the bad config error message 2023-09-15 12:50 Report GLib errors David Bremner 2023-09-15 12:50 ` [PATCH 1/4] test: add known broken test for bad utf8 in config David Bremner 2023-09-15 12:50 ` [PATCH 2/4] CLI: exit with error when load_config returns an error David Bremner @ 2023-09-15 12:50 ` David Bremner 2023-09-15 12:50 ` [PATCH 4/4] Pass error message from GLib ini parser to CLI David Bremner 2023-09-23 11:47 ` Report GLib errors David Bremner 4 siblings, 0 replies; 6+ messages in thread From: David Bremner @ 2023-09-15 12:50 UTC (permalink / raw) To: notmuch This is a bit fragile w.r.t. glib changing their error message, but it already helped me find one formatting bug, so for now I think it's worth it, instead of just grepping for "UTF-8". --- test/T030-config.sh | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/T030-config.sh b/test/T030-config.sh index 8b3ef436..11428e8a 100755 --- a/test/T030-config.sh +++ b/test/T030-config.sh @@ -200,5 +200,14 @@ cp initial-config bad-config printf '[query]\nq3=from:\xff\n' >>bad-config test_expect_code 1 "notmuch --config=./bad-config config list" +test_begin_subtest "Specific error message about bad utf8" +test_subtest_known_broken +notmuch --config=./bad-config config list 2>ERRORS +cat <<EOF > EXPECTED +GLib: Key file contains key “q3” with value “from:�” which is not UTF-8 +Error: unable to load config file. +EOF +test_expect_equal_file EXPECTED ERRORS + test_done -- 2.40.1 \r ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 4/4] Pass error message from GLib ini parser to CLI 2023-09-15 12:50 Report GLib errors David Bremner ` (2 preceding siblings ...) 2023-09-15 12:50 ` [PATCH 3/4] test: add known broken subtest for the bad config error message David Bremner @ 2023-09-15 12:50 ` David Bremner 2023-09-23 11:47 ` Report GLib errors David Bremner 4 siblings, 0 replies; 6+ messages in thread From: David Bremner @ 2023-09-15 12:50 UTC (permalink / raw) To: notmuch The function _notmuch_config_load_from_file is only called in two places in open.cc. Update internal API to match the idiom in open.cc. Adding a newline is needed for consistency with other status strings. Based in part on a patch [1] from Eric Blake. [1]: id:20230906153402.101471-1-eblake@redhat.com --- lib/config.cc | 13 +++++++++++-- lib/notmuch-private.h | 2 +- lib/open.cc | 4 ++-- notmuch.c | 6 ++++++ test/T030-config.sh | 1 - 5 files changed, 20 insertions(+), 6 deletions(-) diff --git a/lib/config.cc b/lib/config.cc index 2323860d..6cd15fab 100644 --- a/lib/config.cc +++ b/lib/config.cc @@ -416,7 +416,8 @@ _expand_path (void *ctx, const char *key, const char *val) notmuch_status_t _notmuch_config_load_from_file (notmuch_database_t *notmuch, - GKeyFile *file) + GKeyFile *file, + char **status_string) { notmuch_status_t status = NOTMUCH_STATUS_SUCCESS; gchar **groups = NULL, **keys, *val; @@ -435,6 +436,7 @@ _notmuch_config_load_from_file (notmuch_database_t *notmuch, for (gchar **keys_p = keys; *keys_p; keys_p++) { char *absolute_key = talloc_asprintf (notmuch, "%s.%s", *grp, *keys_p); char *normalized_val; + GError *gerr = NULL; /* If we opened from a given path, do not overwrite it */ if (strcmp (absolute_key, "database.path") == 0 && @@ -442,7 +444,14 @@ _notmuch_config_load_from_file (notmuch_database_t *notmuch, notmuch->xapian_db) continue; - val = g_key_file_get_string (file, *grp, *keys_p, NULL); + val = g_key_file_get_string (file, *grp, *keys_p, &gerr); + if (gerr) { + if (status_string) + IGNORE_RESULT (asprintf (status_string, + "GLib: %s\n", + gerr->message)); + g_error_free (gerr); + } if (! val) { status = NOTMUCH_STATUS_FILE_ERROR; goto DONE; diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h index c19ee8e2..367e23e6 100644 --- a/lib/notmuch-private.h +++ b/lib/notmuch-private.h @@ -726,7 +726,7 @@ notmuch_status_t _notmuch_config_load_from_database (notmuch_database_t *db); notmuch_status_t -_notmuch_config_load_from_file (notmuch_database_t *db, GKeyFile *file); +_notmuch_config_load_from_file (notmuch_database_t *db, GKeyFile *file, char **status_string); notmuch_status_t _notmuch_config_load_defaults (notmuch_database_t *db); diff --git a/lib/open.cc b/lib/open.cc index 54d1faf3..005872dc 100644 --- a/lib/open.cc +++ b/lib/open.cc @@ -555,7 +555,7 @@ _finish_open (notmuch_database_t *notmuch, goto DONE; if (key_file) - status = _notmuch_config_load_from_file (notmuch, key_file); + status = _notmuch_config_load_from_file (notmuch, key_file, &message); if (status) goto DONE; @@ -961,7 +961,7 @@ notmuch_database_load_config (const char *database_path, } if (key_file) { - status = _notmuch_config_load_from_file (notmuch, key_file); + status = _notmuch_config_load_from_file (notmuch, key_file, &message); if (status) goto DONE; } diff --git a/notmuch.c b/notmuch.c index 69a18131..814b9e42 100644 --- a/notmuch.c +++ b/notmuch.c @@ -563,6 +563,12 @@ main (int argc, char *argv[]) NULL, ¬much, &status_string); + if (status_string) { + fputs (status_string, stderr); + free (status_string); + status_string = NULL; + } + switch (status) { case NOTMUCH_STATUS_NO_CONFIG: if (! (command->mode & NOTMUCH_COMMAND_CONFIG_CREATE)) { diff --git a/test/T030-config.sh b/test/T030-config.sh index 11428e8a..8aee9dc9 100755 --- a/test/T030-config.sh +++ b/test/T030-config.sh @@ -201,7 +201,6 @@ printf '[query]\nq3=from:\xff\n' >>bad-config test_expect_code 1 "notmuch --config=./bad-config config list" test_begin_subtest "Specific error message about bad utf8" -test_subtest_known_broken notmuch --config=./bad-config config list 2>ERRORS cat <<EOF > EXPECTED GLib: Key file contains key “q3” with value “from:�” which is not UTF-8 -- 2.40.1 \r ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: Report GLib errors 2023-09-15 12:50 Report GLib errors David Bremner ` (3 preceding siblings ...) 2023-09-15 12:50 ` [PATCH 4/4] Pass error message from GLib ini parser to CLI David Bremner @ 2023-09-23 11:47 ` David Bremner 4 siblings, 0 replies; 6+ messages in thread From: David Bremner @ 2023-09-23 11:47 UTC (permalink / raw) To: notmuch David Bremner <david@tethera.net> writes: > This series obsoletes the WIP series [0]. I think it's probably worth > doing a point release with this fix as it is not too intrusive, and > upcoming GLib releases will likely cause more users to experience the > problems Eric found [1]. > > [0]: id:20230913005636.135665-1-david@tethera.net > [1]: id:5a7paaqa2dvdo5lmnxvaeacfwhdytfnkr4gfh6mtlotdviki2s@ro4gz4m2aqsw Series applied (with a couple of whitespace and commit message fixups) to release and master. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-09-23 11:47 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-09-15 12:50 Report GLib errors David Bremner 2023-09-15 12:50 ` [PATCH 1/4] test: add known broken test for bad utf8 in config David Bremner 2023-09-15 12:50 ` [PATCH 2/4] CLI: exit with error when load_config returns an error David Bremner 2023-09-15 12:50 ` [PATCH 3/4] test: add known broken subtest for the bad config error message David Bremner 2023-09-15 12:50 ` [PATCH 4/4] Pass error message from GLib ini parser to CLI David Bremner 2023-09-23 11:47 ` Report GLib errors David Bremner
Code repositories for project(s) associated with this public inbox https://yhetil.org/notmuch.git/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).