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

FindWine improvements #7268

Merged
merged 9 commits into from
May 23, 2024
Merged

FindWine improvements #7268

merged 9 commits into from
May 23, 2024

Conversation

tresf
Copy link
Member

@tresf tresf commented May 19, 2024

Wine 5.0 Release notes:

  • Binaries built for a Windows target no longer depend on the libwine library, to enable them to run on Windows without any extra dependencies. The libwine library is no longer built for Windows at all.

Wine 9.0 Release notes

  • The libwine.so library is removed. It was no longer used, and deprecated since Wine 6.0. Winelib ELF applications that were built with Wine 5.0 or older will need a rebuild to run on Wine 9.0.

Fix RemoteVstPlugin.exe.so compilation for WineHQ builds.

-- Found Wine: /opt/wine-staging/bin/wineg++
--   WINE_INCLUDE_DIR:     /opt/wine-staging/include
--   WINE_CXX:             /opt/wine-staging/bin/wineg++
--   WINE_GCC:             /opt/wine-staging/bin/winegcc
--   WINE_32_FLAGS:        -L/opt/wine-staging/bin/../lib/wine/i386-unix/
--   WINE_64_FLAGS:        -L/opt/wine-staging/bin/../lib64/wine/x86_64-unix/

Supersedes #7260

Wine search order: (TBD)

  • wine-staging[-dev], wine-devel[-dev], wine-stable[-dev], <whatever your distro offers>
  • If your build fails after this PR please ensure the appropriate -dev package is installed!

Summary:

  • Uses NO_DEFAULT_PATH when locating files to avoid prefix mixing
  • Removes polluting CMAKE_PREFIX_PATH
  • Moves winegcc references to a new variable WINE_GCC
    • WINE_CXX is solely for wineg++.
  • Echo's found bin|include|libs to the CMake summary

Note, Ubuntu 20.04 ships with Wine 5.0, but WineHQ is 9.0 and these produce Windows VST interfaces that are incompatible with each other.

TODO:

  • RemoteVstPlugin32.exe.so will build with wine-staging, but crashes. ldd output is missing libwine.so.1.
    • I can't find this information anywhere, but libwine.so.1 is no longer linked to executables created with Wine 9's wineg++. Furthemore, those executables seem to be incompatible with older Wine versions. Replacing package wine with package winehq-staging|devel|stable is fine as well, versions don't need to be identical.
  • 64-bit VSTs effects are reported as not working, quoting:

    -1 — Today at 3:08 AM
    No 64bits effect plugins are working. 64bits instrument plugins seem to run properly

    • Cannot reproduce with this plugin. -1 claims a reboot fixed it. Marking as resolved.

@tresf tresf marked this pull request as ready for review May 19, 2024 05:50
@Z3R0C
Copy link
Contributor

Z3R0C commented May 19, 2024

  * 32-bit Windows host       : not found, please install (lib)wine-dev (or similar) - 64 bit systems additionally need gcc-multilib and g++-multilib
  * 64-bit Windows host       : not found, please install (lib)wine-dev (or similar) - 64 bit systems additionally need gcc-multilib and g++-multilib

This fix doesn't work for me. Without a fix it works fine with WineHQ devel.

@Rossmaxx
Copy link
Contributor

This fix doesn't work for me. Without a fix it works fine with WineHQ devel.

I guess you use ubuntu/debian or any other "stable" distro. This is a known issue and is currently under investigation by me and tres. The consensus now is to block older releases to a degree, because somewhere down the line, libwine.so.1 has been removed and that change gets reflected in WineHQ but not in distro wine. I advise you to use wine hq.

@Z3R0C
Copy link
Contributor

Z3R0C commented May 19, 2024

That's right. I use Ubuntu 24.04 and these wine packages are installed:

Bildschirmfoto vom 2024-05-19 17-08-07

I tested vst-fix2 and wine is no longer found. I chose winehq-devel for reasons.

