Fix to allow NSIS packaging to work for non-MSVC Windows builds

[FyiurAmron]

Needed for example for MSYS2, otherwise the paths break due to wrong separator. Backwards-compatible with existing builds (new condition includes the old one).
See also https://discourse.cmake.org/t/platform-id-vs-win32-vs-cmake-system-name/1226 as to why switching to CMAKE_SYSTEM_NAME is a generally good idea (quote from CMake dev):

The WIN32, APPLE, UNIX, etc. variables are “soft” deprecated due to this. We’ll never actually get them removed in the wider ecosystem, so they mean what they mean with all the conflations and ambiguities they come with. CMAKE_SYSTEM_NAME is what I’d use in CMake code

[FyiurAmron]

@Rossmaxx this seems related to the other work you're doing in the project, would you have spare sec for a review here?

[Rossmaxx]

Change look sane. I'll wait for the CI before approval.

[Rossmaxx]

LGTM

[tresf]

I believe this condition is a no-op (always-op?) because we set it for mingw too. Probably best to remove the conditional and just set the value each time.

lmms/cmake/toolchains/common/WinCrossCompile.cmake

Line 2 in 4033406

SET(CMAKE_SYSTEM_NAME Windows)

I too have had this issue. I'm not sure why NSIS is picky about backslashes only in certain parts of the installer codebase (probably a bug) but let's just keep it simple and apply this to all systems.

This used to be detected and handled but we removed msys2 support. https://github.com/LMMS/lmms/pull/7251/files#diff-c353d8c883ae0aa25ad3b3c8a8a6610055fbe93a95a8ef6e7133692d44bf0ce4L48

@FyiurAmron if you want to add msys2 support back in, but modernized, I'd love to discuss the use-case. I wrote the old support and it was very ugly and volatile. It was just recently removed.

[FyiurAmron]

@tresf At this moment, I'd say that the only thing needed for MSYS2 to work from top to bottom (apart from things not really related like compliance with GCC updates etc.) is this single change. I've been able to go the entire way without any hiccups other than this so I would say the removal of those vestigial MSYS2 code was a good call to streamline the process. The corner cases the removed code handled are, well, corner cases - I'm completely OK with no first-class MSYS2 support as long as there are no low-hanging fruits (like this one) to get it running in the way. Anyhow, changing this single line is all that was required for build+package of a complete working app with an installer, and TBH, it's a good change on its own (i.e. getting rid of explicit compiler constants and replacing them with platform-related ones).

As of the use cases, there's one additional that came to my mind: possibility of MSYS2-based Windows builds in CI. Getting the compiled MinGW Windows libs packaged for Linux is no easy feat ATM (taking into account that tobydox PPA is no longer maintained etc.), and MSYS2 repo is bleeding edge, having everything out-of-the-box. I'm not saying that this is a way to go, but having the possibility to switch with virtually 0 upkeep cost (i.e. by just fixing a single CMake if) is IMO a good call.

That said, I don't fully understand how we could remove the conditional here. You mean to use https://cmake.org/cmake/help/latest/command/file.html#to-native-path or something similar? I haven't tested that, I opted for the simplest fix possible, with as little possible side-effects as I could imagine. I guess it can be done otherwise, though.

[tresf]

I don't fully understand how we could remove the conditional here. You mean to use

I mean keep the code, remove the condition.

[FyiurAmron]

@tresf wouldn't that break *n?x (i.e. Mac/Linux) builds?

[Rossmaxx]

wouldn't that break *n?x builds?

I was about to comment that but you beat me to it.

[tresf]

@tresf wouldn't that break *nix (i.e. Mac/Linux) builds?

No more than this PR would, quoting:

because we set it for mingw too.

lmms/cmake/toolchains/common/WinCrossCompile.cmake

Line 2 in 4033406

SET(CMAKE_SYSTEM_NAME Windows)

This PR uses a condition which is always true.

[Rossmaxx]

Ahh i now get what you mean. NSIS is windows only so only windows builds will get affected so looks like this condition is safe to remove.

[tresf]

https://github.com/LMMS/lmms/pull/7287/files#r1617627013

[FyiurAmron]

@tresf re

I too have had this issue. I'm not sure why NSIS is picky about backslashes only in certain parts of the installer codebase (probably a bug) but let's just keep it simple and apply this to all systems.

I digged deeper. It is indeed a known bug in the NSIS installer (see https://cmake.org/cmake/help/book/mastering-cmake/chapter/Packaging%20With%20CPack.html#id5 and https://cmake.org/pipermail/cmake/2008-June/022085.html for more background), but the actual workaround is even simpler than that. Tl;dr just invert the last slash, it magically works then. I tested the updated code in both MSYS2 and Linux MinGW, both seem to work.

[tresf]

Good find ! Let's comment that bug then so we know why this is so weird!

[FyiurAmron]

@tresf amended with the comment. All checks are green.