-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Nightly feedstock build failed #230
Comments
Documenting the errors:
|
Nightly feedstock build failure for tiledbsoma-feedstock at https://dev.azure.com/TileDB-Inc/CI/_build?definitionId=43&_a=summary |
As expected, the R build continues to fail due to C++20 compilation issues. Today I did another experiment. I purposefully skipped the R client build in order to query the status of the Python build with C++20. Note that this build used the commit single-cell-data/TileDB-SOMA@ea3cd1a that was merged earlier this morning. First the good news:
But then the bad news:
The backtrace is super long (600+ lines) and only ends because Python core dumps. Below is the beginning and end of the backtrace. Note that I've also replaced the long conda installation path with
|
I searched for the error message from the failed R package build on linux-64: From what I could find, I think we may need to define |
@jdblischak I agree this is probably |
And for tinkering with the feedstock recipe and/or scripts, you can start from the branch nightly-build. It gets overwritten every night, so if you want your experiments to persist beyond a day, I recommend creating a new branch. |
@jdblischak I believe this will be fixed by #231 which addresses precisely this problem. I'm looking at #231 (comment) and trying to decide what to sequence to make this all work ... 🤔 |
Re #231 (comment) I may be confused but I think we just need to merge #231 to address that bit. |
@jdblischak sorry, I need to sync with you tomorrow ... |
Nightly feedstock build failure for tiledbsoma-feedstock at https://dev.azure.com/TileDB-Inc/CI/_build?definitionId=43&_a=summary |
I already tried applying your commit to update |
OK I'm entirely confused. I'll need to sit down and pencil-and-paper out the repro steps I need to follow to make this happen interactively, debug, and fix. Sorry for the delay. |
Today I setting The results:
|
I created a dedicated branch for troubleshooting the C++20 compilation errors https://github.com/TileDB-Inc/tiledbsoma-feedstock/tree/troubleshoot-cpp20 It pulls from the main branch of TileDB-SOMA (to mimic the nightly builds). I applied your fix from #231 and also updated the macOS SDK to 13.3 To troubleshoot, you can either push commits directly to this branch (or you can spin off your own branch if you'd prefer). Once we have a working feedstock build, then I'll worry about fixing the setup scripts for the nightly build. |
Thanks @jdblischak ! I'll check my notes on how to 'drive' a branch interactively rather than waiting for the nightlies (which is how I usually interact with this repo) |
Quick update. This didn't fix the osx-arm64 build of r-tiledbsoma. And I can tell why now. Despite having conda install version 13.3 of the macOS SDK, the compiler is still passed |
Also, another interesting development. After updating the macOS SDK to 13.3, the error message from the failed import test for Python is more informative: import: 'tiledbsoma'
Traceback (most recent call last):
File "/Users/runner/miniforge3/conda-bld/tiledbsoma_1732303363846/test_tmp/run_test.py", line 2, in <module>
import tiledbsoma
File "$PREFIX/lib/python3.9/site-packages/tiledbsoma/__init__.py", line 166, in <module>
from ._experiment import Experiment
File "$PREFIX/lib/python3.9/site-packages/tiledbsoma/_experiment.py", line 18, in <module>
from ._query import ExperimentAxisQuery
File "$PREFIX/lib/python3.9/site-packages/tiledbsoma/_query.py", line 48, in <module>
from somacore.query.query import (
ImportError: cannot import name 'Numpyable' from 'somacore.query.query' ($PREFIX/lib/python3.9/site-packages/somacore/query/query.py) |
Nightly feedstock build failure for tiledbsoma-feedstock at https://dev.azure.com/TileDB-Inc/CI/_build?definitionId=43&_a=summary |
@jdblischak re
So we do need (at least) an update to |
Closing the loop on 'how to drive': @jdblischak established in Slack:
That's it! I'd failed to realize how much autorun/trigger logic the inestimable John B has set up for this ✨ |
Addendum:
|
Nightly feedstock build failure for tiledbsoma-feedstock at https://dev.azure.com/TileDB-Inc/CI/_build?definitionId=43&_a=summary |
1 similar comment
Nightly feedstock build failure for tiledbsoma-feedstock at https://dev.azure.com/TileDB-Inc/CI/_build?definitionId=43&_a=summary |
@johnkerl Great insight! With your commit a11e54c, we now have a passing osx-64 build on troubleshoot-cpp20 |
Current status:
At this point:
|
Info:
|
@johnkerl thanks for the updates. Love to see the progress you are making. Just two things to add at the moment: Interestingly, your local builds are behaving differently than the Azure builds. For both of the above commits, the r-tiledbsoma builds on Azure continued to fail with the same error message:
That PR was merged into main 4 days ago in single-cell-data/TileDB-SOMA@f17d55c, so it was already included in all your previous builds on the feedstock branch troubleshoot-cpp. Your commit 78f4e01 had no effect on the build because we are using |
Ah, I see my confusion. In
-- and I use the second block for not-tag experiments like this. Whereas in our |
Nightly feedstock build failure for tiledbsoma-feedstock at https://dev.azure.com/TileDB-Inc/CI/_build?definitionId=43&_a=summary |
Today I did some experiments in centralized-tiledb-nightlies because it is able to build nightly TileDB-SOMA-R with C++20. Below are some potentially useful observations I made about this successful non-conda build setup. Main observations:
g++ -std=gnu++20 \
-I"/usr/share/R/include" \
-DNDEBUG \
-I. \
-I../inst/include/ \
-I/home/runner/work/centralized-tiledb-nightlies/centralized-tiledb-nightlies/install-libtiledb/include \
-I/home/runner/work/centralized-tiledb-nightlies/centralized-tiledb-nightlies/install-libtiledbsoma/include \
-I/home/runner/work/centralized-tiledb-nightlies/centralized-tiledb-nightlies/install-libtiledbsoma/include/tiledbsoma \
-D SPDLOG_USE_STD_FORMAT \
-I'/usr/lib/R/site-library/Rcpp/include'
-I'/usr/lib/R/site-library/RcppSpdlog/include' \
-I'/usr/lib/R/site-library/RcppInt64/include' \
-I'/usr/lib/R/site-library/nanoarrow/include' \
-fpic \
-Wno-deprecated-declarations -Wno-deprecated \
-c RcppExports.cpp -o RcppExports.o Minor observations:
|
This seems like a big clue as I believe we "really need to" build as an external project -- @nguyenv can you offer any insight? |
I just pushed a commit on |
No need to apologize! I was busy with other things today, so it took me a while to get to this. The good news is that your commit will pass 😄 |
Is there something about C++20 as compared to C++17 that affects the spdlog requirement? We've been installing spdlog from conda-forge for both libtiledb and libtiledbsoma for a while now: tiledbsoma-feedstock/recipe/meta.yaml Lines 52 to 53 in fd059f0
|
One thing is we don't require |
With C++20 we are setting |
@XanthosXanthopoulos let's please find a way to answer that question outside of the iodiosyncracies of this repo |
Yeah it seems everything just happened to be compatible and working fine before, but with the move to C++20, it's now revealing issues with this setup. I think we should build spdlog as an external project rather than relying on the conda-forge version.
No, with C++20 we should remove the |
Nightly feedstock build failure for tiledbsoma-feedstock at https://dev.azure.com/TileDB-Inc/CI/_build?definitionId=43&_a=summary |
If that ends up being required, we may need to add a CMake flag to force building spdlog as an external project. Even if we remove spdlog from the requirements for libtiledbsoma, it will still be installed in the conda environment where it is built: mamba create --dry-run -n test tiledb | grep -e fmt -e spdlog
## + fmt 11.0.2 h434a139_0 conda-forge Cached
## + spdlog 1.14.1 hed91bc2_1 conda-forge 196kB |
Nightly feedstock build failure for tiledbsoma-feedstock at https://dev.azure.com/TileDB-Inc/CI/_build?definitionId=43&_a=summary |
4 similar comments
Nightly feedstock build failure for tiledbsoma-feedstock at https://dev.azure.com/TileDB-Inc/CI/_build?definitionId=43&_a=summary |
Nightly feedstock build failure for tiledbsoma-feedstock at https://dev.azure.com/TileDB-Inc/CI/_build?definitionId=43&_a=summary |
Nightly feedstock build failure for tiledbsoma-feedstock at https://dev.azure.com/TileDB-Inc/CI/_build?definitionId=43&_a=summary |
Nightly feedstock build failure for tiledbsoma-feedstock at https://dev.azure.com/TileDB-Inc/CI/_build?definitionId=43&_a=summary |
Today I troubleshooted the linux-64 R build. I used the strategy I had previously identified (#230 (comment)) of defining the
CXX20 = $CXX
CXX20FLAGS = $CXXFLAGS -Wno-deprecated-declarations -Wno-deprecated
CXX20PICFLAGS = -fPIC
CXX20STD = -std=c++20
|
Nightly feedstock build failure for tiledbsoma-feedstock at https://dev.azure.com/TileDB-Inc/CI/_build?definitionId=43&_a=summary |
Thanks @jdblischak ! The double-free issue jives with what I saw here: I really appreciate your deep-dive into this:
-- this explains so much! |
Nice! Thanks for the reminder. So the good news is that we are both observing consistent behavior:
Interestingly, searching this long thread, I had also previously observed the same error message when importing the Python package on linux-64 (#230 (comment)). So while all troubleshooting routes are leading us to the same runtime error, which is nice for consistency, the bad news is that we don't know what is causing the error (speaking for myself at least, I'm at a loss of what additional build options to tweak). |
@jdblischak what I'd love to get is an interactive (non-CI) DOcker build that repros the double-free error. (I haven't succeeded.) |
Good idea. I think this should be feasible. If I can get it working, I'll send you instructions |
Nightly feedstock build failure for tiledbsoma-feedstock at https://dev.azure.com/TileDB-Inc/CI/_build?definitionId=43&_a=summary
The text was updated successfully, but these errors were encountered: