laz-perf api uses latest master?

nilsnolde commented 2 years ago

What is the bug or the crash?

Not even sure if you'd consider it an issue: just wanted to build https://github.com/qgis/QGIS/pull/48357 to see if it worked and got the following error message:

Consolidate compiler generated dependencies of target qgis_core
[ 33%] Linking CXX shared library ../../output/lib/libqgis_core.so
/usr/bin/ld: CMakeFiles/qgis_core.dir/pointcloud/qgscopcpointcloudindex.cpp.o: in function `QgsCopcPointCloudIndex::loadSchema(QgsLazInfo&)':
qgscopcpointcloudindex.cpp:(.text+0x6da): undefined reference to `lazperf::copc_info_vlr::fill(char const*, unsigned long)'
/usr/bin/ld: CMakeFiles/qgis_core.dir/pointcloud/qgslazinfo.cpp.o: in function `QgsLazInfo::parseRawVlrEntries(char*, unsigned long)':
qgslazinfo.cpp:(.text+0x57e): undefined reference to `lazperf::vlr_header::fill(char const*, unsigned long)'
/usr/bin/ld: CMakeFiles/qgis_core.dir/pointcloud/qgslazinfo.cpp.o: in function `QgsLazInfo::parseExtrabytes(char*, int, int)':
qgslazinfo.cpp:(.text+0x2dd3): undefined reference to `lazperf::eb_vlr::fill(char const*, unsigned long)'
collect2: error: ld returned 1 exit status

The latest laz-perf release is 3.0.0 and doesn't have that function yet, it's only been added a few months ago: https://github.com/hobuinc/laz-perf/commit/bbb31580fcb367788e23bb3469e2f2d0a4362d7e. I compiled laz-perf master, installed it and now the branch builds smoothly.

I was surprised that even a master QGIS build requires to have custom dependency builds and wonder how noone was bit yet.

Steps to reproduce the issue

Build https://github.com/qgis/QGIS/pull/48357 (and by extensions current master I guess)

Versions

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0//EN" "http://www.w3.org/TR/REC-html40/strict.dtd">

Supported QGIS version

[ ] I'm running a supported QGIS version according to the roadmap.

New profile

[ ] I tried with a new QGIS profile

Additional context

No response

ArchangeGabriel commented 2 years ago

Similarly, while building 3.26 against lazperf 3.0:

FAILED: src/providers/pdal/CMakeFiles/untwine.dir/__/__/__/external/untwine/bu/CopcSupport.cpp.o 
/usr/bin/c++ -DQT_DEPRECATED_WARNINGS -DQT_DISABLE_DEPRECATED_BEFORE=0x050800 -DQT_NO_CAST_TO_ASCII -DQT_USE_QSTRINGBUILDER -DSIP_VERSION=0x060602 -DWITH_QTWEBKIT -I/build/qgis/src/build/src/providers/pdal/untwine_autogen/include -I/build/qgis/src/build -I/build/qgis/src/qgis-3.26.0/external/untwine/api -I/build/qgis/src/qgis-3.26.0/external/untwine/untwine -I/build/qgis/src/qgis-3.26.0/external/untwine/epf -I/build/qgis/src/qgis-3.26.0/external/untwine/bu -I/build/qgis/src/qgis-3.26.0/untwine/api -I/build/qgis/src/build/untwine -march=x86-64 -mtune=generic -O2 -pipe -fno-plt -fexceptions         -Wp,-D_FORTIFY_SOURCE=2 -Wformat -Werror=format-security         -fstack-clash-protection -fcf-protection -Wp,-D_GLIBCXX_ASSERTIONS -flto=auto -Wall -Wextra -Wno-long-long -Wformat-security -Wno-strict-aliasing -Wnon-virtual-dtor -Wno-redundant-move -Wno-misleading-indentation -Wno-deprecated-copy -fvisibility=hidden -MD -MT src/providers/pdal/CMakeFiles/untwine.dir/__/__/__/external/untwine/bu/CopcSupport.cpp.o -MF src/providers/pdal/CMakeFiles/untwine.dir/__/__/__/external/untwine/bu/CopcSupport.cpp.o.d -o src/providers/pdal/CMakeFiles/untwine.dir/__/__/__/external/untwine/bu/CopcSupport.cpp.o -c /build/qgis/src/qgis-3.26.0/external/untwine/bu/CopcSupport.cpp
/build/qgis/src/qgis-3.26.0/external/untwine/bu/CopcSupport.cpp: In member function ‘void untwine::bu::CopcSupport::addEbFields()’:
/build/qgis/src/qgis-3.26.0/external/untwine/bu/CopcSupport.cpp:128:29: error: no matching function for call to ‘lazperf::eb_vlr::addField(lazperf::eb_vlr::ebfield&)’
  128 |             m_ebVlr.addField(f);
      |             ~~~~~~~~~~~~~~~~^~~
In file included from /build/qgis/src/qgis-3.26.0/external/untwine/bu/CopcSupport.hpp:22,
                 from /build/qgis/src/qgis-3.26.0/external/untwine/bu/CopcSupport.cpp:16:
/usr/include/lazperf/vlr.hpp:159:10: note: candidate: ‘void lazperf::eb_vlr::addField()’
  159 |     void addField();
      |          ^~~~~~~~
/usr/include/lazperf/vlr.hpp:159:10: note:   candidate expects 0 arguments, 1 provided

nilsnolde commented 2 years ago

I forgot to update the issue (and it's a while since I did that, plus not in the office right now to check) that qgis can use an internal version of lazperf: https://github.com/qgis/QGIS/blob/415c545bae38c47355d3b46c7dfb1f9b6bac5d40/CMakeLists.txt#L414-L422

that looks like a cmake option, so you should be able to add -DWITH_INTERNAL_LAZPERF or select it in ccmake. if you try that and it works, let us know, then I'll close this issue.

ArchangeGabriel commented 2 years ago

Of course it works, actually it even defaults to TRUE as you can see, so you have to set -DWITH_INTERNAL_LAZPERF=FALSE to be able to compile against an external lazperf. But this is not a proper solution, since as a distro packager I’m expected to use system libraries and not vendored copies used by each project.

nilsnolde commented 2 years ago

true, didn't even see that in the haste. back then I didn't realize either and it didn't work out-of-the-box, wonder why in hindsight.. anyways, would be great if that could be amended. also didn't think about the packaging side and explains why arch doesn't have 3.26.0 yet.

ArchangeGabriel commented 2 years ago

(Arch is blocked by https://github.com/qgis/QGIS/issues/49214 actually, we would live with an internal lazperf for the time being)

qgis / QGIS