bug#35219: 27.0.50; Problems with nnimap groups with non-ASCII characters

bug#35219: 27.0.50; Problems with nnimap groups with non-ASCII characters

[Lars Ingebrigtsen]

This started showing up yesterday after recent changes in Gnus:

To reproduce: Create an nnimap group called "Tést".  Restarting Gnus
will properly create the group "Tést", but Gnus will also say

nnimap read 12k from quimby.gnus.org (initial sync of 3 groups; please wait)

If you then quit Gnus and restart Gnus, this group will appear:

       *: nnimap+quimby.gnus.org:T\351st

If you try to kill it, all the groups in the buffer will disappear.

So something went wrong during whatever the most recent group-related
changes were.  :-)


In GNU Emacs 27.0.50 (build 3, x86_64-pc-linux-gnu, GTK+ Version 3.22.11)
 of 2019-04-09 built on stories
Repository revision: 44b306d3510e54432b76724583ea9405f1c90686
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.11604000
System Description: Debian GNU/Linux 9 (stretch)


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#35219: 27.0.50; Problems with nnimap groups with non-ASCII characters

[Eric Abrahamsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> This started showing up yesterday after recent changes in Gnus:
>
> To reproduce: Create an nnimap group called "Tést".  Restarting Gnus
> will properly create the group "Tést", but Gnus will also say
>
> nnimap read 12k from quimby.gnus.org (initial sync of 3 groups; please wait)
>
> If you then quit Gnus and restart Gnus, this group will appear:
>
>        *: nnimap+quimby.gnus.org:T\351st
>
> If you try to kill it, all the groups in the buffer will disappear.
>
> So something went wrong during whatever the most recent group-related
> changes were.  :-)

Gaah... I've got a pile of tests in place for exactly this! Give me a
bit and I'll try to reproduce.

bug#35219: 27.0.50; Problems with nnimap groups with non-ASCII characters

[Eric Abrahamsen]

[-- Attachment #1: Type: text/plain, Size: 958 bytes --]

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> Lars Ingebrigtsen <larsi@gnus.org> writes:
>
>> This started showing up yesterday after recent changes in Gnus:
>>
>> To reproduce: Create an nnimap group called "Tést".  Restarting Gnus
>> will properly create the group "Tést", but Gnus will also say
>>
>> nnimap read 12k from quimby.gnus.org (initial sync of 3 groups; please wait)
>>
>> If you then quit Gnus and restart Gnus, this group will appear:
>>
>>        *: nnimap+quimby.gnus.org:T\351st
>>
>> If you try to kill it, all the groups in the buffer will disappear.
>>
>> So something went wrong during whatever the most recent group-related
>> changes were.  :-)
>
> Gaah... I've got a pile of tests in place for exactly this! Give me a
> bit and I'll try to reproduce.

Lars, would you give this a shot? I should only have been messing with
encoding for group names that were coming from symbols.

Thanks,
Eric


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: group-encoding-fix.diff --]
[-- Type: text/x-patch, Size: 862 bytes --]

