Deps overhaul

[kryksyh]

This PR moves third-party dependencies to an external submodule: muse_deps

• I signed the CLA as username:
• The title of the PR describes the problem it addresses.
• Each commit's message describes its purpose and effects, and references the issue it resolves. If changes are extensive, there is a sequence of easily reviewable commits.
• The code in the PR follows the coding rules.
• I understand all aspects of the code I'm contributing and I'm able to explain it if requested.
• The code compiles and runs on my machine, preferably after each commit individually. I have manually tested and verified that my changes fulfil their intended purpose.
• No prior attempts to resolve this problem exist, or if they do, I listed them in my PR description and described how I avoided repeating past mistakes.
• There are no unnecessary changes.
• I created a unit test or vtest to verify the changes I made (if applicable).

[rpatters1]

I don't think this is a good idea, at least the submodule part. Submodules complicate rebasing and generate a tremendous amount of commit noise. Each additional submodule has an exponential impact on these issues. I did an entire project with multiple submodules and will never, ever use them again. FetchContent is a way better alternative. (I think this about muse also, but well, nobody asked me. 😅 )

Anyway, the mnxdom dependency has to update its commit number with every change to the mnx spec. It is a work in progress. Hiding the dependency away in a submodule is going to make this much harder to manage.

[cbjeukendrup]

It looks like this makes the whole MUE_COMPILE_MACOS_PRECOMPILED_DEPS_PATH business obsolete, so it might be desirable to remove that completely.

However what I don't like about this solution, is that all dependencies are compiled from scratch, every time, on every clean build, and on every CI run. That was the advantage of the MUE_COMPILE_MACOS_PRECOMPILED_DEPS_PATH setup: dependencies had been precompiled in a reproducible GitHub Actions workflow, and could be consumed (at least in theory) using normal CMake find_package logic.

(If precompilation functionality is ever added to this new system, do watch out that, for macOS, universal binaries are created, and, equally important but less well known, that the deployment target is macOS 10.15.4 or even lower. Also, in my experience distributing binaries via GH Releases works well because it gives fast downloads, while not blowing up the git repo size.)

One thing to keep in mind is that some third-party package managers (among which FlatPak) require being able to compile without internet access, so in that case downloading dependencies source code during the CMake configure step is not an option.

Finally, I was also going to say that this makes updating in-development dependencies like mnxdom significantly more complicated.
(I'm less sure whether I agree with the submodule/FetchContent argument; with FetchContent I'd be afraid to lose local changes inside the sub-repo, given that the content is populated in the build folder. But this is probably not the best place to have that discussion.)

[kryksyh]

@cbjeukendrup I understand you mean these deps: https://raw.githubusercontent.com/cbjeukendrup/musescore_deps/main/musescore_deps_macos.tar.gz ? Yes, we can remove it safely now. I'm not sure if you have looked at the muse_deps repo, but unifying prebuilt dependency delivery was the main goal of this change. So

what I don't like about this solution, is that all dependencies are compiled from scratch, every time, on every clean build, and on every CI run

Should not be the case, unless I did something wrong. Please let me know what exactly you meant by that.

One thing to keep in mind is that some third-party package managers (among which FlatPak) require being able to compile without internet access

This was the other main goal: the unified approach to ALL dependencies allows us to build a package suitable for offline build, see: PrepareDepsSources.cmake.

for macOS, universal binaries are created, and, equally important but less well known, the deployment target is macOS 10.15.4 or even lower

This is something I didn't do, and the deployment target was set to 12. Could you explain why that is needed? Is it only for OS X compatibility, or is there something more to it? From the latest chat with @igorkorsukov, I understand that starting MSS 5, we will have osx12 as the target version.

[kryksyh]

Anyway, the mnxdom dependency has to update its commit number with every change to the mnx spec. It is a work in progress. Hiding the dependency away in a submodule is going to make this much harder to manage.

@rpatters1 I'm not familiar with MNX development story, but if it is indeed an experimental in-development feature, we can keep it in-tree, I guess.

Prior to this refactoring, I see it hadn't been updated for about 4 months, so it didn't look that intensive.

[rpatters1]

Prior to this refactoring, I see it hadn't been updated for about 4 months, so it didn't look that intensive.

There is currently an outstanding PR #33236 that changes it. I had another that I had been holding off opening until that is merged. But now I plan to roll that into yet a third iteration.

Every time the MNX committee releases a new schema I will need to update mnxdom. Those changes seem to come in chunks every few weeks. They have been stuck on how to encode dynamics lately, but they finally released something that will make for much more complete looking imports. (Sorry for the MNX-aside. 😅 )

