Skip to content

Allow easily using the distro Qt binaries - #69

Open
quic-kdybcio wants to merge 4 commits into
qualcomm:mainfrom
quic-kdybcio:topic/host_qt
Open

Allow easily using the distro Qt binaries#69
quic-kdybcio wants to merge 4 commits into
qualcomm:mainfrom
quic-kdybcio:topic/host_qt

Conversation

@quic-kdybcio

Copy link
Copy Markdown

Pull Request

Description
Most Linux users don't want to install a pre-made package from the Qt website and would prefer to use the distro-provided set. This is also more or less also a pre-requisite for distro packaging. This PR also bumps down the Qt requirement from 6.9 to 6.4 through a trivial change, to allow a broader range of non-bleeding-edge distros in.

The current build.sh really insists on having a user-provided path to the Qt toolchain, with a structure that only exists if the website package is used, so that is changed accordingly.

Related Issue
n/a

Type of Change
Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works # built on ubuntu 24.04
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Additional Context
Add any other context or screenshots about the pull request here.

QCheckBox::checkStateChanged() only exists starting with Qt 6.7;
older releases only expose the deprecated stateChanged() signal.
The project has so far only been built against Qt 6.9+ installer
trees, so this never mattered, but tracking whatever Qt a distro's
package manager ships means older minor releases need to keep
working too.

Select the signal to connect to at compile time based on QT_VERSION
so the same source works across the whole range.

Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
qt_standard_project_setup(REQUIRES 6.9) was set while bringing up
the libraries and UI layer, without that being an actual API
dependency anywhere: nothing in the tree calls anything newer than
Qt 6.2, and the requirement was simply copy-pasted into every
CMakeLists.txt that has a project_setup() call.

In practice this forces every contributor and CI job through the Qt
Online Installer, since no mainstream distribution packages anything
close to 6.9 yet. Drop the floor to 6.4 so the build can use whatever
Qt6 a distribution's package manager already provides.

qt_generate_deploy_app_script()'s option to receive the generated
script's path was renamed OUTPUT_SCRIPT in Qt 6.5, and 6.4 only
understands the original FILENAME_VARIABLE name, so pick whichever
keyword matches the Qt version actually found instead of hardcoding
either one.

Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Every Linux contributor was required to install Qt through the Qt
Online Installer and export QTBIN, even though most distributions
already package a perfectly usable Qt6. The Online Installer needs
a Qt account, pulls down components most of this project doesn't
touch, and leaves behind a tree that has to be found and pointed at
manually forever after.

Fall back to the host's qmake6 (and the apt packages it comes from)
when QTBIN isn't set, and report any missing packages up front
instead of failing on whichever tool happens to be checked first.
Runtime bundling only matters when linking against a private Qt
Online Installer tree that won't be on the target machine's library
path, so skip that step when building against the system Qt.

Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
The Linux guide led with the Qt Online Installer and only offered
the distro-based one as a secondary "Option B" that still told readers
to export QTBIN to a path apt never creates, which never actually
worked. Now that build.sh can use the distro Qt package directly,
describe that as the primary path and drop the minimum version quoted
in the prerequisites table down to what's actually required.

Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
@Biswajee

Copy link
Copy Markdown
Member

quic-kdybcio , please point your merge branch to develop per CONTRIBUTING.md.

Comment on lines +19 to +25
// QCheckBox::checkStateChanged() replaced stateChanged() in Qt 6.7; stateChanged()
// is compiled out on newer Qt by QT_DISABLE_DEPRECATED_UP_TO, so branch on version.
#if QT_VERSION >= QT_VERSION_CHECK(6, 7, 0)
connect(_checkBox, &QCheckBox::checkStateChanged, this, [=, this](bool newState){ emit checkStateChanged(newState); });
#else
connect(_checkBox, &QCheckBox::stateChanged, this, [=, this](bool newState){ emit checkStateChanged(newState); });
#endif

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We're looking to maintain a single codebase and would prefer if we can have a single implementation. Addressing this will be really great for acceptance.

qt_generate_deploy_app_script(
TARGET DeviceCatalog
OUTPUT_SCRIPT deploy_script
${QT_DEPLOY_SCRIPT_KEYWORD} deploy_script

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This rename probably does not provide a significant benefit. Please remove same for all other files.

Comment thread build.sh

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Too many changes. We won't accept bulk changes without significant benefits over the existing implementation. Please outline clearly. This will affect acceptance.

Comment thread README.md
**Option A**: Quick installation via apt (recommended)
```bash
sudo apt install qt6-base-dev qt6-serialport-dev qt6-multimedia-dev
sudo apt install cmake ninja-build qt6-base-dev qt6-serialport-dev qt6-multimedia-dev

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ninja is not a hard-requirement but good to have. The option is left to Linux users and therefore not mandated in the command

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants