Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial port to Qt6 #77

Merged
merged 11 commits into from
Mar 5, 2024
Merged

Initial port to Qt6 #77

merged 11 commits into from
Mar 5, 2024

Conversation

luis-pereira
Copy link
Member

We intend to be able to have Qt5 lxqt-build-tools and Qt6 lxqt-build-tools
installed to the same prefix location.

Major changes:
* Package name is lxqt2-build-tools.
* CMake minimum required version is 3.16.0
* start script renamed to startlxqt2
* translation update script renamed to lxqt2-transupdate
* Qt5TanslataionLoader CMake modules renamed to Qt6TranslationLoader.
Qt6 support only. USE_QT5 option dropped.
* LXQT_LIBRARY_NAME set to lxqt2

@luis-pereira luis-pereira mentioned this pull request Jun 17, 2022
@stefonarch
Copy link
Member

As we have to live with those executables for some years - wouldn't lxqt6-foo be a more suitable naming scheme?

@luis-pereira
Copy link
Member Author

we have to live with those executables for some years - wouldn't lxqt6-foo be a more suitable naming scheme?

The executable can stay lxqt (exception: maybe startlxqt2). But not the libraries. That's what KDE handles KF libraries,
It's odd jumping from lxqt1 to lxqt6. My proposition is that LXQt2 uses Qt6., LXQt3 -> Qt7

@luis-pereira luis-pereira requested a review from tsujan July 6, 2022 15:48
@tsujan
Copy link
Member

tsujan commented Jul 6, 2022

It's odd jumping from lxqt1 to lxqt6

I agree.The names of executables shouldn't change; the Qt6 ones will replace those of Qt5.

@tsujan
Copy link
Member

tsujan commented Jul 8, 2022

@luis-pereira

Something has changed about QT_INSTALL_CONFIGURATION. Here, with Qt 6.3.1, I get

$ qmake6 -query QT_INSTALL_CONFIGURATION
**Unknown**

While, for example, QT_INSTALL_BINS and QT_INSTALL_DATA are OK.

As a result:

CMake Warning at cmake/FindInstallConfigPath.cmake:37 (message):
  Failed call: /usr/lib/qt6/bin/qmake -query "QT_INSTALL_CONFIGURATION"
Call Stack (most recent call first):
  CMakeLists.txt:20 (include)


-- You can set it manually with -DLXQT_ETC_XDG_DIR=<value>
--
CMake Error at cmake/FindInstallConfigPath.cmake:39 (message):
  QMake call failed with return code: 101
Call Stack (most recent call first):
  CMakeLists.txt:20 (include)


-- Configuring incomplete, errors occurred!

EDIT: -DLXQT_ETC_XDG_DIR=/etc/xdg works, of course. But, IMHO, it shouldn't be needed.

@tsujan
Copy link
Member

tsujan commented Jul 8, 2022

This is the output of qmake6 -query:

$ qmake6 -query
prefix:/tmp/compilation/install
QT_SYSROOT:
QT_INSTALL_PREFIX:/usr
QT_INSTALL_ARCHDATA:/usr/lib/qt6
QT_INSTALL_DATA:/usr/share/qt6
QT_INSTALL_DOCS:/usr/share/doc/qt6
QT_INSTALL_HEADERS:/usr/include/qt6
QT_INSTALL_LIBS:/usr/lib
QT_INSTALL_LIBEXECS:/usr/lib/qt6
QT_INSTALL_BINS:/usr/lib/qt6/bin
QT_INSTALL_TESTS:/usr/tests
QT_INSTALL_PLUGINS:/usr/lib/qt6/plugins
QT_INSTALL_QML:/usr/lib/qt6/qml
QT_INSTALL_TRANSLATIONS:/usr/share/qt6/translations
QT_INSTALL_CONFIGURATION:
QT_INSTALL_EXAMPLES:/usr/share/doc/qt6/examples
QT_INSTALL_DEMOS:/usr/share/doc/qt6/examples
QT_HOST_PREFIX:/usr
QT_HOST_DATA:/usr/lib/qt6
QT_HOST_BINS:/usr/lib/qt6/bin
QT_HOST_LIBEXECS:/usr/lib/qt6
QT_HOST_LIBS:/usr/lib
QMAKE_SPEC:linux-g++
QMAKE_XSPEC:linux-g++
QMAKE_VERSION:3.1
QT_VERSION:6.3.1

Unlike in Qt5, QT_INSTALL_CONFIGURATION isn't set.

@tsujan
Copy link
Member

tsujan commented Jul 8, 2022

Maybe something likes this:

    if(return_code EQUAL 0)
        string(STRIP "${output}" output)
        file(TO_CMAKE_PATH "${output}" output_path)
    else()
        message(STATUS "Got nothing from: ${_qt_qmake_executable} -query \"${qt_variable}\". LXQT_ETC_XDG_DIR will be set to '/etc/xdg'")
        set(output_path "/etc/xdg")
    endif()

@luis-pereira
Copy link
Member Author

@tsujan Tanks for taking the time to test it. I'm aware of that issue. It's a known fact.
QT_INSTALL_CONFIGURATION can be set with qmake6 -set QT_INSTALL_CONFIGURATION /etc/xdg.
I do agree with you proposed solution. That will be part of the next commits to this Draft PR.

startlxqt is renamed to startlxqt2. What's your view on that ?

@tsujan
Copy link
Member

tsujan commented Jul 8, 2022

I'm aware of that issue. It's a known fact.

@luis-pereira, yes, I guessed that when I saw you'd checked the return code, but it was a surprise to me.

My point is only that, since it can fail, we might need to set /etc/xdg as the default value. #77 (comment) was just a suggestion.

startlxqt is renamed to startlxqt2. What's your view on that ?

Actually, I wanted to ask you about it. Since there will be only one LXQt session, IMHO, startlxqt should be kept as it is. Did you have a specific reason in mind for startlxqt2?

@luis-pereira
Copy link
Member Author

I'm aware of that issue. It's a known fact.

@luis-pereira, yes, I guessed that when I saw you'd checked the return code, but it was a surprise to me.

My point is only that, since it can fail, we might need to set /etc/xdg as the default value. #77 (comment) was just a suggestion.

I understood the spirit of your suggestion.

Actually, I wanted to ask you about it. Since there will be only one LXQt session, IMHO, startlxqt should be kept as it is. Did you have a specific reason in mind for startlxqt2?

It was intended for the developers. It will not help that much. Keeping startlxqt

@luis-pereira
Copy link
Member Author

@tsujan startlxqt2 was just referenced in comments.
What I actually renamed in code was the lxqt-transupdate to lxqt2-transupdate. It's not installed under the lxqt2-build-tools directory and it will collide with the one from lxqt-build-tools. Does it cause an issue ?

@tsujan
Copy link
Member

tsujan commented Jul 15, 2022

Does it cause an issue ?

I don't see any problem. According to your logic, lxqt2-transupdate will be used by Qt6 apps of LXQt2; so, it's good.

@tsujan
Copy link
Member

tsujan commented Jul 15, 2022

startlxqt2 was just referenced in comments.

Yes, I know. I just think we might use startlxqt with Qt6 (keeping its name seems more convenient to me). If so, the comment(s) could be confusing to code readers.

@luis-pereira
Copy link
Member Author

Force pushing:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index e9e4b45..50f8aaa 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -22,7 +22,7 @@ include(cmake/modules/LXQtPreventInSourceBuilds.cmake)
 
 #-----------------------------------------------------------------------------
 # LXQT_DATA_DIR defaults to CMAKE_INSTALL_FULL_DATADIR. It's added to
-#   XDG_DATA_DIRS by the startlxqt2 script
+#   XDG_DATA_DIRS by the startlxqt script
 # Warning: Setting LXQT_DATA_DIR must be done after including GNUInstallDirs
 #-----------------------------------------------------------------------------
 if (NOT DEFINED LXQT_DATA_DIR)

@tsujan
Copy link
Member

tsujan commented Jan 5, 2023