[rpatters1]

Another complication around the mnxdom story is its dependencies, which are nlohmann_json and the json schema validator.

These could be safely separated into the muse_dep submodule (however it is done). But making them available to an mnxdom still residing in-repo is beyond what I know how to do.

mnxdom also embeds the schema from its FetchContent-linked commit of the w3c mnx project, along with the example json files that is uses in its tests. How the internet-free Linux package builds are dealing with this, I do not know.

[cbjeukendrup]

I understand you mean these deps:

Yes, indeed.

I'm not sure if you have looked at the muse_deps repo

I did take a look, and only saw links to source code releases in packages definitions, which together with what I had seen earlier led to assumptions, but now I see there's more... much more. So I'll look again.

This is something I didn't do, and the deployment target was set to 12. Could you explain why that is needed?

MuseScore's deployment target is currently OS X 10.15.4, using a backported version of Qt 6.10.2 (https://github.com/cbjeukendrup/musescore_build_qt/blob/main/.github/workflows/qt6.10.2_macOS_universal_10_15.yml). We originally wanted to keep the deployment target at 10.15, but backporting Qt to that version was significantly more complicated than 10.15.4.

This may be relevant for Audacity 4 as well. I see that Audacity 3 supports all the way back to OS X 10.13; maintaining that with Qt is not realistic, but with my backported Qt version you can at least still have 10.15.4.

Anyway, so since MuseScore's deployment target is 10.15.4, the dependencies would need to support that version too.

The news that the deployment target will be bumped to 12.0 had not reached me yet. It might be good to see how the community responds to that, before fully committing to it.

Note that supporting macOS 12.0 still requires creating a backported version of Qt: by the time that MS5 is released, Qt 6.11.2 and Qt 6.12 will have been released, which support respectively macOS 13.0 and 14.4. That's not a problem but I just wanted to mention it.

[igorkorsukov]

We are considering dropping support for older versions of macOS, which means that support for macOS 13 will be supported (we will use Qt 6.11).

[kryksyh]

@igorkorsukov @rpatters1 I leave the final decision about MNX to you guys. Let me know if we should keep it in the main repo for now as fetchcontent.

[igorkorsukov]

@igorkorsukov @rpatters1 I leave the final decision about MNX to you guys. Let me know if we should keep it in the main repo for now as fetchcontent.

We're currently having trouble with this.
When building applications for distributions and Flatpak, the internet is unavailable. Currently, only MNX requires the internet (other dependencies are system dependencies). I think that's why there is still no Flatpak build for 4.7.

Solution options:

1. Keep fetchcontent, but disable this module for Linux (I don't know how popular it is)
2. Replace fetchcontent with a submodule (submodules are allowed, at least for Flatpak, they are fetched before building)
3. Make it like all the other dependencies. (I would certainly prefer this, in order to have one policy, one infrastructure, one solution).

That is, keeping fetchcontent and this module enabled for all platforms is the worst solution, since it blocks the build for Linux distributions and Flatpak

[kryksyh]

From what I understand, it is now in development and is not ready for regular users. So we could safely disable it on Linux by default until it become stable.

[igorkorsukov]

@rpatters1

Which option would you prefer:

1. Replace fetchcontent with a submodule temporarily - suitable for frequent updates (I understand that you don't like the submodule, but in this case it's a simple technical requirement). When MNX is rarely updated, we would prefer to include it like other dependencies.
2. We're building MNX like other dependencies—it's suitable for infrequent updates, like once per release. You can, of course, work on your own version locally, but the build on our CI will only use the new version when we decide it needs to be updated.

@kryksyh @rpatters1

I was thinking, let's do both options :)
That is, the standard muse_dep dependency option will be the primary one, enabled by default.
And the fetchcontent option will be enabled on request, during the development phase.

[kryksyh]

I was thinking, let's do both options :)

I'm not sure it would work. If the library's api changes, it has to be reflected in the app code. Supporting two versions in the mnx module will be messy.

[igorkorsukov]

Then let's disable it for Linux.
We'll decide later, as development progresses.

[kryksyh]

@igorkorsukov I will keep it enabled in our own CI builds where we produce appimages and have no restrictions, but by default it will be turned off, so distro maintainers are happy.

[cbjeukendrup]

Regarding that it is "in development": it's not that it is unstable or very experimental, it's rather that it should preferably remain easy to update it because there may be frequent changes. But disabling it for that reason seems to me like a too drastic trade-off. Anyway, I think it would be good to await @rpatters1 's opinion before taking action.