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

Fix automatic QML type registration for Qt 6.3.x #10928

Merged
merged 1 commit into from
Sep 30, 2022

Conversation

Holzhaus
Copy link
Member

Works around QTBUG-104373.

@Holzhaus Holzhaus added the bug label Sep 29, 2022
@Holzhaus Holzhaus added this to In progress in QML GUI via automation Sep 29, 2022
@github-actions github-actions bot added the build label Sep 29, 2022
@Holzhaus Holzhaus linked an issue Sep 29, 2022 that may be closed by this pull request
@Holzhaus
Copy link
Member Author

Yikes, now we have a mismatch between CI qmlformat and local qmlformat because it's a system depedency that does not allow pinning :(

This is a workaround for [QTBUG-104373], which broke automatic type
registration of C++ QML types if the QML module version starts with `0`.
As a workaround, we simply increase the version from `0.1` to `1.0`.

Fixes mixxxdj#10854.

[QTBUG-104373]: https://bugreports.qt.io/browse/QTBUG-104373
@Holzhaus
Copy link
Member Author

Holzhaus commented Sep 29, 2022

I removed the formatting changes for now, I'll take care of that in a separate PR.

EDIT: See #10929.

@m0dB
Copy link
Contributor

m0dB commented Sep 29, 2022

Fix confirmed! It actually works with library and playback and all!

@Holzhaus
Copy link
Member Author

Holzhaus commented Sep 29, 2022

Shall we merge then?

@m0dB
Copy link
Contributor

m0dB commented Sep 29, 2022

Sounds good to me. But I wonder how you have been able to commit: When I try to commit qml files (e.g. main.qml without any actual changes) qmllint fails:

Warning: res/qml/main.qml:2:1: Warnings occurred while importing module "Mixxx":
import Mixxx 1.0 as Mixxx
^^^^^^
---
Warning: Failed to import Mixxx. Are your include paths set up properly?
---

Warning: res/qml/main.qml:73:25: Unqualified access
                        Mixxx.PreferencesDialog.show();
                        ^^^^^

Doesn't this happen to you?

@m0dB
Copy link
Contributor

m0dB commented Sep 29, 2022

I see I can run qmllint successfully with these arguments:

qmllint -I build/qml/Mixxx/ -i build/qml/Mixxx/qmldir res/qml/main.qml

@m0dB
Copy link
Contributor

m0dB commented Sep 29, 2022

Yikes, now we have a mismatch between CI qmlformat and local qmlformat because it's a system depedency that does not allow pinning :(

Any suggestions how to deal with this? BTW, I notice that 6.3.2 qmlformat does some weird stuff, like this:

-            Skin.FadeBehavior on visible {
+            Skin.FadeBehavior on visible  {

@Holzhaus
Copy link
Member Author

Holzhaus commented Sep 29, 2022

Shall we merge then?

Sounds good to me.

We have a policy of not merging our own PRs, so this was just a small hint to the other core team members to have a look 😉

But I wonder how you have been able to commit: When I try to commit qml files (e.g. main.qml without any actual changes) qmllint fails

qmllint does not complain on my machine. If it doesn't work for you, you probably have a different qmllint version installed.

$ pre-commit run qmllint --all-files
qmllint..................................................................Passed
$ qmllint --version
qmllint 1.0

Yikes, now we have a mismatch between CI qmlformat and local qmlformat because it's a system depedency that does not allow pinning :(

Any suggestions how to deal with this? BTW, I notice that 6.3.2 qmlformat does some weird stuff, like this:

-            Skin.FadeBehavior on visible {
+            Skin.FadeBehavior on visible  {

Let's discuss this on #10929.

Copy link
Member

@Swiftb0y Swiftb0y left a comment

Choose a reason for hiding this comment

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

Code LGTM. Waiting for my local build so I can confirm with a smoke test. Thanks.

@Swiftb0y
Copy link
Member

fyi I unwatched the repo. So please @ me or request a review if you need my involvement.

@Swiftb0y
Copy link
Member

works with Qt 6.3.1 ✔️

@Swiftb0y Swiftb0y merged commit 57cf055 into mixxxdj:main Sep 30, 2022
QML GUI automation moved this from In progress to Done Sep 30, 2022
@Holzhaus
Copy link
Member Author

Thanks!

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

Successfully merging this pull request may close these issues.

Experimental QML UI does not load with Qt 6.3
3 participants