T610 failing on Fedora rawhide

T610 failing on Fedora rawhide

[Michael J Gruber]

[-- Attachment #1.1: Type: text/plain, Size: 5642 bytes --]

Hi there,

during my first tests for Python 3.13 (hooray...) I noticed that some tests
in T610 started to fail independent of that. It seems that with notmuch
0.38.1 on current Fedora rawhide,  `notmuch_message_get_properties()`
returns properties in a different order, while the tests expect a specific
order. So I'm wondering:
- Is there a particular order which the interface promises to deliver?
- If yes: What could cause it to be different?
If not there's some work to do making the tests independent of the order ...
This is not glib again, is it?

Cheers,
Michael

https://kojipkgs.fedoraproject.org//work/tasks/3691/109443691/build.log

T610-message-property: Testing message property API
 PASS   notmuch_message_{add,get,remove}_property
 PASS   testing string map binary search (via message properties)
 PASS   notmuch_message_get_properties: empty list
 PASS   notmuch_message_properties: one value
 PASS   notmuch_message_remove_all_properties
 PASS   notmuch_message_properties: multiple values
 FAIL   notmuch_message_properties: prefix
--- T610-message-property.7.EXPECTED 2023-11-23 12:39:05.368791821 +0000
+++ T610-message-property.7.OUTPUT 2023-11-23 12:39:05.370791961 +0000
@@ -1,7 +1,7 @@
== stdout ==
testkey1 = alice
-testkey1 = bob
testkey1 = testvalue2
+testkey1 = bob
testkey3 = alice3
testkey3 = bob3
testkey3 = testvalue3
 PASS   notmuch_message_properties: modify during iteration
 FAIL   dump message properties
--- T610-message-property.9.PROPERTIES 2023-11-23 12:39:05.787821095 +0000
+++ T610-message-property.9.OUTPUT 2023-11-23 12:39:05.789821235 +0000
@@ -1 +1 @@
-#= 4EFC743A.3060609@april.org
fancy%20key%20with%20%c3%a1cc%c3%a8nts=import%20value%20with%20=
testkey1=alice testkey1=bob testkey1=testvalue2 testkey3=alice3
testkey3=bob3 testkey3=testvalue3
+#= 4EFC743A.3060609@april.org
fancy%20key%20with%20%c3%a1cc%c3%a8nts=import%20value%20with%20=
testkey1=testvalue2 testkey1=bob testkey1=alice testkey3=alice3
testkey3=bob3 testkey3=testvalue3
 FAIL   dump _only_ message properties
--- T610-message-property.10.EXPECTED 2023-11-23 12:39:05.815823052 +0000
+++ T610-message-property.10.OUTPUT 2023-11-23 12:39:05.817823191 +0000
@@ -1,2 +1,2 @@
#notmuch-dump batch-tag:3 properties
-#= 4EFC743A.3060609@april.org
fancy%20key%20with%20%c3%a1cc%c3%a8nts=import%20value%20with%20=
testkey1=alice testkey1=bob testkey1=testvalue2 testkey3=alice3
testkey3=bob3 testkey3=testvalue3
+#= 4EFC743A.3060609@april.org
fancy%20key%20with%20%c3%a1cc%c3%a8nts=import%20value%20with%20=
testkey1=testvalue2 testkey1=bob testkey1=alice testkey3=alice3
testkey3=bob3 testkey3=testvalue3
 FAIL   restore missing message property (single line)
--- T610-message-property.11.PROPERTIES 2023-11-23 12:39:06.042838911 +0000
+++ T610-message-property.11.OUTPUT 2023-11-23 12:39:06.043838981 +0000
@@ -1 +1 @@
-#= 4EFC743A.3060609@april.org
fancy%20key%20with%20%c3%a1cc%c3%a8nts=import%20value%20with%20=
testkey1=alice testkey1=bob testkey1=testvalue2 testkey3=alice3
testkey3=bob3 testkey3=testvalue3
+#= 4EFC743A.3060609@april.org
fancy%20key%20with%20%c3%a1cc%c3%a8nts=import%20value%20with%20=
testkey1=testvalue2 testkey1=bob testkey1=alice testkey3=alice3
testkey3=bob3 testkey3=testvalue3
 FAIL   restore missing message property (full dump)
--- T610-message-property.12.PROPERTIES 2023-11-23 12:39:06.276855260 +0000
+++ T610-message-property.12.OUTPUT 2023-11-23 12:39:06.277855330 +0000
@@ -1 +1 @@
-#= 4EFC743A.3060609@april.org
fancy%20key%20with%20%c3%a1cc%c3%a8nts=import%20value%20with%20=
testkey1=alice testkey1=bob testkey1=testvalue2 testkey3=alice3
testkey3=bob3 testkey3=testvalue3
+#= 4EFC743A.3060609@april.org
fancy%20key%20with%20%c3%a1cc%c3%a8nts=import%20value%20with%20=
testkey1=testvalue2 testkey1=bob testkey1=alice testkey3=alice3
testkey3=bob3 testkey3=testvalue3
 FAIL   restore clear extra message property
--- T610-message-property.13.PROPERTIES 2023-11-23 12:39:06.485869862 +0000
+++ T610-message-property.13.OUTPUT 2023-11-23 12:39:06.487870002 +0000
@@ -1 +1 @@
-#= 4EFC743A.3060609@april.org
fancy%20key%20with%20%c3%a1cc%c3%a8nts=import%20value%20with%20=
testkey1=alice testkey1=bob testkey1=testvalue2 testkey3=alice3
testkey3=bob3 testkey3=testvalue3
+#= 4EFC743A.3060609@april.org
fancy%20key%20with%20%c3%a1cc%c3%a8nts=import%20value%20with%20=
testkey1=testvalue2 testkey1=bob testkey1=alice testkey3=alice3
testkey3=bob3 testkey3=testvalue3
 PASS   test 'property:' queries: empty
 PASS   test 'property:' queries: single message
 FAIL   msg.get_property (python)
--- T610-message-property.16.EXPECTED 2023-11-23 12:39:06.601877966 +0000
+++ T610-message-property.16.OUTPUT 2023-11-23 12:39:06.603878106 +0000
@@ -1,2 +1,2 @@
-testkey1 = alice
+testkey1 = testvalue2
testkey3 = alice3
 FAIL   msg.get_properties (python)
--- T610-message-property.17.EXPECTED 2023-11-23 12:39:06.677883276 +0000
+++ T610-message-property.17.OUTPUT 2023-11-23 12:39:06.678883346 +0000
@@ -1,3 +1,3 @@
-testkey1 = alice
-testkey1 = bob
testkey1 = testvalue2
+testkey1 = bob
+testkey1 = alice
 FAIL   msg.get_properties (python, prefix)
--- T610-message-property.18.EXPECTED 2023-11-23 12:39:06.779890402 +0000
+++ T610-message-property.18.OUTPUT 2023-11-23 12:39:06.780890472 +0000
@@ -1,6 +1,6 @@
-testkey1 = alice
-testkey1 = bob
testkey1 = testvalue2
+testkey1 = bob
+testkey1 = alice
testkey3 = alice3
testkey3 = bob3
testkey3 = testvalue3
 PASS   msg.get_properties (python, exact)
 PASS   notmuch_message_remove_all_properties_with_prefix
 PASS   edit property on removed message without uncaught exception
 PASS   remove all properties on removed message without uncaught exception

[-- Attachment #1.2: Type: text/html, Size: 6781 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: T610 failing on Fedora rawhide

[David Bremner]

Michael J Gruber <michaeljgruber+grubix+git@gmail.com> writes:

> during my first tests for Python 3.13 (hooray...) I noticed that some tests
> in T610 started to fail independent of that. It seems that with notmuch
> 0.38.1 on current Fedora rawhide,  `notmuch_message_get_properties()`
> returns properties in a different order, while the tests expect a specific
> order. So I'm wondering:
> - Is there a particular order which the interface promises to deliver?
> - If yes: What could cause it to be different?
> If not there's some work to do making the tests independent of the order ...
> This is not glib again, is it?

The order is the result of sorting the keys (property names) using the
system qsort. So the first is potentially explicable, but the second
suggests a strange (to me) collation order. Do you happen to know the
locale used by these runs?

Ultimately the comparisons are done with strcmp (string-map.c).

Re: T610 failing on Fedora rawhide

[David Bremner]

David Bremner <david@tethera.net> writes:

> Michael J Gruber <michaeljgruber+grubix+git@gmail.com> writes:
>
>> during my first tests for Python 3.13 (hooray...) I noticed that some tests
>> in T610 started to fail independent of that. It seems that with notmuch
>> 0.38.1 on current Fedora rawhide,  `notmuch_message_get_properties()`
>> returns properties in a different order, while the tests expect a specific
>> order. So I'm wondering:
>> - Is there a particular order which the interface promises to deliver?
>> - If yes: What could cause it to be different?
>> If not there's some work to do making the tests independent of the order ...
>> This is not glib again, is it?
>
> The order is the result of sorting the keys (property names) using the
> system qsort. So the first is potentially explicable, but the second
> suggests a strange (to me) collation order. Do you happen to know the
> locale used by these runs?
>
> Ultimately the comparisons are done with strcmp (string-map.c).

OK, I misread the output (there seems to be some confusing line
wrapping). The test with #= is not actually a key, just a line
marker. The underlying problem seems to be the same: qsort behaving
differently, or perhaps (but less likely, I think) different input to
qsort from Xapian.  We could force the output to be stable by sorting on
lexicographic order (include the property value in the comparison).

Re: T610 failing on Fedora rawhide

[David Bremner]

Michael J Gruber <michaeljgruber+grubix+git@gmail.com> writes:

> Hi there,
>
> during my first tests for Python 3.13 (hooray...) I noticed that some tests
> in T610 started to fail independent of that. It seems that with notmuch
> 0.38.1 on current Fedora rawhide,  `notmuch_message_get_properties()`
> returns properties in a different order, while the tests expect a specific
> order. So I'm wondering:
> - Is there a particular order which the interface promises to deliver?
> - If yes: What could cause it to be different?
> If not there's some work to do making the tests independent of the order ...
> This is not glib again, is it?

Can you try the following patch?

diff --git a/lib/string-map.c b/lib/string-map.c
index e3a81b4f..99bc2ea2 100644
--- a/lib/string-map.c
+++ b/lib/string-map.c
@@ -86,10 +86,14 @@ _notmuch_string_map_append (notmuch_string_map_t *map,
 static int
 cmppair (const void *pa, const void *pb)
 {
+    int cmp = 0;
     notmuch_string_pair_t *a = (notmuch_string_pair_t *) pa;
     notmuch_string_pair_t *b = (notmuch_string_pair_t *) pb;

-    return strcmp (a->key, b->key);
+    cmp = strcmp (a->key, b->key);
+    if (cmp == 0)
+       cmp = strcmp (a->value, b->value);
+    return cmp;
 }

 static void

Re: T610 failing on Fedora rawhide

[Florian Weimer]

* David Bremner:

> Michael J Gruber <michaeljgruber+grubix+git@gmail.com> writes:
>
>> during my first tests for Python 3.13 (hooray...) I noticed that some tests
>> in T610 started to fail independent of that. It seems that with notmuch
>> 0.38.1 on current Fedora rawhide,  `notmuch_message_get_properties()`
>> returns properties in a different order, while the tests expect a specific
>> order. So I'm wondering:
>> - Is there a particular order which the interface promises to deliver?
>> - If yes: What could cause it to be different?
>> If not there's some work to do making the tests independent of the order ...
>> This is not glib again, is it?
>
> The order is the result of sorting the keys (property names) using the
> system qsort. So the first is potentially explicable, but the second
> suggests a strange (to me) collation order. Do you happen to know the
> locale used by these runs?
>
> Ultimately the comparisons are done with strcmp (string-map.c).

strcmp doesn't follow the locale.

The qsort in glibc was never a stable sort, but it looked like that in
many cases.  The version currently in Fedora rawhide (from the upcoming
glibc upstream version) shows non-stable sorting behavior in more cases.
The test case uses identical sort keys (testkey1, testkey 3), which is
why its output changes.

Still this is useful feedback.  Maybe our qsort was a mostly stable sort
for so long that it's now part of the interface contract.

Thanks,
Florian

Re: T610 failing on Fedora rawhide

[Michael J Gruber]

[-- Attachment #1.1: Type: text/plain, Size: 1614 bytes --]

Am Fr., 24. Nov. 2023 um 14:57 Uhr schrieb David Bremner <david@tethera.net
>:

> Michael J Gruber <michaeljgruber+grubix+git@gmail.com> writes:
>
> > Hi there,
> >
> > during my first tests for Python 3.13 (hooray...) I noticed that some
> tests
> > in T610 started to fail independent of that. It seems that with notmuch
> > 0.38.1 on current Fedora rawhide,  `notmuch_message_get_properties()`
> > returns properties in a different order, while the tests expect a
> specific
> > order. So I'm wondering:
> > - Is there a particular order which the interface promises to deliver?
> > - If yes: What could cause it to be different?
> > If not there's some work to do making the tests independent of the order
> ...
> > This is not glib again, is it?
>
> Can you try the following patch?
>
> diff --git a/lib/string-map.c b/lib/string-map.c
> index e3a81b4f..99bc2ea2 100644
> --- a/lib/string-map.c
> +++ b/lib/string-map.c
> @@ -86,10 +86,14 @@ _notmuch_string_map_append (notmuch_string_map_t *map,
>  static int
>  cmppair (const void *pa, const void *pb)
>  {
> +    int cmp = 0;
>      notmuch_string_pair_t *a = (notmuch_string_pair_t *) pa;
>      notmuch_string_pair_t *b = (notmuch_string_pair_t *) pb;
>
> -    return strcmp (a->key, b->key);
> +    cmp = strcmp (a->key, b->key);
> +    if (cmp == 0)
> +       cmp = strcmp (a->value, b->value);
> +    return cmp;
>  }
>
>  static void
>

So I guess we did not quite sort completely before, ey? ;-)

That patch makes T610 pass again, thanks. It's a good change independent of
any glibc promises, I think.

Glad we got this sorted out!

Cheers
Michael

[-- Attachment #1.2: Type: text/html, Size: 2299 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]