You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@spamassassin.apache.org by bu...@bugzilla.spamassassin.org on 2018/12/06 20:12:07 UTC

[Bug 7666] New: Non-standard build/test handling may cause build/test failures

https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7666

            Bug ID: 7666
           Summary: Non-standard build/test handling may cause build/test
                    failures
           Product: Spamassassin
           Version: 3.4.2
          Hardware: All
                OS: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Building & Packaging
          Assignee: dev@spamassassin.apache.org
          Reporter: eserte12@yahoo.de
  Target Milestone: Undefined

Created attachment 5635
  --> https://bz.apache.org/SpamAssassin/attachment.cgi?id=5635&action=edit
Dockerfile

The attached Dockerfile shows two problems with the current 3.4.2 release, a
build and a test problem.

* the test problem: a possible setup of CPAN Testers smoke machines is to
*test* a distribution without installing it. CPAN.pm normally makes sure that
the dependencies are all available in PERL5LIB, so for a test script it looks
like the dependencies are already installed. However, it seems that the
SpamAssassin test scripts strip out these paths from PERL5LIB, and this leads
to test failures like this:
http://www.cpantesters.org/cpan/report/b4308f14-f984-11e8-b607-f782ef07057d
More test similar test failures may be seen on
http://fast-matrix.cpantesters.org/?dist=Mail-SpamAssassin%203.4.2;os=linux;reports=1#sl=6,1
(actually all reports by SREZIC should have the same problem)

* the build problem: if the deactivated CMD line in the attached Dockerfile is
used instead, then the build fails immediately because of missing prereqs. This
is non-standard behavior. A normal CPAN distribution would warn on the missing
prerequisites, but still write the Makefile and also the MYMETA.* files.
CPAN.pm then would inspect either the Makefile or the MYMETA.* files, install
the necessary dependencies, and go on with building and testing.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7666] Non-standard build/test handling may cause build/test failures

Posted by bu...@bugzilla.spamassassin.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7666

Kevin A. McGrail <km...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |kmcgrail@apache.org

--- Comment #1 from Kevin A. McGrail <km...@apache.org> ---
Any suggest fix?

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7666] Non-standard build/test handling may cause build/test failures

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7666

--- Comment #4 from Sidney Markowitz <si...@sidney.com> ---
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.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7666] Non-standard build/test handling may cause build/test failures

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7666

--- Comment #7 from Sidney Markowitz <si...@sidney.com> ---
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.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7666] Non-standard build/test handling may cause build/test failures

Posted by bu...@bugzilla.spamassassin.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7666

eserte12@yahoo.de changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |eserte12@yahoo.de

--- Comment #2 from eserte12@yahoo.de ---
(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.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7666] Non-standard build/test handling may cause build/test failures

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7666

--- Comment #10 from Sidney Markowitz <si...@sidney.com> ---
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.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7666] Non-standard build/test handling may cause build/test failures

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7666

--- Comment #11 from Sidney Markowitz <si...@sidney.com> ---
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.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7666] Non-standard build/test handling may cause build/test failures

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7666

--- Comment #13 from Sidney Markowitz <si...@sidney.com> ---
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.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7666] Non-standard build/test handling may cause build/test failures

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7666

--- Comment #16 from Sidney Markowitz <si...@sidney.com> ---
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.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7666] Non-standard build/test handling may cause build/test failures

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7666

--- Comment #14 from Sidney Markowitz <si...@sidney.com> ---
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.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7666] Non-standard build/test handling may cause build/test failures

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7666

--- Comment #6 from Sidney Markowitz <si...@sidney.com> ---
(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.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7666] Non-standard build/test handling may cause build/test failures

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7666

--- Comment #9 from Sidney Markowitz <si...@sidney.com> ---
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.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7666] Non-standard build/test handling may cause build/test failures

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7666

--- Comment #12 from Sidney Markowitz <si...@sidney.com> ---
(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.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7666] Non-standard build/test handling may cause build/test failures

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7666

Loren Wilton <lw...@earthlink.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |lwilton@earthlink.net

--- Comment #5 from Loren Wilton <lw...@earthlink.net> ---
> 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.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7666] Non-standard build/test handling may cause build/test failures

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7666

--- Comment #8 from Sidney Markowitz <si...@sidney.com> ---
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?

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7666] Non-standard build/test handling may cause build/test failures

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7666

Sidney Markowitz <si...@sidney.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED

--- Comment #15 from Sidney Markowitz <si...@sidney.com> ---
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.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7666] Non-standard build/test handling may cause build/test failures

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7666

Sidney Markowitz <si...@sidney.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|Undefined                   |4.0.0
                 CC|                            |sidney@sidney.com

--- Comment #3 from Sidney Markowitz <si...@sidney.com> ---
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.

-- 
You are receiving this mail because:
You are the assignee for the bug.