Nightly feedstock build failed

[github-actions[bot]]

Nightly feedstock build failure for tiledbsoma-feedstock at https://dev.azure.com/TileDB-Inc/CI/_build?definitionId=43&_a=summary

[jdblischak]

The r-tiledbsoma build is failing with a linking error related to arrow and liborc. Note that I replaced the long host environment path with $HOST_ENV

** byte-compile and prepare package for lazy loading
sh: line 1:  2086 Aborted                 (core dumped) R_TESTS= '$HOST_ENV/lib/R/bin/R' --no-save --no-restore --no-echo 2>&1 < '/tmp/Rtmptd2Fv5/file7a22fad0e50'
Error in dyn.load(file, DLLpath = DLLpath, ...) : 
  unable to load shared object '$HOST_ENV/lib/R/library/arrow/libs/arrow.so':
  $HOST_ENV/lib/R/library/arrow/libs/../../../.././liborc.so: undefined symbol: _ZN6snappy11RawCompressEPKcmPcPm
Calls: <Anonymous> ... asNamespace -> loadNamespace -> library.dynam -> dyn.load
Execution halted
libgcc_s.so.1 must be installed for pthread_cancel to work
ERROR: lazy loading failed for package ‘tiledbsoma’
* removing ‘$HOST_ENV/lib/R/library/tiledbsoma’

[johnkerl]

@eddelbuettel looping you in for advice, if you have any here

[eddelbuettel]

@johnkerl Happy to help ... but sad to say that may be a new one :-/ Newer fought liborb. The lines above are from snappy, compressor used by Arrow. @jdblischak Is there a chance to bissect and simplify? Any change to see which 'related ingredients' may have changed overnight?

[github-actions[bot]]

Nightly feedstock build failure for tiledbsoma-feedstock at https://dev.azure.com/TileDB-Inc/CI/_build?definitionId=43&_a=summary

[johnkerl]

@ihnorton @aaronwolen @Shelnutt2 for clarity: this breakage puts us in a state wherein we are unable to produce the next tilebsoma release until this is resolved.

For reference please see our established procedure.

[eddelbuettel]

One possible approach would be to not Imports: arrow (following what tiledb-r does) and wrapping all actual access via a test with if (requireNamespace("arrow")). That should not loose functionality but would sidestep the linking error we see.

[johnkerl]

@eddelbuettel thank you! Please put up a branch implementing your sketch.

[eddelbuettel]

The sketch looks good. Too good. (Comments from @aaronwolen and @mojaveazure appreciated!) All I did so far was to move it from Imports: to Suggests: and comment out to roxygen importFrom stanza to clear NAMESPACE. Tests still pass:

Duration: 134.9 s

[ FAIL 0 | WARN 0 | SKIP 0 | PASS 3007 ]

PR coming shortly.

[johnkerl]

Tests still pass:

Yes, as expected. It's nightly feedstock build that's failing, not TileDB-SOMA unit tests.

I'll try to see if I can integrate this TileDB-SOMA package mod of yours with the thing that's failing, which is a feedstock build ... somehow ...

[eddelbuettel]

TileDB-SOMA unit tests

One can very reasonably expect tests to fail when a Suggests: package is used unconditionally (as it was in our case a required Imports) so I was focussing on a different aspect. It is a long-running issue in the R package system to ensure that conditional use is, in fact, conditional. CRAN had been very tolerant and often/always installed Siggests:, this is tightening now -- and that is a Good Thing (TM) in the longer run.

[eddelbuettel]

@jdblischak Can you import a commit via cherry-pick / patch to the feedstock as a short cut?

[johnkerl]

One can very reasonably expect tests to fail when a Suggests: package is used unconditionally (as it was in our case a required Imports) so I was focussing on a different aspect. It is a long-running issue in the R package system to ensure that conditional use is, in fact, conditional. CRAN had been very tolerant and often/always installed Siggests:, this is tightening now -- and that is a Good Thing (TM) in the longer run.

I understand now -- thanks @eddelbuettel !

[eddelbuettel]

It also must mean that all (?) of our arrow use is properly namespaced as in arrow::schema() so removing the two entries from NAMESPACE did not hurt nor did it other cases ... In CI we also ensure all Suggests: are installed so the tests will find it.

[johnkerl]

@eddelbuettel I'll put up a draft PR on this repo momentarily ...

[eddelbuettel]

Straight up edits should work. I also ran my roxygen2::roxygenize() wrapper but it altered zero Rd files so edits / changes to DESCRIPTION and NAMESPACE should work. "Should" -- famous last words.

[jdblischak]

One possible approach would be to not Imports: arrow (following what tiledb-r does) and wrapping all actual access via a test with if (requireNamespace("arrow")). That should not loose functionality but would sidestep the linking error we see.

