Fails to build with pybind 2.8.1

[nileshpatra]

Hi @paoloczi

shasta does not seem to build with pybind API changes in 2.8.1, and the corresponding package is failing to build in debian.

> cd /<<PKGBUILDDIR>>/obj-x86_64-linux-gnu/staticLibrary && /usr/bin/c++ -DBOOST_ERROR_CODE_HEADER_ONLY -DBOOST_SYSTEM_NO_DEPRECATED -DBUILD_ID=0.8.0 -DNDEBUG -DSHASTA_HTTP_SERVER -I/<<PKGBUILDDIR>>/staticLibrary/../src -g -O2 -ffile-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2   -std=c++17 -Wall -Wconversion -Wno-unused-result -g0 -O3 -mcx16 -MD -MT staticLibrary/CMakeFiles/shastaStaticLibrary.dir/__/src/ReadLoader.cpp.o -MF CMakeFiles/shastaStaticLibrary.dir/__/src/ReadLoader.cpp.o.d -o CMakeFiles/shastaStaticLibrary.dir/__/src/ReadLoader.cpp.o -c /<<PKGBUILDDIR>>/src/ReadLoader.cpp
> /<<PKGBUILDDIR>>/src/PythonModule.cpp: In function ‘void pybind11_init_shasta(pybind11::module_& (*)(pybind11::module))’:
> /<<PKGBUILDDIR>>/src/PythonModule.cpp:40:29: error: expected primary-expression before ‘(’ token
>    40 |     class_<OrientedReadPair>(module, "OrientedReadPair")
>       |                             ^
> /<<PKGBUILDDIR>>/src/PythonModule.cpp:40:36: error: expected primary-expression before ‘,’ token
>    40 |     class_<OrientedReadPair>(module, "OrientedReadPair")
>       |                                    ^
> /<<PKGBUILDDIR>>/src/PythonModule.cpp:46:18: error: expected primary-expression before ‘(’ token
>    46 |     class_<Reads>(module, "Reads")
>       |                  ^
> /<<PKGBUILDDIR>>/src/PythonModule.cpp:46:25: error: expected primary-expression before ‘,’ token
>    46 |     class_<Reads>(module, "Reads")
>       |                         ^
> /<<PKGBUILDDIR>>/src/PythonModule.cpp:83:25: error: expected primary-expression before ‘(’ token
>    83 |     class_<AlignOptions>(module, "AlignOptions")
>       |                         ^
> /<<PKGBUILDDIR>>/src/PythonModule.cpp:83:32: error: expected primary-expression before ‘,’ token
>    83 |     class_<AlignOptions>(module, "AlignOptions")
>       |                                ^
> /<<PKGBUILDDIR>>/src/PythonModule.cpp:110:22: error: expected primary-expression before ‘(’ token
>   110 |     class_<Assembler>(module, "Assembler")
>       |                      ^

Full log with Bug report can be found here Could you please fix this?

[paoloczi]

Yes, currently Shasta builds with an old version of pybind11. I will look into fixing this soon. However I need to make sure Shasta continues to build on Ubuntu 20.04, at least until 22.04 comes out. Do you know if that new version of pybind11 would work on Ubuntu 20.04?

There is also another Debian related open PR that I have not been able to work on due to conflicting priorities, but I hope to be able to get additional help soon.

[paoloczi]

Another possibility would be for Debian to only build the Shasta static executable, which does not include the Shasta Python API. The Shasta Python API is currently only used for Shasta development, and the small number of people using it will probably be working on the current Shasta version on GitHub rather than using the Debian package. On the other hand, providing a Debian package for the Shasta static executable is very helpful.

[nileshpatra]

@paoloczi I think you could do it simply on the lines of

#if PYBIND11_VERSION_HEX >= 0x02080000
 [.... new API changes ...]
#else 
 [... old code ...]
#endif 

I saw a similar path, for instance here

I don't know much about this, but this way probably code will be compatible on previous versions as well. Does this sound sensible? (again, I am unaware about pybind11 APIs)

[paoloczi]

Thank you @nileshpatra! I will try that suggestion next week.

[paoloczi]

I was able to reproduce this problem by installing pybind11 2.8.1 on a fresh Ubuntu 20.04 install via pip3 install pybind11==2.8.1. I will work on a fix.

I will not be able to use PYBIND11_VERSION_HEX because older versions of pybind11 don't define it. I can instead test PYBIND11_VERSION_MAJOR, PYBIND11_VERSION_MINOR, and PYBIND11_VERSION_PATCH.

[paoloczi]

It was a much simpler problem, which did not require testing the pybind11 version. The code was using a variable named module, which is now a reserved C++ keyword, due the introduction of modules in C++20. This is a problem even though the code is compiled with -std=c++17 because that option also enables some C++20 features. For some reason this usage of a reserved keyword did not trigger an error in older versions of pybind11, probably due to changes in the definition of a pybind11 macro.

The fix is in commit 3dd42d4 and consists of simply changing the name of the offending variable. I tested it by installing pybind11 2.8.1 in Ubuntu 20.04 via pip3 install pybind11==2.8.1. I am pretty confident it will work in your Debian build environment because the messages I was getting before the fix were identical to the ones shown in the log you attached. But please check on that, and close the issue when you verify that all is good.

[nileshpatra]

Thanks, I have uploaded it. It'll build in the next few hours. I'll close when all's good. Thanks a lot for your help and the patch.

[nileshpatra]

All good, thanks again for the fix!