Bug 3822 - add more useful to text to warning during "make" output if module versions are too low
Summary: add more useful to text to warning during "make" output if module versions ar...
Status: RESOLVED FIXED
Alias: None
Product: Spamassassin
Classification: Unclassified
Component: Documentation (show other bugs)
Version: 3.0.0
Hardware: PC Linux
: P4 normal
Target Milestone: 3.0.3
Assignee: Justin Mason
URL:
Whiteboard:
Keywords:
: 3820 4036 (view as bug list)
Depends on:
Blocks:
 
Reported: 2004-09-25 04:15 UTC by sehh
Modified: 2005-01-12 09:28 UTC (History)
3 users (show)



Attachment Type Modified Status Actions Submitter/CLA Status
Patch File patch None Michael Parker [HasCLA]
proposed patch to fix patch None Daniel Quinlan [HasCLA]

Note You need to log in before you can comment on or make changes to this bug.
Description sehh 2004-09-25 04:15:07 UTC
The documentation of SA v3 says that SA is compatible with at least Perl 5.6.1.

That is not entirely true because SA requires a new version of Net::DNS, which
does not exist in Perl 5.6.1.

In order to run SA under Perl 5.6.1 we have to upgrade the following libraries:

Test-Simple
Test-Harness
Net-DNS


WARNING:
After installing SA on a Perl 5.6.1 system, the admin will NOT get any error
messages or warnings from "perl Makefile.PL" that these upgrades are required.
As a result, ALL NETWORK AND DNS tests will fail to work (for example RBLs will
not work). Running 'spamassassin -D' will show an error that the existing
Net::DNS for perl 5.6.1 is of smaller version than the required one.
Comment 1 Kelsey Cummings 2004-09-27 12:36:25 UTC
Subject: Re:  New: SA is not fully compatible with Perl 5.6.1

> WARNING:
> After installing SA on a Perl 5.6.1 system, the admin will NOT get any error
> messages or warnings from "perl Makefile.PL" that these upgrades are required.
> As a result, ALL NETWORK AND DNS tests will fail to work (for example RBLs will
> not work). Running 'spamassassin -D' will show an error that the existing
> Net::DNS for perl 5.6.1 is of smaller version than the required one.

Sep 27 12:31:04 a spamd[17449]: result: Y 27 -
HTML_30_40,HTML_IMAGE_ONLY_20,HTML_MESSAGE,INFO_TLD,LONGWORDS,MIME_BOUND_DD_DIGITS,MPART_ALT_DIFF,MSGID_FROM_MTA_HEADER,RCVD_HELO_IP_MISMATCH,RCVD_IN_BL_SPAMCOP_NET,RCVD_IN_XBL,RCVD_NUMERIC_HELO,UNIQUE_WORDS,URIBL_AB_SURBL,URIBL_OB_SURBL,URIBL_SBL,URIBL_SC_SURBL,URIBL_WS_SURBL
scantime=0.9,size=4080,mid=<29156170505398.660dwd23994dk@hotmail.com>,autolearn=disabled 

# perl -v
This is perl, v5.6.1 built for i386-linux

I believe you are mistaken; perhaps you are having a platform dependent
problem or a perl build related bug?

The DNS checks are working fine for me with 5.6.1 on Linux and FreeBSD.


Comment 2 Theo Van Dinter 2004-09-27 12:40:15 UTC
"The documentation of SA v3 says that SA is compatible with at least Perl 5.6.1.

That is not entirely true because SA requires a new version of Net::DNS, which
does not exist in Perl 5.6.1."

The document doesn't state that all you need is 5.6.1, it states that you need at least 5.6.1 *AND* the 
other stuff that's listed:

Perl 5.6.1, and its associated modules, are required for SpamAssassin
3.0.0 and later.
[...]
Optional Additional Modules
---------------------------
[...]
  - Net::DNS        (from CPAN)
[...]
    If this is installed and you are using network tests of any variety
    (which is the default), then you need to make sure the Net::DNS
    version is sufficiently up-to-date:

      - version 0.34 or higher on Unix systems
      - version 0.46 or higher on Windows systems


closing as WFM.
Comment 3 Daniel Quinlan 2004-09-27 12:46:30 UTC
I think we could improve how INSTALL specifies this.  I will work on this.
Comment 4 Daniel Quinlan 2004-09-27 16:59:04 UTC
I checked in somewhat clearer documentation in INSTALL.

