search

OpenPHDGuiding / phd2

PHD2 Guiding
https://openphdguiding.org
BSD 3-Clause "New" or "Revised" License
244 stars 111 forks source link

PHD2 no more build in Ubuntu PPA #1152

Closed pchev closed 6 months ago

pchev commented 6 months ago

After the last changes in the PHD2 build system the build failed for the Ubuntu PPA at https://launchpad.net/~pch/+archive/ubuntu/phd2

I already make two change I will add to a PR later, for now this is in branch patrick/fix-ppa

Now the build fail because of gtest, probably because of this change : https://github.com/OpenPHDGuiding/phd2/commit/94cafff72ca4a026f0a80bd786734355f46d7b76

First the new requirement for cmake 3.24 is not available in current stable Ubuntu 22.04, the error message is: CMake Error at CMakeLists.txt:31 (cmake_minimum_required): CMake 3.24 or higher is required. You are running version 3.22.1

This version of cmake is available in Ubuntu 23.10 but then it fail when compiling with gtest: /<>/contributions/MPI_IS_gaussian_process/tests/gaussian_process/math_tools_test.cpp:38:10: fatal error: gtest/gtest.h: No such file or directory 38 | #include <gtest/gtest.h>

The full log is at : https://launchpadlibrarian.net/706009692/buildlog_ubuntu-mantic-amd64.phd2_2.6.13.rev20231227d-0ppa1~ubuntu23.10_BUILDING.txt.gz

I not really understand how gtest is installed but if this is from the network we can reach the same limitation as for INDI. All work fine when compiling on my local computer. Please @agalasso can you take a look at this issue.

agalasso commented 6 months ago

Thanks @pchev I will take a look at the issues.

pchev commented 6 months ago

I just look more in detail at https://github.com/OpenPHDGuiding/phd2/commit/94cafff72ca4a026f0a80bd786734355f46d7b76 and remark it suppress the option USE_SYSTEM_GTEST. There is a libgtest-dev version 1.13 packaged with Ubuntu, maybe the more simple is to revive this option.

agalasso commented 6 months ago

yes, reinstating USE_SYSTEM_GTEST is a good solution; the other thing I was thinking would be to make building the test executables optional and perhaps disabled by default. gtest is only needed for building the test executables, and those are not needed for the debian package.

pchev commented 6 months ago

It is good if we can keep the test, because the test are run as part of the build process and the build fail in case of error in the test. This can detect an issue with the Eigen library like we have some years ago with the i386 version.

agalasso commented 6 months ago

ah, that's great that the test is run as part of the ppa build! USE_SYSTEM_GTEST sounds like the right solution then. I'll put up a pr for that directly against master

agalasso commented 6 months ago

1153 should address the gtest issue

looking into fixing the cmake_minimum_required issue now...

agalasso commented 6 months ago

@pchev I pushed another commit to #1153 's branch which allowed building on focal (cmake 3.16)