Added support for official wine-devel and wine-staging packages. #3278

karmux · 2017-01-20T20:55:45Z

jasp00

Not sure if this is necessary. If so, you should fix it.

jasp00 · 2017-01-22T09:54:52Z

cmake/modules/FindWine.cmake

@@ -7,8 +7,8 @@
 #  WINE_DEFINITIONS - Compiler switches required for using wine
 #

-FIND_PATH(WINE_INCLUDE_DIR windows/windows.h PATH_SUFFIXES wine)
-FIND_LIBRARY(WINE_LIBRARY NAMES wine PATH_SUFFIXES wine i386-linux-gnu/wine)
+FIND_PATH(WINE_INCLUDE_DIR windows/windows.h PATHS /opt/wine-devel/include /opt/wine-staging/include PATH_SUFFIXES wine)


I would argue what official means, since the official packages in Debian and Ubuntu are the wine-development packages. The expected way is to configure with CMAKE_PREFIX_PATH rather than use PATHS, but I guess this does no harm.

I've now written this part using CMAKE_PREFIX_PATH.

jasp00 · 2017-01-22T10:02:01Z

plugins/vst_base/CMakeLists.txt

 		SET(EXTRA_FLAGS ${EXTRA_FLAGS} -nodefaultlibs /usr/lib/i386/wine/libwinecrt0.a -luser32 -lkernel32 -lgdi32)
 	ENDIF()
+	# Development
+	IF("${WINEBUILD_OUTPUT}" MATCHES "/opt/wine-devel/lib64/wine/libwinecrt0.a.*")


I question this workaround for a recent system, specially for a package from WineHQ. There is no way currently to mix 64-bit and 32-bit objects, so this scenario is not possible.

I'm confused by this comment. What I did is just added a 2 paths to look for libwinecrt0.a.
In addition to /usr/lib/x86_64-linux-gnu/wine/ and /usr/lib/lib64/wine/ it now looks also into /opt/wine-devel/lib64/wine/ and /opt/wine-staging/lib64/wine/. I didn't introduce any new mixing of 32bit and 64bit.

This workaround is currently here because some people do not know they have to install 32-bit Wine and do not understand the linker error. If you install the package from WineHQ, you have 64-bit and 32-bit Wine, so you cannot get this error because 32-bit is selected. Did you actually run into this problem?

If I remove those lines, I can't compile with Wine devel or staging.

Fine, it can be fixed later.

jasp00 · 2017-01-22T10:10:28Z

plugins/vst_base/CMakeLists.txt

+		-I/opt/wine-devel/include
+		-L/opt/wine-devel/lib
+		-I/opt/wine-staging/include
+		-I/opt/wine-staging/lib


This breaks if you have multiple versions of Wine. You should include directories depending on the detected configuration. On the other hand, the expected way is to configure with WINE_CXX_FLAGS.

Do you mean -L/opt/wine-staging/lib?

Yes, it's a typo, should be -L.

I agree with @jasp00 here... isn't there a way to assume this in FindWine.cmake once FIND_LIBRARY returns?

I've made a new changeset that detects if devel or stable is installed and includes directories of only one wine version.

jasp00

Workaround can be fixed later. You should not override preferences.

jasp00 · 2017-02-13T20:53:24Z

plugins/vst_base/CMakeLists.txt

 		SET(EXTRA_FLAGS ${EXTRA_FLAGS} -nodefaultlibs /usr/lib/i386/wine/libwinecrt0.a -luser32 -lkernel32 -lgdi32)
 	ENDIF()
+	# Development
+	IF("${WINEBUILD_OUTPUT}" MATCHES "/opt/wine-devel/lib64/wine/libwinecrt0.a.*")


Fine, it can be fixed later.

jasp00 · 2017-02-13T20:58:30Z

cmake/modules/FindWine.cmake

@@ -7,12 +7,14 @@
 #  WINE_DEFINITIONS - Compiler switches required for using wine
 #

+SET(CMAKE_PREFIX_PATH /opt/wine-devel;/opt/wine-staging)


You are overriding user's preference. You should include the current value.

jasp00 · 2017-02-13T21:03:46Z

plugins/vst_base/CMakeLists.txt

+# Staging
+IF(EXISTS "/opt/wine-staging/lib64/libwine.so")
+	SET(WINE_INCLUDES_AND_LIBS -I/opt/wine-staging/include -L/opt/wine-staging/lib)
+ENDIF()


Overriding again. You should use WINE_INCLUDE_DIR and WINE_LIBRARY.

WINE_INCLUDE_DIR gets value /opt/wine-devel/include/wine and wine/*.h headers cannot be found from there. Path needs to be /opt/wine-devel/include.

WINE_LIBRARY gets value /opt/wine-devel/lib/libwine.so and this cannot be linked. Path needs to be /opt/wine-devel/lib.

WINE_LIBRARY gets value /opt/wine-devel/lib/libwine.so

@karmux @jasp00 can't we calculate it in FindWine.cmake then and call it something like WINE_LIBRARY_PATH ?

wine/*.h headers cannot be found from [wine/]

Sounds like a bug with FindWine.cmake, no?

WINE_INCLUDE_DIR gets value /opt/wine-devel/include/wine [...] Path needs to be /opt/wine-devel/include.

WINE_LIBRARY gets value /opt/wine-devel/lib/libwine.so [...] Path needs to be /opt/wine-devel/lib.

So? Use STRING().

jasp00 · 2017-03-28T04:46:18Z

plugins/vst_base/CMakeLists.txt

 ENDIF(LMMS_HOST_X86_64)

 SET(WINE_CXX_FLAGS "" CACHE STRING "Extra flags passed to wineg++")

+STRING(REPLACE "include/wine" "include" WINE_INCLUDE_DIR ${WINE_INCLUDE_DIR})
+STRING(REPLACE "lib/libwine.so" "lib" WINE_LIBRARY ${WINE_LIBRARY})


You should output to new variables.

jasp00 · 2017-03-30T00:28:13Z

plugins/vst_base/CMakeLists.txt

+		"-I\"${CMAKE_INSTALL_PREFIX}/include/wine/windows\""
+		"-I\"${CMAKE_INSTALL_PREFIX}/include\""
+		"-I\"${WINE_INCLUDE_DIR_REPLACED}\""
+		"-L\"${WINE_LIBRARY_REPLACED}\""


With _REPLACED, it looks like you are using an alternate directory and library. Do these variables name directories? Then you should use something like WINE_INCLUDE_BASE_DIR and WINE_LIBRARY_DIR.

You are correct. I'll fix it.

jasp00 · 2017-03-31T23:52:09Z

Code looks good. @karmux, did you test Wine under /usr? If so, I will merge.

karmux · 2017-04-01T11:28:47Z

@jasp00 it compiles and runs with Wine under /usr.

…S#3278) * Added support for wine-devel packages. * Added support for wine-staging packages. * Staging extra flags. * Dynamic Wine branch detection (stable, devel, staging). * Using STRING() to correct WINE_INCLUDE_DIR and WINE_LIBRARY. * Don't override CMAKE_PREFIX_PATH. * REPLACE outputs new variables. * Renamed variables.

karmux added 3 commits January 20, 2017 22:18

Added support for wine-devel packages.

f787369

Added support for wine-staging packages.

a293c06

Staging extra flags.

d2345d7

jasp00 reviewed Jan 22, 2017

View reviewed changes

Dynamic Wine branch detection (stable, devel, staging).

bc83094

jasp00 reviewed Feb 13, 2017

View reviewed changes

karmux added 3 commits March 24, 2017 21:32

Merge branch 'master' into wine-devel

66d3dec

Using STRING() to correct WINE_INCLUDE_DIR and WINE_LIBRARY.

eb7bc41

Don't override CMAKE_PREFIX_PATH.

2de789d

jasp00 reviewed Mar 28, 2017

View reviewed changes

REPLACE outputs new variables.

8db1128

jasp00 reviewed Mar 30, 2017

View reviewed changes

Renamed variables.

3a9b185

jasp00 merged commit 0b43741 into LMMS:master Apr 1, 2017

karmux deleted the wine-devel branch April 1, 2017 12:36

This was referenced Jul 19, 2017

Added support for wine-stable from WineHQ repositories. #3708

Merged

Add AppImage support to Linux builds #3688

Merged

PhysSong mentioned this pull request Oct 16, 2017

LMMS wants to remove wine-staging on Ubuntu #3705

Closed

Uh oh!

Added support for official wine-devel and wine-staging packages. #3278

Added support for official wine-devel and wine-staging packages. #3278

Uh oh!

Conversation

karmux commented Jan 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jasp00 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jasp00 Jan 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jasp00 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tresf Feb 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jasp00 commented Mar 31, 2017

Uh oh!

karmux commented Apr 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

karmux commented Jan 20, 2017 •

edited

Loading

jasp00 Jan 22, 2017 •

edited

Loading

tresf Feb 22, 2017 •

edited

Loading

karmux commented Apr 1, 2017 •

edited

Loading