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

MuseScore can't be built with Qt 6.7 due to changes in private QKeyMapper #23980

Closed
ferdnyc opened this issue Aug 11, 2024 · 15 comments · Fixed by #23982
Closed

MuseScore can't be built with Qt 6.7 due to changes in private QKeyMapper #23980

ferdnyc opened this issue Aug 11, 2024 · 15 comments · Fixed by #23982
Labels
dev Related to developer experience (compiling, code base, CI), rather than end user experience Qt-next Either broken or fixed by (updating to) newer Qt versions than we currently use

Comments

@ferdnyc
Copy link
Contributor

ferdnyc commented Aug 11, 2024

The private QKeyMapper::possibleKeys was changed to return QList<QKeyCombination> instead of QList<int> in QT change 504447 last October.

Originally posted by @ferdnyc in #22979 (comment)

@ferdnyc
Copy link
Contributor Author

ferdnyc commented Aug 11, 2024

I believe I have a fix for this, so formally documenting it as a referenceable issue.

@cbjeukendrup
Copy link
Contributor

I also have a branch with some fixes so that it can be built using Qt 6.7/6.8, but that's actually not very useful because with any Qt version newer than 6.2, the entire top toolbar is not rendered at all, for unknown reasons.

ferdnyc added a commit to ferdnyc/MuseScore that referenced this issue Aug 11, 2024
Qt 6.7 changes the return type of private `QKeyMapper::possibleKeys`
from `QList<int>` to `QList<QKeyCombination>`. A QKeyCombination
can easily be transformed into an `int` by calling its `.toCombined()`
method.

Avoid ugly ifdefs by changing the type of the variable returned from
`possibleKeys()` to `auto`, and write two overloaded functions that
take either a `QList<QKeyCombination>` or `QList<int>` and output
the `QSet<int>` we really want.

Resolves: musescore#23980
@ferdnyc
Copy link
Contributor Author

ferdnyc commented Aug 11, 2024

@cbjeukendrup

