jurplel / install-qt-action

Install Qt on your Github Actions workflows with just one simple action
MIT License
455 stars 78 forks source link

What's the purpose of `Qt5_DIR` environment? #184

Closed wy-luke closed 1 year ago

wy-luke commented 1 year ago

Firstly thank you for your great work! It helps!

I have tried these in my mac.

  1. If the Qt bin directory, .../bin, is appended to the path environment variable, CMake then could find Qt. No need of the Qt5_DIR environment.

  2. If only create the Qt5_DIR environment, using export command, CMake cannot find Qt.

Besides, in CMake the Qt5_DIR environment usually is .../lib/cmake/Qt5, rather the base folder Qt installed, like ${{ github.workspace }}/Qt/5.15.2/clang_64.

So I wonder what's the purpose of creating the Qt5_DIR environment. And you said in the readme that "Qt5_DIR/Qt6_DIR is also set appropriately for cmake." I don't think that CMake would use these two environment. Is there anything I don't konw or misunderstood?

I would appreciate it greatly if you could help me for solving my confusion. Thanks!

ddalcino commented 1 year ago

I’m not 100% sure about this, but I think it’s likely due to confusion about the meaning/intent of the variable. You are correct that for cmake, Qt5_DIR should point to a directory that includes .cmake files. From https://doc.qt.io/qt-5/cmake-get-started.html:

Set the Qt5_DIR in the CMake cache to the location of the Qt5Config.cmake file.

Unfortunately, I imagine that fixing this issue would be a breaking change, and would require bumping up the install-Qt-action major version from 3 to 4. Qt5_DIR has pointed to the qmake directory for a very long time, and it’s possible that users are depending on it for the location of the base Qt directory.

Additionally, I think it would be helpful to add a test project that uses cmake instead of qmake, so we can be confident that the action supports both build systems.

jurplel commented 1 year ago

Yeah, sounds to me like it was a mistake. I remember finding that Qt5_DIR worked as is now but it was probably because of appending to path.

ddalcino commented 1 year ago

The Qt 6 cmake docs (https://doc.qt.io/qt-6/cmake-get-started.html#building-a-c-console-application) do not make any reference to a Qt6_DIR environment variable, but they do say this:

For find_package to be successful, CMake must find the Qt installation. There are different ways you can tell CMake about Qt, but the most common and recommended approach is to set the CMake cache variable CMAKE_PREFIX_PATH to include the Qt 6 installation prefix. Note that Qt Creator will handle this transparently for you.

I am able to successfully configure a cmake project with Qt6 by running this command in the project source directory, where QT_INSTALL_PREFIX points to the Qt architecture directory (gcc_64 on linux, clang_64 or macos on mac, etc):

$ cmake -S . -B ./build -DCMAKE_PREFIX_PATH="${QT_INSTALL_PREFIX}"

I think it definitely makes sense to have an environment variable that points to the Qt architecture directory, equivalent to what we are doing for Qt5_DIR and Qt6_DIR right now. I think it doesn't make sense to call it Qt5_DIR because of the issue raised by OP, and it doesn't make sense to call it Qt6_DIR because CMake cannot use that variable to automatically find where Qt is hiding. I am tempted to call it something like QT_INSTALL_PREFIX or QT_ROOT_DIR; alternative suggestions are welcome.

wy-luke commented 1 year ago

@ddalcino As far as I know, the CMAKE_PREFIX_PATH should be pointed the QT_ROOT_DIR/lib/cmake, rather the QT_ROOT_DIR.

Dose your PATH environment variable have the Qt bin folder already? In CMake, the find_package would find package in the CMAKE_PREFIX_PATH, PATH.... consequently. I think that maybe your command worded was not because of the CMAKE_PREFIX_PATH.

And I alse think that CMake do not make any reference to a Qt6_DIR environment variable to find Qt via the find_package command. As above mentions, find_package will find Qt in a couple of folders defined in some environment variables. After finding the Qt package, *then CMake will set the Qt_DIR, Qt5_DIR, QtCore_DIR ... and so on accoding to the path of Qt package that was found.

I totally agree your idea the add a QT_ROOT_DIR to replace current Qt_DIR in the v4 version.

ddalcino commented 1 year ago

@ddalcino As far as I know, the CMAKE_PREFIX_PATH should be pointed the QT_ROOT_DIR/lib/cmake, rather the QT_ROOT_DIR.

I'm running the command locally. Qt is not in my PATH. Both of the following commands work properly with Qt6, but every other path I have tried has not:

I totally agree your idea the add a QT_ROOT_DIR to replace current Qt_DIR in the v4 version.

Ok, great!

I started to make a CMake build file, and run it in CI here. Right now, it configures properly because the Qt bin folder is in the path already. I was hoping for a failing test before I go any further, but I can't do that without removing the Qt bin folder from the path. I doubt that's something we would want to do.

wy-luke commented 1 year ago

Sorry for the incorrect spelling. It should be Qt5_DIR rather QT5_DIR in CMake. I have corrected it in title and the first comment.

wy-luke commented 1 year ago

@ddalcino Yes you're right. I have tested it on my PC, confiming that using -DCMAKE_PREFIX_PATH="${QT_ROOT_DIR}" also works fine as ./build -DCMAKE_PREFIX_PATH="${QT_ROOT_DIR}/lib/cmake".

And I also comfims that in Windows there is also no need to set CMAKE_PREFIX_PATH after adding Qt bin folder to PATH. I think that in Linux it will work, too.