Any suggest fix?

(In reply to Kevin A. McGrail from comment #1)
> Any suggest fix?

For the build problem: I think the exit here:
https://metacpan.org/source/KMCGRAIL/Mail-SpamAssassin-3.4.2/Makefile.PL#L328
is wrong. At least it's wrong when it comes to missing perl module dependencies --- exiting means that no Makefile would be created, and CPAN.pm cannot continue with automatic dependency installation.

However, the exit 0 might be still OK if required binaries are missing --- CPAN.pm cannot install these automatically. Currently there are none (all binary dependencies are optional), but maybe the code should be prepared to deal with this situation.


For the test problem: probably running spamassassin with -T (taint mode) is causing the existing PERL5LIB to be ineffective. I just learned about the TEST_PERL_TAINT environment variable, and indeed, when setting

    ENV TEST_PERL_TAINT=no

in the Dockerfile before the CMD line, then the test suite runs much better (still there are some test failures, but much less).

Maybe SATest.pm could add a series on -I options (build from PERL5LIB) if tainting is enabled.

I'm putting this on the 4.0 milestone because I'm pretty sure recent things I've done for untainting have fixed this, and I am testing that 4.0 will build on CPAN. Either this issue has already been fixed or I'll make sure that it is.

The test scripts have been worked on to ensure that they all can run in taint mode. Also, I've been checking on the results of CPAN testers and haven't seen this particular problem in recent versions.

I have reproduced the problem shown in the Docker file, in my case reproducing it on a minimal Ubuntu 20.04 VirtualBox VM.

The code we have is the result of the fix to bug 5908 which I now think was a misinterpretation of the following language in CPAN Authors FAQ (now at http://cpanwiki.grango.org/wiki/CPANAuthorNotes instead of the old link that has a broken redirect):

> "How can I stop getting FAIL reports for missing libraries or other
> non-Perl dependencies?"
> If you have some special dependencies and don't want to get CPAN Testers
> reports if a dependency is not available, just exit from the Makefile.PL or
> Build.PL normally (with an exit code of 0) before the Makefile or Build file
> is created.
> 
>   exit 0 unless some_dependency_is_met();

Notice that the FAQ is not referring to dependencies on Perl modules. It looks like we are supposed to build the Makefile even when a required module is missing so that CPAN testers can notice the missing dependency, add the module, and continue testing the Makefile. Then we are supposed to fail in the Makefile in a way that will not generate a failure report email.

There seems like to be some way to do that, as another FAQ entry says

> "Why am I getting a FAIL/UNKNOWN/NA report when prerequisites are not met?"
> Some early versions of CPAN Testers tools had bugs that did not properly
> catch when prerequisites were specified but not met. Please just ignore
> these reports.

In https://metacpan.org/pod/CPAN::Reporter::FAQ it says

> Why was a report sent if a prerequisite is missing?
> As of CPAN::Reporter 0.46, FAIL and UNKNOWN reports with unsatisfied
> prerequisites are discarded. Earlier versions may have sent these reports
> out by mistake as either an NA or UNKNOWN report.
> 
> PASS reports are not discarded because it may be useful to know when
> tests passed despite a missing prerequisite. NA reports are sent because
> information about the lack of support for a platform is relevant
> regardless of prerequisites.

When I test simply removing the exit 0, running the cpan -t command as in the docker file tries to install the missing dependencies before running the generated  Makefile. That seems to be the correct and expected result of installing a CPAN module - It automatically fetches and installs dependencies.

This does not work well on Ubuntu, because for some reason Net::DNS does not install properly from cpan, it has to be installed using Ubuntu's apt package for it. But having Makefile.PL exit without producing the Makefile breaks the ability to let cpan automatically handle the module dependencies.

I'm going to generate a trial package that removes the exit(0) from Makefile.PL to see if the current CPAN test machines generate any reports for it. If they don't, I will propose that as our fix. If we ever do have non-module library non-optional dependencies, we can look at having a hard exit from Makefile.PL for them.

> If we ever do have non-module library non-optional 
> dependencies, we can look at having a hard exit 
> from Makefile.PL for them.

Would it be worth leaving a comment in Makefile.PL about the possible need to do this? So that if the situation occurs, the next person doesn't have to track down everything you did in this comment.

(In reply to Loren Wilton from comment #5)
> Would it be worth leaving a comment in Makefile.PL

Definitely. If that's the change I end up with after testing, I'll make sure there is a comment there about what needs to be done if we ever have a non-optional non-perl dependency.

I have verified that the remaining failures on CPAN test machines of trials I have uploaded are because of the problem described in the initial comment of this issue:

The required modules that are not installed by default on the CPAN test machine are installed into temporary directories that are appended to PERL5LIB. When make test is run, PERL5LIB is not added to INC because tests are run in taint mode.

I recall seeing some documentation about how to address that issue, which I'll look for.

I was wrong about that documentation. As far as I can tell, the only way to get make test to use the PERL5LIB environment variable (and therefore work on CPAN tester machines) is to not have -T in the tests' hashbang lines.

We can still run tests in taint mode using prove -T, which is already in the xt/run_release_test_suite.sh script that we are supposed to run before a release.

It would also work to run a command
  prove -T t/*.t
to get the same effect as make test does now.

So should I remove the -T from the t/*.t hashbang lines and require people to test using prove -T instead of make test when developing?

I'm embarrassed to say that I had a typo in the command line I was using to try different things and that I kept running from history instead of ever re-typing. As a result, I don't know what of what I tried did and did not work, so there may be a workaround or the failed tests may be due to some different cause completely. Sorry for the noise. Back to the drawing board.

It turns out that MakeMaker uses Test::Harness to run tests, which does something that allows PERL5LIB to pass through to tests that are run in taint mode.

However, tests that use sarun, etc., to run spamassassin, etc. do so invoking perl in a shell command, which breaks the use of PERL5LIB because the spamassassin script specifies -T.

This is causing certain tests to fail on CPAN test machines that install required modules in temporary directories that are specified in PERL5LIB.

I think the fix to this is to have SATest.pm use the contents of PERL5LIB to add include options to the perl command line that is used for sarun, etc.

trunk % svn ci -m "bug 7666 - Fix tests that run spamassassin in taint mode not passing through PERL5LIB path" t/SATest.pm
Sending        t/SATest.pm
Transmitting file data .done
Committing transaction...
Committed revision 1903193.

(In reply to Loren Wilton from comment #5)
> Would it be worth leaving a comment in Makefile.PL about the possible need
> to do this?

I was able to go one better. This commit has the check that is called from Makefile.PL issue a warning for missing CPAN modules that are required or optional, a warning for missing binaries that are optional, but exit 0 with an error message if any required binaries are missing.

The commit also has a more complete list of required and optional CPAN modules in the Makefile.PL table that CPAN uses to compile its dependency database.

trunk % svn ci -m "bug 7666 - Fix module dependency checks in Makefile.PL so CPAN tests can install missing modules and continue running"
Sending        Makefile.PL
Sending        lib/Mail/SpamAssassin/Util/DependencyInfo.pm
Transmitting file data ..done
Committing transaction...
Committed revision 1903194.

My commit in comment 11 did fix the problem I found in my tests when required CPAN modules were installed in directories not in INC but passed in via PERL5LIB. However, that is not the problem I'm seeing on the CPAN test machines. All the ones that use PERL5LIB also put the paths for the added modules into INC. On most of the test machines that worked fine even before this patch. On the test machines that fail, the patch doesn't fix it. So more digging is necessary.

Let's see if this is enough...
trunk % svn ci -m "Add defined check for a value that can end up undefined" t/SATest.pm 
Sending        t/SATest.pm
Transmitting file data .done
Committing transaction...
Committed revision 1903224.

Here is the matrix of results from my latest trial upload to CPAN of what is currently in svn trunk.
http://matrix.cpantesters.org/?dist=Mail-SpamAssassin%204.0.0-pre2r-TRIAL

As of the time I am typing this, there are only two groups of failures, neither which I think are problems worth fixing.

There are two testers running Windows 7 and 8 with old versions of Strawberry Perl that use dmake, as opposed to gmake which Strawberry has used since perl 5.26, who get a failure when they try to run dmake. I cannot reproduce that failure using Windows 7 and the same version of Perl with dmake they use. I don't think that is a problem worth pursuing.

The remaining failures are all on machines run by the same tester who consistently gets failures trying to run the t/spamd_hup.t tests with spamd seeming not to run. This is on different Ubuntu and FreeBSD configurations. I haven't figured out the common element on their test machines that would be different from all other testers. Those are the only tests right now showing up with "fail" status and every test report from that person is one of those spamd_hup fails. Again, I don't see this as a problem worth pursuing, at least not as a blocker for 4.0.0.

With this, I am closing this issue.

This issue has been closed, but I'm adding this comment to tie up some loose ends in the comment history.

I tracked the failures on some Windows CPAN testing machines to an unreleased development version of ExtUtills::MakeMaker they installed which has a bug that only shows up in old versions of Strawberry perl that use dmake. The problem does not happen with the current release version of ExtUtils::MakeMaker. I reported the bug to the ExtUtils::MakeMaker project. We don't have to do anything else about it.

The other remaining test failures were on test machines that are configured in a way that triggered bug 8030 which is now fixed. Tests on those machines now pass.