diff --git a/lisp/gnus/gnus-start.el b/lisp/gnus/gnus-start.el
index 2beb685822..606155d741 100644
--- a/lisp/gnus/gnus-start.el
+++ b/lisp/gnus/gnus-start.el
@@ -2146,14 +2146,14 @@ gnus-active-to-gnus-format
 	    (if (and (stringp (progn
 				(setq group (read cur)
 				      group
-				      (encode-coding-string
-				       (cond ((numberp group)
-					      (number-to-string group))
-					     ((symbolp group)
-					      (symbol-name group))
-					     ((stringp group)
-					      group))
-				       'latin-1))))
+				      (cond ((numberp group)
+					     (number-to-string group))
+					    ((symbolp group)
+					     (encode-coding-string
+					      (symbol-name group)
+					      'latin-1))
+					    ((stringp group)
+					     group)))))
 		     (numberp (setq max (read cur)))
 		     (numberp (setq min (read cur)))
 		     (null (progn

bug#35219: 27.0.50; Problems with nnimap groups with non-ASCII characters

[Eric Abrahamsen]

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> Lars Ingebrigtsen <larsi@gnus.org> writes:
>>
>>> This started showing up yesterday after recent changes in Gnus:
>>>
>>> To reproduce: Create an nnimap group called "Tést".  Restarting Gnus
>>> will properly create the group "Tést", but Gnus will also say
>>>
>>> nnimap read 12k from quimby.gnus.org (initial sync of 3 groups; please wait)
>>>
>>> If you then quit Gnus and restart Gnus, this group will appear:
>>>
>>>        *: nnimap+quimby.gnus.org:T\351st
>>>
>>> If you try to kill it, all the groups in the buffer will disappear.
>>>
>>> So something went wrong during whatever the most recent group-related
>>> changes were.  :-)
>>
>> Gaah... I've got a pile of tests in place for exactly this! Give me a
>> bit and I'll try to reproduce.
>
> Lars, would you give this a shot? I should only have been messing with
> encoding for group names that were coming from symbols.

This may not be the full solution (though I think it is), but it's
definitely a necessary part. I'm going to push this first so more people
don't end up with corrupted .newsrc.eld files, and then see where we're at.

bug#35219: 27.0.50; Problems with nnimap groups with non-ASCII characters

[Andy Moreton]

On Thu 18 Apr 2019, Eric Abrahamsen wrote:

> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>>
>>> Lars Ingebrigtsen <larsi@gnus.org> writes:
>>>
>>>> This started showing up yesterday after recent changes in Gnus:
>>>>
>>>> To reproduce: Create an nnimap group called "Tést".  Restarting Gnus
>>>> will properly create the group "Tést", but Gnus will also say
>>>>
>>>> nnimap read 12k from quimby.gnus.org (initial sync of 3 groups; please wait)
>>>>
>>>> If you then quit Gnus and restart Gnus, this group will appear:
>>>>
>>>>        *: nnimap+quimby.gnus.org:T\351st
>>>>
>>>> If you try to kill it, all the groups in the buffer will disappear.
>>>>
>>>> So something went wrong during whatever the most recent group-related
>>>> changes were.  :-)
>>>
>>> Gaah... I've got a pile of tests in place for exactly this! Give me a
>>> bit and I'll try to reproduce.
>>
>> Lars, would you give this a shot? I should only have been messing with
>> encoding for group names that were coming from symbols.
>
> This may not be the full solution (though I think it is), but it's
> definitely a necessary part. I'm going to push this first so more people
> don't end up with corrupted .newsrc.eld files, and then see where we're at.

I see a similar symptom, but with a different recipe:
 - start Gnus
 - open the server buffer, select a server, and subscribe to a new group
 - quit the server buffer
 - in the group buffer, kill the group line for the new group
At this point, emacs is busy but unresponsive. Breaking in with ^G
results in emacs becoming responsive agin, but all of the group lines
disappear from the group buffer.

Something is still not quite right.

    AndyM

bug#35219: 27.0.50; Problems with nnimap groups with non-ASCII characters

[Eric Abrahamsen]

Andy Moreton <andrewjmoreton@gmail.com> writes:

> On Thu 18 Apr 2019, Eric Abrahamsen wrote:
>
>> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>>
>>> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>>>
>>>> Lars Ingebrigtsen <larsi@gnus.org> writes:
>>>>
>>>>> This started showing up yesterday after recent changes in Gnus:
>>>>>
>>>>> To reproduce: Create an nnimap group called "Tést".  Restarting Gnus
>>>>> will properly create the group "Tést", but Gnus will also say
>>>>>
>>>>> nnimap read 12k from quimby.gnus.org (initial sync of 3 groups; please wait)
>>>>>
>>>>> If you then quit Gnus and restart Gnus, this group will appear:
>>>>>
>>>>>        *: nnimap+quimby.gnus.org:T\351st
>>>>>
>>>>> If you try to kill it, all the groups in the buffer will disappear.
>>>>>
>>>>> So something went wrong during whatever the most recent group-related
>>>>> changes were.  :-)
>>>>
>>>> Gaah... I've got a pile of tests in place for exactly this! Give me a
>>>> bit and I'll try to reproduce.
>>>
>>> Lars, would you give this a shot? I should only have been messing with
>>> encoding for group names that were coming from symbols.
>>
>> This may not be the full solution (though I think it is), but it's
>> definitely a necessary part. I'm going to push this first so more people
>> don't end up with corrupted .newsrc.eld files, and then see where we're at.
>
> I see a similar symptom, but with a different recipe:
>  - start Gnus
>  - open the server buffer, select a server, and subscribe to a new group
>  - quit the server buffer
>  - in the group buffer, kill the group line for the new group
> At this point, emacs is busy but unresponsive. Breaking in with ^G
> results in emacs becoming responsive agin, but all of the group lines
> disappear from the group buffer.
>
> Something is still not quite right.

I'm not able to reproduce that, using a nntp group from gmane -- can you
give me more detail about how you trigger it?

Unfortunately, the last round of changes might have left some cruft in
your .newsrc.eld file, if you had groups with non-ascii names. If you've
got some of that cruft in your group list, I suppose it's possible that
trying to kill a group might enter some sort of loop...

bug#35219: 27.0.50; Problems with nnimap groups with non-ASCII characters

[Andy Moreton]

On Thu 18 Apr 2019, Eric Abrahamsen wrote:

> Andy Moreton <andrewjmoreton@gmail.com> writes:
>> I see a similar symptom, but with a different recipe:
>>  - start Gnus
>>  - open the server buffer, select a server, and subscribe to a new group
>>  - quit the server buffer
>>  - in the group buffer, kill the group line for the new group
>> At this point, emacs is busy but unresponsive. Breaking in with ^G
>> results in emacs becoming responsive agin, but all of the group lines
>> disappear from the group buffer.
>>
>> Something is still not quite right.
>
> I'm not able to reproduce that, using a nntp group from gmane -- can you
> give me more detail about how you trigger it?

I used an nntp group from gmane to check this. After hitting ^g in the
recipe above, emacs shows the last 3 lines of the original group buffer,
and typing 'g' toudpate the group buffer restores the display to all of
the missing groups.

After more testing, it seems that this wrong display depends on using
topics in the group buffer. If I toggle topics off ('t' in the group
buffer) then killing the newly added group appears to work normally.

With topics enabled, adding a new group and then saving .newsrc.eld ('s'
in the group buffer) still results in the bad behaviour when killing the
new group.

Killing/yanking a group that was already in .newsrc.eld before gnus was
started works normally.

I've only checked this with my usual config though, rather than
something stripped down to produce a minimal test case.

> Unfortunately, the last round of changes might have left some cruft in
> your .newsrc.eld file, if you had groups with non-ascii names. If you've
> got some of that cruft in your group list, I suppose it's possible that
> trying to kill a group might enter some sort of loop...

All of the groups in my .newsrc.eld have ASCII names.

    AndyM

bug#35219: 27.0.50; Problems with nnimap groups with non-ASCII characters

[Eric Abrahamsen]

[-- Attachment #1: Type: text/plain, Size: 3052 bytes --]

Andy Moreton <andrewjmoreton@gmail.com> writes:

> On Thu 18 Apr 2019, Eric Abrahamsen wrote:
>
>> Andy Moreton <andrewjmoreton@gmail.com> writes:
>>> I see a similar symptom, but with a different recipe:
>>>  - start Gnus
>>>  - open the server buffer, select a server, and subscribe to a new group
>>>  - quit the server buffer
>>>  - in the group buffer, kill the group line for the new group
>>> At this point, emacs is busy but unresponsive. Breaking in with ^G
>>> results in emacs becoming responsive agin, but all of the group lines
>>> disappear from the group buffer.
>>>
>>> Something is still not quite right.
>>
>> I'm not able to reproduce that, using a nntp group from gmane -- can you
>> give me more detail about how you trigger it?
>
> I used an nntp group from gmane to check this. After hitting ^g in the
> recipe above, emacs shows the last 3 lines of the original group buffer,
> and typing 'g' toudpate the group buffer restores the display to all of
> the missing groups.
>
> After more testing, it seems that this wrong display depends on using
> topics in the group buffer. If I toggle topics off ('t' in the group
> buffer) then killing the newly added group appears to work normally.
>
> With topics enabled, adding a new group and then saving .newsrc.eld ('s'
> in the group buffer) still results in the bad behaviour when killing the
> new group.

Okay, this was a fairly intense deep dive! Thanks for the report. It
looks like the flurry of fixes over the past week or so also introduced
some new bugs.

The infloop comes from `gnus-group-goto-group' returning 1 instead of
nil when it can't find a group. So `while' loops in gnus-topic.el never
exit. The attached patch should fix it.

Yamaoka-san, this would revert some of your changes. I think this is
correct because the original `gnus-group-goto-group' function did not
actually check that a group exists in the newsrc or active hashtables:
it used `gnus-intern-safe' which _always returned the group name_, no
matter what. In fact, I don't know why the original function even
bothered checking that, but the point is that the goto function would
always have a group name to work with.

With the attached change, Gnus' original behavior is restored for me,
including the fact that `gnus-group-jump-to-group' will jump to
non-existent groups (creating them in the process).

> Killing/yanking a group that was already in.newsrc.eld before gnus was
> started works normally.
>
> I've only checked this with my usual config though, rather than
> something stripped down to produce a minimal test case.

FWIW, I have a package in ELPA called gnus-mock that I'm using for
development: it's a trashable-and-restorable Gnus installation with
dummy data that you can use to play with Gnus changes. I'm not expecting
anyone else to go to the trouble of using this to report bugs, but it is
there. I'm working on a semi-interactive set of ERT tests that are meant
to run while Gnus is open, and ensure that point ends up in the right
place after various commands, etc.

Eric

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: goto-group-fix.diff --]
[-- Type: text/x-patch, Size: 2476 bytes --]

diff --git a/lisp/gnus/gnus-group.el b/lisp/gnus/gnus-group.el
index b1e4091c97..c757c82fbc 100644
--- a/lisp/gnus/gnus-group.el
+++ b/lisp/gnus/gnus-group.el
@@ -2549,37 +2549,33 @@ gnus-group-jump-to-group
     (gnus-group-position-point)))
 
 (defun gnus-group-goto-group (group &optional far test-marked)
-  "Goto to newsgroup GROUP.
+  "Go to newsgroup GROUP.
 If FAR, it is likely that the group is not on the current line.
-If TEST-MARKED, the line must be marked."
+If TEST-MARKED, the line must be marked.
+
+Return nil if GROUP is not found."
   (when group
-    (let ((start (point))
-	  (active (and (or
-                        ;; Some kind of group may be only there.
-                        (gnus-active group)
-                        ;; All groups (but with exception) are there.
-                        (gnus-group-entry group))
-		       group)))
+    (let ((start (point)))
       (beginning-of-line)
       (cond
        ;; It's quite likely that we are on the right line, so
        ;; we check the current line first.
        ((and (not far)
-	     (equal (get-text-property (point) 'gnus-group) active)
+	     (equal (get-text-property (point) 'gnus-group) group)
 	     (or (not test-marked) (gnus-group-mark-line-p)))
 	(point))
        ;; Previous and next line are also likely, so we check them as well.
        ((and (not far)
 	     (save-excursion
 	       (forward-line -1)
-	       (and (equal (get-text-property (point) 'gnus-group) active)
+	       (and (equal (get-text-property (point) 'gnus-group) group)
 		    (or (not test-marked) (gnus-group-mark-line-p)))))
 	(forward-line -1)
 	(point))
        ((and (not far)
 	     (save-excursion
 	       (forward-line 1)
-	       (and (equal (get-text-property (point) 'gnus-group) active)
+	       (and (equal (get-text-property (point) 'gnus-group) group)
 		    (or (not test-marked) (gnus-group-mark-line-p)))))
 	(forward-line 1)
 	(point))
@@ -2588,7 +2584,7 @@ gnus-group-goto-group
 	(let (found)
 	  (while (and (not found)
 		      (gnus-text-property-search
-		       'gnus-group active 'forward 'goto))
+		       'gnus-group group 'forward 'goto))
 	    (if (gnus-group-mark-line-p)
 		(setq found t)
 	      (forward-line 1)))
@@ -2596,7 +2592,7 @@ gnus-group-goto-group
        (t
 	;; Search through the entire buffer.
 	(if (gnus-text-property-search
-	     'gnus-group active nil 'goto)
+	     'gnus-group group nil 'goto)
 	    (point)
 	  (goto-char start)
 	  nil))))))

bug#35219: 27.0.50; Problems with nnimap groups with non-ASCII characters

[Katsumi Yamaoka]

On Thu, 18 Apr 2019 16:53:54 -0700, Eric Abrahamsen wrote:
>>> Andy Moreton <andrewjmoreton@gmail.com> writes:
>>>> I see a similar symptom, but with a different recipe:
>>>>  - start Gnus
>>>>  - open the server buffer, select a server, and subscribe to a new group
>>>>  - quit the server buffer
>>>>  - in the group buffer, kill the group line for the new group
>>>> At this point, emacs is busy but unresponsive. Breaking in with ^G
>>>> results in emacs becoming responsive agin, but all of the group lines
>>>> disappear from the group buffer.
[...]
>> After more testing, it seems that this wrong display depends on using
>> topics in the group buffer. If I toggle topics off ('t' in the group
>> buffer) then killing the newly added group appears to work normally.
[...]

I found what is happening then, too.  At first such a new group
is registered in only `gnus-newsrc-hashtb', not `gnus-active-hashtb'.
When trying to kill the group in the group buffer of the topic mode,
during the course of the procedures `gnus-group-change-level'
deletes the group from `gnus-newsrc-hashtb', even so
`gnus-group-goto-group' tries to go to the group, and fails.

I also realized what `gnus-group-goto-group' does when the group
is not found in the hash tables is nonsense.

[...]
> Yamaoka-san, this would revert some of your changes.

Not revert but great improve.  What is especially great is that
making `g-g-g-g' needless to refer to the hash tables:

> -    (let ((start (point))
> -	  (active (and (or
> -                        ;; Some kind of group may be only there.
> -                        (gnus-active group)
> -                        ;; All groups (but with exception) are there.
> -                        (gnus-group-entry group))
> -		       group)))
> +    (let ((start (point)))
[...]
>  		      (gnus-text-property-search
> -		       'gnus-group active 'forward 'goto))
> +		       'gnus-group group 'forward 'goto))

Yes, the existing group should be found but nonexistent one
should not be found.

> With the attached change, Gnus' original behavior is restored for me,
> including the fact that `gnus-group-jump-to-group' will jump to
> non-existent groups (creating them in the process).

Wow!  I don't remember it but Gnus in Emacs 26.2 surely does so.
It is probably the right behavior of `gnus-group-jump-to-group':

2011-01-31  Lars Ingebrigtsen  <larsi at gnus.org>

	* gnus-group.el (gnus-group-jump-to-group): Allow jumping to groups
	that Gnus doesn't know exists again.

2011-01-28  Julien Danjou  <julien at danjou.info>

	* gnus-group.el (gnus-group-jump-to-group): Set must match to t.


	Probably Lars or someone made it allow non-existent
	group unconditionally at some time.

	In No Gnus, (gnus-read-active-file-p) was used as the
	must-match flag.


2004-05-21  Lars Magne Ingebrigtsen  <larsi at gnus.org>

	* gnus-group.el (gnus-group-jump-to-group): Don't prompt for
	non-active groups.


Anyway there is no other problem so far, so please push the patch.

Thanks.
Regards,

bug#35219: 27.0.50; Problems with nnimap groups with non-ASCII characters

[Andy Moreton]

On Thu 18 Apr 2019, Eric Abrahamsen wrote:
> Okay, this was a fairly intense deep dive! Thanks for the report. It
> looks like the flurry of fixes over the past week or so also introduced
> some new bugs.
>
> The infloop comes from `gnus-group-goto-group' returning 1 instead of
> nil when it can't find a group. So `while' loops in gnus-topic.el never
> exit. The attached patch should fix it.

Thanks for the speedy fix! The patch fixes the problem for me.

    AndyM

bug#35219: 27.0.50; Problems with nnimap groups with non-ASCII characters

[Eric Abrahamsen]

Katsumi Yamaoka <yamaoka@jpl.org> writes:

> On Thu, 18 Apr 2019 16:53:54 -0700, Eric Abrahamsen wrote:
>>>> Andy Moreton <andrewjmoreton@gmail.com> writes:
>>>>> I see a similar symptom, but with a different recipe:
>>>>>  - start Gnus
>>>>>  - open the server buffer, select a server, and subscribe to a new group
>>>>>  - quit the server buffer
>>>>>  - in the group buffer, kill the group line for the new group
>>>>> At this point, emacs is busy but unresponsive. Breaking in with ^G
>>>>> results in emacs becoming responsive agin, but all of the group lines
>>>>> disappear from the group buffer.
> [...]
>>> After more testing, it seems that this wrong display depends on using
>>> topics in the group buffer. If I toggle topics off ('t' in the group
>>> buffer) then killing the newly added group appears to work normally.
> [...]
>
> I found what is happening then, too.  At first such a new group
> is registered in only `gnus-newsrc-hashtb', not `gnus-active-hashtb'.
> When trying to kill the group in the group buffer of the topic mode,
> during the course of the procedures `gnus-group-change-level'
> deletes the group from `gnus-newsrc-hashtb', even so
> `gnus-group-goto-group' tries to go to the group, and fails.
>
> I also realized what `gnus-group-goto-group' does when the group
> is not found in the hash tables is nonsense.
>
> [...]
>> Yamaoka-san, this would revert some of your changes.
>
> Not revert but great improve.  What is especially great is that
> making `g-g-g-g' needless to refer to the hash tables:

Yes, I like how this is simpler now.

>> - (let ((start (point))
>> -	  (active (and (or
>> -                        ;; Some kind of group may be only there.
>> -                        (gnus-active group)
>> -                        ;; All groups (but with exception) are there.
>> -                        (gnus-group-entry group))
>> -		       group)))
>> +    (let ((start (point)))
> [...]
>>  		      (gnus-text-property-search
>> -		       'gnus-group active 'forward 'goto))
>> +		       'gnus-group group 'forward 'goto))

[...]

>
> Anyway there is no other problem so far, so please push the patch.

Done! Thanks to both of you for testing.

Eric