Bug 6392

> defined(%hash) is deprecated at ../blib/lib/Mail/SpamAssassin/Dns.pm line 757.
> (Maybe you should just omit the defined()?)

A bug indeed.

trunk:
  defined(%hash) is not the way to test a hash for being empty
Sending lib/Mail/SpamAssassin/Dns.pm
Committed revision 928958.

Hi, 5.12 will be out within the next 30 days most likely. The fix is to replace all occurrences of:

defined %

with

%

I believe this is considered the most efficient check for if a hash is empty

sorry, I missed the commit in this ticket. looks good.

> The fix is to replace all occurrences
> sorry, I missed the commit in this ticket. looks good.

I believe that was the only one. Thanks for spotting it.

But I found a similar case in rulesrc/sandbox/jm/EmailBL.pm
-    return 0 unless defined @$emails;
+    return 0 unless @$emails;

Also found two cases of 'if/unless keys %hash', which is suboptimal.
Under perl 5.12 the 'if/unless %hash' supposedly runs much faster
(according to release notes), semantics is the same.


trunk:
  Bug 6392: Test suite fails with perl 5.12.0;
  some more cleanups according to Bug 6392 comment 4
Sending lib/Mail/SpamAssassin/HTML.pm
Sending lib/Mail/SpamAssassin/Plugin/BodyEval.pm
Sending rulesrc/sandbox/jm/EmailBL.pm
Committed revision 929084.


The goto into a construct still needs fixing,
then all of this is to be backported to 3.3.2.

Actually it looks like there's more than that I found doing the test suite from CPAN last night. Should I supply a patch from there? or is there a better source for me to do so?

I'm guess that this link should build as a dev version of the CPAN module?

http://svn.apache.org/viewvc/spamassassin/trunk/

> Actually it looks like there's more than that I found doing the test suite from
> CPAN last night. Should I supply a patch from there? or is there a better
> source for me to do so?

A problem description would do, a patch is even better.
Either against 3.3.1, or better still, against trunk.

> I'm guess that this link should build as a dev version of the CPAN module?
> http://svn.apache.org/viewvc/spamassassin/trunk/

My preferred choice is just to fetch/update directly from svn:
  $ svn checkout https://svn.apache.org/repos/asf/spamassassin/trunk
Build/make within such directory should work just fine
(perl Makefile.PL; make; make test; make install).

Created attachment 4726 [details]
avoid goto run_compiled_method in Plugin::Check

Proposed patch to fix 'Use of "goto" to jump into a construct is deprecated'.

For the sake of patch compactness for review purposes I did not make
indentation change - which would be made when a patch is applied.

trunk:
  Bug 6392: Test suite fails with perl 5.12.0:
  avoid goto run_compiled_method in Plugin::Check, getting rid
  of 'Use of "goto" to jump into a construct is deprecated'
Sending lib/Mail/SpamAssassin/Plugin/Check.pm
Committed revision 929182.

(the r929182 includes indentation change; for easier review
please see the patch attached to previous comment)

It appears we caught all perl 5.12 warnings now (including Bug 6396).

There is another bunch of undef warnings while running a t/dkim2.t
test. I'll send a patch to the author of a Mail::DKIM module, will
probably end up in Mail-DKIM-0.37_8 and 0.38. Should be harmless though,
it's just ugly. I can attach it here if anyone is interested.

Does anyone see anything else? I'm running SpamAssassin trunk
under amavisd-new and Perl 5.12 now on our mailer, looks normal,
and all tests pass cleanly, debug log does not reveal any surprises.

if anyone has copious tuits, it'd be great to install perl 5.12 on hudson-solaris.zones.apache.org so we can run a Hudson job to regularly run the test suite with it.  (we should really add perl 5.10 too while we're at it ;)

*** Bug 6399 has been marked as a duplicate of this bug. ***

Created attachment 4732 [details]
Proposed combo patch for 3.3.

The attached patch combines all changes already committed to trunk
mentioned in this problem report: the 'defined @' and 'defined %'
cases, the goto jump into a construct, unnecessary use of keys()
in a scalar context testing for empty hash.

FYI, Bug 6396 is technically a 5.12 regression too. I believe it's already in trunk.

> FYI, Bug 6396 is technically a 5.12 regression too.
> I believe it's already in trunk.

Yes. It's also already in the 3.3 branch too, so that ticket is
only still left open for considering the Test::NoWarnings.

