bacpop / unitig-caller

Methods to determine sequence element (unitig) presence/absence
Apache License 2.0
18 stars 3 forks source link

Source code compatibility with SeqAn3 3.2 and C++20 #13

Closed jmarshall closed 2 years ago

jmarshall commented 2 years ago

SeqAn3 3.2.0 was released in June and has dropped support for C++17. Attempting to build unitig-caller against current SeqAn3 (in a Bioconda context; see also bioconda/bioconda-recipes#36884) produced complaints about not using ‑std=c++20.

This was easily fixed by adjusting unitig-caller's CMakeLists.txt, line 3:

set(CMAKE_CXX_STANDARD 20)

However even after fixing this to use C++20, there is a cascade of C++ build errors:

[ 25%] Building CXX object CMakeFiles/unitig_query.dir/src/map_strings.cpp.o
…/x86_64-conda-linux-gnu-c++ -DSEQAN3_HAS_BZIP2=1 -DSEQAN3_HAS_ZLIB=1 -Dunitig_query_EXPORTS … -fvisibility-inlines-hidden -std=c++17 -fmessage-length=0 -march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O2 -ffunction-sections -pipe … -DVERSION_INFO=\"1.2.1\" -fconcepts -O3 -DNDEBUG -fPIC -fvisibility=hidden -flto -fno-fat-lto-objects -std=c++20 -pthread -std=gnu++2a -MD -MT CMakeFiles/unitig_query.dir/src/map_strings.cpp.o -MF CMakeFiles/unitig_query.dir/src/map_strings.cpp.o.d -o CMakeFiles/unitig_query.dir/src/map_strings.cpp.o -c …/conda-bld/unitig-caller_1664446063704/work/src/map_strings.cpp
…/conda-bld/unitig-caller_1664446063704/work/src/map_strings.cpp: In function 'void call_strings(const std::vector<std::__cxx11::basic_string<char> >&, const std::vector<std::__cxx11::basic_string<char> >&, const std::vector<std::__cxx11::basic_string<char> >&, const string&, bool, size_t)':
…/conda-bld/unitig-caller_1664446063704/work/src/map_strings.cpp:73:85: error: no match for 'operator|' (operand types are 'std::ranges::transform_view<std::ranges::ref_view<const std::__cxx11::basic_string<char> >, <lambda(auto:56&&)> >' and '<unresolved overloaded function type>')
   73 |       seqan3::dna5_vector query = *unitig_it | seqan3::views::char_to<seqan3::dna5> | seqan3::views::to<std::vector>;
      |                                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                              |                                                       |
      |                                              |                                                       <unresolved overloaded function type>
      |                                              std::ranges::transform_view<std::ranges::ref_view<const std::__cxx11::basic_string<char> >, <lambda(auto:56&&)> >

[…etc…]

The SeqAn3 3.2.0 release notes mention that seqan3::views::to has been substantially changed.

Do you plan to support SeqAn3 3.2.x? I haven't looked at whether the source code changes required would remain workable with earlier versions of SeqAn3…

johnlees commented 2 years ago

Thanks for this detailed report, I'll try and take a look next week, but I think I might replace seqan with sdsl-lite-3 as we're just using an FM-index, and the latter has a more stable API so would hopefully prevent similar problems in future.

johnlees commented 2 years ago

@jmarshall I have made a PR and release which swaps out seqan for sdsl-lite, so this should sort this out. I presume a bioconda PR for the new release will automatically be made shortly -- I can edit and merge when I see it

jmarshall commented 2 years ago

Thanks; that seems like a fairly sensible approach.

I don't know why the bot didn't notice the new release, but I see you've created a bioconda-recipes PR now anyway.