Compatibility with Qt6 does not wreck Qt5

Adapted GeoModelVisualization to Qt6 and to the recently released newversions of Coin (4.0.2) and SoQt (1.6.2). A number of classes were suppressed totally, since they are not used (eg those dealing with "2d systems" none of which are used here. This was preferable to making dead code compile.

This work should be tested on a variety of platforms before getting merged to main. The branch can stay open until such time as.

enabled an automatic merge when the pipeline for cb316274 succeeds

canceled the automatic merge

Hi @boudreau, the CI failures here seem to be related to your changes. Could you please have a look? Thanks.

marked this merge request as draft

added 1 commit

• 5951487c - Fix build of GeoModelVisualization when built by itself

Compare with previous version

added 1 commit

• 31087621 - another fix to the CMakeLists.txt file, this time in FSL

Compare with previous version

marked this merge request as ready

merged

mentioned in commit c608dded

Hi @boudreau , I noticed that the build of the docker image is failing in the CI; which means that the docker image that is used by the GeoModelATLAS CI cannot be built.

The job is marked in yellow, as a warning, because it is formally "allowed to fail". Very often, in fact, the docker build faikled because of network glitches while fetching or pushing to the CERN GitLab Registry. A simple "retry" usually solved the problem. That's why, I set that job as "allowed to fail" to not block the CI.

But this looks as a real issue, because it looks related to the new GeoModelFunCSnippets:

[ 39%] Building CXX object GeoModelTools/GeoModelFuncSnippets/CMakeFiles/GeoModelFuncSnippets.dir/src/StringUtils.cxx.o
/workdir/GeoModel_src/GeoModelTools/GeoModelFuncSnippets/src/StringUtils.cxx: In instantiation of 'void GeoStrUtils::convertToNumber(std::string_view, dType&) [with dType = double; std::string_view = std::basic_string_view<char>]':
/workdir/GeoModel_src/GeoModelTools/GeoModelFuncSnippets/src/StringUtils.cxx:135:36:   required from here

Maybe @jojungge could take a look as well?

Thanks! --Ric.

P.S. @boudreau , I just noticed this branch was merged already. But, if I'm not wrong, you wrote in the MR's description: "This work should be tested on a variety of platforms before getting merged to main. The branch can stay open until such time as."

Did you complete all the tests you intended to perform? Have you noticed any issues on other platforms?

Thanks!

No, I did something else instead. I put the switch so that Qt5 is built unless Qt6 is requested explicitly. So I think the tests can be done before (eventually) updating to Qt6.

OK, thanks.

However, I noticed compilation errors on macOS when both Qt5 and Qt6 are installed on the machine with Brew.

Brew installs Qt6 as qt and Qt5 as qt@5. CMake correctly sees Qt5 as the correct Qt version when we do not use the GEOMODEL_USE_QT6 flag; but then the linker complain because it picks Qt6 instead.

The solution is simply to remove the Qt6 package that was installed with brew, if any:

brew remove qt

Of course, this is a temporary solution until we globally switch to Qt6.

P.S. Concerning the main branch, the build of main on macOS is broken now, due to an incompatibility of one of the C++17 features on Apple Clang, I reported that on this issue: #103 (closed)

We should revive the macOS pipeline soon, otherwise we only test the CI on Ubuntu and we overlook such errors on macOS

I have a fix for that. @jojungge should check my draft MR 253

Thanks, @boudreau. Since the failing code has been merged already into main, I would undraft your MR and merge it; so that we can fix the build of main on macOS.

OK, going for that.

Thanks. Also, I just added a new warning about Qt5 and Qt6 on macOS in the documentation:

https://geomodel.web.cern.ch/home/dev/#third-party-libraries-macos

mentioned in merge request !336 (merged)