bug#34974: 27.0.50; Moving article error with duplicate suppression disabled

bug#34974: 27.0.50; Moving article error with duplicate suppression disabled

[Basil L. Contovounesios]

With gnus-suppress-duplicates left at its default value of nil, trying
to move an article with 'B m <group>' gives me the following backtrace:

Debugger entered--Lisp error: (wrong-type-argument hash-table-p nil)
  remhash("<redacted-message-id>" nil)
  gnus-dup-unsuppress-article(1988)
  gnus-summary-move-article(nil)
  funcall-interactively(gnus-summary-move-article nil)
  call-interactively(gnus-summary-move-article nil nil)
  command-execute(gnus-summary-move-article)

I'm no expert, and I haven't tried reproducing this with a minimal
config, but I think gnus-summary-move-article should not call
gnus-dup-unsuppress-article when gnus-suppress-duplicates is nil, right?

This issue seems to have been uncovered by the switch to hash-tables in
bug#33653.  Previously, gnus-dup-unsuppress-article called unintern,
which would not complain when its second argument gnus-dup-hashtb was
nil, even though it probably should have.

Patch to follow.

Thanks,

-- 
Basil

Gnus v5.13
In GNU Emacs 27.0.50 (build 1, x86_64-pc-linux-gnu, X toolkit, Xaw3d scroll bars)
 of 2019-03-24 built on thunk
Repository revision: dbd6490ad49b0f088d56cdd5f04178bdd62c806a
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12003000
System Description: Debian GNU/Linux buster/sid

bug#34974: 27.0.50; Moving article error with duplicate suppression disabled

[Basil L. Contovounesios]

"Basil L. Contovounesios" <contovob@tcd.ie> writes:

> With gnus-suppress-duplicates left at its default value of nil, trying
> to move an article with 'B m <group>' gives me the following backtrace:
>
> Debugger entered--Lisp error: (wrong-type-argument hash-table-p nil)
>   remhash("<redacted-message-id>" nil)
>   gnus-dup-unsuppress-article(1988)
>   gnus-summary-move-article(nil)
>   funcall-interactively(gnus-summary-move-article nil)
>   call-interactively(gnus-summary-move-article nil nil)
>   command-execute(gnus-summary-move-article)
>
> I'm no expert, and I haven't tried reproducing this with a minimal
> config, but I think gnus-summary-move-article should not call
> gnus-dup-unsuppress-article when gnus-suppress-duplicates is nil, right?
>
> This issue seems to have been uncovered by the switch to hash-tables in
> bug#33653.  Previously, gnus-dup-unsuppress-article called unintern,
> which would not complain when its second argument gnus-dup-hashtb was
> nil, even though it probably should have.
>
> Patch to follow.

Sorry, this is a duplicate of bug#34973, which was reported first and
where I've now sent my suggested patch.

I was going to merge the two bug reports, but then I read that the
debbugs 'merge' command requires reports to be assigned to the same
package.

This report was created by the gnus-bug command, which assigned it to
both packages emacs and gnus and version 5.13, whereas the other report
was presumably created by report-emacs-bug and assigned to the emacs
package, version 27.0.50.

So, which package+version combination should Gnus bugs be assigned to?
How should these two reports be merged?  Does merging keep all parties
CCed, or do they have to be CCed anew?

Thanks,

-- 
Basil

bug#34974: 27.0.50; Moving article error with duplicate suppression disabled

[Eric Abrahamsen]

"Basil L. Contovounesios" <contovob@tcd.ie> writes:

> With gnus-suppress-duplicates left at its default value of nil, trying
> to move an article with 'B m <group>' gives me the following backtrace:
>
> Debugger entered--Lisp error: (wrong-type-argument hash-table-p nil)
>   remhash("<redacted-message-id>" nil)
>   gnus-dup-unsuppress-article(1988)
>   gnus-summary-move-article(nil)
>   funcall-interactively(gnus-summary-move-article nil)
>   call-interactively(gnus-summary-move-article nil nil)
>   command-execute(gnus-summary-move-article)
>
> I'm no expert, and I haven't tried reproducing this with a minimal
> config, but I think gnus-summary-move-article should not call
> gnus-dup-unsuppress-article when gnus-suppress-duplicates is nil, right?
>
> This issue seems to have been uncovered by the switch to hash-tables in
> bug#33653.  Previously, gnus-dup-unsuppress-article called unintern,
> which would not complain when its second argument gnus-dup-hashtb was
> nil, even though it probably should have.
>
> Patch to follow.

