[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#1040001: Role of tibble? (Was: Bug#1040001: Seeking advise how to proceed with the transition / move R stack to testing)



Hi Paul,

On 9 July 2023 at 20:11, Paul Gevers wrote:
| On 09-07-2023 18:41, Dirk Eddelbuettel wrote:
| > On 9 July 2023 at 17:40, Paul Gevers wrote:
| > | Did we already discuss that r-cran-ps also seems to be impacted by the
| > | r-base change of the symbols thingy, as can be seen in r-cran-xopen [1].
| > 
| > Correct me if I am wrong but the "symbols thingy" was not a change in R 4.2.*
| > to R 4.3.*. It was a change by some packages opting into different behavior.
| 
| I don't understand this then. For several packages we're seeing failures 
| in testing even if we only use r-base from unstable and everything else 
| from testing to run the test. They pass with a rebuild r-cran-fnn and/or 
| a rebuild and updated r-cran-ps, and/or r-cran-tibble. (Sorry, in my 
| previous message I think I added r-cran-dplyr by mistake, misremembered).

It would be useful to break this down into concrete reproducible examples.

| > |   33s  10. ps:::not_null(.Call("psll_connections", p))
| > 
| > Tis would be a bug in r-cran-ps and I think a Breaks may be warranted.
| 
| Ok, let's remember this.
| 
| > Given
| > that ps always had 'native symbols', maybe testthat changed?
| 
| But I think (I would need to check for the autopkgtest fallback) in none 
| of the tests, the version of testthat in testing changed between passing 
| and failing tests. Would testthat embed something during the build of 
| the binaries (from the name, I would assume not)?

I don't think it would do anything _explicit_.

I think what we are seeing is simply 'fragility from some packages with
larger tails'. If you install 'testthat' (presumably just to run tests) you
end up with with 30+ packages loaded (without considering Suggests:). It is
similar with other 'tidyverse' packages (dplyr, tibble, vctrs, ...) are all
part of that.
 
| > | Same for r-cran-fnn, which impacts r-cran-uwof [2].

I just looked at FNN, it is very narrow package at its core, it gets a bit of
tail via the Suggests of chemometrics.

| > | I think what we should do is add a versioned Breaks in r-base on
| > | <packages-involved-in:r-graphics-engine>, r-cran-ps, r-cran-tibble,
| > | r-cran-dplyr, r-cran-fnn. I think that's the right thing to do for
| > 
| > I think it would be best to work out to corresponding package pairings and
| > apply the Breaks to them. If we can.
| 
| Sure, lets add the Breaks to the place where it belongs.
| 
| > For spacetime and stars I suspect (based on past experience) possible
| > interaction from the underlying graphics libraries.
| 
| Good to hear, didn't check yet, but will shortly (geospatial).

It's a complex block. spacetime is one of the older ones using 'sp' (and then
'raster' via Suggests), 'stars' is newer using 'sf'. Sometimes these prefer /
work better with a consistent rebuild.
 
| > If I am not mistaken all of these were already in the Excuses list before we
| > made addition of the r-graphics-engine-* tag (which was taken care of for R
| > 4.3.* already, having it may help if another change happens like that).
| 
| Sure, I'd hope the r-graphics-engine-* Provides only reduced issue, so 
| I'm currently considering them to be different. But I might be proven 
| wrong easily.

I don't think it had a net effect this.  The 
| 
| > Unfortunately I find
| > the reports a little hard to read and am hence not very efficient at
| > pinpointing underlying causes. Above you pointed eg at [2] for uwot, I see no
| > error in there :-/
| 
| Well, that log has two tests. The first second one passes, the first one 
| has:
|   51s > library(testthat)
|   51s > library(uwot)
|   51s Loading required package: Matrix
|   53s >
|   53s > test_check("uwot")
|   53s Error in FNN::get.knn(X, k) : DLL requires the use of native symbols
|   53s Calls: test_check ... eval -> create_data -> find_nn -> FNN_nn -> 
| <Anonymous>
|   53s Execution halted

But uwot itself does not force symbols:

  https://github.com/jlmelville/uwot/blob/master/src/RcppExports.cpp#L228-L231

and I mentioned, using just those two lines in common. So the forceSymbols
comes from somewhere else.

Ok, I rechecked.  R 4.3.0 has this

    * Attempting to use a character string naming a foreign function
      entry point in a foreign function call in a package will now
      signal an error if the packages has called R_forceSymbols to
      specify that symbols must be used.

It used to _ignore_ the combinatation of calling R_forceSymbols _and_ use of
non-symbols / quoted identifiers.  Now it errors: if you have R_forceSymbols
each .Call() will require symbols (not strings).

So this is where R 4.3.[01] will possibly break with some older packages.
But the fix is simple because when R 4.3.0 came out all packages at CRAN
complied. We need to have current packages that correspond to the R 4.3.0
standard.

(If one were super picky one could call this an ABI/API change and reason for
forcing _all_ packages to be rebuild. I never advocated for that but I am
getting tired of this process. But we need to throw that bomb at some point
just say 'fsck it' and rebuild _all_ packages. Feels like overkill but so is
wasting weeks on this.

| > Obviously, I too want my package r-base in testing and I will help. But I
| > feel that pinning a large Breaks list on it seems a little inefficient /
| > unfair if the package was not causing the change. We can do if there is (as
| > we r-graphics-engine-*) an overwhelming feel "that we must".
| 
| I want the Breaks in the right location. If we locate a more logical 
| location than r-base, that's where it should go. At least the packages 
| involved in the r-graphics-engine would do nice, as it's really the 
| change in r-base that broke them (will not be needed anymore after the

(Well, it didn't. It offers a higher API version and it gives packages a
choice to die on that. The six identified package choose to call
R_GE_checkVersionOrDie so they impose a run-time equality on the R engine
they built with. My point is that the accept-vs-break comes from the package,
not from R.)

Dirk

| release of trixie as now we have the Provides): r-cran-cairo, 
| r-cran-magick, r-cran-ragg, r-cran-svglite, r-cran-tikzdevice and 
| r-cran-vdiffr.
| 
| Paul
| x[DELETED ATTACHMENT OpenPGP_signature, application/pgp-signature]

-- 
dirk.eddelbuettel.com | @eddelbuettel | edd@debian.org


Reply to: