coin3d / quarter

Coin GUI binding for Qt
BSD 3-Clause "New" or "Revised" License
36 stars 26 forks source link

Fix plugins destination #53

Closed podsvirov closed 2 years ago

podsvirov commented 2 years ago

Best practices say to use relative paths for DESTINATION (from CMake documentation):

As absolute paths are not supported by cpack installer generators, it is preferable to use relative paths throughout. In particular, there is no need to make paths absolute by prepending CMAKE_INSTALL_PREFIX; this prefix is used by default if the DESTINATION is a relative path.

UPDATE: I use similar patch for MSYS2 package.

VolkerEnderlein commented 2 years ago

Thanks for the fix, Konstantin. Unfortunately the static code analysis failed, see the attached build log file. It uses Ubuntu 19.10 (Eoan) and Qt 5.12.4. I inspected the log but did not see an obvious reason for this failure. Can you insert message(STATUS "prefix: ${QT_INSTALL_PREFIX} plugins: ${QT_INSTALL_PLUGINS}") before the file(RELATIVE_PATH ...) command to further debug the issue? 572d27ccc184736990a1f05ccfcd46f50412fe49_build.log

podsvirov commented 2 years ago

From LGTM log:

[2021-11-04 06:25:19] [build-stdout] Semmle autobuild: no supported build system detected.
[2021-11-04 06:25:19] [build-stderr] + '[' -f SConstruct ']'
[2021-11-04 06:25:19] [build-stderr] + '[' -f wscript ']'
[2021-11-04 06:25:19] [build-stderr] + '[' -f Makefile ']'
[2021-11-04 06:25:19] [build-stderr] + '[' -f makefile ']'
[2021-11-04 06:25:19] [build-stderr] + '[' -f GNUmakefile ']'
[2021-11-04 06:25:19] [ERROR] Spawned process exited abnormally (code 1; tried to run: [/opt/dist/tools/linux64/preload_tracer, /opt/dist/cpp/tools/do-build])
[2021-11-04 06:25:19] [build-stderr] + '[' -f build.ninja ']'
[2021-11-04 06:25:19] [build-stderr] + '[' -d ../_lgtm_build_dir ']'
[2021-11-04 06:25:19] [build-stderr] + for f in build build.sh
[2021-11-04 06:25:19] [build-stderr] + '[' -x build ']'
[2021-11-04 06:25:19] [build-stderr] + '[' -f build ']'
[2021-11-04 06:25:19] [build-stderr] + for f in build build.sh
[2021-11-04 06:25:19] [build-stderr] + '[' -x build.sh ']'
[2021-11-04 06:25:19] [build-stderr] + '[' -f setup.py ']'
[2021-11-04 06:25:19] [build-stderr] + echo 'Semmle autobuild: no supported build system detected.'
[2021-11-04 06:25:19] [build-stderr] + exit 1
[2021-11-04 06:25:19] [build-stderr] A fatal error occurred: Exit status 1 from command: [/opt/dist/cpp/tools/do-build]

I would expect to see СMakeLists.txt on this list, but it is not there. It seems to me that all these errors have nothing to do with my changes.

VolkerEnderlein commented 2 years ago

It's at line 577 of the attached log file. file(RELATIVE_PATH ...) fails for unknown reason. Semmle tries automatically to detect what build system needs to be called, so it starts with CMake and if that fails it tries configure.

VolkerEnderlein commented 2 years ago

Attached is the log of the last run. It fails for the same reason. (see line 577) Apparently the CMake variables are all empty. But I don't know why. The only variable that has a proper value is QT_QMAKE_EXECUTABLE. This is for Qt 5.12.4. Since what qmake version qmake -query supports QT_INSTALL_PREFIX and QT_INSTALL_PLUGINS? My quick search did not reveal any results. [ee804c73bbff929797c5f22887d66cb174fc8730_build.log]

[2021-11-04 06:24:37] [build-stderr] qmake-wrapper: first run
[2021-11-04 06:24:37] [build-stderr] /usr
[2021-11-04 06:24:37] [build-stderr] qmake-wrapper: first run
[2021-11-04 06:24:37] [build-stderr] /usr/lib/x86_64-linux-gnu/qt5/plugins
[2021-11-04 06:24:37] [build-stderr] CMake Error at src/plugins/CMakeLists.txt:51 (file):
[2021-11-04 06:24:37] [build-stderr]   file RELATIVE_PATH must be passed a full path to the directory:
[2021-11-04 06:24:37] [build-stdout] -- QT_QMAKE_EXECUTABLE: "/usr/lib/x86_64-linux-gnu/qt5/bin/qmake"
[2021-11-04 06:24:37] [build-stdout] -- QT_INSTALL_PREFIX: ""
[2021-11-04 06:24:37] [build-stdout] -- QT_INSTALL_PLUGINS: ""
[2021-11-04 06:24:37] [build-stdout] -- CMAKE_INSTALL_QTPLUGINSDIR_DEFAULT: ""
[2021-11-04 06:24:37] [build-stdout] -- CMAKE_INSTALL_QTPLUGINSDIR: ""
[2021-11-04 06:24:37] [build-stdout] -- Configuring incomplete, errors occurred!

(https://github.com/coin3d/quarter/files/7473026/ee804c73bbff929797c5f22887d66cb174fc8730_build.log)

podsvirov commented 2 years ago

Summary: code work as expected (CI checks), but LGTM check failed...

From attached log:

[2021-11-03 19:16:24] [build-stderr] qmake-wrapper: first run
[2021-11-03 19:16:24] [build-stderr] /usr
[2021-11-03 19:16:24] [build-stderr] qmake-wrapper: first run
[2021-11-03 19:16:24] [build-stderr] /usr/lib/x86_64-linux-gnu/qt5/plugins
[2021-11-03 19:16:24] [build-stderr] CMake Error at src/plugins/CMakeLists.txt:51 (file):
[2021-11-03 19:16:24] [build-stderr]   file RELATIVE_PATH must be passed a full path to the directory:
[2021-11-03 19:16:24] [build-stdout] -- Configuring incomplete, errors occurred!

This equal in CMakeLists.txt:

  file(RELATIVE_PATH CMAKE_INSTALL_QTPLUGINSDIR_DEFAULT
    "/usr" "/usr/lib/x86_64-linux-gnu/qt5/plugins"
  )

But why /usr is not a full path to the directory?

VolkerEnderlein commented 2 years ago

As this should work out of the box as the CI for various platform runs prove, I think it's time to open an issue with LGTM. Will report back.

podsvirov commented 2 years ago

Can you insert message(STATUS "prefix: ${QT_INSTALL_PREFIX} plugins: ${QT_INSTALL_PLUGINS}") before the file(RELATIVE_PATH ...)

Should I refactor current debug (message per line)?

VolkerEnderlein commented 2 years ago

I don't think so. Let's wait, maybe there are other change requirements from the LGTM folks.

VolkerEnderlein commented 2 years ago

See created issue for further reference.

podsvirov commented 2 years ago

Since what qmake version qmake -query supports QT_INSTALL_PREFIX and QT_INSTALL_PLUGINS?

According to the documentation, they are supported by both Qt5 and Qt6. It seems to me that they have always been.

VolkerEnderlein commented 2 years ago

@podsvirov Thanks for the links. To further debug the issue we could also inspect the ERROR_VARIABLE and RESULT_VARIABLE of the execute_process commands. Maybe they give some additional information what went wrong.

podsvirov commented 2 years ago

I am alarmed by the cryptic wrappers (qmake-wrapper, cmake-wrapper and so on) and how they work with standard streams.

VolkerEnderlein commented 2 years ago

It seems you are right. Now the error looks as follows:

[2021-11-04 09:36:41] [build-stderr] CMake Error at src/plugins/CMakeLists.txt:55 (file):
[2021-11-04 09:36:41] [build-stdout] -- QT_QMAKE_EXECUTABLE: "/usr/lib/x86_64-linux-gnu/qt5/bin/qmake"
[2021-11-04 09:36:41] [build-stderr]   file RELATIVE_PATH must be passed a full path to the directory:
[2021-11-04 09:36:41] [build-stdout] -- QT_INSTALL_PREFIX: ""
[2021-11-04 09:36:41] [build-stdout] -- QT_INSTALL_PREFIX_RESULT: "0"
[2021-11-04 09:36:41] [build-stdout] -- QT_INSTALL_PREFIX_ERROR: "
[2021-11-04 09:36:41] [build-stdout] qmake-wrapper: first run
[2021-11-04 09:36:41] [build-stdout] /usr
[2021-11-04 09:36:41] [build-stdout] "
[2021-11-04 09:36:41] [build-stdout] -- QT_INSTALL_PLUGINS: ""
[2021-11-04 09:36:41] [build-stdout] -- QT_INSTALL_PLUGINS_ERROR: "
[2021-11-04 09:36:41] [build-stdout] qmake-wrapper: first run
[2021-11-04 09:36:41] [build-stdout] /usr/lib/x86_64-linux-gnu/qt5/plugins
[2021-11-04 09:36:41] [build-stdout] "
[2021-11-04 09:36:41] [build-stdout] -- QT_INSTALL_PLUGINS_ERROR: "
[2021-11-04 09:36:41] [build-stdout] qmake-wrapper: first run
[2021-11-04 09:36:41] [build-stdout] /usr/lib/x86_64-linux-gnu/qt5/plugins
[2021-11-04 09:36:41] [build-stdout] "
[2021-11-04 09:36:41] [build-stdout] -- CMAKE_INSTALL_QTPLUGINSDIR_DEFAULT: ""
[2021-11-04 09:36:41] [build-stdout] -- CMAKE_INSTALL_QTPLUGINSDIR: ""

Build Log:

b583226e4f70c5a696648090ef011d1b796cd3e5_build.log

podsvirov commented 2 years ago

Bingo! Wrappers return zero code (mean all good) but redirect modified original output to error stream.

podsvirov commented 2 years ago

Summary: we found root of outside problem and open issue, current code work as expected. Can I remove debug code to prepare for merge?

VolkerEnderlein commented 2 years ago

Yup. I think it's OK to remove the debug stuff. Thank you for your work on this issue.

VolkerEnderlein commented 2 years ago

I just detected another issue. The proposed changes only work when Qt and Coin3d are installed under the same prefix. In MSYS2 it is /usr. When I tested the changes compiling with MSVC the plugins are installed under the Coin3D installation prefix ${CMAKE_INSTALL_PRREFIX}/plugins/designer that in general will differ from the Qt installation prefix.

podsvirov commented 2 years ago

In general, the installation prefix should be the same for all.

On your machine, you have 2 configuration options:

Where is QTDIR the Qt installation prefix.