I think we might want to consider warning at install time since people install
from CPAN so I've changed the Summary accordingly.  Just a warning, though.
Comment 5 Theo Van Dinter 2004-09-27 19:20:38 UTC
I don't know how easy or difficult it is to do in Makefile.PL, but it would be pretty trivial for a general 
test.
Comment 6 Malte S. Stretz 2004-09-29 15:50:18 UTC
Will do the warn part in Makefile.PL 
Comment 7 Michael Parker 2004-10-12 22:17:24 UTC
Created attachment 2446 [details]
Patch File

Here is one possibility.  It checks to see if you have MIME::Base64, DB_File
and Net::DNS installed.  No specific versions, we should probably do that for
at least Net::DNS.  It just warns you that they aren't installed and suggests
where you might be able to find the modules.

Question, do we want to warn on less or more modules?  Razor? Net::SMTP?
Mail::SPF::Query? IP::Country::Fast? anything else?
Comment 8 Bob Apthorpe 2004-10-12 22:56:15 UTC
Assume the user wants Bayes and network checks, check for modules required for
these options, and list the installation status and minimum version of modules,
whether they're required and optional and what major feature they support.

This should short-circuit the support cycle of "<function> isn't working! How do
you know? <log> You don't have <module> installed. Where does it say I need
that? <reference to unread/broken documentation> Grr!"
Comment 9 Justin Mason 2004-10-12 23:25:49 UTC
hmm.

can we do this *after* Makefile.PL, like at the end of the "make" process? that
would be more "on display" as it's on-screen when the build completes, I think.

also, it should check those modules, plus those required for reporting, and for
default-enabled plugins, and versions, and outputs an informational report and
that "go to CPAN" message.

also, refactoring into a separate script ("build/check_module_deps" or
something?) would seem to be cleaner IMO.

thinking... will this work OK for packaged builds?  should we create a script or
spamassassin cmdline flag to do this instead?
Comment 10 Loren Wilton 2004-10-13 03:48:33 UTC
Subject: Re:  warn during "make" if module versions are too low

Apologies if this is a second reply.  Webmail bombed while I was trying to post.)

> Question, do we want to warn on less or more modules?  Razor? Net::SMTP?
> Mail::SPF::Query? IP::Country::Fast? anything else?

Suggested answer from user's viewpoint:

If the product won't run out of the box with all options enabled without a particular module or particular version of the module, then Make should probably warn about it just to give the user a hint and save an hour or two digging to find the obvious error.  For optional parts like net tests you could do a header line of "If using Net tests you will need:\n" followed by a slightly indented list of the requsite modules/versions.

This is only talking about standard SA. If you are running JollyGreenGiantMilter-2v3 to interface to SA, whatever you need for that is your worry, not SA's worry.  (Although if it is a really bog-standard configuration and you happen to KNOW that what worked on 2.64 won't work on 3.0 without an upgrade, it would be real polite to include a line about that.  Optional, but real polite.)

I think I'd probably suggest that these messages/warnings come out at the very end of the make, so they will maybe still be on the terminal screen.  :-)

Comment 11 Michael Parker 2004-10-16 20:19:20 UTC
I think I'm against running something at the end of make.  Doing this in
Makefile.PL has been the traditional method for this sort of thing.  The patch
mimics how DBI does it's check for the optional modules.  We could call an
external script that gets maintained, and can be run separtately, that spits out
a bunch of useful info.  I just attached an example on Bug 3820:
http://bugzilla.spamassassin.org/attachment.cgi?id=2455&action=view

For sure we should run this before make test since some of the optional modules
will turn on/off tests.
Comment 12 Malte S. Stretz 2004-10-17 13:22:53 UTC
Sorry, been out a while.  My plan was: 
1. See if we can put the infos into the file META.yml and extract it from 
there (that file is currently disabled because it made some trouble -- but 
some nice people on the MakeMaker list told me how to do it correctly :) 
2. Read it from a script in build/ (as Justin suggested) 
3. Run that script as late as possible.  Possible means here where 
implementing it is the least hassle -- I dont want to go hacking on EU::MM's 
guts again for this feature. 
Comment 13 Justin Mason 2004-10-17 23:04:43 UTC
that script looks perfect btw!   I really like the standalone script idea, as it
doesn't *need* to be part of Makefile.PL really, at all, and that script can be
run without even downloading the SA distro as it is now.

also, fair enough about running it from Makefile.PL.  I suppose that is the std
form for this in other modules, and the principle of least surprise applies.
we could always run it later in the build *as well*, too ;)
Comment 14 Theo Van Dinter 2004-10-20 20:59:28 UTC
can we get something official to vote on and such?  I'd like to get this closed rsn.
Comment 15 Daniel Quinlan 2004-10-21 15:20:52 UTC
Created attachment 2467 [details]
proposed patch to fix

I think we should only warn for things that are actionable: having an
old version installed for an optional module.

I also do not like the idea of using a separate script, I think changing
Makefile.PL is much less apt to be missed by users, so here is my proposed
fix.  Please approve and I'll check it in.
Comment 16 Daniel Quinlan 2004-10-21 15:22:12 UTC
moving to review state

One note: Test::Harness and Test::Simple do not seem to be requirements, they
must be required for some other module like Net::DNS which is out of scope for
our Makefile.PL.
Comment 17 Justin Mason 2004-10-21 15:24:24 UTC
yeah, ok: +1

I'd prefer an external script, I think our Makefile.PL is bloated.  but if it
works, hey, that's good ;)
Comment 18 Daniel Quinlan 2004-10-21 15:25:35 UTC
Makefile.PL may be bloated, but it is the right place to fix this.

If Makefile.PL is bloated, we should break it up or clean it up somehow,
not add more scripts that users have to contend with.  :-)
Comment 19 Daniel Quinlan 2004-10-21 15:26:33 UTC
changing owner
Comment 20 Michael Parker 2004-10-21 15:29:48 UTC
Subject: Re:  [review] warn during "make" if module versions are too low

+1
Comment 21 Daniel Quinlan 2004-10-21 15:33:32 UTC
committed to 3.0 so we can release 3.0.1 now
Comment 22 Justin Mason 2004-12-16 17:14:58 UTC
reopening.  this needs to use more useful text a la the descriptions from this
patch in bug 3820:

http://bugzilla.spamassassin.org/attachment.cgi?id=2455&action=view

and include support for more modules, including modules required by scripts in
"tools" -- bug 4036:

'its very simply that your version of getopt::long is not recent enough'
Comment 23 Justin Mason 2004-12-16 17:15:09 UTC
*** Bug 3820 has been marked as a duplicate of this bug. ***
Comment 24 Justin Mason 2004-12-16 17:15:40 UTC
*** Bug 4036 has been marked as a duplicate of this bug. ***
Comment 25 Daniel Quinlan 2005-01-11 02:14:15 UTC
Justin, you want to get this one?
Comment 26 Justin Mason 2005-01-11 10:39:49 UTC
yeah, will do. here's what I plan to do:

1. run the check from Makefile.PL

2. *also* make the check available as a script that can be
   run post-build, called "build/check_dependencies" or similar

3. both call into code in Mail::SpamAssassin::Util::DependencyInfo"

4. the code should

    - check presence of modules
    - check min version of modules
    - include a brief description of what the module is used for
    - differentiate between required and optional modules
    - possibly include a link to a wiki page where more complex
      dependency situations can be explained

so basically it'll look a lot like
http://bugzilla.spamassassin.org/attachment.cgi?id=2455&action=view .

--j.
Comment 27 Loren Wilton 2005-01-11 18:41:42 UTC
Subject: Re:  add more useful to text to warning during "make" output if module versions are too low

Couple of minor suggestions Justin, although you were planning on doing it
this way anyway.  :-)

I'd have the makefile simply call the script, rather than duplicating the
code in two places (and having to remember to update it in two places).

If possible, I'd have the code determine the current option settings that
can affect dependencies, and potentially spit out additional dependency info
based on what is turned on, or at least attempted to be turned on.

I'd also as much as possible have the code check for "dependencies of
dependencies" when this info is known.  Yes, it isn't a direct dependency of
SA, but if it isn't right the thing SA is dependent on won't work anyway, so
the user won't get the desired result.  Which at first glance is going to
look like something screwed up in SA, until someone points out to the poor
schnook that it is the Globbitz.pl module used by something he called.  I
think I originally submitted this or a related bug based on someone on the
list having exactly this sort of problem.

Comment 28 Justin Mason 2005-01-12 18:18:54 UTC
ok, checked in -- r125018.

it's mostly based on Michael's script, moved into a module that Makefile.PL
calls directly.
I added Getopt::Long, since that's the cause of that tools/sa-stats.pl bug after
all.
Comment 29 Justin Mason 2005-01-12 18:28:21 UTC
resolved