WIP: Qt6 transition#11651
Conversation
|
Any chance of this making it into v2.7.10? Apparently, Qt 5 is pretty much unsupported at this point. |
No. This needs more work. |
|
@varjolintu Gotcha. Keep up the good work! |
|
QT5 in Gentoo and other (GNU/)Linux distros will be gettting slowly removed. Please see also: https://bugs.gentoo.org/949231 |
|
Stuff under |
|
I may be able to take a look this weekend. Too sad that I couldn't spend more time on this. What is breaking about fdosecrets? It has some custom code to make handling fdosecrets secret service related dbus objects easier. I don't remember anything too out of ordinary from top of my head that would break due to Qt6 though... |
For one, The type ID bool DBusMgr::deliverMethod(...)
{
...
// output args need to be converted before they can be directly sent out:
for (int i = 0; i != outputArgs.size(); ++i) {
auto& outputArg = outputArgs[i];
if (!outputArg.convert(method.outputTargetTypes.at(i))) {
qWarning() << "Internal error: Failed to convert message output to type"
<< method.outputTargetTypes.at(i);
return false;
}
}
...The // Under CollectionProxy of FdoSecretsProxy.h
inline PropertyReply<bool> locked() const
{
IMPL_GET_PROPERTY(Locked); // crashes here, specifically at the QDBusConnection::call call, inside of which stack traces aren't available
} |
I don't think this does what you think it does?
|
|
@Aetf Turns out the https://doc.qt.io/qt-6/qvariant-obsolete.html Same with the What I've done so far: diff --git a/src/fdosecrets/dbus/DBusDispatch.cpp b/src/fdosecrets/dbus/DBusDispatch.cpp
index eee63bdb..0165c62d 100644
--- a/src/fdosecrets/dbus/DBusDispatch.cpp
+++ b/src/fdosecrets/dbus/DBusDispatch.cpp
@@ -20,7 +20,6 @@
#include <QDBusMetaType>
#include <QThread>
-#include <QtDBus>
namespace FdoSecrets
{
@@ -32,29 +31,37 @@ namespace FdoSecrets
return camel.at(0).toUpper() + camel.mid(1);
}
- bool prepareInputParams(const QVector<int>& inputTypes,
+ bool prepareInputParams(const DBusClientPtr& client,
+ const QVector<int>& inputTypes,
+ bool needsCallingClient,
const QVariantList& args,
+ DBusResult& ret,
QVarLengthArray<void*, 10>& params,
QVariantList& auxParams)
{
- // prepare params
+ // reserve to avoid reallocation
+ auxParams.reserve(inputTypes.size() + (needsCallingClient ? 1 : 0));
+
+ // fill auxParams
+ if (needsCallingClient) {
+ auxParams.append(QVariant::fromValue(client));
+ }
+
for (int count = 0; count != inputTypes.size(); ++count) {
const auto& id = inputTypes.at(count);
const auto& arg = args.at(count);
if (arg.userType() == id) {
- // shortcut for no conversion
- params.append(const_cast<void*>(arg.constData()));
+ // skip conversion
+ auxParams.append(arg);
continue;
}
- // we need at least one conversion, allocate a slot in auxParams
- auxParams.append(QVariant(id));
- auto& out = auxParams.last();
- // first handle QDBusArgument to wire types
+ QVariant out{QMetaType(id)};
+
if (arg.userType() == qMetaTypeId<QDBusArgument>()) {
QMetaType wireId(typeToWireType(id).dbusTypeId);
- out = QVariant(wireId, nullptr);
+ out = QVariant(QMetaType(wireId), nullptr);
const auto& in = arg.value<QDBusArgument>();
if (!QDBusMetaType::demarshall(in, wireId, out.data())) {
@@ -63,18 +70,24 @@ namespace FdoSecrets
return false;
}
} else {
- // make a copy to store the converted value
out = arg;
}
- // other conversions are handled here
- if (!out.convert(id)) {
- qDebug() << "Internal error: failed conversion from" << arg << "to type" << QMetaType::typeName(id)
- << id;
+
+ if (!out.convert(QMetaType(id))) {
+ qDebug() << "Internal error: failed conversion from" << arg << "to type"
+ << QMetaType::typeName(id) << id;
return false;
}
- // good to go
- params.append(const_cast<void*>(out.constData()));
+
+ auxParams.append(std::move(out));
+ }
+
+ // fill params from auxParams
+ params.append(&ret); // first slot for return value
+ for (const QVariant& var : auxParams) {
+ params.append(const_cast<void*>(var.constData()));
}
+
return true;
}
@@ -341,16 +354,8 @@ namespace FdoSecrets
QVarLengthArray<void*, 10> params;
QVariantList auxParams;
- // the first one is for return type
- params.append(&ret);
-
- if (method.needsCallingClient) {
- auxParams.append(QVariant::fromValue(client));
- params.append(const_cast<void*>(auxParams.last().constData()));
- }
-
// prepare input
- if (!prepareInputParams(method.inputTypes, args, params, auxParams)) {
+ if (!prepareInputParams(client, method.inputTypes, method.needsCallingClient, args, ret, params, auxParams)) {
qDebug() << "Failed to prepare input params";
return false;
}
@@ -358,7 +363,7 @@ namespace FdoSecrets
// prepare output args
outputArgs.reserve(outputArgs.size() + method.outputTypes.size());
for (const auto& outputType : asConst(method.outputTypes)) {
- outputArgs.append(QVariant(outputType));
+ outputArgs.append(QVariant(QMetaType(outputType)));
params.append(const_cast<void*>(outputArgs.last().constData()));
}
@@ -378,10 +383,17 @@ namespace FdoSecrets
// output args need to be converted before they can be directly sent out:
for (int i = 0; i != outputArgs.size(); ++i) {
auto& outputArg = outputArgs[i];
- if (!outputArg.convert(method.outputTargetTypes.at(i))) {
- qWarning() << "Internal error: Failed to convert message output to type"
- << method.outputTargetTypes.at(i);
+
+ QMetaType fromType = outputArg.metaType();
+ QMetaType toType = QMetaType(method.outputTargetTypes.at(i));
+
+ QVariant result(toType); // default-construct target type
+ if (!QMetaType::convert(fromType, outputArg.constData(), toType, result.data())) {
+ qWarning() << "Internal error: Failed to convert message output from type"
+ << fromType.name() << "to type" << toType.name();
return false;
+ } else {
+ outputArg = result;
}
}This should get a handful of tests to pass. Some test cases under testguifdosecrets are still failing/crashing, I'll try to find out what went wrong with those. For now @varjolintu if you can, please apply the patch above to |
|
|
|
Working on the code now. Also need the following patch to get gui tests to compile, due to diff --git a/tests/gui/TestGuiFdoSecrets.cpp b/tests/gui/TestGuiFdoSecrets.cpp
index fc7e218e..88d20db5 100644
--- a/tests/gui/TestGuiFdoSecrets.cpp
+++ b/tests/gui/TestGuiFdoSecrets.cpp
@@ -131,6 +131,7 @@ char* toString(const QDBusObjectPath& path)
return QTest::toString("ObjectPath(" + path.path() + ")");
}
+TestGuiFdoSecrets::TestGuiFdoSecrets() = default;
TestGuiFdoSecrets::~TestGuiFdoSecrets() = default;
void TestGuiFdoSecrets::initTestCase()
diff --git a/tests/gui/TestGuiFdoSecrets.h b/tests/gui/TestGuiFdoSecrets.h
index 1624eed4..c6809f38 100644
--- a/tests/gui/TestGuiFdoSecrets.h
+++ b/tests/gui/TestGuiFdoSecrets.h
@@ -55,6 +55,7 @@ class TestGuiFdoSecrets : public QObject
Q_OBJECT
public:
+ explicit TestGuiFdoSecrets();
~TestGuiFdoSecrets() override;
private slots:
@@ -145,6 +146,8 @@ private:
}
private:
+ Q_DISABLE_COPY(TestGuiFdoSecrets)
+
QScopedPointer<MainWindow> m_mainWindow;
QPointer<DatabaseTabWidget> m_tabWidget;
QPointer<DatabaseWidget> m_dbWidget; |
|
Thank you :) This makes our job a little bit easier. |
|
@mutativesystems Thanks for spotting the deprecated QVariant methods. That's super hard to debug! However I think your patch for Therefore after the patch even if the code can progress, there are multiple other errors. The real issue is outputArgs are all null somehow after qt_metacall. Here's the log output after I added more debug logging: I still don't get why outputArg is null... Too late for me so I have to stop here. But I feel we are close if we can manage to fix this. |
Edit 1: I take it back, you're right. I'll debug this some more. Edit 2: this can be fixed like so: diff --git a/src/fdosecrets/dbus/DBusDispatch.cpp b/src/fdosecrets/dbus/DBusDispatch.cpp
index 0165c62d..6c5f1abd 100644
--- a/src/fdosecrets/dbus/DBusDispatch.cpp
+++ b/src/fdosecrets/dbus/DBusDispatch.cpp
@@ -363,7 +363,7 @@ namespace FdoSecrets
// prepare output args
outputArgs.reserve(outputArgs.size() + method.outputTypes.size());
for (const auto& outputType : asConst(method.outputTypes)) {
- outputArgs.append(QVariant(QMetaType(outputType)));
+ outputArgs.append(QVariant(QMetaType(outputType), QMetaType(outputType).create()));
params.append(const_cast<void*>(outputArgs.last().constData()));
}
@@ -383,17 +383,12 @@ namespace FdoSecrets
// output args need to be converted before they can be directly sent out:
for (int i = 0; i != outputArgs.size(); ++i) {
auto& outputArg = outputArgs[i];
+ auto toType = QMetaType(method.outputTargetTypes.at(i));
- QMetaType fromType = outputArg.metaType();
- QMetaType toType = QMetaType(method.outputTargetTypes.at(i));
-
- QVariant result(toType); // default-construct target type
- if (!QMetaType::convert(fromType, outputArg.constData(), toType, result.data())) {
+ if (!outputArg.convert(toType)) {
qWarning() << "Internal error: Failed to convert message output from type"
- << fromType.name() << "to type" << toType.name();
+ << outputArg.metaType().name() << "to type" << toType.name();
return false;
- } else {
- outputArg = result;
}
}Basically to make a |
For this I'll wait for someone else to figure this out - I'm not familiar with Qt or KPXC's codebase; for now I'm just self-compiling and using it with fdosecrets disabled in CMake. |
I took a quick look, but couldn't find the reason yet. I'm pretty sure it's related to the new code changes in DBusDispatch.cpp, and not the events or EDIT: If I revert the line |
Ran without more details: |
|
For anyone else who wishes to do some testing on Alpine, there's an APKBUILD for this branch at https://gitlab.alpinelinux.org/alpine/aports/-/merge_requests/101126 Tests are disabled in CI as |
droidmonkey
left a comment
droidmonkey
left a comment
There was a problem hiding this comment.
I have applied my final cleanup and changes.
droidmonkey
force-pushed
the
qt6_ver2
branch
2 times, most recently
from
e784d77 to
a1071b5
Compare
|
Trying to build from the latest push
|
* Improve CsvParser and TextStream handling of codecs for encoding/decoding * Improve Auto-Type guards for plugin load failure * Improve handling of X11 auto-type plugin if display is not available * Fix potential ASAN errors in TestGui
* Apply `u` prefix to all strings used for comparison with XML element names and wrap in QStringView. * Improve readability of XML code by reducing repeat calls to `m_xml.name()` where appropriate.
|
Should be all fixed now |
OK, now I'm only getting
so that seems to be progress 👍 However, while compiling, I also get a lot of repetitions of this message:
EDIT: to clarify, I was building off of |
|
testguifdosecrets also fails for me in DatabaseWidget::save(). Haven't checked yet what change triggered that. Not sure why it doesn't fail in the CI. Might be Wayland related. |
|
Still seeing a single test failure (in a clean chroot) in the latest iteration: |
|
Set your locale to C. |
|
On Sat, 2 May 2026, at 16:06, Janek Bevendorff wrote:
*phoerious* left a comment (keepassxreboot/keepassxc#11651)
<#11651 (comment)>
Set your locale to C.
This workaround was mentioned above already. The problem is that the test scaffolding still isn’t taking care of this. A previous comment mention this was pending and a later comment stated “all fixed”, hence why I’m bringing up again.
|
Try again with the latest changes. |
OK, can confirm that the locale related test failures disappear with the latest fix (at least on Arch), but Test #47 still fails. |
Building When I keep enabled the |
|
Thanks everyone. Lets try to solve the remaining FDO Secrets issues in a separate issue: |
20 participants
Continues work from #10267, using #11003 as the base.
Fixes #7774.
Type of change