@tresf
Copy link
Member Author

tresf commented May 19, 2024

`* 32-bit Windows host : not found, please install (lib)wine-dev (or similar) - 64 bit systems additionally need gcc-multilib and g++-multilib

  • 64-bit Windows host : not found, please install (lib)wine-dev (or similar) - 64 bit systems additionally need gcc-multilib and g++-multilib
    `

This fix doesn't work for me. Without a fix it works fine with WineHQ devel.

You're probably missing the winehq -dev package. This is deliberate, it really should have errored before this PR, but managed to recover in a half-working state due to mangled search paths. I cover this more in the comments in #7260.

Can you install the appropriate -dev packages from WineHQ and try again please?

Note, the warning specifically mentions -dev missing, so I find this to still be an accurate message, assuming it rings true for you.

@tresf
Copy link
Member Author

tresf commented May 19, 2024

I tested vst-fix2 and wine is no longer found. I chose winehq-devel for reasons.

Yeah, but you also have staging, but not staging-dev. It finds wine-staging then can't find the headers because the package wine-staging-dev is missing. Please either remove wine-staging or install wine-staging-dev. You can still use wine executable from devel or stable.

If people don't like this search order, I can reverse it. I'll edit the original PR description to mention this with "TBD" incase it's not well received.

Edit:

the -dev is called wine-staging-dev NOT winehq-staging-dev as one would expect. Not sure why wine chose these package names, sorry if this causes any confusion.

@tresf
Copy link
Member Author

tresf commented May 19, 2024

Feedback from Discord:

image

@Z3R0C

This comment was marked as outdated.

@tresf

This comment was marked as outdated.

@Z3R0C

This comment was marked as off-topic.

@tresf

This comment was marked as off-topic.

@tresf

This comment was marked as off-topic.

@tresf
Copy link
Member Author

tresf commented May 19, 2024

Quoting winehq. Please note the difference between wine-stable and winehq-stable. (hq being operative here).

Click to expand

By splitting a Wine over different packages, it is possible to install different branches side by side.

  • For example: Use Wine stable as the default Wine version and install Wine staging to test other programs.

Install Wine stable:

sudo apt install --install-recommends winehq-stable

Install Wine staging:

sudo apt install --install-recommends wine-staging # (Note the missing hq after wine)

Run a program with Wine stable:

wine program.exe

I'll reiterate one more time before marking this as off-topic...

  • If you have wine-staging installed, it exposes winegcc to the system, so it NEEDS a matching wine-staging-dev package due to our search order, clearly marked as "TBD" in the original post.

We did not make the decision to ship winegcc|g++|cpp WITHOUT matching dev headers, that's a decision by WineHQ. We simply search and use the version installed, newest to oldest. It can be argued that we should just keep searching until we find any suitable Wine, but LMMS has never officially supported this, and the error message from CMake is accurate to the action a developer needs to do to resolve the issue.

If your vote is to reverse the search order, stable|devel|staging|<whatever your distro offers>, simply state that and move on please, but a valid argument ("I don't want it" is not a valid argument) is needed to influence the decision.

Lastly, compiling Windows VST support with a particular Wine version doesn't force it to use that at runtime. That's configured by the winehq (with hq) package.

@tresf

This comment was marked as outdated.

@tresf
Copy link
Member Author

tresf commented May 20, 2024

This came in through Discord... will attempt to confirm: image

I tested in my Linux VM this plugin and could not reproduce, so this crash may be unrelated to LMMS' implementation.

@tresf tresf merged commit bd2362a into LMMS:master May 23, 2024
9 checks passed
@tresf tresf deleted the vst-fix2 branch May 23, 2024 05:25
sakertooth pushed a commit to sakertooth/lmms that referenced this pull request May 27, 2024
Improve WineHQ detection
Closes LMMS#7169 

---------

Co-authored-by: Rossmaxx <mrroshan127@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants