diff mbox

[RFC] Wine package

Message ID 54A6E9C0.7060109@dawncrow.de
State Superseded
Headers show

Commit Message

André Zwing Jan. 2, 2015, 6:56 p.m. UTC 
Am 02.01.2015 um 19:13 schrieb Thomas Petazzoni:> Dear André Hentschel,
>
> On Fri, 02 Jan 2015 18:02:37 +0100, André Hentschel wrote:
>
>> I'm working on the Wine package for upstream buildroot.
>> I attached Config.in and wine.mk for early review, so i don't start totally wrong.
>
> Can you send a proper patch, sent inline (using git send-email) ? This
> is actually what makes review easy as we can just hit "Reply" and put
> some comments inline in the code.

ok, here it is, modified the version and added the uclibc dependency

Comments

Thomas Petazzoni Jan. 2, 2015, 7:29 p.m. UTC | #1 
Dear André Hentschel,

On Fri, 02 Jan 2015 19:56:00 +0100, André Hentschel wrote:

> ok, here it is, modified the version and added the uclibc dependency

Thanks. See comments below.

> diff --git a/package/wine/Config.in b/package/wine/Config.in
> new file mode 100644
> index 0000000..ad0c8f2
> --- /dev/null
> +++ b/package/wine/Config.in
> @@ -0,0 +1,19 @@
> +config BR2_PACKAGE_WINE
> +	bool "Wine"

Lowercase: "wine".

> +	depends on BR2_INET_IPV6
> +	depends on BR2_TOOLCHAIN_HAS_THREADS
> +    depends on !BR2_TOOLCHAIN_USES_UCLIBC

Indentation is not correct for this line. Also, just for curiosity,
could you give some details on the issues you've seen when building
wine against uclibc?

Does it work with the musl C library? In other words, do you need:

	depends on !BR2_TOOLCHAIN_USES_UCLIBC

or:

	depends on BR2_TOOLCHAIN_USES_GLIBC

 ?

> +	# Wine has much CPU specific code
> +	depends on BR2_i386 || BR2_x86_64
> +	help
> +	  Wine is a compatibility layer capable of running
> +	  Windows applications on Linux. Instead of simulating internal
> +	  Windows logic like a virtual machine or emulator,
> +	  Wine translates Windows API calls into POSIX calls on-the-fly,
> +	  eliminating the performance and memory penalties of other methods.
> +
> +	  http://www.winehq.org
> +
> +comment "Wine needs a toolchain w/ IPv6, threads"
> +	depends on BR2_i386 || BR2_x86_64
> +	depends on !BR2_INET_IPV6 || !BR2_TOOLCHAIN_HAS_THREADS

You need to take into account the uClibc issue here, by adding
something like:

	wine needs an (e)glibc toolchain w/ IPv6, threads

Of course assuming what you need is glibc only.

Also, lowercase "wine".

> diff --git a/package/wine/wine.mk b/package/wine/wine.mk
> new file mode 100644
> index 0000000..ce91587
> --- /dev/null
> +++ b/package/wine/wine.mk
> @@ -0,0 +1,203 @@
> +################################################################################
> +#
> +# wine
> +#
> +################################################################################
> +
> +WINE_VERSION = 1.6.2
> +WINE_SOURCE = wine-$(WINE_VERSION).tar.bz2
> +WINE_SITE = http://source.winehq.org/git/wine.git/snapshot/
> +WINE_LICENSE = LGPLv2.1

Can you double check whether it's LGPLv2.1 or LGPLv2.1+ ?

> +WINE_LICENSE_FILES = COPYING.LIB
> +WINE_INSTALL_TARGET = YES
> +WINE_DEPENDENCIES = host-wine
> +WINE_CONF_OPTS += --disable-tests --with-wine-tools=../host-wine-$(WINE_VERSION)

Having the target wine poke directly inside the source directory of
host-wine isn't that great. Ideally, we should be able to remove the
source directory of a package immediately after it has been built. The
host wine package gets installed to $(HOST_DIR). Aren't the tools you
need also installed there? If not, then please add a comment above this
line that explains what is going on.

> +
> +ifeq ($(or $(BR2_aarch64),$(BR2_x86_64)),y)
> +WINE_CONF_OPTS += --enable-win64
> +endif

Your Config.in file only allows the wine package to be built on i386
and x86-64, so the aarch64 part seems superfluous here. So maybe just:

WINE_CONF_OPTS += $(if $(BR2_ARCH_IS_64),--enable-win64)

> +ifeq ($(BR2_PACKAGE_XORG7),y)
> +	WINE_CONF_OPTS += --with-x
> +	WINE_DEPENDENCIES += x11r7
> +else
> +	WINE_CONF_OPTS += --without-x
> +endif

There is no package named 'x11r7' in Buildroot, so clearly this hasn't
been tested.

Did you test all those optional dependencies? If not, then please
simply disable the ones you haven't tested (i.e force --without-<foo>
in WINE_CONF_OPTS). Developers interested in enabling more optional
dependencies can do so later on.

Other than that, looks good. Do you have a pointer to a simple
command-line only Windows program that we could potentially use to test
this wine package?

Thanks,

Thomas
diff mbox

Patch

diff --git a/package/Config.in b/package/Config.in
index a4f5d8b..e59b458 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -997,6 +997,7 @@  menu "Miscellaneous"
 	source "package/snowball-init/Config.in"
 	source "package/sound-theme-borealis/Config.in"
 	source "package/sound-theme-freedesktop/Config.in"
+	source "package/wine/Config.in"
 endmenu

 menu "Networking applications"
diff --git a/package/wine/Config.in b/package/wine/Config.in
new file mode 100644
index 0000000..ad0c8f2
--- /dev/null
+++ b/package/wine/Config.in
@@ -0,0 +1,19 @@ 
+config BR2_PACKAGE_WINE
+	bool "Wine"
+	depends on BR2_INET_IPV6
+	depends on BR2_TOOLCHAIN_HAS_THREADS
+    depends on !BR2_TOOLCHAIN_USES_UCLIBC
+	# Wine has much CPU specific code
+	depends on BR2_i386 || BR2_x86_64
+	help
+	  Wine is a compatibility layer capable of running
+	  Windows applications on Linux. Instead of simulating internal
+	  Windows logic like a virtual machine or emulator,
+	  Wine translates Windows API calls into POSIX calls on-the-fly,
+	  eliminating the performance and memory penalties of other methods.
+
+	  http://www.winehq.org
+
+comment "Wine needs a toolchain w/ IPv6, threads"
+	depends on BR2_i386 || BR2_x86_64
+	depends on !BR2_INET_IPV6 || !BR2_TOOLCHAIN_HAS_THREADS
diff --git a/package/wine/wine.mk b/package/wine/wine.mk
new file mode 100644
index 0000000..ce91587
--- /dev/null
+++ b/package/wine/wine.mk
@@ -0,0 +1,203 @@ 
+################################################################################
+#
+# wine
+#
+################################################################################
+
+WINE_VERSION = 1.6.2
+WINE_SOURCE = wine-$(WINE_VERSION).tar.bz2
+WINE_SITE = http://source.winehq.org/git/wine.git/snapshot/
+WINE_LICENSE = LGPLv2.1
+WINE_LICENSE_FILES = COPYING.LIB
+WINE_INSTALL_TARGET = YES
+WINE_DEPENDENCIES = host-wine
+WINE_CONF_OPTS += --disable-tests --with-wine-tools=../host-wine-$(WINE_VERSION)
+
+ifeq ($(or $(BR2_aarch64),$(BR2_x86_64)),y)
+WINE_CONF_OPTS += --enable-win64
+endif
+
+ifeq ($(BR2_PACKAGE_CUPS),y)
+	WINE_CONF_OPTS += --with-cups
+	WINE_DEPENDENCIES += cups
+else
+	WINE_CONF_OPTS += --without-cups
+endif
+
+ifeq ($(BR2_PACKAGE_DBUS),y)
+	WINE_CONF_OPTS += --with-dbus
+	WINE_DEPENDENCIES += dbus
+else
+	WINE_CONF_OPTS += --without-dbus
+endif
+
+ifeq ($(BR2_PACKAGE_FONTCONFIG),y)
+	WINE_CONF_OPTS += --with-fontconfig
+	WINE_DEPENDENCIES += fontconfig
+else
+	WINE_CONF_OPTS += --without-fontconfig
+endif
+
+ifeq ($(BR2_PACKAGE_FREETYPE),y)
+	WINE_CONF_OPTS += --with-freetype
+	WINE_DEPENDENCIES += freetype
+else
+	WINE_CONF_OPTS += --without-freetype
+endif
+
+ifeq ($(BR2_PACKAGE_GNUTLS),y)
+	WINE_CONF_OPTS += --with-gnutls
+	WINE_DEPENDENCIES += gnutls
+else
+	WINE_CONF_OPTS += --without-gnutls
+endif
+
+ifeq ($(BR2_PACKAGE_GST_PLUGINS_BASE),y)
+	WINE_CONF_OPTS += --with-gstreamer
+	WINE_DEPENDENCIES += gst-plugins-base
+else
+	WINE_CONF_OPTS += --without-gstreamer
+endif
+
+ifeq ($(BR2_PACKAGE_JPEG),y)
+	WINE_CONF_OPTS += --with-jpeg
+	WINE_DEPENDENCIES += jpeg
+else
+	WINE_CONF_OPTS += --without-jpeg
+endif
+
+ifeq ($(BR2_PACKAGE_LCMS2),y)
+	WINE_CONF_OPTS += --with-cms
+	WINE_DEPENDENCIES += lcms2
+else
+	WINE_CONF_OPTS += --without-cms
+endif
+
+ifeq ($(BR2_PACKAGE_LIBPCAP),y)
+	WINE_CONF_OPTS += --with-pcap
+	WINE_DEPENDENCIES += libpcap
+else
+	WINE_CONF_OPTS += --without-pcap
+endif
+
+ifeq ($(BR2_PACKAGE_LIBPNG),y)
+	WINE_CONF_OPTS += --with-png
+	WINE_DEPENDENCIES += libpng
+else
+	WINE_CONF_OPTS += --without-png
+endif
+
+ifeq ($(BR2_PACKAGE_LIBV4L),y)
+	WINE_CONF_OPTS += --with-v4l
+	WINE_DEPENDENCIES += libv4l
+else
+	WINE_CONF_OPTS += --without-v4l
+endif
+
+ifeq ($(BR2_PACKAGE_LIBXML2),y)
+	WINE_CONF_OPTS += --with-xml
+	WINE_DEPENDENCIES += xml2
+else
+	WINE_CONF_OPTS += --without-xml
+endif
+
+ifeq ($(BR2_PACKAGE_XORG7),y)
+	WINE_CONF_OPTS += --with-x
+	WINE_DEPENDENCIES += x11r7
+else
+	WINE_CONF_OPTS += --without-x
+endif
+
+ifeq ($(BR2_PACKAGE_LIBXSLT),y)
+	WINE_CONF_OPTS += --with-xslt
+	WINE_DEPENDENCIES += libxslt
+else
+	WINE_CONF_OPTS += --without-xslt
+endif
+
+ifeq ($(BR2_PACKAGE_MPG123),y)
+	WINE_CONF_OPTS += --with-mpg123
+	WINE_DEPENDENCIES += mpg123
+else
+	WINE_CONF_OPTS += --without-mpg123
+endif
+
+ifeq ($(BR2_PACKAGE_NCURSES),y)
+	WINE_CONF_OPTS += --with-curses
+	WINE_DEPENDENCIES += ncurses
+else
+	WINE_CONF_OPTS += --without-curses
+endif
+
+ifeq ($(BR2_PACKAGE_SANE_BACKENDS),y)
+	WINE_CONF_OPTS += --with-sane
+	WINE_DEPENDENCIES += sane-backends
+else
+	WINE_CONF_OPTS += --without-sane
+endif
+
+ifeq ($(BR2_PACKAGE_TIFF),y)
+	WINE_CONF_OPTS += --with-tiff
+	WINE_DEPENDENCIES += tiff
+else
+	WINE_CONF_OPTS += --without-tiff
+endif
+
+ifeq ($(BR2_PACKAGE_ZLIB),y)
+	WINE_CONF_OPTS += --with-zlib
+	WINE_DEPENDENCIES += zlib
+else
+	WINE_CONF_OPTS += --without-zlib
+endif
+
+HOST_WINE_CONF_OPTS += \
+	--disable-tests \
+	--disable-win16 \
+	--without-alsa \
+	--without-capi \
+	--without-cms \
+	--without-coreaudio \
+	--without-cups \
+	--without-curses \
+	--without-dbus \
+	--without-fontconfig \
+	--without-freetype \
+	--without-gettext \
+	--without-gettextpo \
+	--without-gphoto \
+	--without-glu \
+	--without-gnutls \
+	--without-gsm \
+	--without-gstreamer \
+	--without-hal \
+	--without-jpeg \
+	--without-ldap \
+	--without-mpg123 \
+	--without-netapi \
+	--without-openal \
+	--without-opencl \
+	--without-opengl \
+	--without-osmesa \
+	--without-oss \
+	--without-pcap \
+	--without-png \
+	--without-sane \
+	--without-tiff \
+	--without-v4l \
+	--without-x \
+	--without-xcomposite \
+	--without-xcursor \
+	--without-xinerama \
+	--without-xinput \
+	--without-xinput2 \
+	--without-xml \
+	--without-xrandr \
+	--without-xrender \
+	--without-xshape \
+	--without-xshm \
+	--without-xslt \
+	--without-xxf86vm \
+	--without-zlib
+
+$(eval $(autotools-package))
+$(eval $(host-autotools-package))