(In reply to comment #9)
> There is another bunch of undef warnings while running a t/dkim2.t
> test. I'll send a patch to the author of a Mail::DKIM module, will
> probably end up in Mail-DKIM-0.37_8 and 0.38. Should be harmless though,
> it's just ugly.

Now fixed with Mail-DKIM-0.38  (already in FreeBSD ports now).

+1 on 4732

I just ran make on branch 3.3 and got the below errors. Was this missed in trunk? or just not backmerged into 3.3?

rulesrc/sandbox/wtogami/20_vanity.cf: 1 active rules, 22 other
Use of "goto" to jump into a construct is deprecated at lib/Mail/SpamAssassin/Plugin/Check.pm line 409.

> rulesrc/sandbox/wtogami/20_vanity.cf: 1 active rules, 22 other
> Use of "goto" to jump into a construct is deprecated at
> lib/Mail/SpamAssassin/Plugin/Check.pm line 409.

> I just ran make on branch 3.3 and got the below errors.
> Was this missed in trunk?

No, it *is* in the trunk.

> or just not backmerged into 3.3?

Indeed. As the whiteboard on the top of this bug report indicates,
the proposed patch is waiting one more vote for applying to 3.3 branch.

(In reply to comment #18)
> > rulesrc/sandbox/wtogami/20_vanity.cf: 1 active rules, 22 other
> > Use of "goto" to jump into a construct is deprecated at
> > lib/Mail/SpamAssassin/Plugin/Check.pm line 409.
> 
> > I just ran make on branch 3.3 and got the below errors.
> > Was this missed in trunk?
> 
> No, it *is* in the trunk.
> 
> > or just not backmerged into 3.3?
> 
> Indeed. As the whiteboard on the top of this bug report indicates,
> the proposed patch is waiting one more vote for applying to 3.3 branch.

+1 to apply combo patch. KAM

Thanks!


3.3:
  Bug 6392: Test suite fails with perl 5.12.0
Sending lib/Mail/SpamAssassin/Dns.pm
Sending lib/Mail/SpamAssassin/HTML.pm
Sending lib/Mail/SpamAssassin/Plugin/BodyEval.pm
Sending lib/Mail/SpamAssassin/Plugin/Check.pm
Sending rulesrc/sandbox/jm/EmailBL.pm
Committed revision 943933.

*** Bug 6463 has been marked as a duplicate of this bug. ***

*** Bug 6474 has been marked as a duplicate of this bug. ***

has anyone regressed test this on perl 5.8.8 5.8.9, and 5.10?

(In reply to comment #23)
> has anyone regressed test this on perl 5.8.8 5.8.9, and 5.10?

I suggest letting CPAN testers do it for you.

Bump the version to have an underscore and it'll show up on CPAN as a dev version. This would not show up in the cpan client as available for install.

$VERSION = "3.003001_1"

Once it's up there, you would have access to all the various platforms and versions running the test suite: http://www.cpantesters.org/show/Mail-SpamAssassin.html#Mail-SpamAssassin-3.3.1

Created attachment 4805 [details]
Changes made to release a CPAN dev version of SA

I just released 3.3.1_01 to CPAN. This will not be indexed by PAUSE since it's a dev version, however this will give CPAN testers access to it so we can see if there are currently any regressions with the 3.3 branch. I'll post the results in a couple of days.

The link doesn't exist yet but I believe it will show up here:
http://search.cpan.org/~toddr/Mail-SpamAssassin-3.3.1_01/

This link has test reports from the 3.3.2 candidate. So far 5.10 and 5.12 pass without issue.

http://goo.gl/Y3K7

*** Bug 6493 has been marked as a duplicate of this bug. ***

OK, now that I can comment on this bug - please release a new version on CPAN. A bug should not be marked as fixed until it is fixed in a stable release. "It's not over until the fat lady sings.". We don't have to suffer through this warning ever since perl-5.12.x came out on what was invalid syntax.

Please release a new version.

Regards,

-- Shlomi Fish

(In reply to comment #28)
> OK, now that I can comment on this bug - please release a new version on CPAN.

Why would you not have been able to comment on this bug before I closed your duplicate report?

> A bug should not be marked as fixed until it is fixed in a stable release.

Incorrect. As I just explained to you, Status RESOLVED FIXED means that the issue has been fixed in SVN. Status CLOSED would indicate a release, but we don't usually use anything beyond RESOLVED in this bug tracker.

(In reply to comment #23)
> has anyone regressed test this on perl 5.8.8 5.8.9, and 5.10?

So far 5.10 and 5.12 are relativeley green per CPAN testers. There seems to be a limited amount of 5.8 and below automated testers these days.

http://www.cpantesters.org/distro/M/Mail-SpamAssassin.html#Mail-SpamAssassin-3.3.1_01?grade=1&perlmat=1&patches=1&oncpan=2&distmat=3&perlver=ALL&osname=ALL&version=3.3.1_01

(In reply to comment #30)
> (In reply to comment #23)
> > has anyone regressed test this on perl 5.8.8 5.8.9, and 5.10?
> 
> So far 5.10 and 5.12 are relativeley green per CPAN testers. There seems to be
> a limited amount of 5.8 and below automated testers these days.
> 
> http://www.cpantesters.org/distro/M/Mail-SpamAssassin.html#Mail-SpamAssassin-3.3.1_01?grade=1&perlmat=1&patches=1&oncpan=2&distmat=3&perlver=ALL&osname=ALL&version=3.3.1_01

us FreeBsd guys seem to be on the ball :-)

I'm really getting sick of seeing these messages in my logs, especially when they have been fixed in the mainline trunk.  Please push out version 3.3.2 to CPAN so this lameness may disappear from everyone's syslog.

[This is a follow-up request.  Such was originally requested in July 2010, yet no one has done it.]

(In reply to comment #32)
> I'm really getting sick of seeing these messages in my logs, especially when
> they have been fixed in the mainline trunk.  Please push out version 3.3.2 to
> CPAN so this lameness may disappear from everyone's syslog.
> 
> [This is a follow-up request.  Such was originally requested in July 2010, yet
> no one has done it.]

I second that.

Regards,

-- Shlomi Fish

I think it would be helpful for us to know what's holding up the release of 3.3.2 from the 3.3 branch (not trunk). From what I can tell, no commits have gone into it since July 2010

http://svn.apache.org/repos/asf/spamassassin/branches/3.3

trunk:
  Bug 6392: fix one more case of a 'goto into a construct',
  this one occured with sa-compile
Sending lib/Mail/SpamAssassin/Plugin/BodyRuleBaseExtractor.pm
Committed revision 1027163.

Created attachment 4815 [details]
a backport of  trunk's r1027163 applicable to 3.2

The attachment is a backported BodyRuleBaseExtractor.pm patch r1027163
from trunk, applicable to 3.2 and proposed for inclusion into 3.2.
A corresponding indentation change makes the patch appear more
obfuscated than it really is.

As this 'goto into a construct' is just another instance
of similar cases already covered and fixed here, I'm just
reopening it - I hope it's alright.

one more case of a 'goto into a construct' to be fixed

> Created an attachment (id=4815) [details]
> a backport of  trunk's r1027163 applicable to 3.2

Oops, sorry, that was indeed for 3.2, please disregards the attached patch.
Will provide one for 3.3.

Created attachment 4816 [details]
the r1027163 applicable to 3.3

Ok, here is the correct one for 3.3.
In fact the patch for 3.3 is identical to trunk's r1027163.

Still waiting for you all to push SA 3.3.2 out to CPAN so that EVERYONE can eliminate these warnings....

Hi all,

please release a new version already.

Regards,

-- Shlomi Fish

Yet another 3 months later....  What's the cause of the delay?

(In reply to comment #35)
> trunk:
>   Bug 6392: fix one more case of a 'goto into a construct',
>   this one occured with sa-compile
> Sending lib/Mail/SpamAssassin/Plugin/BodyRuleBaseExtractor.pm
> Committed revision 1027163.

Applying this one to 3.3 as well:

Sending lib/Mail/SpamAssassin/Plugin/BodyRuleBaseExtractor.pm
Committed revision 1099478.

I think we have a go on this one, all 'goto' fixes are now backported
from trunk to 3.3.  Closing.

*** Bug 6590 has been marked as a duplicate of this bug. ***

Hi all.

I appreciate the patches, even that will save me the trouble of
patching it myself, but a new CPAN release would be very useful.

Any idea when that should be expected, and/or what the holdup is?

thanks

-- frosty

> I appreciate the patches, even that will save me the trouble of
> patching it myself, but a new CPAN release would be very useful.
> 
> Any idea when that should be expected, and/or what the holdup is?

3.3.2 has received enough votes to be released.  I have currently uploaded a trial version to CPAN at http://cpan.perl.org/authors/id/K/KM/KMCGRAIL/Mail-SpamAssassin-3.3.2-TRIAL.tar.gz

We are working on all the rest of the bits of the release process.

Regards,
KAM