To restart the Qt6 port after LXQt 1.2.0, I set /etc/xdg as the default global config dir, removed conflicts, and rebased the branch.

Updated FindInstallConfigPath accordingly.
QT_QMAKE_EXECUTABLE cached variable renamed to QUERY_EXECUTABLE. QT prefix
removed to avoid clash with Qt CMake variables.
CMakeLists.txt Outdated
@@ -30,7 +30,7 @@ if (NOT DEFINED LXQT_DATA_DIR)
)
endif()

set(LXQT_LIBRARY_NAME "lxqt" CACHE STRING "lxqt")
set(LXQT_LIBRARY_NAME "lxqt2" CACHE STRING "lxqt2")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@luis-pereira
I think this instance of lxqt could be kept as it was (no change to lxqt2). The components that can be installed in parallel with Qt5-based ones shouldn't be affected, and the other components shouldn't be installed in parallel. Therefore, for example, there's no reason to have different LXQT_TRANSLATIONS_DIR or LXQT_GRAPHICS_DIR folders.

Am I missing something?

Copy link
Member

@stefonarch stefonarch Feb 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made some tests with globalshortcuts-wip_qt6:

  • translations will only be built and installed if for all *.ts and *.yaml files lxqt- prefix is renamed to lxqt2-.
  • If LXQT_LIBRARY_NAME "lxqt" is used translations are not loaded, they are looked for in /usr/share/lxqt2/translations by the applications.

So not sure what the best approach is - but renaming all files will break all weblate translations settings. Just not using any lxqt2- prefixes if port is finished?
@luis-pereira ?

Copy link
Member

@gfgit gfgit Feb 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should try remove lxqt2 folder prefix and use plain lxqt and then try to build same library with both Qt6 and Qt5 and link some apps to either of them.
See if some files get overwritten or other kind of conflicts happen. This way you are sure there are no side effects.

@redtide
Copy link

redtide commented Jul 10, 2023

I'm not sure if this is the right place but I tried to update lxqt-transupdate.
I'm not a shell script expert, so I'd appreciate some feedback on what's good, what should be changed and what's not.

This version:

  • uses functions instead global scope
  • removed QT_SELECT: IMO a translation update tool shouldn't touch, nor impose an env var like that
  • checks the qt version installed, by detecting the one in use using qmake and setting an array of supported versions (5 and 6)
  • let the user to pass the path of the source directory, to avoid to work on the wrong one and delete all translations
  • using printf instead echo as proposed in fix bashisms in lxqt-transupdate for better portability with non-bash #84

@redtide
Copy link

redtide commented Jul 26, 2023

FTR: you can use the versionless commands to update translations, so it might work also with future Qt versions. I did a "version agnostic" of LXQtTranslateTs, though qt_create_translation is going to be deprecated.

Instead of hardcoding the project name in several instances, use the project
name variable.
gfgit and others added 5 commits March 3, 2024 16:32
CMake: test normal `lxqt` prefix
This has no effect on build itself, just for consistency
Remove "2" suffix from docstring
@tsujan
Copy link
Member

tsujan commented Mar 3, 2024

@gfgit
Thanks for changing those two instances of lxqt2 back to lxqt!

We usually don't make changes in other devs' branches without their consent, but this one can be seen as an exception — it was needed.

@tsujan tsujan marked this pull request as ready for review March 5, 2024 12:09
@tsujan
Copy link
Member

tsujan commented Mar 5, 2024

Any objection to merging? It's tested by several devs.

@gfgit
Copy link
Member

gfgit commented Mar 5, 2024

None

@tsujan tsujan merged commit 86c3fb6 into master Mar 5, 2024
@tsujan tsujan deleted the lxqt2-qt6 branch March 5, 2024 16:20
@tsujan
Copy link
Member

tsujan commented Mar 5, 2024

@gfgit
lxqt-build-tools uses QLibraryInfo::location, which is deprecated. Would you please fix that?

@tsujan tsujan mentioned this pull request Mar 6, 2024
@gfgit
Copy link
Member

gfgit commented Mar 7, 2024

See #93

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

6 participants