Re: notmuch breaks on \. in config file with upgrade from glib2 2.76.1 to 2.76.5

2023-09-11 Thread Eric Blake
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 -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org


Re: notmuch breaks on \. in config file with upgrade from glib2 2.76.1 to 2.76.5

2023-09-10 Thread David Bremner
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 -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org


Re: notmuch breaks on \. in config file with upgrade from glib2 2.76.1 to 2.76.5

2023-09-09 Thread David Bremner
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 -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org


Re: notmuch breaks on \. in config file with upgrade from glib2 2.76.1 to 2.76.5

2023-09-06 Thread Eric Blake
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 -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org


Re: notmuch breaks on \. in config file with upgrade from glib2 2.76.1 to 2.76.5

2023-09-06 Thread Michael J Gruber
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 -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org