If we do this, can we add a test to the recipe to confirm that the r-tiledbsoma conda binary can still use the optional arrow functionality?

Is there a chance to bissect and simplify? Any change to see which 'related ingredients' may have changed overnight?

@eddelbuettel I assume this means that there have been no upstream changes in TileDB-SOMA that would affect the ability of the R package client to link to arrow. If yes, that would narrow it down to an update in the dependencies installed in the conda environment. I'll investigate

[eddelbuettel]

If we do this, can we add a test to the recipe to confirm that the r-tiledbsoma conda binary can still use the optional arrow functionality?

Yes that seems useful.

[...] would narrow it down to an update in the dependencies installed in the conda environment.

That is my suspicion but one never knows. We made very few changes that could have caused this. We still compile and link the same nanoarrow and have no other changes I can think of. But as an Imports: arrow leads to load check at build time I suspect "something somewhere" ruffled a feather and now we have a sneeze.

[johnkerl]

and now we have a sneeze.

This is life in Condaland; I am 0% surprised. There is never a release goes by that something doesn't change at some level with one of our direct or indirect dependencies.

[jdblischak]

Yes that seems useful.

I'm not as familiar with r-tiledbsoma/arrow. Is there a quick one-liner via R -e that would confirm the arrow functionality is intact? If it would require multiple lines to be more readable/maintainable, we could also create a test script.

[eddelbuettel]

So we use arrow (the R package) to create Arrow (the abstract model and spec) representations of, say, an 'Arrow Table' (think data.frame). We never write or read using arrow. The writing is for now still passed off to tiledb-r the reading picks up from TileDB Core and instantiates nanoarrow (in essence: two C pointers to data, no linking) and only once all that is done do we become an (R) Arrow object via arrow. (You can also go directly from the two Arrow pointers to other Arrow consumers such as duckdb or polars.)

Basically all we do is this:

nrows <- 12
as <- arrow::schema(arrow::field("foo", arrow::int32(), nullable = FALSE),
                    arrow::field("soma_joinid", arrow::int64(), nullable = FALSE),
                    arrow::field("bar", arrow::float64(), nullable = FALSE),
                    arrow::field("baz", arrow::large_utf8(), nullable = FALSE))
at <- arrow::arrow_table(foo = seq.int(nrows) + 1000L,
                         soma_joinid = bit64::seq.integer64(from = 0L, to = nrows - 1L),
                         bar = seq(nrows) + 0.1,
                         baz = as.character(seq.int(nrows) + 1000L),
                         schema = as)
print(at)

print(tibble::as_tibble(at))

and only because arrow gets loaded as an Imports: do we hit the conda build pothole. (Or so I think ...)

I can expand this into a full write-then-read example as we have in unit tests too...

[johnkerl]

If we do this, can we add a test to the recipe to confirm that the r-tiledbsoma conda binary can still use the optional arrow functionality?

Yes that seems useful.

@eddelbuettel can you please propose a concrete modification for this file that might accomplish this goal?

https://github.com/TileDB-Inc/tiledbsoma-feedstock/blob/main/recipe/build-r-tiledbsoma.sh

[eddelbuettel]

@johnkerl I am not sure I follow. I understood John B to have asked about a 'litmus test example' of Arrow use in the context of the build to provide a 'proof' that arrow loads. I do not yet understand how that relates to the feedstock script. Maybe we three need to huddle...

[johnkerl]

@eddelbuettel I'm going to be in the car a while. Let me say that there are two things going on:

• Litmus test example -- yes, this is one thing. And I have an ask for you ☝️ .
  • https://github.com/TileDB-Inc/tiledbsoma-feedstock/blob/main/recipe/build-r-tiledbsoma.sh is a bash script wherein I'd like you to find a place to put a check that arrow got installed OK. This is a non-Conda ask for you: just a bash-scripting/R-scripting ask for you.
• Conda-solver / version-pin issues involving version pins -- completely orthogonal. I will experiment with. And these are @jdblischak + me things which I won't bug you about.

[eddelbuettel]

@johnkerl Thanks, that is clearer. I can work with something like

Rscript -e 'arrow::arrow_info()'
Rscript -e 'names(unclass(arrow::arrow_info()))'
Rscript -e 'length(unclass(arrow::arrow_info()))'  

which will presumably error rather than return output I can pick up and shell-wise compare to expectations...

[johnkerl]

For reference https://github.com/TileDB-Inc/tiledbsoma-feedstock/pull/128

[jdblischak]

Last night's nightly builds all passed. All the builds installed snappy 1.1 and aws-crt-cpp >=0.26.4 (some envs installed 0.26.6)