Thanks, Basil!

It looks like the only other call to `gnus-dup-unsuppress-articles' is
wrapped in a check for `gnus-suppress-duplicates' -- I suppose it would
be enough just to wrap this one as well?

Eric

bug#34974: 27.0.50; Moving article error with duplicate suppression disabled

[Eric Abrahamsen]

"Basil L. Contovounesios" <contovob@tcd.ie> writes:

> "Basil L. Contovounesios" <contovob@tcd.ie> writes:
>
>> With gnus-suppress-duplicates left at its default value of nil, trying
>> to move an article with 'B m <group>' gives me the following backtrace:
>>
>> Debugger entered--Lisp error: (wrong-type-argument hash-table-p nil)
>>   remhash("<redacted-message-id>" nil)
>>   gnus-dup-unsuppress-article(1988)
>>   gnus-summary-move-article(nil)
>>   funcall-interactively(gnus-summary-move-article nil)
>>   call-interactively(gnus-summary-move-article nil nil)
>>   command-execute(gnus-summary-move-article)
>>
>> I'm no expert, and I haven't tried reproducing this with a minimal
>> config, but I think gnus-summary-move-article should not call
>> gnus-dup-unsuppress-article when gnus-suppress-duplicates is nil, right?
>>
>> This issue seems to have been uncovered by the switch to hash-tables in
>> bug#33653.  Previously, gnus-dup-unsuppress-article called unintern,
>> which would not complain when its second argument gnus-dup-hashtb was
>> nil, even though it probably should have.
>>
>> Patch to follow.
>
> Sorry, this is a duplicate of bug#34973, which was reported first and
> where I've now sent my suggested patch.
>
> I was going to merge the two bug reports, but then I read that the
> debbugs 'merge' command requires reports to be assigned to the same
> package.

Bah, and I'd sent in the "merge" command moments before. Trying too hard
to be helpful.

> This report was created by the gnus-bug command, which assigned it to
> both packages emacs and gnus and version 5.13, whereas the other report
> was presumably created by report-emacs-bug and assigned to the emacs
> package, version 27.0.50.
>
> So, which package+version combination should Gnus bugs be assigned to?
> How should these two reports be merged?  Does merging keep all parties
> CCed, or do they have to be CCed anew?
>
> Thanks,

bug#34974: 27.0.50; Moving article error with duplicate suppression disabled

[Basil L. Contovounesios]

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> "Basil L. Contovounesios" <contovob@tcd.ie> writes:
>
>> With gnus-suppress-duplicates left at its default value of nil, trying
>> to move an article with 'B m <group>' gives me the following backtrace:
>>
>> Debugger entered--Lisp error: (wrong-type-argument hash-table-p nil)
>>   remhash("<redacted-message-id>" nil)
>>   gnus-dup-unsuppress-article(1988)
>>   gnus-summary-move-article(nil)
>>   funcall-interactively(gnus-summary-move-article nil)
>>   call-interactively(gnus-summary-move-article nil nil)
>>   command-execute(gnus-summary-move-article)
>>
>> I'm no expert, and I haven't tried reproducing this with a minimal
>> config, but I think gnus-summary-move-article should not call
>> gnus-dup-unsuppress-article when gnus-suppress-duplicates is nil, right?
>>
>> This issue seems to have been uncovered by the switch to hash-tables in
>> bug#33653.  Previously, gnus-dup-unsuppress-article called unintern,
>> which would not complain when its second argument gnus-dup-hashtb was
>> nil, even though it probably should have.
>>
>> Patch to follow.
>
> It looks like the only other call to `gnus-dup-unsuppress-articles' is
> wrapped in a check for `gnus-suppress-duplicates' -- I suppose it would
> be enough just to wrap this one as well?

That is what the proposed patch[1] does.  Is it okay to push this to
master?

[1]: https://debbugs.gnu.org/34973#8

As I mention there, though, I'm not sure such a guard is sufficient in
the case that gnus-suppress-duplicates is enabled at a later stage, but
I will investigate and report this as a separate bug if needed.

Thanks,

-- 
Basil

bug#34974: 27.0.50; Moving article error with duplicate suppression disabled

[Eric Abrahamsen]

On 03/24/19 17:08 PM, Basil L. Contovounesios wrote:
> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> "Basil L. Contovounesios" <contovob@tcd.ie> writes:
>>
>>> With gnus-suppress-duplicates left at its default value of nil, trying
>>> to move an article with 'B m <group>' gives me the following backtrace:
>>>
>>> Debugger entered--Lisp error: (wrong-type-argument hash-table-p nil)
>>>   remhash("<redacted-message-id>" nil)
>>>   gnus-dup-unsuppress-article(1988)
>>>   gnus-summary-move-article(nil)
>>>   funcall-interactively(gnus-summary-move-article nil)
>>>   call-interactively(gnus-summary-move-article nil nil)
>>>   command-execute(gnus-summary-move-article)
>>>
>>> I'm no expert, and I haven't tried reproducing this with a minimal
>>> config, but I think gnus-summary-move-article should not call
>>> gnus-dup-unsuppress-article when gnus-suppress-duplicates is nil, right?
>>>
>>> This issue seems to have been uncovered by the switch to hash-tables in
>>> bug#33653.  Previously, gnus-dup-unsuppress-article called unintern,
>>> which would not complain when its second argument gnus-dup-hashtb was
>>> nil, even though it probably should have.
>>>
>>> Patch to follow.
>>
>> It looks like the only other call to `gnus-dup-unsuppress-articles' is
>> wrapped in a check for `gnus-suppress-duplicates' -- I suppose it would
>> be enough just to wrap this one as well?
>
> That is what the proposed patch[1] does.  Is it okay to push this to
> master?
>
> [1]: https://debbugs.gnu.org/34973#8

Already done! Our messages keep passing each other...

bug#34974: 27.0.50; Moving article error with duplicate suppression disabled

[Basil L. Contovounesios]

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> On 03/24/19 17:08 PM, Basil L. Contovounesios wrote:
>> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>>
>>> It looks like the only other call to `gnus-dup-unsuppress-articles' is
>>> wrapped in a check for `gnus-suppress-duplicates' -- I suppose it would
>>> be enough just to wrap this one as well?
>>
>> That is what the proposed patch[1] does.  Is it okay to push this to
>> master?
>>
>> [1]: https://debbugs.gnu.org/34973#8
>
> Already done! Our messages keep passing each other...

Thanks for looking into this so quickly and working on Gnus in general!
And good luck with the rest of the fallout. ;)

-- 
Basil

bug#34974: 27.0.50; Moving article error with duplicate suppression disabled

[Basil L. Contovounesios]

tags 34974 fixed
close 34974
quit

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> "Basil L. Contovounesios" <contovob@tcd.ie> writes:
>
>> Sorry, this is a duplicate of bug#34973, which was reported first and
>> where I've now sent my suggested patch.
>>
>> I was going to merge the two bug reports, but then I read that the
>> debbugs 'merge' command requires reports to be assigned to the same
>> package.
>
> Bah, and I'd sent in the "merge" command moments before. Trying too hard
> to be helpful.
>
>> This report was created by the gnus-bug command, which assigned it to
>> both packages emacs and gnus and version 5.13, whereas the other report
>> was presumably created by report-emacs-bug and assigned to the emacs
>> package, version 27.0.50.
>>
>> So, which package+version combination should Gnus bugs be assigned to?
>> How should these two reports be merged?  Does merging keep all parties
>> CCed, or do they have to be CCed anew?

I see that Glenn reassigned bug#34973 to both packages emacs and gnus
before merging these two reports, thanks Glenn!

The suggested fix has been merged[1][2], so I'm closing this report.

[1]: https://debbugs.gnu.org/34973#22
[2]: https://debbugs.gnu.org/34974#22

Thanks,

-- 
Basil