Use CMake for Windows installer branding and version number #679
Conversation
As a side effect, fixes an issue where the installer wrote the incorrect version to the registry.
|
I just tested this using Wine, as I don't have access to a Windows VM rn. It seems like the version is still not set. |
|
Maybe we need this: https://stackoverflow.com/a/36507708 |
|
oh no what the fuck did i do |
|
kthchew just got scrumped |
|
Why does windows documentation say use X.X.X but NSIS wants X.X.X.X, and why does it have to be set in so many places ;-; Thanks @Scrumplex |
|
I have no idea. AFAIK Windows always formats it as A.B.X.Y no matter what you set yourself, so I guess that's the common denominator. |
|
After testing this yet again on Wine, it seems like this didn't fix it. Still no version number in wine's uninstaller.exe. Not sure how to proceed otherwise |
|
are we sure this doesn't happen on actual windows? |
|
No idea. I have no Windows VM rn |
I vaguely remember seeing the correct number on Windows somewhere with my original PR, but I could be remembering wrong. I remember checking the registry key directly as well so I might be thinking of when I did that. I will see if I can test it later. |
|
Also, I’m pretty sure the changes in 2ce4ce9 affect the version on the installer file’s properties, and doesn’t affect the actual install at all. So it probably wouldn’t change anything anyway in regards to whether WINE shows the installed version or not (but it still makes sense to make that change). |
|
Let's merge this for now. Maybe we should just switch to a different installer framework. WiX looked interesting, last time I looked at it. |
|
Found some time to test in a Windows VM.
I did remember wrong. Seems like the way to test this is to check Control Panel (not Settings) – the version is missing. And the most recent commits do not resolve it either.
My assumption here is also wrong in theory, upon reading the docs again. What I did here: WriteRegStr HKCU "${UNINST_KEY}" "ProductVersion" "@Launcher_RELEASE_VERSION_NAME4@"was incorrect. When the documentation says "Derived from ProductVersion property", it actually means the property from the installer, not another registry key. Which begs the question as to why VIProductVersion "@Launcher_RELEASE_VERSION_NAME4@"doesn't resolve this. (Actually, it doesn't even set the info when you right click the installer and look at its details. I have no idea what this does.
|
Trying this locally, I now see the version in Control Panel: I want to assume this would fix the original issue. I'll push the changes so you can see them. Edit: actually, I think that requires a new PR. The commit is https://github.com/kthchew/PolyMC/commit/9f039cef72d32dd2cfeee1038323e5c30af4957a. I'll wait for the CI on my fork to make an installer I can test before opening another PR. |
Fixes #644