Re: notmuch breaks on \. in config file with upgrade from glib2 2.76.1 to 2.76.5
On Sun, Sep 10, 2023 at 08:58:39AM -0300, David Bremner wrote: > David Bremner writes: > > > I usually like to start with a failing test, but it seems that may not > > be possible here, since the actual failure only happens with specific > > versions of glib. > > I guess the more interesting issue is making sure we propagate parsing > errors from glib properly. Do you have an example of an init file syntax > error that does consistently trigger a NULL return value (i.e. not bad > escape, since that is apparently in flux). Sure - put in some invalid UTF-8 (unlike invalid escape sequences changing behavior over time, invalid UTF-8 has always been invalid in g_key_file_get_string). For example, printf 'q3=from:\xff\n' >> .notmuch-config into a config file that previously contains valid UTF-8 and ends in a [query] block. Even with glib2-2.76.1 installed, I'm back to the scenario where 'notmuch --config=.notmuch-config config list' outputs nothing with exit status 0 (when it SHOULD have been reporting glib's error, "Key file contains key “%s” with value “%s” which is not UTF-8"). -- Eric Blake, Principal Software Engineer Red Hat, Inc. Virtualization: qemu.org | libguestfs.org ___ notmuch mailing list -- [email protected] To unsubscribe send an email to [email protected]
Re: notmuch breaks on \. in config file with upgrade from glib2 2.76.1 to 2.76.5
David Bremner writes: > I usually like to start with a failing test, but it seems that may not > be possible here, since the actual failure only happens with specific > versions of glib. I guess the more interesting issue is making sure we propagate parsing errors from glib properly. Do you have an example of an init file syntax error that does consistently trigger a NULL return value (i.e. not bad escape, since that is apparently in flux). d ___ notmuch mailing list -- [email protected] To unsubscribe send an email to [email protected]
Re: notmuch breaks on \. in config file with upgrade from glib2 2.76.1 to 2.76.5
Eric Blake writes: > $ tail -n3 .notmuch-config > [query] > q1=from:/.*\\.example\\.org/ > q2=from:"/.*\.example\.org/" > glib2-2.76.5-1.fc38.x86_64 > glib2-2.76.5-1.fc38.i686 > $ notmuch --config=$PWD/.notmuch-config config list > $ echo $? > 0 > $ notmuch --config=$PWD/.notmuch-config count > $ echo $? > 1 I could not duplicate the glib issue with glib 2.77.2 on Debian. Looking at the source, it looks like we don't have 71b7efd08a1fe (or equivalent). ╰─ (git)-[release]-% tail -3 bad-config [query] q1=from:/.*\\.example\\.org/ q2=from:"/.*\.example\.org/" ╭─ motzkin:upstream/notmuch/test/tmp.T030-config ╰─ (git)-[release]-% ../../notmuch --config=./bad-config count 52 ╭─ motzkin:upstream/notmuch/test/tmp.T030-config ╰─ (git)-[release]-% echo $? 0 I usually like to start with a failing test, but it seems that may not be possible here, since the actual failure only happens with specific versions of glib. From the discussion surrounding the revert / fix, it seems like the behaviour of setting the GError, but also returning a string is itself an error and can be expected to change. The assumption in the notmuch code [1] is also that the status string only needs to be checked on non-success status code. It should be OK to check the string anyway, but I guess there might be some bugs uncovered. [1]: notmuch.c, around line 547 ___ notmuch mailing list -- [email protected] To unsubscribe send an email to [email protected]
Re: notmuch breaks on \. in config file with upgrade from glib2 2.76.1 to 2.76.5
On Wed, Sep 06, 2023 at 04:37:57PM +0200, Michael J Gruber wrote: > Hi there > > [snip] > > Last night, I filed > > https://bugzilla.redhat.com/show_bug.cgi?id=2237562. Later, I found > > this about glib 2.77 introducing regressions: > > https://bugzilla.redhat.com/show_bug.cgi?id=2225257; looks like Fedora > > backported enough of that into 2.76.5 to cause similar issues in > > relation to 2.76.1, even though a patchlevel release of glib shouldn't > > be changing behaviors. > > Fedora has no related patches in 2.76.5-1 at all (only hmac/eperm). > So, if that's the same regression as in 2.77 it was introduced > earlier, and purely in upstream. In the meantime, I've pinpointed where the problem was introduced in glib (71b7efd08a on mainline was backported as 3fd1b63453 on glib-2-76 just before 2.76.5 was cut); and they have a patch pending to fix it which I have now tested: https://gitlab.gnome.org/GNOME/glib/-/merge_requests/3565 > > > I presume that 'notmuch config set' should be the preferred way to > > modify the config file - but since it IS a human-readable file, > > notmuch should do a much better job of reporting errors whenever > > glib's gkeyfile API cannot parse the file (even if that failure to > > parse is caused by an unintended regression in glib behavior for > > rejecting something it used to accept). > > Yes. This round of glib2 gave us quite some headaches to get config > back working at all. The typical response from glib2 upstream was that > what we called regressions were fixes to wrong behaviour in glib2 and > that we should not rely on established behaviour (my words) but only > on the documentation, the latter exposing a sense of humour which I do > appreciate at times. > > In particular, glib2's read and write results are the authoritative > answer. And probably the older glib2 was "wrong" in what it accepted > leniently ... Does notmuch even get the chance to read partially? GError are intended to be used as recoverable errors - we have every right to refactor the logic to ignore keys that are unreadable while still moving forward with the rest of the file, if we want to do partial reads. But that's more invasive than the patch I just proposed, which merely prints the GError message to the user (serves as a warning to a user that their hand-written invalid escapes are being accepted anyways with 2.76.1, and gives the user an explanation why notmuch isn't working with 2.76.5). -- Eric Blake, Principal Software Engineer Red Hat, Inc. Virtualization: qemu.org | libguestfs.org ___ notmuch mailing list -- [email protected] To unsubscribe send an email to [email protected]
Re: notmuch breaks on \. in config file with upgrade from glib2 2.76.1 to 2.76.5
Hi there [snip] > Last night, I filed > https://bugzilla.redhat.com/show_bug.cgi?id=2237562. Later, I found > this about glib 2.77 introducing regressions: > https://bugzilla.redhat.com/show_bug.cgi?id=2225257; looks like Fedora > backported enough of that into 2.76.5 to cause similar issues in > relation to 2.76.1, even though a patchlevel release of glib shouldn't > be changing behaviors. Fedora has no related patches in 2.76.5-1 at all (only hmac/eperm). So, if that's the same regression as in 2.77 it was introduced earlier, and purely in upstream. > I presume that 'notmuch config set' should be the preferred way to > modify the config file - but since it IS a human-readable file, > notmuch should do a much better job of reporting errors whenever > glib's gkeyfile API cannot parse the file (even if that failure to > parse is caused by an unintended regression in glib behavior for > rejecting something it used to accept). Yes. This round of glib2 gave us quite some headaches to get config back working at all. The typical response from glib2 upstream was that what we called regressions were fixes to wrong behaviour in glib2 and that we should not rely on established behaviour (my words) but only on the documentation, the latter exposing a sense of humour which I do appreciate at times. In particular, glib2's read and write results are the authoritative answer. And probably the older glib2 was "wrong" in what it accepted leniently ... Does notmuch even get the chance to read partially? Michael ___ notmuch mailing list -- [email protected] To unsubscribe send an email to [email protected]