I don't think that's happening, but I admit I'm not too familiar with MuseScore's interface. (I'm a coder, not a musician!) Am I missing a toolbar without realizing? This is built from my PR branch, against Fedora's system Qt 6.7.2.

image

@Jojo-Schmitz
Copy link
Contributor

The menu bar is missing
image

@cbjeukendrup
Copy link
Contributor

The fact that the file/edit/view/... menus are missing might be because the system-wide menu bar is used (that is supported on some Linux distributions).

But the Home/Score/Publish tab bar is missing. I somehow thought that much more was missing, but apparently I misremembered that. Or it got magically fixed meanwhile.

@cbjeukendrup cbjeukendrup added the dev Related to developer experience (compiling, code base, CI), rather than end user experience label Aug 11, 2024
@ferdnyc
Copy link
Contributor Author

ferdnyc commented Aug 12, 2024

Aha! No, my distro doesn't have Mac-style systemwide menus, so if there's supposed to be a menubar on MuseScore then it is indeed missing.

Qt Creator also has an annoying habit of opening with its menubar hidden, come to think of it. Coupled with the keyboard combo supposedly bound to UN-hiding it not working, necessitating the diddling of its .ini file to change the appropriate setting by hand.

But I just looked in MuseScore's rather spare $HOME/.config/MuseScore/MuseScore4Development.ini, and there's no such setting in there to even change. (Though perhaps adding one would work; I'll have to check what the correct setting is in QtCreator.ini.)

@ferdnyc
Copy link
Contributor Author

ferdnyc commented Aug 12, 2024

Hrm, nope. In Qt Creator it's

[MainWindow]
MenubarVisible=true

MuseScore's .ini has no such section, but just for S&G I tried adding MenubarVisible=true to both the [ui] and [application] sections, as well as a [MainWindow] section I also created... and still no go from any of it.

There's a ton of Qml errors spewed out onto the console at startup. Nothing explicitly mentioning a menu, but plenty that could be related anyway.

igorkorsukov pushed a commit that referenced this issue Aug 12, 2024
Qt 6.7 changes the return type of private `QKeyMapper::possibleKeys`
from `QList<int>` to `QList<QKeyCombination>`. A QKeyCombination
can easily be transformed into an `int` by calling its `.toCombined()`
method.

Avoid ugly ifdefs by changing the type of the variable returned from
`possibleKeys()` to `auto`, and write two overloaded functions that
take either a `QList<QKeyCombination>` or `QList<int>` and output
the `QSet<int>` we really want.

Resolves: #23980
@cbjeukendrup
Copy link
Contributor

MuseScore has a custom menubar, see AppMenuBar.qml. But it's interesting that that's missing as well. We could look what AppMenuBar.qml and MainToolbar.qml have in common.

@igorkorsukov
Copy link
Contributor

@ferdnyc Just in case, we are not planning to switch to Qt6.7 in any case. It's not that MuseScore doesn't work with it - we can just do it and fix it. The thing is that we try to support the oldest versions of MacOS, so we always use the minimum possible version of Qt.

@ferdnyc
Copy link
Contributor Author

ferdnyc commented Aug 26, 2024

@igorkorsukov

@ferdnyc Just in case, we are not planning to switch to Qt6.7 in any case. It's not that MuseScore doesn't work with it - we can just do it and fix it. The thing is that we try to support the oldest versions of MacOS, so we always use the minimum possible version of Qt.

That make sense, but Linux distributions always build their own packages with the system-installed Qt, so the choices on e.g. Fedora 40 are to to build with either 5.15.12 or 6.7.2. If MuseScore won't build with whatever system version is available, it's marked Failed To Build From Source (FTBFS). And if it builds but doesn't work correctly, that's in some ways worse, as users will get a broken application.

@igorkorsukov
Copy link
Contributor

I don't really understand the point of this approach.
As far as I know, there are two reasons:

  • Saving space - but this was relevant 40 years ago, it is not relevant now.
  • The ability to fix a bug in a shared library so that all applications receive this fix. But in practice, applications may simply not work correctly or not work at all, because they were not tested and did not take into account the peculiarity of the new version. And it is even more strange to do this with such complex packages as Qt, because each version has its own peculiarities.

As a result, distributions have either outdated software or non-functional software.
For Linux users we provide AppImage and hopefully will soon make Flatpak. To bypass this approach of distributions.

@ferdnyc
Copy link
Contributor Author

ferdnyc commented Aug 30, 2024

@igorkorsukov I understand that viewpoint, and you're not the first to express it. From an application developer perspective, there's certainly an appeal to the idea of having full control over your complete software distribution.

But there's another side to that coin. While it's not really about package size / disk space, the Fedora package repos have 946 packages linked with libQt5Core.so.5, and 733 linked with libQt6Core.so.6. If each of those bundled their own copy of just that one library, the distribution grows by some 10.5 GB. Now multiply that by all of the other Qt libraries.

And while obviously no user is going to install every package that uses a library, my own desktop system currently contains 244 packages dependent on Qt5Core and 264 dependent on Qt6Core, so we're talking a fair amount of space saved.

Then there's the maintenance. The last updates to qt5-qtbase and qt6-qtbase (the packages containing the core Qt libs) were a fix for CVE-2024-39936. Which only had to be applied and rebuilt in two places, instead of tracking down every package that uses Qt[56]Network to ensure that they were patched with that fix, then rebuilt and updated.

Fedora currently maintains a set of 40 patches to the official Qt 5.15.14 distribution, and 11 to Qt 6.7.2, to make them less buggy and better integrated with the OS — particularly since it's a GNOME-first distro.

Like I said, for an application developer, there's a certain appeal to Flatpak. (I've never liked AppImages, since their reliance on "oldest possible dependencies" is a nice theory that has, in my experience, completely fallen apart in practice. The notion that 4-5 year old dependencies are always forward-compatible to modern Linux environments has been disproven more times than I can count.) And Fedora has embraced Flatpaks as well, giving them equal standing with distro packages in the GNOME Software app. But they're not looking to replace all distro packages with them, just affording users that option if they choose to take it.

As a user, I've always found Flatpaks to be lacking — the sandboxing, while vital from a security perspective, gets in the way of the interoperability that's always been a hallmark of Linux packaging, unlike other OSes. In Linux packages can work together, they can enhance and rely on each other — which makes it easier for each to "do one thing and do it well". Flatpak blocks that in the name of security, which isn't my favorite tradeoff.

(The latest blow was learning that, while they've managed to sort out Flatpak distribution of application plugins/extensions separate from the application itself — despite the fact that it punches holes in their application siloing — turns out that updating an application won't update any of its separately-distributed plugins. Even if the newer version breaks compatibility with those plugins. AND EVEN IF there's also a newer version of the plugin that would be compatible. They just break until you think to update them manually.)

Linux distro packaging has had 20 years to wrestle with these issues, and has come up with solutions. Flatpak is reinventing all of this from scratch, and facing all of the same challenges since they seemingly learned little from what came before.

@ferdnyc
Copy link
Contributor Author

ferdnyc commented Aug 30, 2024

my own desktop system currently contains 244 packages dependent on Qt5Core and 264 dependent on Qt6Core

I should qualify those stats by admitting that I didn't adjust for the fact that many of those packages — perhaps as many as 50 each? — are other parts of the Qt distribution itself, so those shouldn't really count. Let's just round down to 200 each.

@cbjeukendrup
Copy link
Contributor

Incidentally, a similar discussion was had recently at #24235. I think the policy should be:

  • Keep providing AppImage for who likes that, and we have the infrastructure for that anyway
  • Make official FlatPaks, for who prefers that
  • When distro maintainers want to create distro-specific packages, and need changes at our side to support that, we're happy to consider these changes
  • Users of distro packages should somehow be made aware that these packages are not created nor verified by us, so if they experience bugs they should check whether they should report them to us or to the distro maintainers.

Every approach has advantages and disadvantages. We cannot and should not stop the practice of creating distro-specific packages. But what we don't want is that distro package users experience bugs because of how the app was packaged for their distro and then complain to us that we deliver low-quality software. As long as that doesn't happen, we have no reason to be unhappy.

@darix
Copy link

darix commented Sep 12, 2024

I agree with the comments on why distros like to avoid duplicated libraries. :)

@cbjeukendrup cbjeukendrup added the Qt-next Either broken or fixed by (updating to) newer Qt versions than we currently use label Sep 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev Related to developer experience (compiling, code base, CI), rather than end user experience Qt-next Either broken or fixed by (updating to) newer Qt versions than we currently use
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants