possible bug in win R-devel in check/test environment

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

possible bug in win R-devel in check/test environment

Serguei Sokol
Hi,

During my recent r2sundials development, I've came across a strange test
failing during 'R CMD check' exclusively on win R-devel which I could
reproduce with a minimal example that I present here.
The toy packages testarma1 [1] and testarma2 [2] are minimal
modifications of a skeleton package produced by
RcppArmadillo.package.skeleton().
They are almost identical. The first one fails to passe its tests on win
R-devel [3] while the second one is OK [4]. The reason of test failing
in testarma1 boils down to not finding a package during tests (here
RcppArmadillo) although it is well present in LinkingTo field of the
DESCRIPTION file (the mechanism of the error is detailed in [5]). To
make the tests pass, I had to add RcppArmadillo and r2sundials to
'Suggests:' field too (as can be seen in testarma2)

In my understanding, the presence of a package name in the LinkingTo
field should be sufficient for finding it during the test phase.
Instead, one have to add it to 'Suggests:' field too.

Am I wrong or this behavior is unexpected?

Best,
Serguei.

[1] https://github.com/sgsokol/testarma1 
<https://github.com/sgsokol/testarma2>
[2] https://github.com/sgsokol/testarma2
[3] https://win-builder.r-project.org/v0nBoFleT48y/00check.log
[4] https://win-builder.r-project.org/TMKbnEBncFNc/00check.log
[5] https://github.com/RcppCore/Rcpp/issues/1026

<https://github.com/sgsokol/testarma2>

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: possible bug in win R-devel in check/test environment

Dirk Eddelbuettel

Hi Serguei,

Nice analysis!

On 14 January 2020 at 11:00, Serguei Sokol wrote:
| During my recent r2sundials development, I've came across a strange test
| failing during 'R CMD check' exclusively on win R-devel which I could
| reproduce with a minimal example that I present here.
| The toy packages testarma1 [1] and testarma2 [2] are minimal
| modifications of a skeleton package produced by
| RcppArmadillo.package.skeleton().
| They are almost identical. The first one fails to passe its tests on win
| R-devel [3] while the second one is OK [4]. The reason of test failing
| in testarma1 boils down to not finding a package during tests (here
| RcppArmadillo) although it is well present in LinkingTo field of the
| DESCRIPTION file (the mechanism of the error is detailed in [5]). To
| make the tests pass, I had to add RcppArmadillo and r2sundials to
| 'Suggests:' field too (as can be seen in testarma2)
|
| In my understanding, the presence of a package name in the LinkingTo
| field should be sufficient for finding it during the test phase.

I thought so too. But thinking about it a little more it clears up a little.

A bit more context: One can be more fine-grained on Depends. And Debian does
that, and R sometimes followed Debian's model of declaring dependencies. One
element we are missing here is to distinguish between _build-time_ needs (we
call that Build-Depends: in Debian) and _run_time_ needs.  We currently only
have the latter as Depends:, which for example pains a million dplyr users on
Windows who have to download 120mb worth of our BH package because it is used
to _build_ the binary zipfile, but not thereafter.  That is a wart.

Now, _LinkingTo_ always implies build-dependecies or else it would croak at
that stage.

And I had assumed that this would cover all run-time but ...

| Instead, one have to add it to 'Suggests:' field too.

... tests are indeed treated differently and this may just be a different
code path.

If you have something in Suggests: and test for it, you should condition the
test. I have argued that part a few times but mostly to no avail so I too now
mostly give up and _unconditionally_ install Suggests to support tests when I
run bulk tests for reverse dependencies.  But it is still wrong.

So here the ball is in your court. Your tests for r2sundials should probably
condition on RcppArmadillo being present and skip tests requiring it if it is
not present.  Or, if you don't like that, make it an Imports: too.

Hope this helps.

Cheers, Dirk

| Am I wrong or this behavior is unexpected?
|
| Best,
| Serguei.
|
| [1] https://github.com/sgsokol/testarma1 
| <https://github.com/sgsokol/testarma2>
| [2] https://github.com/sgsokol/testarma2
| [3] https://win-builder.r-project.org/v0nBoFleT48y/00check.log
| [4] https://win-builder.r-project.org/TMKbnEBncFNc/00check.log
| [5] https://github.com/RcppCore/Rcpp/issues/1026
|
| <https://github.com/sgsokol/testarma2>
|
| ______________________________________________
| [hidden email] mailing list
| https://stat.ethz.ch/mailman/listinfo/r-devel

--
http://dirk.eddelbuettel.com | @eddelbuettel | [hidden email]

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: possible bug in win R-devel in check/test environment

Serguei Sokol
Hi Dirk,

Thanks for sharing your thoughts on the subject.  I have few notes next
to it.

Le 14/01/2020 à 15:59, Dirk Eddelbuettel a écrit :

> Hi Serguei,
>
> Nice analysis!
>
> On 14 January 2020 at 11:00, Serguei Sokol wrote:
> | During my recent r2sundials development, I've came across a strange test
> | failing during 'R CMD check' exclusively on win R-devel which I could
> | reproduce with a minimal example that I present here.
> | The toy packages testarma1 [1] and testarma2 [2] are minimal
> | modifications of a skeleton package produced by
> | RcppArmadillo.package.skeleton().
> | They are almost identical. The first one fails to passe its tests on win
> | R-devel [3] while the second one is OK [4]. The reason of test failing
> | in testarma1 boils down to not finding a package during tests (here
> | RcppArmadillo) although it is well present in LinkingTo field of the
> | DESCRIPTION file (the mechanism of the error is detailed in [5]). To
> | make the tests pass, I had to add RcppArmadillo and r2sundials to
> | 'Suggests:' field too (as can be seen in testarma2)
> |
> | In my understanding, the presence of a package name in the LinkingTo
> | field should be sufficient for finding it during the test phase.
>
> I thought so too. But thinking about it a little more it clears up a little.
It remains not clear for me why the tests of testarma1 fail on win
R-devel and run OK on Linux R-devel (
https://builder.r-hub.io/status/testarma1_1.0.tar.gz-37bca609ce3b49149daa2f97d035098c 
)
If the reasoning you describe is the really intended one, it should work
in similar way on all platforms, should not it?

>
> A bit more context: One can be more fine-grained on Depends. And Debian does
> that, and R sometimes followed Debian's model of declaring dependencies. One
> element we are missing here is to distinguish between _build-time_ needs (we
> call that Build-Depends: in Debian) and _run_time_ needs.  We currently only
> have the latter as Depends:, which for example pains a million dplyr users on
> Windows who have to download 120mb worth of our BH package because it is used
> to _build_ the binary zipfile, but not thereafter.  That is a wart.
+1

>
> Now, _LinkingTo_ always implies build-dependecies or else it would croak at
> that stage.
Currently WRE states about LinkingTo:
"Specifying a package in ‘LinkingTo’ suffices if these are C++ headers
containing source code or static linking is done at installation: the
packages do not need to be (and usually should not be) listed in the
‘Depends’ or ‘Imports’ fields. This includes CRAN package BH and almost
all users of RcppArmadillo and RcppEigen."

No mention of build-time or test-time is made. Moreover, regarding your
advice to add packages to 'Imports', this phrase explicitly advises
against it: "and usually should not be ...".

If it is a real intention of R developers, a little phrase in WRE like
the following one could clarify the things:
"Note that packages listed in fields Depends, Imports and Suggests are
visible during the test stage of 'R CMD check' command while those in
LinkingTo are not."

>
> And I had assumed that this would cover all run-time but ...
>
> | Instead, one have to add it to 'Suggests:' field too.
>
> ... tests are indeed treated differently and this may just be a different
> code path.
>
> If you have something in Suggests: and test for it, you should condition the
> test.
Is it documented somewhere in such or similar words? (I mean official R
documentation.)

>   I have argued that part a few times but mostly to no avail so I too now
> mostly give up and _unconditionally_ install Suggests to support tests when I
> run bulk tests for reverse dependencies.  But it is still wrong.
I am not so resolved to call it "wrong". After all why not? The main
thing is to have a widely accepted consensus about it.
The packages underlying the tests like testthat, RUnit and alike are
explicitly required to be listed in Suggests. So if the packages in
Suggests are to be considered as optional including those ones you don't
even have a chance to check the presence of packages like RcppArmadillo
as the code containing this check cannot be run without testthat, RUnit
and so on.

>
> So here the ball is in your court. Your tests for r2sundials should probably
> condition on RcppArmadillo being present and skip tests requiring it if it is
> not present.
In this case, this is not an option for me. I do want the tests to be
run, not skipped.

>    Or, if you don't like that, make it an Imports: too.
I confirm, putting packages in Import, makes the tests run OK on win
R-devel too (cf imports branch of testarma1 and the check log on
https://win-builder.r-project.org/GQaZBdmn2U1x/00check.log )
But I prefer to leave them in Suggests if no their functions are used in
the body of the package (hence no real import is required).

Best,
Serguei.

>
> Hope this helps.
>
> Cheers, Dirk
>
> | Am I wrong or this behavior is unexpected?
> |
> | Best,
> | Serguei.
> |
> | [1] https://github.com/sgsokol/testarma1
> | <https://github.com/sgsokol/testarma2>
> | [2] https://github.com/sgsokol/testarma2
> | [3] https://win-builder.r-project.org/v0nBoFleT48y/00check.log
> | [4] https://win-builder.r-project.org/TMKbnEBncFNc/00check.log
> | [5] https://github.com/RcppCore/Rcpp/issues/1026
> |
> | <https://github.com/sgsokol/testarma2>
> |
> | ______________________________________________
> | [hidden email] mailing list
> | https://stat.ethz.ch/mailman/listinfo/r-devel
>

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel